morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[GH-ISSUE #5144] [Bug] Process capture errors retain cleanup_incomplete after confirmed owner retry #3163

New issue

Closed

opened 2026-10-31 00:37:23 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-31 00:37:23 +00:00

Owner

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

Before submitting

Searched open/closed capture cleanup retry/stale and ProcessCaptureResourceScope reports. Related #4467/#4869/#4734 fixed the same status/ownership distinction in other owners; #5140 is a separate unretained snapshot-handle issue.
No credentials or private target evidence included.

Area

Process capture or runtime observation.

Observed behavior

A compiled production-boundary probe retained a real temporary directory after controlled removal denial. ProcessCaptureResourceScope.retryCleanup() then successfully removed that exact directory, and scope.close() succeeded. The originating ProcessCaptureError.cleanupIncomplete remained true and projectAnalysisError() still returned cleanup_incomplete.

{"rootAbsent":true,"removalAttempts":2,"before":{"cleanupIncomplete":true,"projection":"cleanup_incomplete"},"after":{"cleanupIncomplete":true,"projection":"cleanup_incomplete"}}

Reproduction

At d6ca7b3f20, imported compiled ProcessCaptureResourceScope, resolveProcessResult and error projection. Created a real temporary directory. Called scope.release() with no process/renderer and a removal host that initially rejects, then produced the error via resolveProcessResult(undefined, typedPrimaryFailure, report). Restored removal to actual fs.rm, awaited scope.retryCleanup() and scope.close(), verified stat() reports ENOENT, then projected the same error again. Reproduced once under that controlled schedule; all owned resources were released. This is not a full CLI/MCP or natural-denial reproduction.

Expected behavior

After the exact owner confirms release, clear the current cleanup-incomplete marker on originating and composed errors while preserving original failure and historical cleanup report. An unrelated or still-pending owner must remain incomplete. docs/tool-design.md#own-the-lifecycle requires owner-confirmed recovery and explicit cleanup capabilities rather than reparsing diagnostics.

Root cause / proposed correction

ProcessCaptureResourceScope retains private owners in a Set and deletes them after successful retry, but its reports and resolveProcessResult errors are not bound to those owners with cleanup leases/dependencies. The scope knows recovery happened; the published errors do not. Reuse exact owner leases and the existing AnalysisError cleanup dependency model, including original and retry aggregates. Do not rewrite the historical report or treat an empty unrelated scope as recovery proof.

Impact and scope

Confirmed stale status after a real root removal. CLI capture closes its scope before projecting the capture error, so this composition can affect output (source-derived; public CLI not run for this probe). Frequency under natural filesystem denial is NOT_ENOUGH_INFO. Mixed-child and repeated-failure behavior still require regression coverage.

Version / runtime / target / interface

REA source d6ca7b3f20; Node24.18.0, macOS arm64. Synthetic owned temporary directory; no native provider or target process. Other/compiled process cleanup boundary. Earliest stage cleanup retry/error projection.

Originally created by @morluto on GitHub (Oct 19, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5144 ## Before submitting - [x] Searched open/closed capture cleanup retry/stale and ProcessCaptureResourceScope reports. Related #4467/#4869/#4734 fixed the same status/ownership distinction in other owners; #5140 is a separate unretained snapshot-handle issue. - [x] No credentials or private target evidence included. ## Area Process capture or runtime observation. ## Observed behavior A compiled production-boundary probe retained a real temporary directory after controlled removal denial. `ProcessCaptureResourceScope.retryCleanup()` then successfully removed that exact directory, and `scope.close()` succeeded. The originating `ProcessCaptureError.cleanupIncomplete` remained true and `projectAnalysisError()` still returned `cleanup_incomplete`. ```json {"rootAbsent":true,"removalAttempts":2,"before":{"cleanupIncomplete":true,"projection":"cleanup_incomplete"},"after":{"cleanupIncomplete":true,"projection":"cleanup_incomplete"}} ``` ## Reproduction At d6ca7b3f20a273823c50d6253f0b6f34f3876f53, imported compiled `ProcessCaptureResourceScope`, `resolveProcessResult` and error projection. Created a real temporary directory. Called `scope.release()` with no process/renderer and a removal host that initially rejects, then produced the error via `resolveProcessResult(undefined, typedPrimaryFailure, report)`. Restored removal to actual `fs.rm`, awaited `scope.retryCleanup()` and `scope.close()`, verified `stat()` reports ENOENT, then projected the same error again. Reproduced once under that controlled schedule; all owned resources were released. This is not a full CLI/MCP or natural-denial reproduction. ## Expected behavior After the exact owner confirms release, clear the current cleanup-incomplete marker on originating and composed errors while preserving original failure and historical cleanup report. An unrelated or still-pending owner must remain incomplete. `docs/tool-design.md#own-the-lifecycle` requires owner-confirmed recovery and explicit cleanup capabilities rather than reparsing diagnostics. ## Root cause / proposed correction `ProcessCaptureResourceScope` retains private owners in a Set and deletes them after successful retry, but its reports and `resolveProcessResult` errors are not bound to those owners with cleanup leases/dependencies. The scope knows recovery happened; the published errors do not. Reuse exact owner leases and the existing AnalysisError cleanup dependency model, including original and retry aggregates. Do not rewrite the historical report or treat an empty unrelated scope as recovery proof. ## Impact and scope Confirmed stale status after a real root removal. CLI capture closes its scope before projecting the capture error, so this composition can affect output (source-derived; public CLI not run for this probe). Frequency under natural filesystem denial is NOT_ENOUGH_INFO. Mixed-child and repeated-failure behavior still require regression coverage. ## Version / runtime / target / interface REA source d6ca7b3f20a273823c50d6253f0b6f34f3876f53; Node24.18.0, macOS arm64. Synthetic owned temporary directory; no native provider or target process. Other/compiled process cleanup boundary. Earliest stage cleanup retry/error projection.

bsakaguchi

2026-10-31 00:37:23 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-31 00:37:24 +00:00

Author

Owner

@Varshith-Kali commented on GitHub (Oct 19, 2026):

Hi, I'd like to take this on.

I read the reproduction and the proposed correction, and the mechanism matches the code: ProcessCaptureResourceOwner instances are retained in the scope's private set and deleted after a successful retryCleanup, but the ProcessCaptureError produced by resolveProcessResult is never bound to those owners with a cleanup lease/dependency. So cleanupIncomplete stays true and projectAnalysisError keeps returning cleanup_incomplete even after the exact owner confirms release.

My plan: bind the produced error to the scope's owner via the existing AnalysisError cleanup dependency model (reuse bindCleanupDependencies with the owner leases, following the #4467/#4869/#4734 pattern in the sibling owners), so owner-confirmed recovery clears the current marker while an unrelated or still-pending owner keeps it incomplete. Original failure facts and the historical cleanup report stay intact, and no empty unrelated scope counts as recovery proof. I'll add regression tests covering the retry-confirmed path and the unrelated-owner-stays-incomplete path.

@Varshith-Kali commented on GitHub (Oct 19, 2026): Hi, I'd like to take this on. I read the reproduction and the proposed correction, and the mechanism matches the code: ProcessCaptureResourceOwner instances are retained in the scope's private set and deleted after a successful retryCleanup, but the ProcessCaptureError produced by resolveProcessResult is never bound to those owners with a cleanup lease/dependency. So cleanupIncomplete stays true and projectAnalysisError keeps returning cleanup_incomplete even after the exact owner confirms release. My plan: bind the produced error to the scope's owner via the existing AnalysisError cleanup dependency model (reuse bindCleanupDependencies with the owner leases, following the #4467/#4869/#4734 pattern in the sibling owners), so owner-confirmed recovery clears the current marker while an unrelated or still-pending owner keeps it incomplete. Original failure facts and the historical cleanup report stay intact, and no empty unrelated scope counts as recovery proof. I'll add regression tests covering the retry-confirmed path and the unrelated-owner-stays-incomplete path.

bsakaguchi commented

2026-10-31 00:37:24 +00:00

Author

Owner

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

Fixed on main in 974487f00.

Cleanup now returns the historical report together with the exact lease that can confirm release. Initial capture errors, finalization copies and every typed retry aggregate bind to those owners. Successful retry clears current cleanup uncertainty while preserving the original failure, partial observations and cleanup report. Failed peers remain incomplete, and admission stays blocked until their owners confirm release.

The nearby consumer audit also fixed #5163: MCP close preserves typed cleanup errors and composes simultaneous session/process failures, including returned, rejected and synchronous typed failures. #5166 separates report snapshots from private owner state, so editing a report cannot authorize skipping an actual cleanup attempt.

Validation: clean dfe8378 baseline owner tests failed 5/5, the public compiled CLI reproduced stale status after two actual removal attempts and real root release, and SDK close tests reproduced hidden cleanup facts. The broader lifecycle run passed 156 tests/16 files. After final report hardening, 72 affected tests/7 files passed, including a new six-case real-root owner suite, actual compiled CLI recovery, and public SDK mixed-failure controls. Full static/type/lint/architecture, documentation checks and test discovery passed.

Twelve final measured workflows (six official-SDK successful captures and six compiled CLI cleanup-fault captures) retain completed-workload and filesystem/cleanup oracles. Corrected CLI live error fields intentionally differ; clock fields are excluded from semantic equality, with full originals retained. These small measurements do not establish a speedup, actual model-context savings, true cold-cache behavior or other-host performance.

Scope: macOS arm64, pinned Node24.18.0, controlled real temporary resources and actual Node captures. MCP session failures/removal denials are controlled production seams; no analysis-engine teardown or natural-failure frequency claim. Snapshot-handle ownership #5140 remains a separate pending issue.

@morluto commented on GitHub (Oct 19, 2026): Fixed on main in [`974487f00`](https://github.com/morluto/rea/commit/974487f00f2d68912c7c2c9e033a2760a343a82e). Cleanup now returns the historical report together with the exact lease that can confirm release. Initial capture errors, finalization copies and every typed retry aggregate bind to those owners. Successful retry clears current cleanup uncertainty while preserving the original failure, partial observations and cleanup report. Failed peers remain incomplete, and admission stays blocked until their owners confirm release. The nearby consumer audit also fixed #5163: MCP close preserves typed cleanup errors and composes simultaneous session/process failures, including returned, rejected and synchronous typed failures. #5166 separates report snapshots from private owner state, so editing a report cannot authorize skipping an actual cleanup attempt. Validation: clean dfe8378 baseline owner tests failed 5/5, the public compiled CLI reproduced stale status after two actual removal attempts and real root release, and SDK close tests reproduced hidden cleanup facts. The broader lifecycle run passed 156 tests/16 files. After final report hardening, 72 affected tests/7 files passed, including a new six-case real-root owner suite, actual compiled CLI recovery, and public SDK mixed-failure controls. Full static/type/lint/architecture, documentation checks and test discovery passed. Twelve final measured workflows (six official-SDK successful captures and six compiled CLI cleanup-fault captures) retain completed-workload and filesystem/cleanup oracles. Corrected CLI live error fields intentionally differ; clock fields are excluded from semantic equality, with full originals retained. These small measurements do not establish a speedup, actual model-context savings, true cold-cache behavior or other-host performance. Scope: macOS arm64, pinned Node24.18.0, controlled real temporary resources and actual Node captures. MCP session failures/removal denials are controlled production seams; no analysis-engine teardown or natural-failure frequency claim. Snapshot-handle ownership #5140 remains a separate pending issue.

bsakaguchi referenced this issue

2026-10-31 01:08:17 +00:00

[PR #3163] [MERGED] fix(native): derive Mach-O word size and byte order from the selected slice #5652

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#3163

No description provided.

Rows
Columns