mirror of
https://github.com/morluto/rea.git
synced 2026-10-18 20:10:13 -05:00
fix(mcp): keep a snapshot open ahead of later calls on the same connection (#3661)
open_binary read snapshot_path before it entered the session's lifecycle order, so a current_document or close_binary dispatched after it could run first: the read returned Evidence for the previous target and the close left the snapshot target open. The session now loads the snapshot inside the open's own turn. Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
ce89706145
commit
73c72d5745
5 files changed
+92
-20
No files matched your search
@@ -216,12 +216,12 @@ export class BinarySession
|
||||
options: BinarySessionOpenOptions = {},
|
||||
): Promise<Result<BinaryTarget, AnalysisError>> {
|
||||
return this.#open(
|
||||
() =>
|
||||
(admitted) =>
|
||||
resolveSessionOpen({
|
||||
router: this.#providerRouter,
|
||||
current: this.#active,
|
||||
path,
|
||||
options,
|
||||
options: admitted,
|
||||
stagedSnapshotMatches: (target, profile) =>
|
||||
this.matchesSnapshot(target, profile),
|
||||
}),
|
||||
@@ -250,12 +250,33 @@ export class BinarySession
|
||||
}
|
||||
|
||||
#open(
|
||||
resolve: () => Promise<Result<ResolvedSessionOpen, AnalysisError>>,
|
||||
options: Pick<BinarySessionOpenOptions, "signal" | "snapshot">,
|
||||
resolve: (
|
||||
admitted: BinarySessionOpenOptions,
|
||||
) => Promise<Result<ResolvedSessionOpen, AnalysisError>>,
|
||||
options: BinarySessionOpenOptions,
|
||||
): Promise<Result<BinaryTarget, AnalysisError>> {
|
||||
return this.#serialize((admittedThrough) =>
|
||||
this.#openWithinTransition(admittedThrough, resolve, options),
|
||||
);
|
||||
return this.#serialize(async (admittedThrough) => {
|
||||
const admitted = await this.#withLoadedSnapshot(options);
|
||||
if (!admitted.ok) return admitted;
|
||||
return this.#openWithinTransition(
|
||||
admittedThrough,
|
||||
() => resolve(admitted.value),
|
||||
admitted.value,
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
/** Read the staged snapshot only after earlier lifecycle transitions have settled. */
|
||||
async #withLoadedSnapshot(
|
||||
options: BinarySessionOpenOptions,
|
||||
): Promise<Result<BinarySessionOpenOptions, AnalysisError>> {
|
||||
if (options.loadSnapshot === undefined) return ok(options);
|
||||
if (isAborted(options.signal))
|
||||
return err(new AnalysisCancelledError("open_binary"));
|
||||
const loaded = await options.loadSnapshot();
|
||||
if (!loaded.ok) return loaded;
|
||||
const { loadSnapshot: _loaded, ...rest } = options;
|
||||
return ok({ ...rest, snapshot: loaded.value });
|
||||
}
|
||||
|
||||
async #openWithinTransition(
|
||||
|
||||
@@ -19,6 +19,11 @@ import { err, type Result } from "../../domain/result.js";
|
||||
import type { SessionProviderRoute } from "./SessionProviderRouter.js";
|
||||
import { SessionProviderRouter } from "./SessionProviderRouter.js";
|
||||
|
||||
/** Read a staged snapshot once the open holds its place in the session order. */
|
||||
export type AnalysisSnapshotLoader = () => Promise<
|
||||
Result<AnalysisSnapshot, AnalysisError>
|
||||
>;
|
||||
|
||||
export interface BinarySessionOpenOptions {
|
||||
readonly signal?: AbortSignal;
|
||||
readonly targetKind?: BinaryTarget["kind"];
|
||||
@@ -26,6 +31,11 @@ export interface BinarySessionOpenOptions {
|
||||
readonly architecture?: BinaryArchitecture;
|
||||
readonly entryPoint?: string;
|
||||
readonly snapshot?: AnalysisSnapshot;
|
||||
/**
|
||||
* Load the snapshot inside the open's turn instead of before it, so calls
|
||||
* dispatched after the open cannot overtake its file read.
|
||||
*/
|
||||
readonly loadSnapshot?: AnalysisSnapshotLoader;
|
||||
readonly providerId?: AnalysisProviderSelector;
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import type { AnalysisProviderSelector } from "../../contracts/providerSelection
|
||||
import type { AnalysisError } from "../../domain/analysisErrorBase.js";
|
||||
import type { Result } from "../../domain/result.js";
|
||||
import type { EvidenceIntegrityError } from "../../domain/evidenceErrors.js";
|
||||
import type { AnalysisSnapshotLoader } from "./BinarySessionOpen.js";
|
||||
import type { WorkflowSnapshotRecordInput } from "./BinarySessionRecords.js";
|
||||
|
||||
import type {
|
||||
@@ -71,6 +72,7 @@ export interface BinarySessionPort
|
||||
readonly architecture?: BinaryArchitecture;
|
||||
readonly entryPoint?: string;
|
||||
readonly snapshot?: AnalysisSnapshot;
|
||||
readonly loadSnapshot?: AnalysisSnapshotLoader;
|
||||
readonly providerId?: AnalysisProviderSelector;
|
||||
},
|
||||
): Promise<Result<BinaryTarget, AnalysisError>>;
|
||||
|
||||
@@ -12,7 +12,6 @@ import { captureProcessScenario } from "../process/capture/ProcessHarness.js";
|
||||
import { ProcessCaptureResourceScope } from "../process/capture/ProcessCaptureLifecycle.js";
|
||||
import { ProcessCaptureError } from "../process/capture/ProcessCaptureError.js";
|
||||
import { toolContract } from "../contracts/toolContracts.js";
|
||||
import type { AnalysisSnapshot } from "../domain/analysisSnapshot.js";
|
||||
import { binaryTargetSummary } from "../application/binary/BinaryTargetSummary.js";
|
||||
import { UnknownRegistryError } from "../domain/unknownRegistryError.js";
|
||||
import type { AnalysisError } from "../domain/analysisErrorBase.js";
|
||||
@@ -178,17 +177,7 @@ const registerOpenLifecycleTool = (
|
||||
openContract.name,
|
||||
toolRegistrationOptions(openContract),
|
||||
async (input, context) => {
|
||||
let snapshot: AnalysisSnapshot | undefined;
|
||||
if (input.snapshot_path !== undefined) {
|
||||
const loaded = await readAnalysisSnapshot(
|
||||
input.snapshot_path,
|
||||
context.mcpReq.signal,
|
||||
artifactResources,
|
||||
);
|
||||
if (!loaded.ok)
|
||||
return server.delivery.toCallToolResult(loaded, openContract);
|
||||
snapshot = loaded.value;
|
||||
}
|
||||
const snapshotPath = input.snapshot_path;
|
||||
const opened = await logToolExecution(logger, openContract.name, () =>
|
||||
session.open(input.path, {
|
||||
signal: context.mcpReq.signal,
|
||||
@@ -202,7 +191,18 @@ const registerOpenLifecycleTool = (
|
||||
...(input.provider_id === undefined
|
||||
? {}
|
||||
: { providerId: input.provider_id }),
|
||||
...(snapshot === undefined ? {} : { snapshot }),
|
||||
...(snapshotPath === undefined
|
||||
? {}
|
||||
: {
|
||||
// Read inside the open's turn: calls issued after it must not
|
||||
// overtake a file read that precedes its place in the order.
|
||||
loadSnapshot: () =>
|
||||
readAnalysisSnapshot(
|
||||
snapshotPath,
|
||||
context.mcpReq.signal,
|
||||
artifactResources,
|
||||
),
|
||||
}),
|
||||
}),
|
||||
);
|
||||
return opened.ok
|
||||
|
||||
@@ -481,6 +481,45 @@ describe("target-free MCP workflow", () => {
|
||||
}, 10_000);
|
||||
});
|
||||
|
||||
describe("MCP snapshot open ordering", () => {
|
||||
it("keeps a snapshot open ahead of later reads and closes on the same connection", async () => {
|
||||
directory = await createTestTempDirectory("rea-mcp-snapshot-open-order-");
|
||||
const { mcp, first, second } = await createSessionMcpHarness(
|
||||
directory,
|
||||
provider,
|
||||
resources,
|
||||
);
|
||||
const snapshotPath = join(directory, "analysis.json");
|
||||
await mcp.callTool({ name: "open_binary", arguments: { path: second } });
|
||||
expect(
|
||||
(
|
||||
await mcp.callTool({
|
||||
name: "close_binary",
|
||||
arguments: { snapshot_path: snapshotPath },
|
||||
})
|
||||
).isError,
|
||||
).not.toBe(true);
|
||||
await mcp.callTool({ name: "open_binary", arguments: { path: first } });
|
||||
|
||||
// Dispatched without awaiting, like a client issuing one turn's calls.
|
||||
const opening = mcp.callTool({
|
||||
name: "open_binary",
|
||||
arguments: { path: second, snapshot_path: snapshotPath },
|
||||
});
|
||||
const reading = mcp.callTool({ name: "current_document", arguments: {} });
|
||||
const closing = mcp.callTool({ name: "close_binary", arguments: {} });
|
||||
|
||||
expect((await opening).isError, JSON.stringify(await opening)).not.toBe(
|
||||
true,
|
||||
);
|
||||
expect(text(await reading)).toContain("second.hop");
|
||||
expect((await closing).isError).not.toBe(true);
|
||||
expect(
|
||||
text(await mcp.callTool({ name: "binary_session", arguments: {} })),
|
||||
).toContain('"open":false');
|
||||
}, 10_000);
|
||||
});
|
||||
|
||||
describe("session filesystem path boundaries over MCP", () => {
|
||||
it("rejects relative snapshot and export paths with an absolute-path error", async () => {
|
||||
directory = await createTestTempDirectory("rea-mcp-path-boundary-");
|
||||
|
||||
Reference in new issue
Block a user