From a5edab8fbfe169965487cb90dd2ec821cd7716dc Mon Sep 17 00:00:00 2001 From: link1345 Date: Fri, 2 Oct 2026 15:16:33 +0900 Subject: [PATCH] fix(mcp): retain terminal action outcome on disconnect --- packages/mcp/README.md | 22 +++ packages/mcp/src/index.ts | 98 ++++++++++--- packages/mcp/test/action-outcome.test.ts | 172 +++++++++++++++++++++++ 3 files changed, 274 insertions(+), 18 deletions(-) create mode 100644 packages/mcp/test/action-outcome.test.ts diff --git a/packages/mcp/README.md b/packages/mcp/README.md index 6bd33eb7..0ee93366 100644 --- a/packages/mcp/README.md +++ b/packages/mcp/README.md @@ -33,6 +33,28 @@ AI clients can inspect the tree and invoke all protocol v1 semantic actions: When the bridge returns a `requestId`, action tools poll the correlated host completion event. Enqueue acceptance alone is not reported as completion. +Semantic action tool failures retain `isError: true`. Transport failures also +include `outcome`, `stage`, and `requestSent` in the JSON text content: + +- `not_sent` / `before_send`: the action was not submitted to the WebSocket, + including failure to connect during the UI preflight. +- `completion_unconfirmed` / `awaiting_receipt`: the action was submitted, but + its acceptance response was not obtained. `bridgeCommandId` identifies the + submitted bridge command; a host `requestId` is not invented. +- `completion_unconfirmed` / `awaiting_completion`: acceptance supplied a + `requestId`, but the correlated completion could not be obtained. Both IDs + are retained when available. +- `completion_unconfirmed` / `awaiting_observation`: a legacy bridge returned + a null receipt, but the subsequent UI observation could not be obtained. + +Submission means the WebSocket send returned; it does not prove host receipt or +execution. Completion polling and legacy post-action observation stay on that +connection and stop on disconnect. A normal close, game log, or process exit +code (including zero) does not confirm the operation. Do not automatically +retry an unconfirmed action, particularly an exit action. Successful correlated +completion, explicit bridge rejection, and the legacy null-receipt success +format remain compatible. + World tools are read-only and use the observation profile fixed by the host. Set `GUA_OBSERVATION_PROFILE=player` on the native host for player-facing MCP use; tool arguments cannot elevate it to debug. `find_world_objects` accepts ID, diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index e7462c2f..bfff9318 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -108,6 +108,29 @@ type ToolResult = { }; interface GuaActionReceipt { requestId: number } +type ActionFailureStage = "before_send" | "awaiting_receipt" | "awaiting_completion" | "awaiting_observation"; + +export class GuaActionOutcomeError extends Error { + constructor( + readonly stage: ActionFailureStage, + readonly requestSent: boolean, + readonly requestId?: number, + readonly bridgeCommandId?: number, + ) { + super(requestSent + ? "Gua action was sent, but completion could not be confirmed. Do not retry automatically." + : "Gua action was not sent to the bridge."); + } + + diagnostics(): Record { + return compactResult({ error: this.message, + outcome: this.requestSent ? "completion_unconfirmed" : "not_sent", + stage: this.stage, requestSent: this.requestSent, + requestId: this.requestId, bridgeCommandId: this.bridgeCommandId }); + } +} + +class BridgeRejectionError extends Error {} interface GuaActionEvent { requestId: number; action: number; @@ -505,6 +528,7 @@ async function callTool(params: unknown, bridge: GuaBridgeClient, automation: Gu const result = await executeTool(name, isRecord(params.arguments) ? params.arguments : {}, bridge, automation, signal); return textResult(result); } catch (error) { + if (error instanceof GuaActionOutcomeError) return textResult(error.diagnostics(), true); return textResult({ error: (error as Error).message }, true); } } @@ -798,14 +822,36 @@ async function performAndRecord( timeoutMs = 10000, signal?: AbortSignal, ): Promise<{ ok: true; requestId?: number; completion?: GuaActionEvent }> { - const before = await bridge.getUiTree(); + let before: GuaUiTree; + try { before = await bridge.getUiTree(); } + catch (error) { + if (error instanceof RpcFailure) throw error; + throw new GuaActionOutcomeError("before_send", false); + } throwIfAborted(signal); - const receipt = await bridge.performAction(input); - const completion = receipt === null ? undefined : await bridge.waitForAction(receipt.requestId, timeoutMs); + let sent: { socket: WebSocket; id: number } | undefined; + let receipt: GuaActionReceipt | null; + try { + receipt = await bridge.performAction(input, (socket, id) => { sent = { socket, id }; }); + } catch (error) { + if (error instanceof BridgeRejectionError || error instanceof RpcFailure) throw error; + throw new GuaActionOutcomeError(sent ? "awaiting_receipt" : "before_send", sent !== undefined, undefined, sent?.id); + } + if (receipt !== null && (!isRecord(receipt) || !Number.isSafeInteger(receipt.requestId) || receipt.requestId <= 0)) { + throw new GuaActionOutcomeError("awaiting_receipt", true, undefined, sent?.id); + } + let completion: GuaActionEvent | undefined; + let after: GuaUiTree | undefined; + try { + completion = receipt === null ? undefined : await bridge.waitForAction(receipt.requestId, timeoutMs, sent?.socket); + if (completion === undefined) after = await bridge.getUiTree(sent?.socket); + } catch (error) { + if (error instanceof RpcFailure) throw error; + throw new GuaActionOutcomeError(receipt === null ? "awaiting_observation" : "awaiting_completion", true, receipt?.requestId, sent?.id); + } if (completion?.succeeded === false) { throw new Error(`Gua action ${input.action} failed with error ${completion.error}.`); } - const after = completion === undefined ? await bridge.getUiTree() : undefined; automation?.recordAction({ action: input.action, requestId: receipt?.requestId, @@ -1075,8 +1121,8 @@ export class GuaBridgeClient { ) { } - async getUiTree(): Promise { - return this.request({ type: "get_ui_tree" }); + async getUiTree(ownerSocket?: WebSocket): Promise { + return this.request({ type: "get_ui_tree" }, this.requestTimeoutMs, undefined, ownerSocket); } async getWorldObjectTree(): Promise { @@ -1195,7 +1241,7 @@ export class GuaBridgeClient { throw new Error("Timed out waiting for Gua clock run_for host completion."); } - async performAction(input: SemanticActionInput): Promise { + async performAction(input: SemanticActionInput, onSent?: (socket: WebSocket, id: number) => void): Promise { const type = actionCommandType(input.action); return this.request(compactResult({ type, @@ -1208,15 +1254,22 @@ export class GuaBridgeClient { deltaY: input.deltaY, scrollUnit: input.scrollUnit, sensitive: input.sensitive, - }) as BridgeCommandInput); + }) as BridgeCommandInput, this.requestTimeoutMs, undefined, undefined, onSent); } - async waitForAction(requestId: number, timeoutMs: number): Promise { - const startedAt = Date.now(); - while (Date.now() - startedAt <= timeoutMs) { - const event = await this.request({ type: "poll_events", requestId }); - if (event !== null) return event; - await sleep(25); + async waitForAction(requestId: number, timeoutMs: number, ownerSocket = this.socket): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (ownerSocket === null) throw new Error("Gua action connection is inactive."); + const event = await this.request({ type: "poll_events", requestId }, + Math.min(this.requestTimeoutMs, deadline - Date.now()), undefined, ownerSocket); + if (event !== null) { + if (!isRecord(event) || event.requestId !== requestId || typeof event.succeeded !== "boolean") { + throw new Error("Invalid correlated Gua action completion."); + } + return event; + } + await sleep(Math.min(25, Math.max(0, deadline - Date.now()))); } throw new Error(`Timed out waiting for Gua action request ${requestId}.`); } @@ -1250,11 +1303,11 @@ export class GuaBridgeClient { } private async request(command: BridgeCommandInput, timeoutMs = this.requestTimeoutMs, - signal?: AbortSignal, ownerSocket?: WebSocket): Promise { + signal?: AbortSignal, ownerSocket?: WebSocket, onSent?: (socket: WebSocket, id: number) => void): Promise { throwIfAborted(signal); const deadline = Date.now() + timeoutMs; const socket = ownerSocket ?? await this.connectForRequest(command.type, timeoutMs, signal); - if (ownerSocket && (socket !== this.socket || socket.readyState !== WebSocket.OPEN)) throw new Error("Observe subscription belongs to an inactive connection."); + if (ownerSocket && (socket !== this.socket || socket.readyState !== WebSocket.OPEN)) throw new Error("Gua request belongs to an inactive connection."); throwIfAborted(signal); const remainingTimeoutMs = deadline - Date.now(); if (remainingTimeoutMs <= 0) { @@ -1295,7 +1348,16 @@ export class GuaBridgeClient { }); signal?.addEventListener("abort", aborted, { once: true }); - socket.send(JSON.stringify(payload)); + try { + if (socket.readyState !== WebSocket.OPEN) throw new Error("Gua bridge connection is inactive."); + socket.send(JSON.stringify(payload)); + onSent?.(socket, id); + } catch (error) { + clearTimeout(timeoutId); + this.pending.delete(id); + signal?.removeEventListener("abort", aborted); + reject(error); + } }); } @@ -1440,7 +1502,7 @@ export class GuaBridgeClient { } else if (response.ok) { pending.resolve(response.result); } else { - pending.reject(new Error(response.error)); + pending.reject(new BridgeRejectionError(response.error)); } } diff --git a/packages/mcp/test/action-outcome.test.ts b/packages/mcp/test/action-outcome.test.ts new file mode 100644 index 00000000..f8819b3b --- /dev/null +++ b/packages/mcp/test/action-outcome.test.ts @@ -0,0 +1,172 @@ +import { expect, test } from "bun:test"; +import { spawn } from "node:child_process"; +import { createInterface } from "node:readline"; +import path from "node:path"; +import { GuaActionOutcomeError, GuaBridgeClient, replayRecording } from "../src/index"; + +const tree = { screen: "exit_confirmation", revision: 1, nodes: [] }; +const completion = { requestId: 7, action: 1, succeeded: true, error: 0, nodeId: "confirm_exit", + value: "", sensitive: false, sessionEpoch: 1, frameSequence: 2, revision: 2 }; +type Scenario = "normal_disconnect" | "abnormal_disconnect" | "lost_receipt" | "success" | "rejected" | "legacy" | "legacy_disconnect" | "wrong_completion" | "invalid_receipt" | "failed_completion"; + +function fixture(scenario: Scenario) { + const commands: any[] = []; let connections = 0; + const server = Bun.serve({ hostname: "127.0.0.1", port: 0, + fetch(request, server) { if (server.upgrade(request)) return; return new Response("upgrade", { status: 426 }); }, + websocket: { + open() { connections++; }, + message(socket, data) { + const command = JSON.parse(String(data)); commands.push(command); + const respond = (result: unknown) => socket.send(JSON.stringify({ id: command.id, ok: true, result })); + if (command.type === "get_ui_tree") { + if (scenario === "legacy_disconnect" && commands.some(c => c.type === "click_node")) socket.close(); + else respond(tree); + } else if (command.type === "click_node") { + if (scenario === "lost_receipt") socket.terminate(); + else if (scenario === "rejected") socket.send(JSON.stringify({ id: command.id, ok: false, error: "Gua action rejected: disabled" })); + else respond(scenario.startsWith("legacy") ? null : scenario === "invalid_receipt" ? {} : { requestId: 7 }); + } else if (command.type === "poll_events") { + if (scenario === "normal_disconnect") socket.close(1000, "normal shutdown"); + else if (scenario === "abnormal_disconnect") socket.terminate(); + else { respond(scenario === "wrong_completion" ? { ...completion, requestId: 8 } + : scenario === "failed_completion" ? { ...completion, succeeded: false, error: -4 } : completion); socket.close(); } + } + }, + }, + }); + return { server, commands, url: `ws://127.0.0.1:${server.port}`, connections: () => connections }; +} + +async function callTool(url: string, name = "click_node", args: unknown = { nodeId: "confirm_exit" }) { + const child = spawn(process.execPath, [path.resolve(import.meta.dir, "../src/cli.ts"), "mcp"], { + env: { ...process.env, GUA_BRIDGE_URL: url }, stdio: ["pipe", "pipe", "pipe"], + }); + const lines = createInterface({ input: child.stdout }); child.stderr.resume(); + try { + const response = new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error("MCP test response timed out")), 3000); + lines.on("line", line => { const rpc = JSON.parse(line); if (rpc.id === 2) { clearTimeout(timeout); resolve(rpc.result); } }); + child.once("error", error => { clearTimeout(timeout); reject(error); }); + }); + child.stdin.write(JSON.stringify({ jsonrpc: "2.0", id: 1, method: "initialize", params: { + protocolVersion: "2024-11-05", capabilities: {}, clientInfo: { name: "action-outcome-test", version: "1" }, + } }) + "\n"); + child.stdin.write(JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" }) + "\n"); + child.stdin.write(JSON.stringify({ jsonrpc: "2.0", id: 2, method: "tools/call", params: { name, arguments: args } }) + "\n"); + const result = await response; + return { isError: result.isError, body: JSON.parse(result.content[0].text) }; + } finally { lines.close(); child.kill(); } +} + +for (const scenario of ["normal_disconnect", "abnormal_disconnect"] as const) { + test(`${scenario}: accepted exit remains unconfirmed with correlation and no reconnect/retry`, async () => { + const f = fixture(scenario); + try { + const result = await callTool(f.url); + expect(result).toMatchObject({ isError: true, body: { outcome: "completion_unconfirmed", + stage: "awaiting_completion", requestSent: true, requestId: 7 } }); + expect(result.body.bridgeCommandId).toBeInteger(); + expect(result.body.error).toContain("Do not retry automatically"); + expect(f.commands.filter(c => c.type === "click_node")).toHaveLength(1); + expect(f.connections()).toBe(1); + } finally { f.server.stop(true); } + }); +} + +test("initial connection refusal reports that the action was never sent", async () => { + const f = fixture("success"); f.server.stop(true); + const result = await callTool(f.url); + expect(result).toMatchObject({ isError: true, body: { outcome: "not_sent", stage: "before_send", requestSent: false } }); + expect(result.body.requestId).toBeUndefined(); expect(result.body.bridgeCommandId).toBeUndefined(); + expect(f.commands).toHaveLength(0); +}); + +test("lost acceptance response preserves send evidence without inventing a host request ID", async () => { + const f = fixture("lost_receipt"); + try { + const result = await callTool(f.url); + expect(result).toMatchObject({ isError: true, body: { outcome: "completion_unconfirmed", stage: "awaiting_receipt", requestSent: true } }); + expect(result.body.bridgeCommandId).toBeInteger(); expect(result.body.requestId).toBeUndefined(); + expect(f.commands.filter(c => c.type === "click_node")).toHaveLength(1); + } finally { f.server.stop(true); } +}); + +test("correlated completion stays successful when the runtime closes immediately afterwards", async () => { + const f = fixture("success"); + try { + expect(await callTool(f.url)).toEqual({ isError: false, body: { ok: true, requestId: 7, completion } }); + expect(f.commands.filter(c => c.type === "get_ui_tree")).toHaveLength(1); + } finally { f.server.stop(true); } +}); + +test("explicit bridge rejection keeps its existing error", async () => { + const f = fixture("rejected"); + try { expect(await callTool(f.url)).toEqual({ isError: true, body: { error: "Gua action rejected: disabled" } }); } + finally { f.server.stop(true); } +}); + +test("correlated host failure keeps its existing error", async () => { + const f = fixture("failed_completion"); + try { expect(await callTool(f.url)).toEqual({ isError: true, body: { error: "Gua action click failed with error -4." } }); } + finally { f.server.stop(true); } +}); + +test("malformed acceptance cannot fabricate a host request ID", async () => { + const f = fixture("invalid_receipt"); + try { + const result = await callTool(f.url); + expect(result).toMatchObject({ isError: true, body: { outcome: "completion_unconfirmed", stage: "awaiting_receipt", requestSent: true } }); + expect(result.body.requestId).toBeUndefined(); + expect(f.commands.some(c => c.type === "poll_events")).toBe(false); + } finally { f.server.stop(true); } +}); + +test("legacy null receipt keeps the existing successful response", async () => { + const f = fixture("legacy"); + try { expect(await callTool(f.url)).toEqual({ isError: false, body: { ok: true } }); } + finally { f.server.stop(true); } +}); + +test("legacy observation failure is sent but unconfirmed without a fabricated request ID", async () => { + const f = fixture("legacy_disconnect"); + try { + const result = await callTool(f.url); + expect(result).toMatchObject({ isError: true, body: { outcome: "completion_unconfirmed", stage: "awaiting_observation", requestSent: true } }); + expect(result.body.requestId).toBeUndefined(); + } finally { f.server.stop(true); } +}); + +test("a mismatched completion cannot confirm the exit action", async () => { + const f = fixture("wrong_completion"); + try { expect(await callTool(f.url)).toMatchObject({ isError: true, body: { outcome: "completion_unconfirmed", requestId: 7 } }); } + finally { f.server.stop(true); } +}); + +test("run_test stops on an uncertain exit without sending its next step", async () => { + const f = fixture("normal_disconnect"); + try { + expect(await callTool(f.url, "run_test", { steps: [{ action: "click_node", nodeId: "confirm_exit" }, + { action: "click_node", nodeId: "other" }] })).toMatchObject({ isError: true, body: { outcome: "completion_unconfirmed", requestId: 7 } }); + expect(f.commands.filter(c => c.type === "click_node")).toHaveLength(1); + } finally { f.server.stop(true); } +}); + +test("replay completion timeout is bounded and retains acceptance evidence", async () => { + const server = Bun.serve({ port: 0, hostname: "127.0.0.1", + fetch(req, server) { if (server.upgrade(req)) return; return new Response("upgrade", { status: 426 }); }, + websocket: { message(socket, data) { const command = JSON.parse(String(data)); + if (command.type !== "poll_events") socket.send(JSON.stringify({ id: command.id, ok: true, + result: command.type === "get_ui_tree" ? tree : { requestId: 7 } })); } }, + }); + const bridge = new GuaBridgeClient(`ws://127.0.0.1:${server.port}`, 5000); + try { + const start = performance.now(); + let failure: unknown; + try { await replayRecording({ schemaVersion: 1, steps: [{ action: "click", target: { id: "confirm_exit" }, + relativeMilliseconds: 0, preRevision: 1, postRevision: 2, sensitive: false }] }, bridge, {}, "preserve_delays", 75); } + catch (error) { failure = error; } + expect(failure).toBeInstanceOf(GuaActionOutcomeError); + expect((failure as GuaActionOutcomeError).diagnostics()).toMatchObject({ outcome: "completion_unconfirmed", stage: "awaiting_completion", requestId: 7 }); + expect(performance.now() - start).toBeLessThan(1000); + } finally { bridge.close(); server.stop(true); } +});