fix(mcp): preserve terminal action uncertainty on disconnect

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,

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<string, unknown> {
+    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<GuaUiTree> {
-    return this.request<GuaUiTree>({ type: "get_ui_tree" });
+  async getUiTree(ownerSocket?: WebSocket): Promise<GuaUiTree> {
+    return this.request<GuaUiTree>({ type: "get_ui_tree" }, this.requestTimeoutMs, undefined, ownerSocket);
   }
 
   async getWorldObjectTree(): Promise<GuaWorldObjectTree> {
@@ -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<GuaActionReceipt | null> {
+  async performAction(input: SemanticActionInput, onSent?: (socket: WebSocket, id: number) => void): Promise<GuaActionReceipt | null> {
     const type = actionCommandType(input.action);
     return this.request<GuaActionReceipt | null>(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<GuaActionEvent> {
-    const startedAt = Date.now();
-    while (Date.now() - startedAt <= timeoutMs) {
-      const event = await this.request<GuaActionEvent | null>({ type: "poll_events", requestId });
-      if (event !== null) return event;
-      await sleep(25);
+  async waitForAction(requestId: number, timeoutMs: number, ownerSocket = this.socket): Promise<GuaActionEvent> {
+    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<GuaActionEvent | null>({ 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<T>(command: BridgeCommandInput, timeoutMs = this.requestTimeoutMs,
-    signal?: AbortSignal, ownerSocket?: WebSocket): Promise<T> {
+    signal?: AbortSignal, ownerSocket?: WebSocket, onSent?: (socket: WebSocket, id: number) => void): Promise<T> {
     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));
     }
   }
 

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<any>((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); }
+});