[GH-ISSUE #4266] [Bug] Stable file hashing loses descriptor ownership and primary failures when close rejects #1688

New issue

Closed

opened 2026-10-16 21:09:05 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-16 21:09:05 +00:00

Owner

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

`hashStableFile` drops the descriptor owner when close fails

src/filesystem/StableFileHash.ts:15-18,52-53 obtains a descriptor from openRegularFile and unconditionally runs await handle.close() in finally. If that close rejects while the descriptor remains live, the rejection escapes without the handle owner or any cleanup facts. Callers receive an ordinary close error and cannot retain/retry the still-open descriptor. hashStableFile has 14 production call sites across ADB, Android/JADX, Linux Hopper, and Kuna.

Controlled reproduction

Baseline: 0e1f0bf2d06943e59253b8a66923a1ae6005e81e; Node v24.18.0 on macOS arm64. A disposable Vitest probe mocks only the openRegularFile boundary: it opens a real temporary regular file, wraps its real descriptor in OwnedFileHandle, and substitutes a close operation that throws before closing. The owner remains live after hashStableFile rejects; the caught error has no owner, cleanupResources, or cleanup property. The control then closes the same real owner successfully. This deterministically proves the ownership/result gap without claiming native close failures are frequent.

Command:

PATH=/opt/homebrew/opt/node@24/bin:$PATH node_modules/.bin/vitest run --config .cache/codebase-audit/artifacts/stableFileHashProbe.config.ts --reporter=dot

Result: 1 file passed, 1 test passed. Probe source/config are disposable under .cache/codebase-audit/artifacts/.

Impact and ownership

A close failure can turn a successful hash or an earlier read/cancellation failure into a bare close error while leaving the descriptor live. Callers that clean a surrounding temporary root cannot retry closing the lost descriptor. The concrete upstream ADB snapshot path (copyFile followed by hashStableFile in src/adb/AdbProvider.ts) shares this boundary; the local in-progress ADB implementation uses explicitly owned source/destination handles for its snapshot path, but other ADB hashes and the other 13 call sites still use hashStableFile.

The root fix belongs in StableFileHash and its owner contract: preserve the primary failure and expose a retryable owned handle/cleanup observation when close fails, then let each caller lifecycle adopt it where applicable. Avoid an unowned global retry registry. Add fault tests for close failure after a successful read and after a primary read/cancellation failure, plus a successful-close control.

Dedupe and limits

Read-only GitHub issue search for StableFileHash close owned handle descriptor cleanup returned no focused issue. This is a static helper-level, injected-close-failure proof; ordinary filesystem behavior and native close-failure frequency were not measured, and no ADB device or external tool was used.

Originally created by @morluto on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4266 ## `hashStableFile` drops the descriptor owner when close fails `src/filesystem/StableFileHash.ts:15-18,52-53` obtains a descriptor from `openRegularFile` and unconditionally runs `await handle.close()` in `finally`. If that close rejects while the descriptor remains live, the rejection escapes without the handle owner or any cleanup facts. Callers receive an ordinary close error and cannot retain/retry the still-open descriptor. `hashStableFile` has 14 production call sites across ADB, Android/JADX, Linux Hopper, and Kuna. ### Controlled reproduction Baseline: `0e1f0bf2d06943e59253b8a66923a1ae6005e81e`; Node v24.18.0 on macOS arm64. A disposable Vitest probe mocks only the `openRegularFile` boundary: it opens a real temporary regular file, wraps its real descriptor in `OwnedFileHandle`, and substitutes a close operation that throws before closing. The owner remains live after `hashStableFile` rejects; the caught error has no `owner`, `cleanupResources`, or `cleanup` property. The control then closes the same real owner successfully. This deterministically proves the ownership/result gap without claiming native close failures are frequent. Command: ```sh PATH=/opt/homebrew/opt/node@24/bin:$PATH node_modules/.bin/vitest run --config .cache/codebase-audit/artifacts/stableFileHashProbe.config.ts --reporter=dot ``` Result: 1 file passed, 1 test passed. Probe source/config are disposable under `.cache/codebase-audit/artifacts/`. ### Impact and ownership A close failure can turn a successful hash or an earlier read/cancellation failure into a bare close error while leaving the descriptor live. Callers that clean a surrounding temporary root cannot retry closing the lost descriptor. The concrete upstream ADB snapshot path (`copyFile` followed by `hashStableFile` in `src/adb/AdbProvider.ts`) shares this boundary; the local in-progress ADB implementation uses explicitly owned source/destination handles for its snapshot path, but other ADB hashes and the other 13 call sites still use `hashStableFile`. The root fix belongs in `StableFileHash` and its owner contract: preserve the primary failure and expose a retryable owned handle/cleanup observation when close fails, then let each caller lifecycle adopt it where applicable. Avoid an unowned global retry registry. Add fault tests for close failure after a successful read and after a primary read/cancellation failure, plus a successful-close control. ### Dedupe and limits Read-only GitHub issue search for `StableFileHash close owned handle descriptor cleanup` returned no focused issue. This is a static helper-level, injected-close-failure proof; ordinary filesystem behavior and native close-failure frequency were not measured, and no ADB device or external tool was used.

bsakaguchi

2026-10-16 21:09:05 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-16 21:09:06 +00:00

Author

Owner

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

Implemented in draft #4274, commit 5c6005b88.

The regular-file boundary now has one cleanup owner shared by stable hashing and regular reads. A close failure carries the exact descriptor and a typed completed-value or failed-cause outcome. Primary, cleanup, and retention failures survive together, including a thrown undefined value. Callers retain failed owners under their actual session, inspection, cache, or staging lifetime.

The change also covers pre-session Kuna inspection and Doctor, the Kuna cache/command lifetimes, JADX, all ADB hash consumers, Linux publisher staging, and JavaScript verifier callers. A completed ADB hash preserves known bytes/digest through cleanup failure; an incomplete hash remains unknown. Late profile work remains owned after a caller stops waiting. Parent staging stays until descriptor cleanup succeeds.

Executed evidence includes real-descriptor failure/retry controls, delayed profile cancellation and close, warm-cache retry, ADB completed/incomplete hash observations, a portable signed-archive staging dependency test, and compiled verifier hash controls. Source typecheck, repository checks, the full source suite (8,420 passed), and the MCP boundary suite (799 passed) passed. The final combined ownership selection passed: 29 files, 333 tests passed and 21 native-Linux cases skipped.

Limits: faults are controlled, not a measurement of native close-failure frequency. Real Kuna/JADX/device workflows were not exercised; native Linux execution was unavailable locally; the portable archive test ran on macOS.

@morluto commented on GitHub (Oct 15, 2026): Implemented in draft #4274, commit 5c6005b88. The regular-file boundary now has one cleanup owner shared by stable hashing and regular reads. A close failure carries the exact descriptor and a typed completed-value or failed-cause outcome. Primary, cleanup, and retention failures survive together, including a thrown undefined value. Callers retain failed owners under their actual session, inspection, cache, or staging lifetime. The change also covers pre-session Kuna inspection and Doctor, the Kuna cache/command lifetimes, JADX, all ADB hash consumers, Linux publisher staging, and JavaScript verifier callers. A completed ADB hash preserves known bytes/digest through cleanup failure; an incomplete hash remains unknown. Late profile work remains owned after a caller stops waiting. Parent staging stays until descriptor cleanup succeeds. Executed evidence includes real-descriptor failure/retry controls, delayed profile cancellation and close, warm-cache retry, ADB completed/incomplete hash observations, a portable signed-archive staging dependency test, and compiled verifier hash controls. Source typecheck, repository checks, the full source suite (8,420 passed), and the MCP boundary suite (799 passed) passed. The final combined ownership selection passed: 29 files, 333 tests passed and 21 native-Linux cases skipped. Limits: faults are controlled, not a measurement of native close-failure frequency. Real Kuna/JADX/device workflows were not exercised; native Linux execution was unavailable locally; the portable archive test ran on macOS.

bsakaguchi commented

2026-10-16 21:09:07 +00:00

Author

Owner

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

Fixed on main in 131fd5ddc. Stable hashing preserves the exact descriptor owner, original failure and any completed digest after close rejection; callers retain and retry the owner under their actual lifecycle. Follow-up owner-linked recovery fixes are also on main in 6e74704f9 and 1bd5c5c87.

Controlled real-file close-failure/retry tests passed, including ADB consumers and JADX recovery. Native close-failure frequency and real Kuna/JADX/device workflows were not measured.

Hosted main CI is still pending; this closure is based on the implemented fix and the scoped regression evidence.

@morluto commented on GitHub (Oct 16, 2026): Fixed on `main` in [131fd5ddc](https://github.com/morluto/rea/commit/131fd5ddcd3dc7124be118c7a3e56b8e8df38baf). Stable hashing preserves the exact descriptor owner, original failure and any completed digest after close rejection; callers retain and retry the owner under their actual lifecycle. Follow-up owner-linked recovery fixes are also on main in 6e74704f9 and 1bd5c5c87. Controlled real-file close-failure/retry tests passed, including ADB consumers and JADX recovery. Native close-failure frequency and real Kuna/JADX/device workflows were not measured. Hosted main CI is still pending; this closure is based on the implemented fix and the scoped regression evidence.

bsakaguchi referenced this issue

2026-10-31 00:54:42 +00:00

[PR #1688] [MERGED] docs(website): list the setup guide first on the Guides page #4755

bsakaguchi referenced this issue

2026-10-31 00:54:48 +00:00

[PR #1699] [MERGED] style(website): kbd, install-node width and nested collapsible indentation #4761