[GH-ISSUE #2753] [Bug] Failed open_binary cleanup clears all retained Evidence and Unknown history #992

New issue

Closed

opened 2026-10-15 18:30:10 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 18:30:10 +00:00

Owner

Originally created by @Adkid-Zephyr on GitHub (Oct 13, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2753

Observed behavior

open_binary clears the entire retained investigation when retiring the previous provider returns cleanup_incomplete. This affects switching targets and reopening the same target after a provider failure. The connection survives, but get_evidence_bundle and a later export have no Evidence or Unknown history left.

On current main ab02f3c8d665ead35391c94db74cd0fb0cfed0ed, two MCP regressions observe 4 Evidence records and 2 Unknown revisions becoming 0 and 0. One record comes from actual static analysis of a source-owned, single-line JavaScript file; the others are a provider observation and Unknown mutation evidence.

Reproduction

Connect the official MCP SDK to REA's production createServer and production-composed BinarySession, using a controlled provider whose close() can return ProviderCleanupError.
open_binary a tiny fixture; call address_name, analyze_javascript_application, record_unknown, and update_unknown. Confirm the bundle contains 4 records and 2 revisions.
Make cleanup unavailable. Call open_binary for a different fixture, or report the retained provider as exited and reopen the same fixture.
Observe cleanup_incomplete, then call get_evidence_bundle: both collections are empty. Retrying cleanup cannot restore the discarded records.

The accompanying regression is tests/boundary/mcp/sessionEvidenceRecovery.test.ts; run npm run test:focused -- tests/boundary/mcp/sessionEvidenceRecovery.test.ts. Both cases fail on the main revision above at the record-preservation assertion.

Expected behavior and cause

The documented lifetime preserves records across target opens and clears them on explicit close_binary. A failed open should retain the investigation for export/recovery while keeping the old provider unavailable and its cleanup pending.

BinarySession.#open calls clearSessionRecords() when retiring the old client fails. Clearing only the retired provider's snapshot cache preserves the investigation without allowing stale replay or starting another provider before cleanup succeeds.

Impact and scope

Proposed P1: an ordinary recovery/target-switch operation destroys all unexported investigation Evidence and manually maintained Unknown revision history, including results independent of the failing provider. This is data loss, beyond the cleanup error itself. Previously exported files and inline replies are unaffected. The occurrence rate of real provider cleanup failures is not measured here.

This behavior was previously identified in an unresolved P1 review on merged #1776. This report adds current-main execution evidence and a focused fix; it does not claim first discovery. #1260 retains cleanup ownership; #2701 aggregates child cleanup diagnostics. Neither preserves this discarded ledger. #721 establishes the existing explicit-close lifetime, which this fix preserves.

Verification environment

Node 24.18.0, npm 11.16.0, macOS arm64. Real MCP Client/InMemoryTransport and JavaScript file analysis; provider failure/health are controlled fixtures. No real Hopper/Ghidra/IDA failure, native Windows/Linux run, paid model API, or user client configuration is involved.

Prepared with OpenAI Codex assistance. Open/closed issues, associated PRs, complete relevant comments/review threads, and current main were checked before filing.

Originally created by @Adkid-Zephyr on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2753 ## Observed behavior `open_binary` clears the entire retained investigation when retiring the previous provider returns `cleanup_incomplete`. This affects switching targets and reopening the same target after a provider failure. The connection survives, but `get_evidence_bundle` and a later export have no Evidence or Unknown history left. On current main `ab02f3c8d665ead35391c94db74cd0fb0cfed0ed`, two MCP regressions observe **4 Evidence records and 2 Unknown revisions becoming 0 and 0**. One record comes from actual static analysis of a source-owned, single-line JavaScript file; the others are a provider observation and Unknown mutation evidence. ## Reproduction 1. Connect the official MCP SDK to REA's production `createServer` and production-composed `BinarySession`, using a controlled provider whose `close()` can return `ProviderCleanupError`. 2. `open_binary` a tiny fixture; call `address_name`, `analyze_javascript_application`, `record_unknown`, and `update_unknown`. Confirm the bundle contains 4 records and 2 revisions. 3. Make cleanup unavailable. Call `open_binary` for a different fixture, or report the retained provider as exited and reopen the same fixture. 4. Observe `cleanup_incomplete`, then call `get_evidence_bundle`: both collections are empty. Retrying cleanup cannot restore the discarded records. The accompanying regression is `tests/boundary/mcp/sessionEvidenceRecovery.test.ts`; run `npm run test:focused -- tests/boundary/mcp/sessionEvidenceRecovery.test.ts`. Both cases fail on the main revision above at the record-preservation assertion. ## Expected behavior and cause The [documented lifetime](https://github.com/morluto/rea/blob/ab02f3c8d665ead35391c94db74cd0fb0cfed0ed/docs/mcp-contracts.md#retained-application-evidence-inputs) preserves records across target opens and clears them on explicit `close_binary`. A failed open should retain the investigation for export/recovery while keeping the old provider unavailable and its cleanup pending. `BinarySession.#open` calls `clearSessionRecords()` when retiring the old client fails. Clearing only the retired provider's snapshot cache preserves the investigation without allowing stale replay or starting another provider before cleanup succeeds. ## Impact and scope Proposed **P1**: an ordinary recovery/target-switch operation destroys all unexported investigation Evidence and manually maintained Unknown revision history, including results independent of the failing provider. This is data loss, beyond the cleanup error itself. Previously exported files and inline replies are unaffected. The occurrence rate of real provider cleanup failures is not measured here. This behavior was previously identified in an [unresolved P1 review on merged #1776](https://github.com/morluto/rea/pull/1776#discussion_r4242283747). This report adds current-main execution evidence and a focused fix; it does not claim first discovery. #1260 retains cleanup ownership; #2701 aggregates child cleanup diagnostics. Neither preserves this discarded ledger. #721 establishes the existing explicit-close lifetime, which this fix preserves. ## Verification environment Node 24.18.0, npm 11.16.0, macOS arm64. Real MCP Client/InMemoryTransport and JavaScript file analysis; provider failure/health are controlled fixtures. No real Hopper/Ghidra/IDA failure, native Windows/Linux run, paid model API, or user client configuration is involved. Prepared with OpenAI Codex assistance. Open/closed issues, associated PRs, complete relevant comments/review threads, and current main were checked before filing.

bsakaguchi

2026-10-15 18:30:10 +00:00