morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #4812] [Bug] Apktool file and workspace finalization loses ownership and completed outcomes #2502

New issue

Closed

opened 2026-10-17 21:05:49 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-17 21:05:49 +00:00

Owner

Originally created by @morluto on GitHub (Oct 17, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4812

Before submitting

I searched the open and closed issues for related reports.
I removed credentials and sensitive target data.

Area

Evidence, artifacts, or reconstruction

Observed behavior

Source-only finding at 11bcf44252cc095a77a4c6d10290df48f491fdde: Apktool resource finalization does not retain one truthful ownership/result boundary. decodeResources returns success after a failed workspace removal (only a limitation), does not check cancellation after that awaited removal, and its failure path samples a later aborted signal after cleanup, potentially replacing an already observed physical failure. Failed source closes are retried in a suppressing finally with no retained descriptor owner. readBoundedText uses raw blocking open("r") and closes without retention; decoded FIFO/special-file admission and failed descriptor close therefore escape the shared artifact lifecycle. Directory enumeration likewise relies on implicit iterator close.

Reproduction

NOT_ENOUGH_INFO: no natural OS cleanup denial or real Apktool reproduction was run for this new report. Inspected production source src/apktool/ApktoolProvider.ts: readBoundedText, countDecodedFiles, #drainAndClean, and decodeResources. Proposed controlled checks: pause/deny removal after successful projection, abort during removal, combine recorded decode failure with later abort, deny file close, and reject a decoded FIFO without blocking.

Expected behavior

Per docs/tool-design.md#own-the-lifecycle, retain exact descriptor/workspace cleanup capabilities until confirmed release, preserve completed observations and physical failures, and observe cancellation during final cleanup with its exact reason. Block replacement acquisition while cleanup remains unresolved. Successful peer cleanup must not be repeated.

Impact and scope

Source-derived: successful decode classification can hide incomplete workspace cleanup; original errors or completed facts can be lost; unconfirmed descriptors are not retained for retry. Runtime frequency and platform-specific OS failures are unknown. This is provider-level file/workspace ownership; #4808 fixed command/helper ownership. Closed #3935 addressed CLI preservation and does not establish provider descriptor/finalization recovery. #3927 concerns a different Android application service.

REA version

Source revision 11bcf44252cc095a77a4c6d10290df48f491fdde.

Runtime and platform

Source audit on macOS arm64; pinned Node 24.18.0. Real Apktool/JVM and native Windows behavior unverified.

Relevant output or evidence

Smallest source anchors: raw open(path, "r") and finally file.close() in readBoundedText; implicit opendir iterator ownership in countDecodedFiles; cleanupFailure strings and success limitations in decodeResources; signal?.aborted classification after await removeWorkspace(); final source?.close().catch(() => undefined). Shared withArtifactRegularFile, ArtifactResourceScope, AnalysisCleanupLease, and DirectoryArtifactReader already implement applicable ownership boundaries.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4812 ### Before submitting - [x] I searched the open and closed issues for related reports. - [x] I removed credentials and sensitive target data. ### Area Evidence, artifacts, or reconstruction ### Observed behavior Source-only finding at `11bcf44252cc095a77a4c6d10290df48f491fdde`: Apktool resource finalization does not retain one truthful ownership/result boundary. `decodeResources` returns success after a failed workspace removal (only a limitation), does not check cancellation after that awaited removal, and its failure path samples a later aborted signal after cleanup, potentially replacing an already observed physical failure. Failed source closes are retried in a suppressing `finally` with no retained descriptor owner. `readBoundedText` uses raw blocking `open("r")` and closes without retention; decoded FIFO/special-file admission and failed descriptor close therefore escape the shared artifact lifecycle. Directory enumeration likewise relies on implicit iterator close. ### Reproduction NOT_ENOUGH_INFO: no natural OS cleanup denial or real Apktool reproduction was run for this new report. Inspected production source `src/apktool/ApktoolProvider.ts`: `readBoundedText`, `countDecodedFiles`, `#drainAndClean`, and `decodeResources`. Proposed controlled checks: pause/deny removal after successful projection, abort during removal, combine recorded decode failure with later abort, deny file close, and reject a decoded FIFO without blocking. ### Expected behavior Per `docs/tool-design.md#own-the-lifecycle`, retain exact descriptor/workspace cleanup capabilities until confirmed release, preserve completed observations and physical failures, and observe cancellation during final cleanup with its exact reason. Block replacement acquisition while cleanup remains unresolved. Successful peer cleanup must not be repeated. ### Impact and scope Source-derived: successful decode classification can hide incomplete workspace cleanup; original errors or completed facts can be lost; unconfirmed descriptors are not retained for retry. Runtime frequency and platform-specific OS failures are unknown. This is provider-level file/workspace ownership; #4808 fixed command/helper ownership. Closed #3935 addressed CLI preservation and does not establish provider descriptor/finalization recovery. #3927 concerns a different Android application service. ### REA version Source revision `11bcf44252cc095a77a4c6d10290df48f491fdde`. ### Runtime and platform Source audit on macOS arm64; pinned Node 24.18.0. Real Apktool/JVM and native Windows behavior unverified. ### Relevant output or evidence Smallest source anchors: raw `open(path, "r")` and `finally file.close()` in `readBoundedText`; implicit `opendir` iterator ownership in `countDecodedFiles`; `cleanupFailure` strings and success limitations in `decodeResources`; `signal?.aborted` classification after `await removeWorkspace()`; final `source?.close().catch(() => undefined)`. Shared `withArtifactRegularFile`, `ArtifactResourceScope`, `AnalysisCleanupLease`, and `DirectoryArtifactReader` already implement applicable ownership boundaries.

bsakaguchi

2026-10-17 21:05:49 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-17 21:05:51 +00:00

Author

Owner

@morluto commented on GitHub (Oct 17, 2026):

Fixed in 4844ae812b357922496d570fcd57e9f0d1e849c8 on main.

One workspace owns source, snapshot and decoded-file descriptors, traversal and private-root removal. Failed closes retain exact capabilities, unresolved resources fence acquisition, and shutdown attempts independent peers.
Completed Evidence survives removal failure and cancellation during final cleanup. Physical decode/projection failures remain primary; successful retries clear original and retry cleanup status. Removed raw reads, implicit traversal and suppressing closes.
Shared directory traversal now bounds examined entries before omitting special files, retaining the existing capacity limit.

Validation: 163 focused tests across 20 files, build, typecheck/lint, docs, test discovery (1541 files exactly once), and commit/push hooks passed. Two read-only reviews cleared the final implementation. Six new public provider controls fail on clean baseline 11bcf4425 for the expected missing outcomes. Small controls exercise descriptor denial/retry, a FIFO without writer, malformed UTF-8 versus late cancellation, completed Evidence, mixed workspaces, late allocation during shutdown, and a six-entry regular/FIFO capacity fixture.

24 completed actual CLI/stdio MCP tasks used an authored inert decoder producing either 3 or 35 files. Target digest, metadata, strings, count and coverage were independently asserted; replies were byte-identical before/after. Repeated-process medians (baseline → fixed):

Task	Elapsed seconds	CPU seconds	Sampled family RSS MiB	Reply token estimate
CLI, 3 files	0.905 → 0.885	1.154 → 1.092	495.3 → 487.1	491 → 491
CLI, 35 files	0.889 → 0.906	1.137 → 1.132	491.4 → 487.8	492 → 492
MCP, 3 files	1.057 → 1.056	1.307 → 1.302	613.2 → 618.6	1046 → 1046
MCP, 35 files	1.053 → 1.056	1.312 → 1.302	617.7 → 610.8	1048 → 1048

No speedup claim. Two alternating repeats plus first processes, with OS caches intact. CPU uses wait4; sampled RSS may miss brief/reparented members. cl100k reply estimates exclude catalog discovery, requests, retries and agent continuations. A first measurement attempt failed because the authored launcher used CommonJS inside an ES-module package; correcting that fixture changed no production code. That failed attempt is excluded from completed comparisons.

Context7's Node 24 filesystem documentation confirms that explicit descriptor ownership is required with autoClose disabled; automatic directory iteration close likewise does not provide a retained retry capability after failure.

Actual Apktool/JVM, natural OS close/removal denial, native Windows and cold ownership-helper compilation remain unverified. Heavy stress/profiling remained stopped. Locale discovery (#4612), target-refusal attribution (#3340), and shared locale decoding (#4777) are separate audit work.

@morluto commented on GitHub (Oct 17, 2026): Fixed in `4844ae812b357922496d570fcd57e9f0d1e849c8` on main. - One workspace owns source, snapshot and decoded-file descriptors, traversal and private-root removal. Failed closes retain exact capabilities, unresolved resources fence acquisition, and shutdown attempts independent peers. - Completed Evidence survives removal failure and cancellation during final cleanup. Physical decode/projection failures remain primary; successful retries clear original and retry cleanup status. Removed raw reads, implicit traversal and suppressing closes. - Shared directory traversal now bounds examined entries before omitting special files, retaining the existing capacity limit. Validation: 163 focused tests across 20 files, build, typecheck/lint, docs, test discovery (1541 files exactly once), and commit/push hooks passed. Two read-only reviews cleared the final implementation. Six new public provider controls fail on clean baseline `11bcf4425` for the expected missing outcomes. Small controls exercise descriptor denial/retry, a FIFO without writer, malformed UTF-8 versus late cancellation, completed Evidence, mixed workspaces, late allocation during shutdown, and a six-entry regular/FIFO capacity fixture. 24 completed actual CLI/stdio MCP tasks used an authored inert decoder producing either 3 or 35 files. Target digest, metadata, strings, count and coverage were independently asserted; replies were byte-identical before/after. Repeated-process medians (baseline → fixed): | Task | Elapsed seconds | CPU seconds | Sampled family RSS MiB | Reply token estimate | | --- | --- | --- | --- | --- | | CLI, 3 files | 0.905 → 0.885 | 1.154 → 1.092 | 495.3 → 487.1 | 491 → 491 | | CLI, 35 files | 0.889 → 0.906 | 1.137 → 1.132 | 491.4 → 487.8 | 492 → 492 | | MCP, 3 files | 1.057 → 1.056 | 1.307 → 1.302 | 613.2 → 618.6 | 1046 → 1046 | | MCP, 35 files | 1.053 → 1.056 | 1.312 → 1.302 | 617.7 → 610.8 | 1048 → 1048 | No speedup claim. Two alternating repeats plus first processes, with OS caches intact. CPU uses wait4; sampled RSS may miss brief/reparented members. cl100k reply estimates exclude catalog discovery, requests, retries and agent continuations. A first measurement attempt failed because the authored launcher used CommonJS inside an ES-module package; correcting that fixture changed no production code. That failed attempt is excluded from completed comparisons. Context7's [Node 24 filesystem documentation](https://nodejs.org/docs/latest-v24.x/api/fs.html#filehandlecreatereadstreamoptions) confirms that explicit descriptor ownership is required with autoClose disabled; automatic directory iteration close likewise does not provide a retained retry capability after failure. Actual Apktool/JVM, natural OS close/removal denial, native Windows and cold ownership-helper compilation remain unverified. Heavy stress/profiling remained stopped. Locale discovery (#4612), target-refusal attribution (#3340), and shared locale decoding (#4777) are separate audit work.

bsakaguchi referenced this issue

2026-10-31 01:05:24 +00:00

[PR #2502] [CLOSED] fix(cli): report analysis failures as not-ok in full-output envelopes #5279

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#2502

No description provided.

Rows
Columns