[GH-ISSUE #4277] [Bug] Session cleanup status omits retained snapshot staging #1691

New issue

Closed

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

bsakaguchi commented

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

Owner

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

Observed behavior

binary_session.cleanup_pending remains false when snapshot staging cleanup is retained by the session without a provider client. Reproduced at audit head 5c6005b88a4d20dfd4042476f4e056344afe29bb on macOS, Node 24.18.0, using a real filesystem snapshot publication and controlled OwnedStagedOutput.close() rejection. No engine or GUI was involved.

Smallest reproduction

Construct the production binary session with a provider factory that refuses invocation. Write a valid snapshot through session.writeAnalysisSnapshot(snapshot, path, false) while staging close is denied. The write returns cleanupIncomplete: true with a publication receipt, and the published snapshot reads back correctly. session.status() reports open: false, cleanup_pending: false despite the retained owner. session.close() must retry that owner.

Executed: env PATH=/opt/homebrew/opt/node@24/bin:$PATH node_modules/.bin/vitest run tests/boundary/filesystem/analysisSnapshot.test.ts -t 'reports retained snapshot staging'. New regression failed specifically on expected cleanup_pending: true, actual false; prior assertions confirmed the retained cleanup and publication receipt. This is a source-boundary session/filesystem reproduction, not a live external-client or provider-engine claim.

Expected behavior and root cause

Status should derive cleanup pending from every resource scope owned by the binary session. Current status checks the retained provider client and the new profile-descriptor scope, but omits BinarySessionRecords.#writeResources. This is another case of active-target state hiding retained resource ownership (#4227); it is separate from a failed file close losing its owner (#4266).

Proposed fix and verification

Use one file-resource status getter combining snapshot-write and profile-file scopes. Keep open independent. Retain the existing publication/error semantics; assert status stays pending while close fails and clears only after successful retry. Add the regression to the existing filesystem snapshot suite and update the owning MCP contract documentation.

Scope and limits

Read/write only owned snapshot fixtures; no new target or process authority. No credentials or private samples. The test uses the real session/resource owner with a controlled cleanup fault; native failure frequency is unknown. Related implementation: #4274.

Originally created by @morluto on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4277 ### Observed behavior `binary_session.cleanup_pending` remains false when snapshot staging cleanup is retained by the session without a provider client. Reproduced at audit head `5c6005b88a4d20dfd4042476f4e056344afe29bb` on macOS, Node 24.18.0, using a real filesystem snapshot publication and controlled `OwnedStagedOutput.close()` rejection. No engine or GUI was involved. ### Smallest reproduction Construct the production binary session with a provider factory that refuses invocation. Write a valid snapshot through `session.writeAnalysisSnapshot(snapshot, path, false)` while staging close is denied. The write returns `cleanupIncomplete: true` with a publication receipt, and the published snapshot reads back correctly. `session.status()` reports `open: false, cleanup_pending: false` despite the retained owner. `session.close()` must retry that owner. Executed: `env PATH=/opt/homebrew/opt/node@24/bin:$PATH node_modules/.bin/vitest run tests/boundary/filesystem/analysisSnapshot.test.ts -t 'reports retained snapshot staging'`. New regression failed specifically on expected `cleanup_pending: true`, actual false; prior assertions confirmed the retained cleanup and publication receipt. This is a source-boundary session/filesystem reproduction, not a live external-client or provider-engine claim. ### Expected behavior and root cause Status should derive cleanup pending from every resource scope owned by the binary session. Current status checks the retained provider client and the new profile-descriptor scope, but omits `BinarySessionRecords.#writeResources`. This is another case of active-target state hiding retained resource ownership (#4227); it is separate from a failed file close losing its owner (#4266). ### Proposed fix and verification Use one file-resource status getter combining snapshot-write and profile-file scopes. Keep `open` independent. Retain the existing publication/error semantics; assert status stays pending while close fails and clears only after successful retry. Add the regression to the existing filesystem snapshot suite and update the owning MCP contract documentation. ### Scope and limits Read/write only owned snapshot fixtures; no new target or process authority. No credentials or private samples. The test uses the real session/resource owner with a controlled cleanup fault; native failure frequency is unknown. Related implementation: #4274.

bsakaguchi

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

closed this issue
added the
triage

bug
labels

bsakaguchi commented

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

Author

Owner

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

Fixed in 907c9d6d5, included in #4274. Session cleanup status now combines snapshot-write and profile-file resource scopes. It remains true with no active provider while staging cleanup fails, and clears only after successful retry.

The regression first failed at 5c6005b88 on cleanup_pending: false after asserting valid published snapshot content and an incomplete-cleanup receipt. After the fix, all 26 snapshot/session tests passed; 29 MCP lifecycle/session/identity tests and final-head static/build/docs checks passed on Node 24.18.0. The fresh stdio fixture exercise also passed. This is controlled filesystem/session coverage, not a claim about native failure frequency.

@morluto commented on GitHub (Oct 15, 2026): Fixed in `907c9d6d5`, included in #4274. Session cleanup status now combines snapshot-write and profile-file resource scopes. It remains true with no active provider while staging cleanup fails, and clears only after successful retry. The regression first failed at `5c6005b88` on `cleanup_pending: false` after asserting valid published snapshot content and an incomplete-cleanup receipt. After the fix, all 26 snapshot/session tests passed; 29 MCP lifecycle/session/identity tests and final-head static/build/docs checks passed on Node 24.18.0. The fresh stdio fixture exercise also passed. This is controlled filesystem/session coverage, not a claim about native failure frequency.

bsakaguchi commented

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

Author

Owner

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

Fixed on main in 839410b66. Session cleanup status now includes retained snapshot-write and provider-candidate resources even without an active client.

The real-filesystem snapshot regression covers retained staging, failed close and successful cleanup retry.

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 [839410b66](https://github.com/morluto/rea/commit/839410b663525b51c1aacf8a4027d7bb1f3da059). Session cleanup status now includes retained snapshot-write and provider-candidate resources even without an active client. The real-filesystem snapshot regression covers retained staging, failed close and successful cleanup retry. 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:44 +00:00

[PR #1691] [MERGED] perf(mcp): keep Playwright and type-only imports off MCP startup #4757

bsakaguchi referenced this issue

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

[PR #1694] [MERGED] perf(evidence): fuse streamed JSON token handling #4760