morluto/rea

Fork 0

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

[GH-ISSUE #4002] [Bug] close_binary overtakes an earlier call-path request and leaves dangling Evidence #1592

New issue

Closed

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

bsakaguchi commented

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

Owner

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

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

MCP or agent integration

Observed behavior

An earlier build_call_path request can publish successful Evidence after a later close_binary completes and clears its source records. The session then contains only the new call-path result, with an evidence_links entry that cannot be resolved.

The recorded baseline reproduction used the production MCP registration, SDK client, linked in-memory transports, and repository-owned Evidence fixtures. The serial call-path control succeeded. In all five barrier-controlled executions, the later close completed before the earlier call-path request resumed; the resumed request returned status: "found" and retained a record whose source link was missing. A subsequent get_evidence_bundle also returned success. No binary was active and no native analysis engine or target execution was involved.

Reproduction

At source commit 4692bb33300c7cb77b095b6cc0c33da861c1b860, install the locked dependencies with npm ci and save the following as src/server/investigationCloseReproduction.test.ts.
Run npm run test:local -- src/server/investigationCloseReproduction.test.ts using Node 24.18.0. These assertions describe the observed defect and passed in the recorded baseline run.
Remove the temporary test afterward.

The canonical INVESTIGATION_EXAMPLES.build_call_path input and all its source Evidence come from the repository. The transport barrier holds completion of the first progress notification so the later public lifecycle request overlaps the earlier workflow deterministically.

import { expect, it } from 'vitest';
import { Client, InMemoryTransport } from '@modelcontextprotocol/client';
import { createTestBinarySession, createDeferred } from '../../tests/fixtures/binarySession.js';
import { createServer } from '../server/createServer.js';
import { INVESTIGATION_EXAMPLES } from '../contracts/investigationExamples.js';
import { parseEvidence } from '../domain/evidence.js';
import { observed } from '../../tests/fixtures/analysisExecution.js';
import { ok } from '../domain/result.js';

it('observes close during a supported derived call-path request', async () => {
  const session = createTestBinarySession(() => ({ execute: async () => observed(null), close: async () => ok(null) }));
  const server = createServer({ kind: 'session', session });
  const client = new Client({ name: 'investigation-admission', version: '1' });
  const [ct, st] = InMemoryTransport.createLinkedPair();
  const prepared = createDeferred<void>();
  const release = createDeferred<void>();
  const send = st.send.bind(st);
  st.send = async (message, options) => {
    await send(message, options);
    if (message.method === 'notifications/progress' && message.params?.message === 'prepare: Preparing build_call_path') {
      prepared.resolve();
      await release.promise;
    }
  };
  await server.connect(st);
  await client.connect(ct);
  try {
    const input = INVESTIGATION_EXAMPLES.build_call_path;
    for (const evidence of input.functions) expect(session.recordEvidence(evidence).ok).toBe(true);
    const control = await client.callTool({ name: 'build_call_path', arguments: input });
    expect(control.isError).not.toBe(true);
    const request = client.callTool({ name: 'build_call_path', arguments: input }, { onprogress: () => {} });
    await prepared.promise;
    const closed = await client.callTool({ name: 'close_binary', arguments: {} });
    expect(closed.isError).not.toBe(true);
    expect(session.exportEvidenceBundle().records).toHaveLength(0);
    release.resolve();
    const response = await request;
    expect(response.isError).not.toBe(true);
    const evidence = parseEvidence(response.structuredContent);
    const bundle = session.exportEvidenceBundle();
    console.log(JSON.stringify({ closedBeforeDerivedFinished: true, status: (evidence.normalized_result as any).status, retainedOperations: bundle.records.map(x => x.operation), danglingLinks: evidence.evidence_links.filter(id => !session.evidenceById(id)) }));
    expect(bundle.records).toHaveLength(1);
    expect(evidence.evidence_links.length).toBeGreaterThan(0);
    expect(session.evidenceById(evidence.evidence_links[0]!)).toBeUndefined();
    const exported = await client.callTool({ name: 'get_evidence_bundle', arguments: {} });
    console.log(JSON.stringify({bundleQueryError: exported.isError === true, bundleQueryContent: exported.isError ? exported.content : undefined}));
  } finally {
    release.resolve();
    await client.close();
    await server.close();
    await session.close();
    expect(session.exportEvidenceBundle().records).toHaveLength(0);
  }
}, 30000);

Expected behavior

A successful close_binary must drain earlier owned work before clearing the retained records. Once both requests settle, an earlier call-path request must not repopulate the cleared ledger with a successful result pointing to sources removed by close.

docs/mcp-contracts.md:313-317 says close clears retained records even without an active binary. Its recovery procedure at lines 588-590 says close drains owned work before clearing retained records. src/application/binary/BinarySession.ts:652-676 defines admission for a composed request through result production and session bookkeeping. A serial call-path request must continue to succeed with its source Evidence resolvable before the requested close.

Impact and scope

P2. Overlapping supported MCP requests can leave successful derived Evidence without its retained sources after a successful close. The recorded reproduction establishes a cleanup and provenance failure with simulated Evidence. Natural scheduling frequency and real-provider consequences were not measured.

The demonstrated workaround is to await build_call_path before sending close_binary. The serial control returned found with its source retained, and subsequent cleanup cleared the records.

REA version

rea 6.5.0 at source commit 4692bb3330. The failing path still holds by source inspection on origin/main at 6fcfc5b5e0c063300cd5278dd8bad5b1ab1374f6; the reproduction was not rerun on that revision.

Runtime and platform

Node 24.18.0; macOS arm64

Target and artifact context

Canonical INVESTIGATION_EXAMPLES.build_call_path input and previously retained repository-owned function Evidence. No active binary, native analysis engine, or target execution. Production MCP registration and SDK client over linked in-memory transports.

Interface

MCP

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

The recorded baseline test output was:

{"closedBeforeDerivedFinished":true,"status":"found","retainedOperations":["build_call_path"],"danglingLinks":["ev_e86610c7e7c6e1142b59de382743437a6e61ac14831822259cb1fec6eea8caca"]}
{"bundleQueryError":false}

The test directly checks that close succeeds and the ledger is empty while the earlier request waits. After releasing the request, it checks that exactly one derived record remains and its source ID is absent. The final focused run passed these defect assertions; the same lifecycle failure appeared in five controlled executions.

Source references at the reproduced baseline:

Typed outcome or recovery expected

With the same progress barrier and overlapping close, await both requests and require that successful close leaves the ledger empty. It must never leave a newly retained derived record whose sources close removed. Keep the serial control and require every source link to resolve before close. These are proposed restoration checks, not a verified fix.

Analysis or suspected cause

Source-derived reasoning: src/server/registerInvestigationTools.ts:120-166 obtains the source closure, awaits runDerivedOperation, and records the result without session admission around that complete workflow. Close can therefore clear the already-checked source records during the await, before the workflow publishes its result.

The registration, derived-operation helper, result-recording helper, and close contract are unchanged between the reproduced baseline and current origin/main. The intervening BinarySession.ts change in 73c72d574 concerns loading snapshots during open_binary; it does not admit this call-path workflow.

Additional context

Related closed #3334 concerned a snapshot open being overtaken by later requests. Its snapshot-loading correction does not address a derived call-path request publishing after close. Open #3333 and PR #3503 concern unknown-registry collisions between distinct unresolved requests, a separate failure with different inputs and recovery behavior.

Originally created by @coygeek on GitHub (Oct 15, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4002 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area MCP or agent integration ### Observed behavior An earlier `build_call_path` request can publish successful Evidence after a later `close_binary` completes and clears its source records. The session then contains only the new call-path result, with an `evidence_links` entry that cannot be resolved. The recorded baseline reproduction used the production MCP registration, SDK client, linked in-memory transports, and repository-owned Evidence fixtures. The serial call-path control succeeded. In all five barrier-controlled executions, the later close completed before the earlier call-path request resumed; the resumed request returned `status: "found"` and retained a record whose source link was missing. A subsequent `get_evidence_bundle` also returned success. No binary was active and no native analysis engine or target execution was involved. ### Reproduction 1. At source commit `4692bb33300c7cb77b095b6cc0c33da861c1b860`, install the locked dependencies with `npm ci` and save the following as `src/server/investigationCloseReproduction.test.ts`. 2. Run `npm run test:local -- src/server/investigationCloseReproduction.test.ts` using Node 24.18.0. These assertions describe the observed defect and passed in the recorded baseline run. 3. Remove the temporary test afterward. The canonical `INVESTIGATION_EXAMPLES.build_call_path` input and all its source Evidence come from the repository. The transport barrier holds completion of the first progress notification so the later public lifecycle request overlaps the earlier workflow deterministically. ```ts import { expect, it } from 'vitest'; import { Client, InMemoryTransport } from '@modelcontextprotocol/client'; import { createTestBinarySession, createDeferred } from '../../tests/fixtures/binarySession.js'; import { createServer } from '../server/createServer.js'; import { INVESTIGATION_EXAMPLES } from '../contracts/investigationExamples.js'; import { parseEvidence } from '../domain/evidence.js'; import { observed } from '../../tests/fixtures/analysisExecution.js'; import { ok } from '../domain/result.js'; it('observes close during a supported derived call-path request', async () => { const session = createTestBinarySession(() => ({ execute: async () => observed(null), close: async () => ok(null) })); const server = createServer({ kind: 'session', session }); const client = new Client({ name: 'investigation-admission', version: '1' }); const [ct, st] = InMemoryTransport.createLinkedPair(); const prepared = createDeferred<void>(); const release = createDeferred<void>(); const send = st.send.bind(st); st.send = async (message, options) => { await send(message, options); if (message.method === 'notifications/progress' && message.params?.message === 'prepare: Preparing build_call_path') { prepared.resolve(); await release.promise; } }; await server.connect(st); await client.connect(ct); try { const input = INVESTIGATION_EXAMPLES.build_call_path; for (const evidence of input.functions) expect(session.recordEvidence(evidence).ok).toBe(true); const control = await client.callTool({ name: 'build_call_path', arguments: input }); expect(control.isError).not.toBe(true); const request = client.callTool({ name: 'build_call_path', arguments: input }, { onprogress: () => {} }); await prepared.promise; const closed = await client.callTool({ name: 'close_binary', arguments: {} }); expect(closed.isError).not.toBe(true); expect(session.exportEvidenceBundle().records).toHaveLength(0); release.resolve(); const response = await request; expect(response.isError).not.toBe(true); const evidence = parseEvidence(response.structuredContent); const bundle = session.exportEvidenceBundle(); console.log(JSON.stringify({ closedBeforeDerivedFinished: true, status: (evidence.normalized_result as any).status, retainedOperations: bundle.records.map(x => x.operation), danglingLinks: evidence.evidence_links.filter(id => !session.evidenceById(id)) })); expect(bundle.records).toHaveLength(1); expect(evidence.evidence_links.length).toBeGreaterThan(0); expect(session.evidenceById(evidence.evidence_links[0]!)).toBeUndefined(); const exported = await client.callTool({ name: 'get_evidence_bundle', arguments: {} }); console.log(JSON.stringify({bundleQueryError: exported.isError === true, bundleQueryContent: exported.isError ? exported.content : undefined})); } finally { release.resolve(); await client.close(); await server.close(); await session.close(); expect(session.exportEvidenceBundle().records).toHaveLength(0); } }, 30000); ``` ### Expected behavior A successful `close_binary` must drain earlier owned work before clearing the retained records. Once both requests settle, an earlier call-path request must not repopulate the cleared ledger with a successful result pointing to sources removed by close. `docs/mcp-contracts.md:313-317` says close clears retained records even without an active binary. Its recovery procedure at lines 588-590 says close drains owned work before clearing retained records. `src/application/binary/BinarySession.ts:652-676` defines admission for a composed request through result production and session bookkeeping. A serial call-path request must continue to succeed with its source Evidence resolvable before the requested close. ### Impact and scope P2. Overlapping supported MCP requests can leave successful derived Evidence without its retained sources after a successful close. The recorded reproduction establishes a cleanup and provenance failure with simulated Evidence. Natural scheduling frequency and real-provider consequences were not measured. The demonstrated workaround is to await `build_call_path` before sending `close_binary`. The serial control returned `found` with its source retained, and subsequent cleanup cleared the records. ### REA version rea 6.5.0 at source commit 4692bb33300c7cb77b095b6cc0c33da861c1b860. The failing path still holds by source inspection on origin/main at `6fcfc5b5e0c063300cd5278dd8bad5b1ab1374f6`; the reproduction was not rerun on that revision. ### Runtime and platform Node 24.18.0; macOS arm64 ### Target and artifact context Canonical `INVESTIGATION_EXAMPLES.build_call_path` input and previously retained repository-owned function Evidence. No active binary, native analysis engine, or target execution. Production MCP registration and SDK client over linked in-memory transports. ### Interface MCP ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence The recorded baseline test output was: ```json {"closedBeforeDerivedFinished":true,"status":"found","retainedOperations":["build_call_path"],"danglingLinks":["ev_e86610c7e7c6e1142b59de382743437a6e61ac14831822259cb1fec6eea8caca"]} {"bundleQueryError":false} ``` The test directly checks that close succeeds and the ledger is empty while the earlier request waits. After releasing the request, it checks that exactly one derived record remains and its source ID is absent. The final focused run passed these defect assertions; the same lifecycle failure appeared in five controlled executions. Source references at the reproduced baseline: - [Call-path source closure, derived execution, and result registration](https://github.com/morluto/rea/blob/4692bb33300c7cb77b095b6cc0c33da861c1b860/src/server/registerInvestigationTools.ts#L120). - [Composed-request session admission](https://github.com/morluto/rea/blob/4692bb33300c7cb77b095b6cc0c33da861c1b860/src/application/binary/BinarySession.ts#L652). - [Close drains owned work and clears retained records](https://github.com/morluto/rea/blob/4692bb33300c7cb77b095b6cc0c33da861c1b860/docs/mcp-contracts.md#L588). ### Typed outcome or recovery expected With the same progress barrier and overlapping close, await both requests and require that successful close leaves the ledger empty. It must never leave a newly retained derived record whose sources close removed. Keep the serial control and require every source link to resolve before close. These are proposed restoration checks, not a verified fix. ### Analysis or suspected cause Source-derived reasoning: `src/server/registerInvestigationTools.ts:120-166` obtains the source closure, awaits `runDerivedOperation`, and records the result without session admission around that complete workflow. Close can therefore clear the already-checked source records during the await, before the workflow publishes its result. The registration, derived-operation helper, result-recording helper, and close contract are unchanged between the reproduced baseline and current origin/main. The intervening `BinarySession.ts` change in `73c72d574` concerns loading snapshots during `open_binary`; it does not admit this call-path workflow. ### Additional context Related closed [#3334](https://github.com/morluto/rea/issues/3334) concerned a snapshot open being overtaken by later requests. Its snapshot-loading correction does not address a derived call-path request publishing after close. Open [#3333](https://github.com/morluto/rea/issues/3333) and [PR #3503](https://github.com/morluto/rea/pull/3503) concern unknown-registry collisions between distinct unresolved requests, a separate failure with different inputs and recovery behavior.

bsakaguchi closed this issue

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

bsakaguchi commented

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

Author

Owner

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

Confirmed — this is a lifecycle race between close_binary draining/clearing source records and an in-flight build_call_path workflow resuming afterwards. The resumed request validates nothing against the already-cleared source records, so it still publishes "found" Evidence whose evidence_links can no longer be resolved. The barrier-based reproduction makes the window deterministic and easy to reason about.

A reasonable fix would be to re-validate source-record existence when a paused workflow resumes (or fail it as cancelled if its sources were drained in the meantime), so a completed close can never be followed by dangling Evidence. Alternatively, mark workflows that depend on the closing session as invalid at close time. It would also be worth asserting in the regression test that no unresolvable evidence_links survive a close that overtakes an earlier call-path request.

@XinwangTao commented on GitHub (Oct 15, 2026): Confirmed — this is a lifecycle race between close_binary draining/clearing source records and an in-flight build_call_path workflow resuming afterwards. The resumed request validates nothing against the already-cleared source records, so it still publishes "found" Evidence whose evidence_links can no longer be resolved. The barrier-based reproduction makes the window deterministic and easy to reason about. A reasonable fix would be to re-validate source-record existence when a paused workflow resumes (or fail it as cancelled if its sources were drained in the meantime), so a completed close can never be followed by dangling Evidence. Alternatively, mark workflows that depend on the closing session as invalid at close time. It would also be worth asserting in the regression test that no unresolvable evidence_links survive a close that overtakes an earlier call-path request.

bsakaguchi commented

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

Author

Owner

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

Integrated the session-ownership fix on main in 663d07f02. All four investigation handlers hold existing session admission through derivation and Evidence recording, so close_binary drains earlier operations before clearing records. Redundant post-derivation closure repairs and private-state clearing tests were not retained. A related publication guard now honors cancellation arriving during awaited terminal progress.

The public MCP race failed on the original registration with close settling first; fixed ordering and an empty post-close ledger pass. The terminal-cancellation regression also failed before its guard and passes afterward. Main CI passed all36 jobs: https://github.com/morluto/rea/actions/runs/38792163880. Closing as addressed.

@morluto commented on GitHub (Oct 15, 2026): Integrated the session-ownership fix on main in 663d07f02. All four investigation handlers hold existing session admission through derivation and Evidence recording, so close_binary drains earlier operations before clearing records. Redundant post-derivation closure repairs and private-state clearing tests were not retained. A related publication guard now honors cancellation arriving during awaited terminal progress. The public MCP race failed on the original registration with close settling first; fixed ordering and an empty post-close ledger pass. The terminal-cancellation regression also failed before its guard and passes afterward. Main CI passed all36 jobs: https://github.com/morluto/rea/actions/runs/38792163880. Closing as addressed.

bsakaguchi referenced this issue

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

[PR #1592] [MERGED] feat(ghidra): recover signed AArch64 relative jump tables #2992

bsakaguchi referenced this issue

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

[PR #2977] feat(ghidra): report computed branches without a recovered jump table #5533

bsakaguchi referenced this issue

2026-11-03 20:41:07 +00:00

[GH-ISSUE #487] [Feature] Improve Ghidra AArch64 jump-table recovery #261