fix(agents): preserve successful cancellation outcomes

steipete · steipete · commit 574a5684a33e · 2026-06-14T02:17:09.000-04:00
diff --git a/extensions/codex/src/app-server/dynamic-tools.test.ts b/extensions/codex/src/app-server/dynamic-tools.test.ts
@@ -1081,6 +1081,42 @@ describe("createCodexDynamicToolBridge", () => {
     });
   });
 
+  it("preserves explicitly successful cancellation outcomes", async () => {
+    const onAgentToolResult = vi.fn();
+    const cancelledResult = textToolResult("Approval rejected.", {
+      ok: true,
+      status: "cancelled",
+    });
+    const bridge = createCodexDynamicToolBridge({
+      tools: [
+        createTool({
+          name: "lobster",
+          execute: vi.fn(async () => cancelledResult),
+        }),
+      ],
+      signal: new AbortController().signal,
+    });
+
+    const result = await bridge.handleToolCall(
+      {
+        threadId: "thread-1",
+        turnId: "turn-1",
+        callId: "call-1",
+        namespace: null,
+        tool: "lobster",
+        arguments: {},
+      },
+      { onAgentToolResult },
+    );
+
+    expect(result).toMatchObject({ success: true });
+    expect(onAgentToolResult).toHaveBeenCalledWith({
+      toolName: "lobster",
+      result: cancelledResult,
+      isError: false,
+    });
+  });
+
   it("reports sanitized dynamic tool results to the private result observer", async () => {
     const onAgentToolResult = vi.fn();
     const bridge = createCodexDynamicToolBridge({
diff --git a/extensions/codex/src/app-server/dynamic-tools.ts b/extensions/codex/src/app-server/dynamic-tools.ts
@@ -12,6 +12,7 @@ import {
   embeddedAgentLog,
   type EmbeddedRunAttemptParams,
   isToolWrappedWithBeforeToolCallHook,
+  isToolResultError,
   isMessagingTool,
   isMessagingToolSendAction,
   normalizeHeartbeatToolResponse,
@@ -735,10 +736,16 @@ function readPositiveInteger(value: unknown): number | undefined {
 }
 
 function isCodexToolResultError(result: AgentToolResult<unknown>): boolean {
+  if (isToolResultError(result)) {
+    return true;
+  }
   const details = result.details;
   if (!isRecord(details)) {
     return false;
   }
+  if (details.ok === true || details.success === true) {
+    return false;
+  }
   if (details.timedOut === true) {
     return true;
   }
diff --git a/src/agents/embedded-agent-subscribe.tools.test.ts b/src/agents/embedded-agent-subscribe.tools.test.ts
@@ -134,6 +134,12 @@ describe("isToolResultError", () => {
     expect(isToolResultError({ details: { status: "completed", timedOut: true } })).toBe(true);
     expect(isToolResultError({ details: { status: "completed", exitCode: 1 } })).toBe(true);
     expect(isToolResultError({ details: { status: "completed", exitCode: 0 } })).toBe(false);
+    expect(isToolResultError({ details: { ok: true, status: "cancelled" } })).toBe(false);
+    expect(isToolResultError({ details: { success: true, status: "canceled" } })).toBe(false);
+    expect(isToolResultError({ details: { ok: false, status: "completed" } })).toBe(true);
+    expect(isToolResultError({ details: { ok: true, status: "cancelled", timedOut: true } })).toBe(
+      true,
+    );
   });
 });
 
diff --git a/src/agents/embedded-agent-subscribe.tools.ts b/src/agents/embedded-agent-subscribe.tools.ts
@@ -543,7 +543,11 @@ export function extractToolResultMediaPaths(result: unknown): string[] {
 export function isToolResultError(result: unknown): boolean {
   const details = readToolResultDetails(result);
   const normalized = readToolResultStatus(result);
-  if (
+  const explicitlySuccessful = details?.ok === true || details?.success === true;
+  if (details?.ok === false || details?.success === false) {
+    return true;
+  }
+  const hasFailureStatus =
     normalized === "error" ||
     normalized === "failed" ||
     normalized === "failure" ||
@@ -559,8 +563,8 @@ export function isToolResultError(result: unknown): boolean {
     normalized === "cancelled" ||
     normalized === "canceled" ||
     normalized === "killed" ||
-    normalized === "invalid"
-  ) {
+    normalized === "invalid";
+  if (hasFailureStatus && !explicitlySuccessful) {
     return true;
   }
   if (details?.timedOut === true || Boolean(details?.error)) {
