morluto/rea

Fork 0

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

[GH-ISSUE #6216] [Bug] open_binary returns an untyped error when the selected snapshot descriptor cannot close #3784

New issue

Closed

opened 2026-10-31 00:44:01 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

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

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

A registered MCP open_binary request with path: "/bin/ls" and a valid snapshot_path returns an SDK text-only error when the snapshot descriptor's final close and immediate retry reject. The same snapshot imports successfully without the injected fault. The returned error has no typed analysis-error classification, resource list or recovery instructions.

With close denied, open_binary returns isError: true and a single text content item, Regular file cleanup failed for <selected snapshot path>, without structuredContent. There are two snapshot close attempts: finalization and the retained-owner retry. binary_session then reports open: false, cleanup_pending: false, and close_binary returns result: null. Neither call releases or retries this descriptor. A subsequent server shutdown rejects with AggregateError: REA session cleanup failed on the third close attempt. Restoring the real close and retrying server shutdown succeeds on the fourth attempt.

Reproduction

Use the prepared source checkout with its locked dependencies. Save the full test below as src/server/snapshotClose.repro.test.ts. Set REA_VERIFY_ROOT to an existing writable disposable directory; put HOME, DSH_HOME, XDG_CONFIG_HOME, and XDG_CACHE_HOME in that directory before running. Run:

node node_modules/vitest/vitest.mjs run src/server/snapshotClose.repro.test.ts --maxWorkers=1

The same registered MCP workflow reproduced the primary failure in three runs. The initial run failed only because the expected shutdown aggregate message was incorrect; two subsequent runs passed, with the final fixture below also asserting the successful control target path and open session status; it passed its assertions for the current bug, valid-snapshot control and exact-descriptor recovery. The published filename replaces the local temporary verification name. This test asserts the current faulty response; it is an executable reproduction, not a passing restoration test. Delete the test after running it.

import { Client, InMemoryTransport } from "@modelcontextprotocol/client";
import { mkdtemp, rm, writeFile } from "node:fs/promises";
import { join } from "node:path";
import { expect, it, vi } from "vitest";
import { createServer } from "./createServer.js";
import { createCacheProvider, createTestBinarySession } from "../../tests/fixtures/binarySession.js";

const fault = vi.hoisted(() => ({ path: "", denied: false, attempts: 0 }));
vi.mock("node:fs/promises", async (importOriginal) => {
  const fs = await importOriginal<typeof import("node:fs/promises")>();
  return {
    ...fs,
    open: async (...args: Parameters<typeof fs.open>) => {
      const handle = await fs.open(...args);
      if (String(args[0]) !== fault.path) return handle;
      return new Proxy(handle, {
        get(target, key) {
          if (key === "close") return async () => {
            fault.attempts++;
            if (fault.denied) throw Object.assign(new Error("controlled snapshot close EIO"), { code: "EIO" });
            return target.close();
          };
          const value = Reflect.get(target, key, target);
          return typeof value === "function" ? value.bind(target) : value;
        },
      });
    },
  };
});

it("observes an untyped snapshot cleanup failure through registered MCP", async () => {
  const root = await mkdtemp(join(process.env.REA_VERIFY_ROOT!, "snapshot-"));
  fault.path = join(root, "snapshot.json");
  const session = createTestBinarySession(createCacheProvider([]));
  const server = createServer({ kind: "session", session });
  const client = new Client({ name: "snapshot-close-repro", version: "1" });
  const [ct, st] = InMemoryTransport.createLinkedPair();
  try {
    await server.connect(st);
    await client.connect(ct);
    const open = (snapshot = false) => client.callTool({ name: "open_binary", arguments: { path: "/bin/ls", ...(snapshot ? { snapshot_path: fault.path } : {}) } });
    const close = () => client.callTool({ name: "close_binary", arguments: {} });
    expect((await open()).isError).not.toBe(true);
    const exported = session.exportAnalysisSnapshot();
    if (!exported.ok) throw exported.error;
    await writeFile(fault.path, JSON.stringify(exported.value));
    expect((await close()).isError).not.toBe(true);
    const control = await open(true);
    expect(control.isError).not.toBe(true);
    expect(control.structuredContent).toHaveProperty("result.path", "/bin/ls");
    expect((await client.callTool({ name: "binary_session", arguments: {} })).structuredContent).toMatchObject({ result: { open: true } });
    expect((await close()).isError).not.toBe(true);
    fault.attempts = 0;
    fault.denied = true;
    const opened = await open(true);
    const status = await client.callTool({ name: "binary_session", arguments: {} });
    const closed = await close();
    console.log(JSON.stringify({ phase: "denied", opened, status: { open: status.structuredContent?.result?.open, cleanup_pending: status.structuredContent?.result?.cleanup_pending }, closed, attempts: fault.attempts }));
    expect(fault.attempts).toBe(2);
    expect(opened.isError).toBe(true);
    expect(opened.structuredContent).toBeUndefined();
    expect(opened.content).toEqual([{ type: "text", text: `Regular file cleanup failed for ${fault.path}` }]);
    expect(status.structuredContent).toMatchObject({ result: { open: false, cleanup_pending: false } });
    expect(closed.structuredContent).toMatchObject({ result: null });
    let shutdown: unknown;
    try { await server.close(); } catch (cause) { shutdown = cause; }
    console.log(JSON.stringify({ phase: "shutdown", error: shutdown instanceof Error ? shutdown.message : String(shutdown), attempts: fault.attempts }));
    expect(shutdown).toBeInstanceOf(AggregateError);
    expect((shutdown as Error).message).toBe("REA session cleanup failed");
    fault.denied = false;
    await server.close();
    console.log(JSON.stringify({ phase: "recovered", shutdownSucceeded: true, attempts: fault.attempts }));
  } finally {
    fault.denied = false;
    await client.close();
    await server.close();
    await session.close();
    await rm(root, { recursive: true, force: true });
  }
}, 20000);

Expected behavior

An unresolved descriptor-close failure during a selected snapshot read should reach the caller through REA's typed analysis-error result, preserving the remaining resource and the owner's recovery instructions. The documented cleanup_incomplete classification describes this condition; a successful target open must not be implied. The filesystem owner already retains the descriptor, so that ownership should survive error projection instead of becoming only an SDK text error.

The lifecycle contract requires preserving partial observations, remaining resources, recovery instructions and exact descriptor ownership. The error-model rule requires expected failures to use the tagged algebra and Result; the cleanup classification identifies resources whose release remains unverified.

Impact and scope

P2: this bounded filesystem-failure path removes the typed cleanup information needed by automation and leaves a retained descriptor undiscoverable in the observed session status and close result. Successful snapshot imports were observed before injection, and the same retained descriptor was released after close was restored. No input corruption or target execution was observed. This was controlled EIO injection, not a naturally occurring OS fault; natural frequency, CLI output, stdio MCP and real-engine behavior were not measured. No practical user workaround was established: close_binary did not retry the resource, and the recovery control required restoring the close boundary and invoking server shutdown.

REA version

Source commit c465ff5a81 (main)

Runtime and platform

Node.js v26.12.0; macOS 27.0.1 arm64; locked Vitest 5.0.3 and MCP SDK 2.3.1. The repository pins Node 24.18.0; that runtime was not exercised. No Hopper or Ghidra engine was used.

Target and artifact context

/bin/ls read as a native artifact through the repository's simulated analysis provider, plus a matching valid persisted analysis snapshot exported from that session. Real snapshot FileHandles; injected EIO rejection at close only. No target execution.

Interface

MCP

Earliest failure stage

Cleanup, cancellation, or lifecycle

Relevant output or evidence

Relevant output from the final run, reduced to the fields that establish the failure and with the fixture path replaced by a neutral placeholder:

{"opened":{"isError":true,"content":[{"type":"text","text":"Regular file cleanup failed for <snapshot.json>"}]},"structuredContentPresent":false,"status":{"open":false,"cleanup_pending":false},"closed":{"result":null},"snapshotCloseAttempts":2}
{"shutdownError":"REA session cleanup failed","snapshotCloseAttempts":3}
{"shutdownSucceededAfterCloseRestored":true,"snapshotCloseAttempts":4}

The final focused run passed one test. The failure injection changes only the real snapshot handle's close() method; reads, file-state checks, ownership, retry, tool registration, delivery and SDK transport remain production code. The non-faulted control exports and successfully reopens a matching snapshot using REA's existing simulated-provider fixture. This establishes the file/MCP lifecycle, not real-provider compatibility.

Typed outcome or recovery expected

A typed cleanup_incomplete analysis result naming the unresolved snapshot resource and preserving its cleanup owner's recovery action. Retain the exact descriptor until release is confirmed. binary_session and close_binary currently do not expose or retry this server-artifact-scope resource in the observed case; report truthful recovery through the owning interface rather than assuming session closure released it.

Analysis or suspected cause

Source-derived: the shared reader retries the retained owner and rethrows when cleanup still fails. The snapshot loader explicitly rethrows that RegularFileCleanupFailure, the session awaits the loader without translating it, and the registered MCP handler awaits session.open before normal typed delivery. This explains the observed SDK text-only response. No fix was implemented or tested.

Additional context

Related reports were read: #5968 concerns cancellation operation attribution in this same snapshot loader; #6077 concerns typed cleanup in an Evidence-file analysis-view consumer; #6065 concerns managed-snapshot cancellation; #6047 and #4502 concern different snapshot producers; #4910 concerns configuration helpers without an explicit resource scope. The neighboring snapshot-publication report points to PR #1960; its relevant session and registration diff was also inspected, and it leaves this snapshot-loader cleanup rejection unadapted. Each has a different failing contract or consumer, and no duplicate of this open_binary snapshot-import failure was identified.

Fetched upstream main ce5bc8cb0149c230e571126e2e7d9f43c86a823d has no changes since the tested baseline in the cited snapshot loader, shared reader, session or MCP registration, nor the artifact scope/server cleanup files. That newer revision was inspected, not executed. The broader open_binary search encountered a search-rate-limit response; nine other open/closed issue and PR keyword searches and the related issue reads completed. No regression range is established.

#6215 reports a similar unretained descriptor after a failed final close, but for the target file descriptor during target admission; it has a separate owner and was verified independently.

Originally created by @coygeek on GitHub (Oct 22, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/6216 ### 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 A registered MCP `open_binary` request with `path: "/bin/ls"` and a valid `snapshot_path` returns an SDK text-only error when the snapshot descriptor's final close and immediate retry reject. The same snapshot imports successfully without the injected fault. The returned error has no typed analysis-error classification, resource list or recovery instructions. With close denied, `open_binary` returns `isError: true` and a single text content item, `Regular file cleanup failed for <selected snapshot path>`, without `structuredContent`. There are two snapshot close attempts: finalization and the retained-owner retry. `binary_session` then reports `open: false, cleanup_pending: false`, and `close_binary` returns `result: null`. Neither call releases or retries this descriptor. A subsequent server shutdown rejects with `AggregateError: REA session cleanup failed` on the third close attempt. Restoring the real close and retrying server shutdown succeeds on the fourth attempt. ### Reproduction Use the prepared source checkout with its locked dependencies. Save the full test below as `src/server/snapshotClose.repro.test.ts`. Set `REA_VERIFY_ROOT` to an existing writable disposable directory; put `HOME`, `DSH_HOME`, `XDG_CONFIG_HOME`, and `XDG_CACHE_HOME` in that directory before running. Run: ```sh node node_modules/vitest/vitest.mjs run src/server/snapshotClose.repro.test.ts --maxWorkers=1 ``` The same registered MCP workflow reproduced the primary failure in three runs. The initial run failed only because the expected shutdown aggregate message was incorrect; two subsequent runs passed, with the final fixture below also asserting the successful control target path and open session status; it passed its assertions for the current bug, valid-snapshot control and exact-descriptor recovery. The published filename replaces the local temporary verification name. This test asserts the current faulty response; it is an executable reproduction, not a passing restoration test. Delete the test after running it. ```ts import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { expect, it, vi } from "vitest"; import { createServer } from "./createServer.js"; import { createCacheProvider, createTestBinarySession } from "../../tests/fixtures/binarySession.js"; const fault = vi.hoisted(() => ({ path: "", denied: false, attempts: 0 })); vi.mock("node:fs/promises", async (importOriginal) => { const fs = await importOriginal<typeof import("node:fs/promises")>(); return { ...fs, open: async (...args: Parameters<typeof fs.open>) => { const handle = await fs.open(...args); if (String(args[0]) !== fault.path) return handle; return new Proxy(handle, { get(target, key) { if (key === "close") return async () => { fault.attempts++; if (fault.denied) throw Object.assign(new Error("controlled snapshot close EIO"), { code: "EIO" }); return target.close(); }; const value = Reflect.get(target, key, target); return typeof value === "function" ? value.bind(target) : value; }, }); }, }; }); it("observes an untyped snapshot cleanup failure through registered MCP", async () => { const root = await mkdtemp(join(process.env.REA_VERIFY_ROOT!, "snapshot-")); fault.path = join(root, "snapshot.json"); const session = createTestBinarySession(createCacheProvider([])); const server = createServer({ kind: "session", session }); const client = new Client({ name: "snapshot-close-repro", version: "1" }); const [ct, st] = InMemoryTransport.createLinkedPair(); try { await server.connect(st); await client.connect(ct); const open = (snapshot = false) => client.callTool({ name: "open_binary", arguments: { path: "/bin/ls", ...(snapshot ? { snapshot_path: fault.path } : {}) } }); const close = () => client.callTool({ name: "close_binary", arguments: {} }); expect((await open()).isError).not.toBe(true); const exported = session.exportAnalysisSnapshot(); if (!exported.ok) throw exported.error; await writeFile(fault.path, JSON.stringify(exported.value)); expect((await close()).isError).not.toBe(true); const control = await open(true); expect(control.isError).not.toBe(true); expect(control.structuredContent).toHaveProperty("result.path", "/bin/ls"); expect((await client.callTool({ name: "binary_session", arguments: {} })).structuredContent).toMatchObject({ result: { open: true } }); expect((await close()).isError).not.toBe(true); fault.attempts = 0; fault.denied = true; const opened = await open(true); const status = await client.callTool({ name: "binary_session", arguments: {} }); const closed = await close(); console.log(JSON.stringify({ phase: "denied", opened, status: { open: status.structuredContent?.result?.open, cleanup_pending: status.structuredContent?.result?.cleanup_pending }, closed, attempts: fault.attempts })); expect(fault.attempts).toBe(2); expect(opened.isError).toBe(true); expect(opened.structuredContent).toBeUndefined(); expect(opened.content).toEqual([{ type: "text", text: `Regular file cleanup failed for ${fault.path}` }]); expect(status.structuredContent).toMatchObject({ result: { open: false, cleanup_pending: false } }); expect(closed.structuredContent).toMatchObject({ result: null }); let shutdown: unknown; try { await server.close(); } catch (cause) { shutdown = cause; } console.log(JSON.stringify({ phase: "shutdown", error: shutdown instanceof Error ? shutdown.message : String(shutdown), attempts: fault.attempts })); expect(shutdown).toBeInstanceOf(AggregateError); expect((shutdown as Error).message).toBe("REA session cleanup failed"); fault.denied = false; await server.close(); console.log(JSON.stringify({ phase: "recovered", shutdownSucceeded: true, attempts: fault.attempts })); } finally { fault.denied = false; await client.close(); await server.close(); await session.close(); await rm(root, { recursive: true, force: true }); } }, 20000); ``` ### Expected behavior An unresolved descriptor-close failure during a selected snapshot read should reach the caller through REA's typed analysis-error result, preserving the remaining resource and the owner's recovery instructions. The documented `cleanup_incomplete` classification describes this condition; a successful target open must not be implied. The filesystem owner already retains the descriptor, so that ownership should survive error projection instead of becoming only an SDK text error. The [lifecycle contract](https://github.com/morluto/rea/blob/c465ff5a814b8e1a386fc547cbcd94df5c35f086/docs/tool-design.md#L132) requires preserving partial observations, remaining resources, recovery instructions and exact descriptor ownership. The [error-model rule](https://github.com/morluto/rea/blob/c465ff5a814b8e1a386fc547cbcd94df5c35f086/AGENTS.md#L31) requires expected failures to use the tagged algebra and Result; the [cleanup classification](https://github.com/morluto/rea/blob/c465ff5a814b8e1a386fc547cbcd94df5c35f086/docs/cli.md#L226) identifies resources whose release remains unverified. ### Impact and scope P2: this bounded filesystem-failure path removes the typed cleanup information needed by automation and leaves a retained descriptor undiscoverable in the observed session status and close result. Successful snapshot imports were observed before injection, and the same retained descriptor was released after close was restored. No input corruption or target execution was observed. This was controlled EIO injection, not a naturally occurring OS fault; natural frequency, CLI output, stdio MCP and real-engine behavior were not measured. No practical user workaround was established: `close_binary` did not retry the resource, and the recovery control required restoring the close boundary and invoking server shutdown. ### REA version Source commit c465ff5a814b8e1a386fc547cbcd94df5c35f086 (main) ### Runtime and platform Node.js v26.12.0; macOS 27.0.1 arm64; locked Vitest 5.0.3 and MCP SDK 2.3.1. The repository pins Node 24.18.0; that runtime was not exercised. No Hopper or Ghidra engine was used. ### Target and artifact context `/bin/ls` read as a native artifact through the repository's simulated analysis provider, plus a matching valid persisted analysis snapshot exported from that session. Real snapshot FileHandles; injected EIO rejection at close only. No target execution. ### Interface MCP ### Earliest failure stage Cleanup, cancellation, or lifecycle ### Relevant output or evidence Relevant output from the final run, reduced to the fields that establish the failure and with the fixture path replaced by a neutral placeholder: ```json {"opened":{"isError":true,"content":[{"type":"text","text":"Regular file cleanup failed for <snapshot.json>"}]},"structuredContentPresent":false,"status":{"open":false,"cleanup_pending":false},"closed":{"result":null},"snapshotCloseAttempts":2} {"shutdownError":"REA session cleanup failed","snapshotCloseAttempts":3} {"shutdownSucceededAfterCloseRestored":true,"snapshotCloseAttempts":4} ``` The final focused run passed one test. The failure injection changes only the real snapshot handle's `close()` method; reads, file-state checks, ownership, retry, tool registration, delivery and SDK transport remain production code. The non-faulted control exports and successfully reopens a matching snapshot using REA's existing simulated-provider fixture. This establishes the file/MCP lifecycle, not real-provider compatibility. ### Typed outcome or recovery expected A typed `cleanup_incomplete` analysis result naming the unresolved snapshot resource and preserving its cleanup owner's recovery action. Retain the exact descriptor until release is confirmed. `binary_session` and `close_binary` currently do not expose or retry this server-artifact-scope resource in the observed case; report truthful recovery through the owning interface rather than assuming session closure released it. ### Analysis or suspected cause Source-derived: [the shared reader](https://github.com/morluto/rea/blob/c465ff5a814b8e1a386fc547cbcd94df5c35f086/src/application/RegularFileRead.ts#L183) retries the retained owner and rethrows when cleanup still fails. [The snapshot loader](https://github.com/morluto/rea/blob/c465ff5a814b8e1a386fc547cbcd94df5c35f086/src/application/binary/AnalysisSnapshotFiles.ts#L79) explicitly rethrows that RegularFileCleanupFailure, [the session](https://github.com/morluto/rea/blob/c465ff5a814b8e1a386fc547cbcd94df5c35f086/src/application/binary/BinarySession.ts#L500) awaits the loader without translating it, and [the registered MCP handler](https://github.com/morluto/rea/blob/c465ff5a814b8e1a386fc547cbcd94df5c35f086/src/server/registerSessionTools.ts#L179) awaits session.open before normal typed delivery. This explains the observed SDK text-only response. No fix was implemented or tested. ### Additional context Related reports were read: #5968 concerns cancellation operation attribution in this same snapshot loader; #6077 concerns typed cleanup in an Evidence-file analysis-view consumer; #6065 concerns managed-snapshot cancellation; #6047 and #4502 concern different snapshot producers; #4910 concerns configuration helpers without an explicit resource scope. The neighboring snapshot-publication report points to PR #1960; its relevant session and registration diff was also inspected, and it leaves this snapshot-loader cleanup rejection unadapted. Each has a different failing contract or consumer, and no duplicate of this `open_binary` snapshot-import failure was identified. Fetched upstream main `ce5bc8cb0149c230e571126e2e7d9f43c86a823d` has no changes since the tested baseline in the cited snapshot loader, shared reader, session or MCP registration, nor the artifact scope/server cleanup files. That newer revision was inspected, not executed. The broader `open_binary` search encountered a search-rate-limit response; nine other open/closed issue and PR keyword searches and the related issue reads completed. No regression range is established. #6215 reports a similar unretained descriptor after a failed final close, but for the target file descriptor during target admission; it has a separate owner and was verified independently.

bsakaguchi closed this issue

2026-10-31 00:44:02 +00:00

bsakaguchi referenced this issue

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

[PR #3807] [MERGED] feat(il2cpp): inventory Unity IL2CPP global-metadata images, types and methods #5913

bsakaguchi referenced this issue

2026-11-01 21:43:47 +00:00

[PR #5326] [MERGED] feat(il2cpp): read member names, string literals and metadata versions 24 and 27 #6881