morluto/rea

Fork 0

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

[GH-ISSUE #4735] [Bug] Signature inspection returns success after cancellation during final cleanup #2468

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/4735

Before submitting

Searched signature/cancellation issues and open signature PRs. #4734 covers stale cleanup ownership and was fixed separately; #3927 and #3923 concern Android and firmware workflows.
Evidence uses generated inert bytes and a controlled root owner; no native target execution.

Area

Evidence, artifacts, or reconstruction

Observed behavior

withSignatureTarget returns success even though the caller signal aborts during awaited final snapshot-root cleanup. The controlled close removes the directory successfully, then aborts before returning. The helper still returns the completed success value with no cancellation error.

Reproduction

At source 4f3b573b0 (same helper completion path as e09a503f4), use a tiny authored file with its correct registered digest and a generated snapshot directory. Call production withSignatureTarget with an inspection callback returning {completed: 'signature facts'}. Have the root owner's close await directory removal, then controller.abort({caller:'during-final-root-close'}). The completed local module probe reported:

{"result":{"ok":true,"value":{"completed":"signature facts"}},"inspected":true,"removed":true,"aborted":true,"reasonPreserved":false,"pending":false}

Executed Node24.18.0 on macOS arm64; real tiny file admission/copy/hash/seal/descriptor close and directory removal, controlled inspection callback and root-close timing. No codesign or public CLI in this probe. Probe source and raw output retained locally under .cache/codebase-audit/signature-cancellation-*.

Expected behavior

After the last awaited finalization, a completed operation should return typed cancellation with the exact caller reason and retain the completed signature facts. An already failed physical operation must remain primary; cleanup failure must retain its own owner and classification. Apply the same completion rule across descriptor and root finalization without deriving authority from abort-reason shape.

Impact and scope

Reproduced production-helper completion defect. The native provider calls this helper around signature inspection, so the same root-close window is source-derived applicable there. NOT_ENOUGH_INFO: public CLI/MCP signal-timing reproduction, real OS cleanup failure, Windows and observed external-user frequency.

REA version / target

Source 4f3b573b0; authored inert bytes with synthetic Mach-O metadata. Target is not launched.

Interface / earliest failure stage

Other: production signature helper / cleanup, cancellation or lifecycle.

Relevant output or evidence

src/native/SignatureTargetBinding.ts, withSignatureTarget: await resources.release(owner), then return result after successful cleanup, with no completion signal check. bindSignatureTarget similarly awaits descriptor finalization after its final signal check; that second window is source-derived and not exercised by this probe. Existing cancellation controls target acquisition/captures, not final root cleanup.

Proposed checks

Cover successful/failed root cleanup with late cancellation, descriptor finalization, preservation of completed signature facts, exact null/primitive/typed-error caller reasons, mixed primary failure and cleanup retry. Preserve exact lease ownership from #4734. The generic helper currently has no completed native signature observation contract; resolve that producing boundary rather than patching only a CLI adapter.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4735 ### Before submitting - [x] Searched signature/cancellation issues and open signature PRs. #4734 covers stale cleanup ownership and was fixed separately; #3927 and #3923 concern Android and firmware workflows. - [x] Evidence uses generated inert bytes and a controlled root owner; no native target execution. ### Area Evidence, artifacts, or reconstruction ### Observed behavior withSignatureTarget returns success even though the caller signal aborts during awaited final snapshot-root cleanup. The controlled close removes the directory successfully, then aborts before returning. The helper still returns the completed success value with no cancellation error. ### Reproduction At source 4f3b573b0 (same helper completion path as e09a503f4), use a tiny authored file with its correct registered digest and a generated snapshot directory. Call production withSignatureTarget with an inspection callback returning {completed: 'signature facts'}. Have the root owner's close await directory removal, then controller.abort({caller:'during-final-root-close'}). The completed local module probe reported: ```json {"result":{"ok":true,"value":{"completed":"signature facts"}},"inspected":true,"removed":true,"aborted":true,"reasonPreserved":false,"pending":false} ``` Executed Node24.18.0 on macOS arm64; real tiny file admission/copy/hash/seal/descriptor close and directory removal, controlled inspection callback and root-close timing. No codesign or public CLI in this probe. Probe source and raw output retained locally under .cache/codebase-audit/signature-cancellation-*. ### Expected behavior After the last awaited finalization, a completed operation should return typed cancellation with the exact caller reason and retain the completed signature facts. An already failed physical operation must remain primary; cleanup failure must retain its own owner and classification. Apply the same completion rule across descriptor and root finalization without deriving authority from abort-reason shape. ### Impact and scope Reproduced production-helper completion defect. The native provider calls this helper around signature inspection, so the same root-close window is source-derived applicable there. NOT_ENOUGH_INFO: public CLI/MCP signal-timing reproduction, real OS cleanup failure, Windows and observed external-user frequency. ### REA version / target Source 4f3b573b0; authored inert bytes with synthetic Mach-O metadata. Target is not launched. ### Interface / earliest failure stage Other: production signature helper / cleanup, cancellation or lifecycle. ### Relevant output or evidence src/native/SignatureTargetBinding.ts, withSignatureTarget: await resources.release(owner), then return result after successful cleanup, with no completion signal check. bindSignatureTarget similarly awaits descriptor finalization after its final signal check; that second window is source-derived and not exercised by this probe. Existing cancellation controls target acquisition/captures, not final root cleanup. ### Proposed checks Cover successful/failed root cleanup with late cancellation, descriptor finalization, preservation of completed signature facts, exact null/primitive/typed-error caller reasons, mixed primary failure and cleanup retry. Preserve exact lease ownership from #4734. The generic helper currently has no completed native signature observation contract; resolve that producing boundary rather than patching only a CLI adapter.

bsakaguchi added the

bug

label

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

bsakaguchi commented

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

Author

Owner

@GoodSamaritan-bot-by-EG commented on GitHub (Oct 17, 2026):

Thanks for the thorough investigation. A focused change addressing the signature inspection cancellation handling is ready for maintainer review. The Good Samaritan contribution refines the completion logic to ensure proper cancellation signals are propagated during final cleanup. Please review the updated implementation and let us know if any adjustments are needed.

@GoodSamaritan-bot-by-EG commented on GitHub (Oct 17, 2026): Thanks for the thorough investigation. A focused change addressing the signature inspection cancellation handling is ready for maintainer review. The Good Samaritan contribution refines the completion logic to ensure proper cancellation signals are propagated during final cleanup. Please review the updated implementation and let us know if any adjustments are needed.

bsakaguchi commented

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

Author

Owner

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

Published in 715c9c8a8 on main.

The snapshot/inspection owner now accepts the existing named InspectSignature contract, retains completed signature facts in typed cancellation, and checks cancellation after both descriptor and root finalization. InspectSignature is an admitted partial observation; the provider consumes that parsed result without an intermediate arbitrary wrapper/reparse. Explicit shared cancellation checkpoints retain the caller's exact reason. Catch-time signal-state guesses were removed, so concurrent physical read or root-creation failure stays primary. An arbitrary thrown inspection error does not confer cleanup authority. Exact leases and mixed-owner retry semantics from #4734 remain in place.

Baseline: 9 new regression controls failed / 6 controls passed. Final: 156 checks passed across eight files, including 24 signature lifecycle controls, 29 provider target-binding controls (two new completed-provider finalization controls), shared cleanup/error projections, plist overlap and two public CLI checks covering eight native signature invocations. Existing real codesign ancestor-swap and CLI controls used signed copies of /usr/bin/true without executing them. Late cancellation/fault cases use controlled root/descriptor outcomes and command captures; no public CLI/MCP signal-timing, real OS close-failure, LLDB or Windows qualification. Static checks and commit/push hooks passed.

Research: Context7 Node24 API confirms throwIfAborted throws signal.reason, whose type is any. DeepWiki was cross-checked against that API and the repository's explicit RegularFileOperationCancelled owner; an aborted signal in a catch is not cancellation provenance.

Small completed provider workflow comparison: six byte-identical complete AnalysisExecution replies, 425 raw cl100k_base tokens. First process baseline/candidate: elapsed 0.314/0.310 s, CPU 0.476/0.462 s, sampled process-family RSS 305,381,376/257,277,952 bytes. Two warm-repeat medians: elapsed 0.314/0.296 s, CPU 0.495/0.452 s, RSS 307,527,680/255,197,184 bytes. Real tiny snapshot copy/hash/identity/seal/descriptor cleanup with controlled command captures and directory owner. These samples establish no performance improvement; whole-agent, public CLI/MCP, real native helper/codesign, large-target and injected-failure costs unmeasured. Owned comparison worktree verified and removed normally. Heavy profiling/stress stayed stopped.

@morluto commented on GitHub (Oct 17, 2026): Published in 715c9c8a8 on main. The snapshot/inspection owner now accepts the existing named InspectSignature contract, retains completed signature facts in typed cancellation, and checks cancellation after both descriptor and root finalization. InspectSignature is an admitted partial observation; the provider consumes that parsed result without an intermediate arbitrary wrapper/reparse. Explicit shared cancellation checkpoints retain the caller's exact reason. Catch-time signal-state guesses were removed, so concurrent physical read or root-creation failure stays primary. An arbitrary thrown inspection error does not confer cleanup authority. Exact leases and mixed-owner retry semantics from #4734 remain in place. Baseline: 9 new regression controls failed / 6 controls passed. Final: 156 checks passed across eight files, including 24 signature lifecycle controls, 29 provider target-binding controls (two new completed-provider finalization controls), shared cleanup/error projections, plist overlap and two public CLI checks covering eight native signature invocations. Existing real codesign ancestor-swap and CLI controls used signed copies of /usr/bin/true without executing them. Late cancellation/fault cases use controlled root/descriptor outcomes and command captures; no public CLI/MCP signal-timing, real OS close-failure, LLDB or Windows qualification. Static checks and commit/push hooks passed. Research: Context7 Node24 API confirms throwIfAborted throws signal.reason, whose type is any. DeepWiki was cross-checked against that API and the repository's explicit RegularFileOperationCancelled owner; an aborted signal in a catch is not cancellation provenance. Small completed provider workflow comparison: six byte-identical complete AnalysisExecution replies, 425 raw cl100k_base tokens. First process baseline/candidate: elapsed 0.314/0.310 s, CPU 0.476/0.462 s, sampled process-family RSS 305,381,376/257,277,952 bytes. Two warm-repeat medians: elapsed 0.314/0.296 s, CPU 0.495/0.452 s, RSS 307,527,680/255,197,184 bytes. Real tiny snapshot copy/hash/identity/seal/descriptor cleanup with controlled command captures and directory owner. These samples establish no performance improvement; whole-agent, public CLI/MCP, real native helper/codesign, large-target and injected-failure costs unmeasured. Owned comparison worktree verified and removed normally. Heavy profiling/stress stayed stopped.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

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

This is a real correctness gap: reporting ok:true after the caller already aborted during final root cleanup makes the client believe the inspection fully completed, which is worse than a plain cancellation error because downstream logic can act on stale 'success' facts. The fix direction in 715c9c8 — checking the completion signal after the last awaited finalization and returning typed cancellation while retaining the completed facts — looks right, and keeping an already-failed physical operation primary avoids masking real cleanup errors. Worth adding a regression test that aborts specifically in the root-close window (like the probe does) so this exact interleaving stays covered; the descriptor path is handled via #4734 but the root-close window is a separate branch that could silently regress.

@XinwangTao commented on GitHub (Oct 18, 2026): This is a real correctness gap: reporting ok:true after the caller already aborted during final root cleanup makes the client believe the inspection fully completed, which is worse than a plain cancellation error because downstream logic can act on stale 'success' facts. The fix direction in 715c9c8 — checking the completion signal after the last awaited finalization and returning typed cancellation while retaining the completed facts — looks right, and keeping an already-failed physical operation primary avoids masking real cleanup errors. Worth adding a regression test that aborts specifically in the root-close window (like the probe does) so this exact interleaving stays covered; the descriptor path is handled via #4734 but the root-close window is a separate branch that could silently regress.

bsakaguchi referenced this issue

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

[PR #2468] [MERGED] fix(process): preserve nested path identities during normalization #5252

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

No description provided.

Rows
Columns