morluto/rea

Fork 0

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

[GH-ISSUE #4734] [Bug] Signature snapshot cleanup errors retain released resource ownership #2467

New issue

Closed

opened 2026-10-17 21:05:12 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

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

Owner

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

Before submitting

Searched signature-titled issues and open signature PRs. #4293 concerns universal-slice coverage; #4723 concerns plist descriptor admission. #4578 covers the shared dependency-set owner, not these unbound signature owners.
Evidence uses authored inert files and controlled cleanup failures.

Area

Evidence, artifacts, or reconstruction

Observed behavior

Signature snapshot cleanup retries can succeed and delete the exact owned root while the originating analysis error still reports cleanupIncomplete true and the removed root as a live resource. A subsequent client-close cleanup error is also unbound to the retained root owner.

Reproduction

At main e09a503f44, strengthen the existing SignatureTargetBinding tests to assert the original error after resources.close/client.close successfully removes the root. The two controls fail: expected false, received true. Command executed: npm run test:local -- src/native/SignatureTargetBinding.test.ts; 2 failed / 2 passed. These controls use real tiny generated files, controlled root-close rejection, and an intentionally mismatched digest so native commands never run.

Expected behavior

The exact root/file owners should update every bound error when their cleanup succeeds. Mixed cleanup must shrink the live resource set as individual owners release, preserving the original typed primary and historical cleanup facts.

Impact and scope

Reproduced on macOS arm64 / pinned Node24.18.0, production provider/helper boundary. Physical cleanup succeeds; live diagnostic ownership remains stale. No codesign, LLDB, Windows or real OS close-failure reproduction is claimed. NOT_ENOUGH_INFO: external provider/user incidence.

REA version / target

Source e09a503f44; authored inert target bytes with synthetic Mach-O metadata.

Interface / earliest failure stage

Other: provider/helper boundary / cleanup, cancellation or lifecycle.

Relevant output or evidence

src/native/SignatureTargetBinding.test.ts:95 and :160 fail cleanupIncomplete assertions after successful cleanup; roots are deleted by controlled owner close. Source-derived: SignatureTargetBinding retains private-runtime-root/file-handle owners directly in ArtifactResourceScope, but combineCleanupFailure binds neither the error nor its composition to these capabilities. NativeMacOSProvider close can bind only known exact leases; the signature scope does not currently supply them.

Proposed checks

Transfer every signature root/descriptor through AnalysisCleanupLease and bind exact failed sets, including failed retries. Cover mixed root/descriptor outcomes, partial/final release, repeated retries, and preserved primary error. Keep target digest/identity checks and snapshot behavior unchanged. Separate cancellation attribution/finalization questions are not proved by these two controls.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4734 ### Before submitting - [x] Searched signature-titled issues and open signature PRs. #4293 concerns universal-slice coverage; #4723 concerns plist descriptor admission. #4578 covers the shared dependency-set owner, not these unbound signature owners. - [x] Evidence uses authored inert files and controlled cleanup failures. ### Area Evidence, artifacts, or reconstruction ### Observed behavior Signature snapshot cleanup retries can succeed and delete the exact owned root while the originating analysis error still reports cleanupIncomplete true and the removed root as a live resource. A subsequent client-close cleanup error is also unbound to the retained root owner. ### Reproduction At main e09a503f44fba1639b71907a018d71161360f5ae, strengthen the existing SignatureTargetBinding tests to assert the original error after resources.close/client.close successfully removes the root. The two controls fail: expected false, received true. Command executed: npm run test:local -- src/native/SignatureTargetBinding.test.ts; 2 failed / 2 passed. These controls use real tiny generated files, controlled root-close rejection, and an intentionally mismatched digest so native commands never run. ### Expected behavior The exact root/file owners should update every bound error when their cleanup succeeds. Mixed cleanup must shrink the live resource set as individual owners release, preserving the original typed primary and historical cleanup facts. ### Impact and scope Reproduced on macOS arm64 / pinned Node24.18.0, production provider/helper boundary. Physical cleanup succeeds; live diagnostic ownership remains stale. No codesign, LLDB, Windows or real OS close-failure reproduction is claimed. NOT_ENOUGH_INFO: external provider/user incidence. ### REA version / target Source e09a503f44fba1639b71907a018d71161360f5ae; authored inert target bytes with synthetic Mach-O metadata. ### Interface / earliest failure stage Other: provider/helper boundary / cleanup, cancellation or lifecycle. ### Relevant output or evidence src/native/SignatureTargetBinding.test.ts:95 and :160 fail cleanupIncomplete assertions after successful cleanup; roots are deleted by controlled owner close. Source-derived: SignatureTargetBinding retains private-runtime-root/file-handle owners directly in ArtifactResourceScope, but combineCleanupFailure binds neither the error nor its composition to these capabilities. NativeMacOSProvider close can bind only known exact leases; the signature scope does not currently supply them. ### Proposed checks Transfer every signature root/descriptor through AnalysisCleanupLease and bind exact failed sets, including failed retries. Cover mixed root/descriptor outcomes, partial/final release, repeated retries, and preserved primary error. Keep target digest/identity checks and snapshot behavior unchanged. Separate cancellation attribution/finalization questions are not proved by these two controls.

bsakaguchi added the

bug

label

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

bsakaguchi commented

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

Author

Owner

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

Published in 4f3b573b0 on main. Every signature snapshot root/source/snapshot descriptor now transfers through an exact AnalysisCleanupLease; failed sets and composed errors bind to the owning leases/dependencies. This removes the unbound root/file-handle representation. Retry errors from the signature scope now carry the exact leases consumed by client close. Live resources shrink on partial release and clear after final confirmation; original typed digest mismatch and historical cleanup facts survive.

Baseline: two strengthened existing controls failed (stale cleanupIncomplete after confirmed root removal). Final: 74 focused checks across five files, including mixed root/descriptor failure and retry, prior target-identity/mutation controls, plist overlap and core cleanup; two public CLI checks exercised eight native signature CLI invocations. Existing real codesign control against ad-hoc-signed copies of /usr/bin/true passed; those copies were not executed. Cleanup faults are controlled, not ordinary OS close failures. Static checks, commit/push hooks passed. No LLDB or Windows qualification.

Small completed production snapshot-helper comparison: six byte-identical complete replies, 51 raw cl100k_base tokens. Two warm-repeat medians baseline/candidate: elapsed 0.143/0.168 s, CPU 0.250/0.254 s, sampled process-family RSS 186,146,816/180,576,256 bytes. First-process measurements and all raw records are retained locally. Real tiny file copy/digest/identity/descriptor cleanup and controlled directory-owner removal; native root helper, codesign, public CLI/MCP, whole-agent and injected-failure costs unmeasured. No speed or memory improvement claim. Owned baseline worktree removed normally after identity/cleanliness/process checks; heavy profiling stayed stopped.

Separate remaining defect found during audit: cancellation arriving during successful final root close can still return success. This ownership change does not alter cancellation classification or introduce a completed-signature observation contract; that boundary needs its own regression and fix. It is not evidence that released ownership remains unresolved.

@morluto commented on GitHub (Oct 17, 2026): Published in 4f3b573b0 on main. Every signature snapshot root/source/snapshot descriptor now transfers through an exact AnalysisCleanupLease; failed sets and composed errors bind to the owning leases/dependencies. This removes the unbound root/file-handle representation. Retry errors from the signature scope now carry the exact leases consumed by client close. Live resources shrink on partial release and clear after final confirmation; original typed digest mismatch and historical cleanup facts survive. Baseline: two strengthened existing controls failed (stale cleanupIncomplete after confirmed root removal). Final: 74 focused checks across five files, including mixed root/descriptor failure and retry, prior target-identity/mutation controls, plist overlap and core cleanup; two public CLI checks exercised eight native signature CLI invocations. Existing real codesign control against ad-hoc-signed copies of /usr/bin/true passed; those copies were not executed. Cleanup faults are controlled, not ordinary OS close failures. Static checks, commit/push hooks passed. No LLDB or Windows qualification. Small completed production snapshot-helper comparison: six byte-identical complete replies, 51 raw cl100k_base tokens. Two warm-repeat medians baseline/candidate: elapsed 0.143/0.168 s, CPU 0.250/0.254 s, sampled process-family RSS 186,146,816/180,576,256 bytes. First-process measurements and all raw records are retained locally. Real tiny file copy/digest/identity/descriptor cleanup and controlled directory-owner removal; native root helper, codesign, public CLI/MCP, whole-agent and injected-failure costs unmeasured. No speed or memory improvement claim. Owned baseline worktree removed normally after identity/cleanliness/process checks; heavy profiling stayed stopped. Separate remaining defect found during audit: cancellation arriving during successful final root close can still return success. This ownership change does not alter cancellation classification or introduce a completed-signature observation contract; that boundary needs its own regression and fix. It is not evidence that released ownership remains unresolved.

bsakaguchi commented

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

Author

Owner

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

Reproduced on fresh main e09a503f44fba1639b71907a018d71161360f5ae (macOS arm64, Node 24.18.0/npm 11.16.0). I added one post-retry assertion to the existing SignatureTargetBinding.test.ts helper control: after resources.close() removes the exact snapshot root (stat returns ENOENT), the original analysis error still has cleanupIncomplete === true. The complete file finished 1 failed / 3 passed; the failure is precisely the stale cleanup flag.

This uses real authored target files and real directory removal with a controlled first root-close rejection. The intentional digest mismatch prevents native commands from running. It does not demonstrate a natural OS close failure or a descriptor leak. I am checking the exact root/descriptor cleanup capabilities and retry-error composition; native utility and public adapter verification are still pending.

Update: upstream independently published 4f3b573b0922bfe04fb8246f52449629ea42c2ce for this same ownership defect. I read its actual root/source/snapshot lease and dependency changes, then ran the upstream tests on that exact commit: 23 passed across SignatureTargetBinding, AnalysisCleanupLease and analysisErrorCleanup. The same stale-root assertion is now covered; partial root/descriptor release also has an upstream regression. My separate local experiment reached 5 passing checks but was not committed, pushed or submitted and is now retained read-only. This is an independent upstream fix, with no claim of my PR merging or author credit. The separately reported late-cancellation classification issue is outside this ownership report.

@xu91102 commented on GitHub (Oct 17, 2026): Reproduced on fresh main `e09a503f44fba1639b71907a018d71161360f5ae` (macOS arm64, Node 24.18.0/npm 11.16.0). I added one post-retry assertion to the existing `SignatureTargetBinding.test.ts` helper control: after `resources.close()` removes the exact snapshot root (`stat` returns `ENOENT`), the original analysis error still has `cleanupIncomplete === true`. The complete file finished **1 failed / 3 passed**; the failure is precisely the stale cleanup flag. This uses real authored target files and real directory removal with a controlled first root-close rejection. The intentional digest mismatch prevents native commands from running. It does not demonstrate a natural OS close failure or a descriptor leak. I am checking the exact root/descriptor cleanup capabilities and retry-error composition; native utility and public adapter verification are still pending. Update: upstream independently published `4f3b573b0922bfe04fb8246f52449629ea42c2ce` for this same ownership defect. I read its actual root/source/snapshot lease and dependency changes, then ran the upstream tests on that exact commit: **23 passed across SignatureTargetBinding, AnalysisCleanupLease and analysisErrorCleanup**. The same stale-root assertion is now covered; partial root/descriptor release also has an upstream regression. My separate local experiment reached 5 passing checks but was **not committed, pushed or submitted** and is now retained read-only. This is an independent upstream fix, with no claim of my PR merging or author credit. The separately reported late-cancellation classification issue is outside this ownership report.

bsakaguchi closed this issue

2026-10-31 00:34:16 +00:00

bsakaguchi commented

2026-10-31 00:34:18 +00:00

Author

Owner

@XinwangTao commented on GitHub (Oct 18, 2026):

Confirmed and reproducible: the failing assertions (expected false, received true) show the stale owner outlives the successful cleanup, which is misleading for any caller that consults cleanupIncomplete/live diagnostics after recovery. The direction in the fix commits — binding cleanup to exact leases so a successful retry no longer reports the root as live — is right, and keeping the original analysis error primary while the physical cleanup succeeds matches how #4735's root-close window is being handled. One suggestion: since this class of bug keeps appearing around signature snapshot lifecycle (see also #4723, #4578), it would be worth adding a shared regression helper that simulates 'cleanup succeeds after N controlled failures' so all three cleanup paths exercise the same retry-error composition.

@XinwangTao commented on GitHub (Oct 18, 2026): Confirmed and reproducible: the failing assertions (expected false, received true) show the stale owner outlives the successful cleanup, which is misleading for any caller that consults cleanupIncomplete/live diagnostics after recovery. The direction in the fix commits — binding cleanup to exact leases so a successful retry no longer reports the root as live — is right, and keeping the original analysis error primary while the physical cleanup succeeds matches how #4735's root-close window is being handled. One suggestion: since this class of bug keeps appearing around signature snapshot lifecycle (see also #4723, #4578), it would be worth adding a shared regression helper that simulates 'cleanup succeeds after N controlled failures' so all three cleanup paths exercise the same retry-error composition.

bsakaguchi referenced this issue

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

[PR #2467] [MERGED] fix(windows): preserve transient runtime admission failures #5250

bsakaguchi referenced this issue

2026-10-31 01:07:03 +00:00

[PR #2911] [CLOSED] fix(sqlite): report a selected database held by another process as retryable #5489