[GH-ISSUE #7431] [Bug] gdb_console returns success when cancelled during analysis session retirement #4436

New issue

Closed

opened 2026-10-31 00:49:47 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:49:47 +00:00

Owner

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

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

The registered gdb_console handler returns successful Evidence when its request signal is aborted after the GDB command completes but while closing the active binary-analysis client. The result has isError: false, an evidence_id, and no typed cancellation. The real investigation store receives the completed Evidence through the ordinary success path. A pre-aborted request correctly returns cancelled; an uncancelled control succeeds.

Reproduction

Use Node 24.22.1 and the source commit below. A fresh checkout needs npm ci && npm run build:cached; the prepared verifier checkout already had matching locked dependencies, so only the build was rerun. Save and run this focused probe from the repository root. Two focused runs reproduced the late-cancellation failure; the final three-case probe below ran once with real Evidence recording. Its assertions characterize the defect. The filename below is the public-safe spelling of the executed temporary test filename.

export PATH="$(mise where node@24.22.1)/bin:$PATH"
WORK=$(mktemp -d)
export HOME="$WORK/home"
export XDG_CONFIG_HOME="$WORK/config" XDG_DATA_HOME="$WORK/data" XDG_CACHE_HOME="$WORK/cache" XDG_STATE_HOME="$WORK/state" TMPDIR="$WORK/tmp"
mkdir -p "$HOME" "$XDG_CONFIG_HOME" "$XDG_DATA_HOME" "$XDG_CACHE_HOME" "$XDG_STATE_HOME" "$TMPDIR"
cat > tests/boundary/mcp/retirementCancellationProbe.test.ts <<'EOF'
import type { CallToolResult, McpServer } from '@modelcontextprotocol/server';
import { expect, it, vi } from 'vitest';
import { registerGdbTools, type GdbToolManager } from '../../../src/server/registerGdbTools.js';
import { createToolResultDelivery } from '../../../src/server/toolResult.js';
import { silentLogger } from '../../../src/logger.js';
import { createEvidence } from '../../../src/domain/evidence.js';
import { ok } from '../../../src/domain/result.js';
import { createBinarySessionTargets, createDeferred, createTestBinarySession } from '../../fixtures/binarySession.js';

it('compares cancellation before dispatch, during retirement, and an uncancelled control', { timeout: 8000 }, async () => {
  for (const mode of ['pre-aborted', 'during-retirement', 'control'] as const) {
    const [target] = await createBinarySessionTargets();
    const entered = createDeferred<void>();
    const released = createDeferred<void>();
    let closes = 0;
    const session = createTestBinarySession(() => ({
      execute: async () => ok({ operation: 'health', value: null }),
      close: async () => { closes++; entered.resolve(); await released.promise; return ok(null); },
    }) as never);
    expect((await session.open(target)).ok).toBe(true);
    const id = '00000000-0000-4000-8000-000000000001';
    const evidence = createEvidence(undefined, { id: 'gdb', name: 'GDB', version: null }, {
      predicateType: 'rea.debugger.command-observation', operation: 'gdb_console',
      parameters: { session_id: id, command: 'info files' },
      result: { command: 'info files', completion_status: 'complete' },
      rawResult: { command: 'info files' },
    });
    let handler!: (input: unknown, context: unknown) => Promise<CallToolResult>;
    const server = { registerTool(name: string, _options: unknown, fn: typeof handler) {
      if (name === 'gdb_console') handler = fn;
    }} as unknown as McpServer;
    const execute = vi.fn(async () => ok({ evidence }));
    const manager = { execute, retryCleanupFailures: async () => ok(null) } as unknown as GdbToolManager;
    const record = vi.fn((value: typeof evidence) => session.recordEvidence(value));
    registerGdbTools(server, manager, silentLogger, createToolResultDelivery(undefined), record, session);
    const controller = new AbortController();
    const reason = { phase: mode };
    if (mode === 'pre-aborted') controller.abort(reason);
    try {
      const pending = handler({ session_id: id, command: 'info files', detail: 'complete' }, { mcpReq: { signal: controller.signal } });
      if (mode !== 'pre-aborted') {
        await entered.promise;
        expect(execute).toHaveBeenCalledTimes(1);
        expect(record).not.toHaveBeenCalled();
        if (mode === 'during-retirement') controller.abort(reason);
      }
      released.resolve();
      const result = await pending;
      const body = result.structuredContent as any;
      const observation = { mode, aborted: controller.signal.aborted, executions: execute.mock.calls.length,
        retirementCloses: closes, recordings: record.mock.calls.length, isError: result.isError === true,
        code: body.error?.code ?? null, hasEvidence: typeof body.evidence_id === 'string', stored: session.hasEvidence(evidence.evidence_id) };
      console.log(JSON.stringify(observation));
      expect(observation).toEqual({ mode, aborted: mode !== 'control', executions: mode === 'pre-aborted' ? 0 : 1,
        retirementCloses: mode === 'pre-aborted' ? 0 : 1, recordings: mode === 'pre-aborted' ? 0 : 1,
        isError: mode === 'pre-aborted', code: mode === 'pre-aborted' ? 'cancelled' : null, hasEvidence: mode !== 'pre-aborted', stored: mode !== 'pre-aborted' });
      if (mode === 'during-retirement') expect(record.mock.calls[0]?.[0]).toEqual(evidence);
    } finally {
      released.resolve();
      await session.close();
    }
  }
});
EOF
npx vitest run tests/boundary/mcp/retirementCancellationProbe.test.ts
rm tests/boundary/mcp/retirementCancellationProbe.test.ts

Expected behavior

After awaited retirement, classify the aborted call as typed cancellation and retain its completed Evidence as the partial observation. docs/tool-design.md:181-194 requires cancellation through awaited cleanup, a signal check after finalization, and preservation of the exact caller reason. src/contracts/gdbToolContracts.ts:37-50 makes active analysis-session retirement part of this MCP command. The uncancelled case should continue to succeed.

Impact and scope

P2: a bounded timing window yields a misleading server-side success instead of cancellation and its partial-observation reference. Evidence remains valid and available; no target damage or data loss was observed. A client can disregard results after its own abort, but this does not recover the missing typed provenance. The production registration, BinarySession, delivery, and real Evidence recording were exercised with simulated GDB and analysis clients. No MCP transport, real GDB process, device, or external service ran; whether a cancelled client receives a late response and the natural frequency are unknown.

REA version

Exact node scripts/rea.mjs --version output: 6.8.0. Rebuilt source main: a116f2a7ae8e7ae18b6d0a24211abdb2b920a7db.

Runtime and platform

Node v24.22.1; macOS 27.0.1; darwin arm64; locked Vitest 5.0.3. The controlled handler test does not establish native GDB compatibility on this host.

Target and artifact context

An inert local analysis fixture created inline by createBinarySessionTargets; a synthetic completed info files Evidence object. No executable target or inferior ran. A deferred analysis-client close provides the cancellation barrier.

Interface

MCP

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Observed: one focused test passed with these three sequential cases. code: null below means the instrumented result has no error.code; it is not a literal field in the product result. stored checks the real session investigation store.

{"mode":"pre-aborted","aborted":true,"executions":0,"retirementCloses":0,"recordings":0,"isError":true,"code":"cancelled","hasEvidence":false,"stored":false}
{"mode":"during-retirement","aborted":true,"executions":1,"retirementCloses":1,"recordings":1,"isError":false,"code":null,"hasEvidence":true,"stored":true}
{"mode":"control","aborted":false,"executions":1,"retirementCloses":1,"recordings":1,"isError":false,"code":null,"hasEvidence":true,"stored":true}

Typed outcome or recovery expected

Internally, AnalysisCancelledError should preserve signal.reason as its cause and the completed Evidence as partialObservation. MCP delivery should report cancelled and retain/reference that Evidence through the existing partial-error delivery. Recording valid Evidence is expected in either case; the cancellation envelope is missing. No arbitrary cause-object identity on the wire is asserted.

Analysis or suspected cause

Source-derived: src/server/createServer.ts:761-771 supplies the real session to the registered handler. src/server/registerGdbTools.ts:66-118 sends the request signal through src/server/externalMutationAdmission.ts:30-60. BinarySession.withAdmittedExternalMutation checks cancellation before dispatch, but src/application/binary/BinarySession.ts:777-779 awaits successful retirement and returns the saved command result without a final signal check. Delivery then records and returns that success.

Additional context

Open #7378 concerns the separate one-shot CLI finalizer. Closed #6958 concerns missing reasons at existing pre-operation guards, and closed #6115 concerns failed retirement during open_binary. Closed #4256 and #5082 address similar finalization defects in browser and Frida owners. Proposed restoration check: rerun the same barrier/control matrix expecting typed cancellation with retained Evidence only in the aborted cases. No fix was tested; sibling tools sharing admission were not exercised.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: The registered-handler probe returns successful Evidence after the request signal aborts during real BinarySession retirement, whereas its pre-aborted control correctly reports cancellation. Completed Evidence and successful cleanup survive; no severe harm was observed.
Workaround status: unverified
Workaround basis: A caller can ignore a late result using its own signal, but no tested workaround restores the missing canonical cancellation classification and partial-observation reference.

Originally created by @coygeek on GitHub (Oct 27, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7431 ## 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 The registered `gdb_console` handler returns successful Evidence when its request signal is aborted after the GDB command completes but while closing the active binary-analysis client. The result has `isError: false`, an `evidence_id`, and no typed cancellation. The real investigation store receives the completed Evidence through the ordinary success path. A pre-aborted request correctly returns `cancelled`; an uncancelled control succeeds. ## Reproduction Use Node 24.22.1 and the source commit below. A fresh checkout needs `npm ci && npm run build:cached`; the prepared verifier checkout already had matching locked dependencies, so only the build was rerun. Save and run this focused probe from the repository root. Two focused runs reproduced the late-cancellation failure; the final three-case probe below ran once with real Evidence recording. Its assertions characterize the defect. The filename below is the public-safe spelling of the executed temporary test filename. ```sh export PATH="$(mise where node@24.22.1)/bin:$PATH" WORK=$(mktemp -d) export HOME="$WORK/home" export XDG_CONFIG_HOME="$WORK/config" XDG_DATA_HOME="$WORK/data" XDG_CACHE_HOME="$WORK/cache" XDG_STATE_HOME="$WORK/state" TMPDIR="$WORK/tmp" mkdir -p "$HOME" "$XDG_CONFIG_HOME" "$XDG_DATA_HOME" "$XDG_CACHE_HOME" "$XDG_STATE_HOME" "$TMPDIR" cat > tests/boundary/mcp/retirementCancellationProbe.test.ts <<'EOF' import type { CallToolResult, McpServer } from '@modelcontextprotocol/server'; import { expect, it, vi } from 'vitest'; import { registerGdbTools, type GdbToolManager } from '../../../src/server/registerGdbTools.js'; import { createToolResultDelivery } from '../../../src/server/toolResult.js'; import { silentLogger } from '../../../src/logger.js'; import { createEvidence } from '../../../src/domain/evidence.js'; import { ok } from '../../../src/domain/result.js'; import { createBinarySessionTargets, createDeferred, createTestBinarySession } from '../../fixtures/binarySession.js'; it('compares cancellation before dispatch, during retirement, and an uncancelled control', { timeout: 8000 }, async () => { for (const mode of ['pre-aborted', 'during-retirement', 'control'] as const) { const [target] = await createBinarySessionTargets(); const entered = createDeferred<void>(); const released = createDeferred<void>(); let closes = 0; const session = createTestBinarySession(() => ({ execute: async () => ok({ operation: 'health', value: null }), close: async () => { closes++; entered.resolve(); await released.promise; return ok(null); }, }) as never); expect((await session.open(target)).ok).toBe(true); const id = '00000000-0000-4000-8000-000000000001'; const evidence = createEvidence(undefined, { id: 'gdb', name: 'GDB', version: null }, { predicateType: 'rea.debugger.command-observation', operation: 'gdb_console', parameters: { session_id: id, command: 'info files' }, result: { command: 'info files', completion_status: 'complete' }, rawResult: { command: 'info files' }, }); let handler!: (input: unknown, context: unknown) => Promise<CallToolResult>; const server = { registerTool(name: string, _options: unknown, fn: typeof handler) { if (name === 'gdb_console') handler = fn; }} as unknown as McpServer; const execute = vi.fn(async () => ok({ evidence })); const manager = { execute, retryCleanupFailures: async () => ok(null) } as unknown as GdbToolManager; const record = vi.fn((value: typeof evidence) => session.recordEvidence(value)); registerGdbTools(server, manager, silentLogger, createToolResultDelivery(undefined), record, session); const controller = new AbortController(); const reason = { phase: mode }; if (mode === 'pre-aborted') controller.abort(reason); try { const pending = handler({ session_id: id, command: 'info files', detail: 'complete' }, { mcpReq: { signal: controller.signal } }); if (mode !== 'pre-aborted') { await entered.promise; expect(execute).toHaveBeenCalledTimes(1); expect(record).not.toHaveBeenCalled(); if (mode === 'during-retirement') controller.abort(reason); } released.resolve(); const result = await pending; const body = result.structuredContent as any; const observation = { mode, aborted: controller.signal.aborted, executions: execute.mock.calls.length, retirementCloses: closes, recordings: record.mock.calls.length, isError: result.isError === true, code: body.error?.code ?? null, hasEvidence: typeof body.evidence_id === 'string', stored: session.hasEvidence(evidence.evidence_id) }; console.log(JSON.stringify(observation)); expect(observation).toEqual({ mode, aborted: mode !== 'control', executions: mode === 'pre-aborted' ? 0 : 1, retirementCloses: mode === 'pre-aborted' ? 0 : 1, recordings: mode === 'pre-aborted' ? 0 : 1, isError: mode === 'pre-aborted', code: mode === 'pre-aborted' ? 'cancelled' : null, hasEvidence: mode !== 'pre-aborted', stored: mode !== 'pre-aborted' }); if (mode === 'during-retirement') expect(record.mock.calls[0]?.[0]).toEqual(evidence); } finally { released.resolve(); await session.close(); } } }); EOF npx vitest run tests/boundary/mcp/retirementCancellationProbe.test.ts rm tests/boundary/mcp/retirementCancellationProbe.test.ts ``` ## Expected behavior After awaited retirement, classify the aborted call as typed cancellation and retain its completed Evidence as the partial observation. `docs/tool-design.md:181-194` requires cancellation through awaited cleanup, a signal check after finalization, and preservation of the exact caller reason. `src/contracts/gdbToolContracts.ts:37-50` makes active analysis-session retirement part of this MCP command. The uncancelled case should continue to succeed. ## Impact and scope P2: a bounded timing window yields a misleading server-side success instead of cancellation and its partial-observation reference. Evidence remains valid and available; no target damage or data loss was observed. A client can disregard results after its own abort, but this does not recover the missing typed provenance. The production registration, BinarySession, delivery, and real Evidence recording were exercised with simulated GDB and analysis clients. No MCP transport, real GDB process, device, or external service ran; whether a cancelled client receives a late response and the natural frequency are unknown. ## REA version Exact `node scripts/rea.mjs --version` output: `6.8.0`. Rebuilt source main: `a116f2a7ae8e7ae18b6d0a24211abdb2b920a7db`. ## Runtime and platform Node `v24.22.1`; macOS `27.0.1`; `darwin arm64`; locked Vitest `5.0.3`. The controlled handler test does not establish native GDB compatibility on this host. ## Target and artifact context An inert local analysis fixture created inline by `createBinarySessionTargets`; a synthetic completed `info files` Evidence object. No executable target or inferior ran. A deferred analysis-client close provides the cancellation barrier. ## Interface MCP ## Earliest failure stage Cleanup, cancellation, or lifecycle ## Relevant output or evidence Observed: one focused test passed with these three sequential cases. `code: null` below means the instrumented result has no `error.code`; it is not a literal field in the product result. `stored` checks the real session investigation store. ```jsonl {"mode":"pre-aborted","aborted":true,"executions":0,"retirementCloses":0,"recordings":0,"isError":true,"code":"cancelled","hasEvidence":false,"stored":false} {"mode":"during-retirement","aborted":true,"executions":1,"retirementCloses":1,"recordings":1,"isError":false,"code":null,"hasEvidence":true,"stored":true} {"mode":"control","aborted":false,"executions":1,"retirementCloses":1,"recordings":1,"isError":false,"code":null,"hasEvidence":true,"stored":true} ``` ## Typed outcome or recovery expected Internally, `AnalysisCancelledError` should preserve `signal.reason` as its cause and the completed Evidence as `partialObservation`. MCP delivery should report `cancelled` and retain/reference that Evidence through the existing partial-error delivery. Recording valid Evidence is expected in either case; the cancellation envelope is missing. No arbitrary cause-object identity on the wire is asserted. ## Analysis or suspected cause Source-derived: `src/server/createServer.ts:761-771` supplies the real session to the registered handler. `src/server/registerGdbTools.ts:66-118` sends the request signal through `src/server/externalMutationAdmission.ts:30-60`. `BinarySession.withAdmittedExternalMutation` checks cancellation before dispatch, but `src/application/binary/BinarySession.ts:777-779` awaits successful retirement and returns the saved command result without a final signal check. Delivery then records and returns that success. ## Additional context Open [#7378](https://github.com/morluto/rea/issues/7378) concerns the separate one-shot CLI finalizer. Closed [#6958](https://github.com/morluto/rea/issues/6958) concerns missing reasons at existing pre-operation guards, and closed [#6115](https://github.com/morluto/rea/issues/6115) concerns failed retirement during `open_binary`. Closed [#4256](https://github.com/morluto/rea/issues/4256) and [#5082](https://github.com/morluto/rea/issues/5082) address similar finalization defects in browser and Frida owners. Proposed restoration check: rerun the same barrier/control matrix expecting typed cancellation with retained Evidence only in the aborted cases. No fix was tested; sibling tools sharing admission were not exercised. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: The registered-handler probe returns successful Evidence after the request signal aborts during real BinarySession retirement, whereas its pre-aborted control correctly reports cancellation. Completed Evidence and successful cleanup survive; no severe harm was observed. - Workaround status: unverified - Workaround basis: A caller can ignore a late result using its own signal, but no tested workaround restores the missing canonical cancellation classification and partial-observation reference.

bsakaguchi closed this issue

2026-10-31 00:49:47 +00:00

bsakaguchi referenced this issue

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

[PR #4442] [CLOSED] fix(evidence): accept legacy null coverage identity #6247