morluto/rea

Fork 0

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

[GH-ISSUE #5082] [Bug] Frida one-shot MCP instrumentation records success after cancellation during final detach #3119

New issue

Closed

opened 2026-10-31 00:36:51 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:36:51 +00:00

Owner

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

Before submitting

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

Area

Process capture or runtime observation

Observed behavior

A valid instrument_with_frida request cancelled while final native detach is pending still reaches ordinary success delivery after detach finishes. The server records completed instrumentation Evidence instead of classifying it as typed cancellation with partial Evidence.

This was reproduced with the production MCP registration, service, manager and SDK in-memory transport, using simulated native Frida objects. The client promise rejects locally on cancellation; the defect is the server's subsequent success classification and record. No real target process or native Frida binding ran.

Reproduction

From a checkout at the source revision below with its locked dependencies prepared, save the following as $WORK/probe.mts in a disposable directory. Run mkdir -p "$WORK/home" and HOME="$WORK/home" node --import tsx "$WORK/probe.mts" from the repository root. The fixture uses no user configuration or external service. Node v26.12.0 was used.

The first case is an uncancelled control. The second waits until final native detach starts, cancels, uses MCP ping as an ordering barrier, confirms the server's signal was aborted, and then releases detach. Assertions describe current behavior, including the defect.

import assert from "node:assert/strict";
import { pathToFileURL } from "node:url";
const from = p => import(pathToFileURL(`${process.cwd()}/${p}`).href);
const { Client, InMemoryTransport } = await from("node_modules/@modelcontextprotocol/client/dist/index.mjs");
const { McpServer } = await from("node_modules/@modelcontextprotocol/server/dist/index.mjs");
const { FridaInstrumentationManager } = await from("src/frida/FridaInstrumentationManager.ts");
const { registerFridaTools } = await from("src/server/registerFridaTools.ts");
const { createToolResultDelivery } = await from("src/server/toolResult.ts");
const { silentLogger } = await from("src/logger.ts");
async function probe(cancel) {
  let enter, release, observedSignal;
  const entered = new Promise(r => { enter = r; });
  const held = new Promise(r => { release = r; });
  let detached = false, unloads = 0;
  const nativeSession = {
    pid: 512, detached: { connect() {}, disconnect() {} },
    isDetached: () => detached,
    async detach() { enter(); await held; detached = true; },
    async createScript() {
      let destroyed = false, listener;
      return {
        get isDestroyed() { return destroyed; },
        message: { connect(h) { listener = h; }, disconnect() { listener = undefined; } },
        async load() { listener?.({ type: "send", payload: "marker" }, null); },
        async unload() { destroyed = true; unloads++; },
      };
    },
  };
  const device = {
    id: "local", name: "Fixture", type: "local",
    async attach() { return nativeSession; }, async spawn() { return 512; },
    async resume() {}, async enumerateProcesses() { return []; },
    async getProcessByPid() { return { pid: 512, name: "Fixture" }; },
  };
  const manager = new FridaInstrumentationManager({ deviceManager: {
    async getDeviceById() { return device; }, async enumerateDevices() { return [device]; },
    async addRemoteDevice() { return device; }, async removeRemoteDevice() {},
  } });
  const originalLoad = manager.loadScript.bind(manager);
  manager.loadScript = (id, source, signal) => {
    observedSignal = signal;
    return originalLoad(id, source, signal);
  };
  let finishRecord;
  const recorded = new Promise(r => { finishRecord = r; });
  const server = new McpServer({ name: "cleanup-probe", version: "1" });
  const deliveryCalls = [];
  const delivery = new Proxy(createToolResultDelivery(undefined), {
    get(target, key) {
      const value = Reflect.get(target, key);
      return typeof value !== "function" ? value : (...args) => {
        deliveryCalls.push(key);
        return value.apply(target, args);
      };
    },
  });
  registerFridaTools(server, manager, silentLogger, delivery, evidence => {
    finishRecord(evidence);
    return { ok: true, value: null };
  });
  const client = new Client({ name: "cleanup-probe", version: "1" });
  const [a, b] = InMemoryTransport.createLinkedPair();
  await server.connect(b); await client.connect(a);
  const controller = new AbortController();
  try {
    const pending = client.callTool({ name: "instrument_with_frida", arguments: {
      mode: "attach", pid: 512, device_id: "local", source_kind: "inline",
      source: "send('marker')", duration_ms: 0,
    } }, { signal: controller.signal }).catch(e => ({ clientRejected: e.message }));
    await entered;
    assert.equal(detached, false);
    if (cancel) { controller.abort(new Error("cancel during final detach")); await client.ping(); }
    assert.equal(observedSignal.aborted, cancel);
    release();
    const evidence = await recorded;
    const result = await pending;
    await client.ping();
    assert.deepEqual(deliveryCalls, ["toEvidenceToolResult"]);
    if (!cancel) assert.notEqual(result.isError, true);
    assert.equal(detached, true);
    assert.equal(unloads, 1);
    assert.equal(evidence.operation, "instrument_with_frida");
    assert.ok(evidence.normalized_result.messages.some(m => m.payload === "marker"));
    assert.equal(evidence.normalized_result.cleanup_error, null);
    assert.equal(manager.status(evidence.parameters.session_id), undefined);
    console.log(JSON.stringify({ cancel, serverSignalAborted: observedSignal.aborted,
      clientRejected: result.clientRejected ?? null, clientIsError: result.isError ?? null,
      recordedOperation: evidence.operation, deliveryCalls,
      recordedMarker: true, cleanupError: evidence.normalized_result.cleanup_error,
      nativeDetached: detached, unloads, sessionReleased: true }));
  } finally {
    release(); await client.close(); await server.close(); await manager.closeAll();
  }
}
await probe(false);
await probe(true);

Expected behavior

After successful final cleanup, an aborted operation should reach typed cancellation delivery with the completed observation and actual cleanup facts preserved. The uncancelled control should continue to succeed.

The lifecycle contract requires a signal check after the last awaited finalization step and retention of observations under typed cancellation. This case has no earlier failure or incomplete cleanup whose classification needs precedence.

Impact and scope

The controlled interleaving deterministically records an aborted one-shot observation as ordinary successful Evidence. Captured messages survive and the session is released, so the established impact is bounded completion/retention classification. Real-provider frequency and target-side consequences are NOT_ENOUGH_INFO; neither was measured. A contract-preserving caller workaround was not established.

REA version

Source SHA 3537e58c1ab29309f06eabf50bc52adba9817a0c, package version 6.7.0, current main at verification. Relevant Frida source is identical to the rea-agents-6.7.0 tag; the stable release runtime was not executed.

Runtime and platform

Node v26.12.0; macOS 27.0.1; arm64. The repository's exact pinned Node 24.18.0 was not exercised. Native Frida version/device compatibility: NOT_ENOUGH_INFO, simulated bindings only.

Target and artifact context

Simulated local attach session, synthetic PID 512, inline marker script, zero-duration observation. No real process was attached or executed. The production manager creates the session and script identities and releases both through its normal cleanup path.

Interface

MCP

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Final executed probe, exit code 0:

{"cancel":false,"serverSignalAborted":false,"clientRejected":null,"clientIsError":null,"recordedOperation":"instrument_with_frida","deliveryCalls":["toEvidenceToolResult"],"recordedMarker":true,"cleanupError":null,"nativeDetached":true,"unloads":1,"sessionReleased":true}
{"cancel":true,"serverSignalAborted":true,"clientRejected":"Error: cancel during final detach","clientIsError":null,"recordedOperation":"instrument_with_frida","deliveryCalls":["toEvidenceToolResult"],"recordedMarker":true,"cleanupError":null,"nativeDetached":true,"unloads":1,"sessionReleased":true}

The proxy observes the actual delivery method, delegating unchanged to the production delivery object. In both cases it records toEvidenceToolResult; the cancelled case should reach cancellation delivery instead. The recording callback is a simulated persistence boundary; this does not prove behavior of a persistent Evidence database.

Source-derived explanation: instrument checks the signal before closeSession, awaits it and returns ok without checking again. The MCP handler forwards the signal and delivers the resulting success. The manager's cleanup awaits unload/detach before releasing the session.

Typed outcome or recovery expected

For this successful-cleanup case, AnalysisCancelledError("instrument_with_frida") with the server-observed abort reason and partial Evidence containing the completed marker observation and successful cleanup facts. Preserve existing primary/cleanup failure precedence in other cases. Client-local cancellation alone does not establish the server's completion classification.

Analysis or suspected cause

The application service omits the post-close signal check required by the lifecycle contract. Its existing earlier cancellation path already constructs partial instrumentation Evidence, but the final-close path returns ordinary success. No repair was implemented or verified.

Additional context

Related reports #2544 and #3979 concern cancellation during persistent script loading, before this one-shot final session-close boundary. #4939 concerns a generic CLI finalizer. The observed trigger here occurs through MCP in the shared Frida one-shot service. Real Frida scheduling, CLI interruption output, cleanup failures, and stable-release execution remain untested.

Originally created by @coygeek on GitHub (Oct 18, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/5082 ## 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 Process capture or runtime observation ## Observed behavior A valid `instrument_with_frida` request cancelled while final native detach is pending still reaches ordinary success delivery after detach finishes. The server records completed instrumentation Evidence instead of classifying it as typed cancellation with partial Evidence. This was reproduced with the production MCP registration, service, manager and SDK in-memory transport, using simulated native Frida objects. The client promise rejects locally on cancellation; the defect is the server's subsequent success classification and record. No real target process or native Frida binding ran. ## Reproduction From a checkout at the source revision below with its locked dependencies prepared, save the following as `$WORK/probe.mts` in a disposable directory. Run `mkdir -p "$WORK/home"` and `HOME="$WORK/home" node --import tsx "$WORK/probe.mts"` from the repository root. The fixture uses no user configuration or external service. Node v26.12.0 was used. The first case is an uncancelled control. The second waits until final native detach starts, cancels, uses MCP ping as an ordering barrier, confirms the server's signal was aborted, and then releases detach. Assertions describe current behavior, including the defect. ```ts import assert from "node:assert/strict"; import { pathToFileURL } from "node:url"; const from = p => import(pathToFileURL(`${process.cwd()}/${p}`).href); const { Client, InMemoryTransport } = await from("node_modules/@modelcontextprotocol/client/dist/index.mjs"); const { McpServer } = await from("node_modules/@modelcontextprotocol/server/dist/index.mjs"); const { FridaInstrumentationManager } = await from("src/frida/FridaInstrumentationManager.ts"); const { registerFridaTools } = await from("src/server/registerFridaTools.ts"); const { createToolResultDelivery } = await from("src/server/toolResult.ts"); const { silentLogger } = await from("src/logger.ts"); async function probe(cancel) { let enter, release, observedSignal; const entered = new Promise(r => { enter = r; }); const held = new Promise(r => { release = r; }); let detached = false, unloads = 0; const nativeSession = { pid: 512, detached: { connect() {}, disconnect() {} }, isDetached: () => detached, async detach() { enter(); await held; detached = true; }, async createScript() { let destroyed = false, listener; return { get isDestroyed() { return destroyed; }, message: { connect(h) { listener = h; }, disconnect() { listener = undefined; } }, async load() { listener?.({ type: "send", payload: "marker" }, null); }, async unload() { destroyed = true; unloads++; }, }; }, }; const device = { id: "local", name: "Fixture", type: "local", async attach() { return nativeSession; }, async spawn() { return 512; }, async resume() {}, async enumerateProcesses() { return []; }, async getProcessByPid() { return { pid: 512, name: "Fixture" }; }, }; const manager = new FridaInstrumentationManager({ deviceManager: { async getDeviceById() { return device; }, async enumerateDevices() { return [device]; }, async addRemoteDevice() { return device; }, async removeRemoteDevice() {}, } }); const originalLoad = manager.loadScript.bind(manager); manager.loadScript = (id, source, signal) => { observedSignal = signal; return originalLoad(id, source, signal); }; let finishRecord; const recorded = new Promise(r => { finishRecord = r; }); const server = new McpServer({ name: "cleanup-probe", version: "1" }); const deliveryCalls = []; const delivery = new Proxy(createToolResultDelivery(undefined), { get(target, key) { const value = Reflect.get(target, key); return typeof value !== "function" ? value : (...args) => { deliveryCalls.push(key); return value.apply(target, args); }; }, }); registerFridaTools(server, manager, silentLogger, delivery, evidence => { finishRecord(evidence); return { ok: true, value: null }; }); const client = new Client({ name: "cleanup-probe", version: "1" }); const [a, b] = InMemoryTransport.createLinkedPair(); await server.connect(b); await client.connect(a); const controller = new AbortController(); try { const pending = client.callTool({ name: "instrument_with_frida", arguments: { mode: "attach", pid: 512, device_id: "local", source_kind: "inline", source: "send('marker')", duration_ms: 0, } }, { signal: controller.signal }).catch(e => ({ clientRejected: e.message })); await entered; assert.equal(detached, false); if (cancel) { controller.abort(new Error("cancel during final detach")); await client.ping(); } assert.equal(observedSignal.aborted, cancel); release(); const evidence = await recorded; const result = await pending; await client.ping(); assert.deepEqual(deliveryCalls, ["toEvidenceToolResult"]); if (!cancel) assert.notEqual(result.isError, true); assert.equal(detached, true); assert.equal(unloads, 1); assert.equal(evidence.operation, "instrument_with_frida"); assert.ok(evidence.normalized_result.messages.some(m => m.payload === "marker")); assert.equal(evidence.normalized_result.cleanup_error, null); assert.equal(manager.status(evidence.parameters.session_id), undefined); console.log(JSON.stringify({ cancel, serverSignalAborted: observedSignal.aborted, clientRejected: result.clientRejected ?? null, clientIsError: result.isError ?? null, recordedOperation: evidence.operation, deliveryCalls, recordedMarker: true, cleanupError: evidence.normalized_result.cleanup_error, nativeDetached: detached, unloads, sessionReleased: true })); } finally { release(); await client.close(); await server.close(); await manager.closeAll(); } } await probe(false); await probe(true); ``` ## Expected behavior After successful final cleanup, an aborted operation should reach typed cancellation delivery with the completed observation and actual cleanup facts preserved. The uncancelled control should continue to succeed. The [lifecycle contract](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/docs/tool-design.md#L101-L107) requires a signal check after the last awaited finalization step and retention of observations under typed cancellation. This case has no earlier failure or incomplete cleanup whose classification needs precedence. ## Impact and scope The controlled interleaving deterministically records an aborted one-shot observation as ordinary successful Evidence. Captured messages survive and the session is released, so the established impact is bounded completion/retention classification. Real-provider frequency and target-side consequences are NOT_ENOUGH_INFO; neither was measured. A contract-preserving caller workaround was not established. ## REA version Source SHA `3537e58c1ab29309f06eabf50bc52adba9817a0c`, package version 6.7.0, current main at verification. Relevant Frida source is identical to the `rea-agents-6.7.0` tag; the stable release runtime was not executed. ## Runtime and platform Node v26.12.0; macOS 27.0.1; arm64. The repository's exact pinned Node 24.18.0 was not exercised. Native Frida version/device compatibility: NOT_ENOUGH_INFO, simulated bindings only. ## Target and artifact context Simulated local attach session, synthetic PID 512, inline marker script, zero-duration observation. No real process was attached or executed. The production manager creates the session and script identities and releases both through its normal cleanup path. ## Interface MCP ## Earliest failure stage Cleanup, cancellation, or lifecycle ## Relevant output or evidence Final executed probe, exit code 0: ```jsonl {"cancel":false,"serverSignalAborted":false,"clientRejected":null,"clientIsError":null,"recordedOperation":"instrument_with_frida","deliveryCalls":["toEvidenceToolResult"],"recordedMarker":true,"cleanupError":null,"nativeDetached":true,"unloads":1,"sessionReleased":true} {"cancel":true,"serverSignalAborted":true,"clientRejected":"Error: cancel during final detach","clientIsError":null,"recordedOperation":"instrument_with_frida","deliveryCalls":["toEvidenceToolResult"],"recordedMarker":true,"cleanupError":null,"nativeDetached":true,"unloads":1,"sessionReleased":true} ``` The proxy observes the actual delivery method, delegating unchanged to the production delivery object. In both cases it records `toEvidenceToolResult`; the cancelled case should reach cancellation delivery instead. The recording callback is a simulated persistence boundary; this does not prove behavior of a persistent Evidence database. Source-derived explanation: [instrument](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/application/frida/FridaInstrumentationService.ts#L321-L353) checks the signal before `closeSession`, awaits it and returns `ok` without checking again. The [MCP handler](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/server/registerFridaTools.ts#L275-L319) forwards the signal and delivers the resulting success. The [manager's cleanup](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/frida/FridaInstrumentationManager.ts#L670-L736) awaits unload/detach before releasing the session. ## Typed outcome or recovery expected For this successful-cleanup case, `AnalysisCancelledError("instrument_with_frida")` with the server-observed abort reason and partial Evidence containing the completed marker observation and successful cleanup facts. Preserve existing primary/cleanup failure precedence in other cases. Client-local cancellation alone does not establish the server's completion classification. ## Analysis or suspected cause The application service omits the post-close signal check required by the lifecycle contract. Its [existing earlier cancellation path](https://github.com/morluto/rea/blob/3537e58c1ab29309f06eabf50bc52adba9817a0c/src/application/frida/FridaInstrumentationService.ts#L129-L179) already constructs partial instrumentation Evidence, but the final-close path returns ordinary success. No repair was implemented or verified. ## Additional context Related reports [#2544](https://github.com/morluto/rea/issues/2544) and [#3979](https://github.com/morluto/rea/issues/3979) concern cancellation during persistent script loading, before this one-shot final session-close boundary. [#4939](https://github.com/morluto/rea/issues/4939) concerns a generic CLI finalizer. The observed trigger here occurs through MCP in the shared Frida one-shot service. Real Frida scheduling, CLI interruption output, cleanup failures, and stable-release execution remain untested.

bsakaguchi closed this issue

2026-10-31 00:36:51 +00:00

bsakaguchi commented

2026-10-31 00:36:52 +00:00

Author

Owner

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

Fixed on main in 44b6ec97ee.

The one-shot service now checks cancellation after the final awaited close. Earlier cancellation and final-close cancellation use one pure error/Evidence builder with the already observed session and close result, so cleanup runs once. Partial Evidence retains messages, source identity, the exact server-observed abort cause, successful release or retained cleanup failure, and explicit resume observations. The consumer no longer infers REA resume from a displayed running state; it consumes the owning port's resume facts.

Registered MCP regression: before the fix, cancellation during a held native detach reached ordinary success (1 failed,38 passed). Final controls exercise cancellation on/off with detach success/failure, verify server delivery and recorded Evidence after client rejection, and retry retained detach without unloading the successful script again. Application controls also preserve prior load failures across cancellation before/during cleanup.

Validation on pinned Node24.18.0:124 tests passed in5 files across manager/service/CLI/registeredMCP; check and docs:check passed. Simulated native Frida handles only. The public gated test uses attach mode; spawn resume facts are covered separately at manager/service boundaries. No real target, real-provider frequency, native Linux/Windows, or performance improvement claim. The nearby resume-failure precedence defect is separately tracked as #5232.

@morluto commented on GitHub (Oct 19, 2026): Fixed on main in 44b6ec97ee65dba280e0d177545bcd63881287fd. The one-shot service now checks cancellation after the final awaited close. Earlier cancellation and final-close cancellation use one pure error/Evidence builder with the already observed session and close result, so cleanup runs once. Partial Evidence retains messages, source identity, the exact server-observed abort cause, successful release or retained cleanup failure, and explicit resume observations. The consumer no longer infers REA resume from a displayed running state; it consumes the owning port's resume facts. Registered MCP regression: before the fix, cancellation during a held native detach reached ordinary success (1 failed,38 passed). Final controls exercise cancellation on/off with detach success/failure, verify server delivery and recorded Evidence after client rejection, and retry retained detach without unloading the successful script again. Application controls also preserve prior load failures across cancellation before/during cleanup. Validation on pinned Node24.18.0:124 tests passed in5 files across manager/service/CLI/registeredMCP; check and docs:check passed. Simulated native Frida handles only. The public gated test uses attach mode; spawn resume facts are covered separately at manager/service boundaries. No real target, real-provider frequency, native Linux/Windows, or performance improvement claim. The nearby resume-failure precedence defect is separately tracked as #5232.

bsakaguchi referenced this issue

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

[PR #3119] [MERGED] fix(ci): check authored documentation code #5626

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

No description provided.

Rows
Columns