fix: surface deduplication review failure reasons

sdk/typescript/src/deduplication/codex-review.ts

@@ -21,7 +21,7 @@ import {
 } from "../runtime.js";
 import { CODEX_SECURITY_THREAD_SOURCES } from "../thread-source.js";
 import { VERSION } from "../version.js";
-import { CodexSecurityError } from "../errors.js";
+import { CodexSecurityError, safeErrorMessage } from "../errors.js";
 import {
   reviewSubmissionInstructions,
   sourceReviewInstructions,
@@ -44,15 +44,15 @@ type StartCodex = (
 interface Message {
   id?: string | number;
   method?: string;
-  error?: unknown;
+  error?: { message: string };
   result?: {
     thread?: { id: string; ephemeral: boolean; path: string | null };
     turn?: { id: string };
   };
   params?: {
     threadId: string;
     turnId?: string;
-    turn?: { id: string; status: string };
+    turn?: { id: string; status: string; error?: { message: string } | null };
     tool?: string;
     namespace?: string | null;
     arguments?: unknown;
@@ -202,6 +202,7 @@ export class CodexReviewRunner {
       let threadId: string | undefined;
       let turnId: string | undefined;
       let accepted: T | undefined;
+      let validationFailure: string | undefined;
       try {
         send({
           id: 1,
@@ -215,7 +216,12 @@ export class CodexReviewRunner {
           input: child.stdout,
           crlfDelay: Infinity,
         })) {
-          const message = JSON.parse(line) as Message;
+          let message: Message;
+          try {
+            message = JSON.parse(line) as Message;
+          } catch {
+            throw new Error("Codex returned malformed JSON");
+          }
           const params = message.params;
           if (message.id !== undefined && message.method !== undefined) {
             if (
@@ -237,6 +243,7 @@ export class CodexReviewRunner {
               } catch (error) {
                 accepted = undefined;
                 if (error instanceof Error) rejection = error.message;
+                validationFailure = rejection;
               }
               send({
                 id: message.id,
@@ -259,7 +266,9 @@ export class CodexReviewRunner {
               });
             }
           } else if (message.error !== undefined) {
-            throw new Error("Codex rejected the review request");
+            throw new Error(
+              message.error?.message ?? "Codex rejected the review request",
+            );
           } else if (message.id === 1) {
             send({ method: "initialized" });
             if (apiKey)
@@ -307,8 +316,18 @@ export class CodexReviewRunner {
             params.threadId === threadId &&
             params.turn?.id === turnId
           ) {
-            if (params.turn?.status !== "completed" || accepted === undefined) {
-              throw new Error("Codex did not complete a validated review");
+            if (params.turn.status !== "completed") {
+              throw new Error(
+                params.turn.error?.message ??
+                  `Codex review turn ${params.turn.status}`,
+              );
+            }
+            if (accepted === undefined) {
+              throw new Error(
+                validationFailure
+                  ? `Review validation failed: ${validationFailure}`
+                  : "Codex did not submit a validated review",
+              );
             }
             return accepted;
           }
@@ -319,10 +338,10 @@ export class CodexReviewRunner {
         if (child.exitCode === null) child.kill();
         await closed;
       }
-    } catch {
+    } catch (error) {
       this.signal?.throwIfAborted();
       throw new CodexSecurityError(
-        "Codex did not complete a validated deduplication review. Findings are unchanged; retry the command.",
+        `Codex did not complete a validated deduplication review. Findings are unchanged; retry the command. Reason: ${safeErrorMessage(error)}`,
       );
     } finally {
       await rm(directory, { recursive: true, force: true });

sdk/typescript/tests-ts/codex-review.test.ts

@@ -14,14 +14,24 @@ const fixture = fileURLToPath(
   new URL("fixtures/codex-review.mjs", import.meta.url),
 );
 
+const failureReasons: Record<string, string> = {
+  "text-only": "Codex did not submit a validated review",
+  "failed-turn": "Rate limit exceeded",
+  "request-error": "Authentication required",
+  "credential-error": "[redacted]",
+  "invalid-json": "Codex returned malformed JSON",
+  "invalid-submission": "Review validation failed: Invalid decision",
+  exit: "Codex exited before completing the review",
+};
+
 const transportCases: {
   scenario: string;
   name?: string;
   environmentNames?: readonly [string, string, string];
   extraEnvironment?: Record<string, string>;
   windowsOnly?: boolean;
 }[] = [
-  ...["correction", "text-only", "failed-turn", "exit", "cancel"].map(
+  ...["correction", ...Object.keys(failureReasons), "cancel"].map(
     (scenario) => ({ scenario }),
   ),
   {
@@ -138,10 +148,12 @@ for (const {
         await expect(result).rejects.toBe("synthetic cancellation");
       } else {
         await expect(result).rejects.toMatchObject({
-          message:
-            "Codex did not complete a validated deduplication review. Findings are unchanged; retry the command.",
+          name: "CodexSecurityError",
+          message: `Codex did not complete a validated deduplication review. Findings are unchanged; retry the command. Reason: ${failureReasons[scenario]}`,
         });
-        expect(validations).toBe(scenario === "failed-turn" ? 1 : 0);
+        expect(validations).toBe(
+          ["failed-turn", "invalid-submission"].includes(scenario) ? 1 : 0,
+        );
       }
       expect(args).toContain('cli_auth_credentials_store="ephemeral"');
       expect(args.join(" ")).not.toContain("synthetic-review-key");

sdk/typescript/tests-ts/fixtures/codex-review.mjs

@@ -17,12 +17,12 @@ const submit = (id, arguments_, overrides = {}) =>
       ...overrides,
     },
   });
-const complete = (status = "completed") =>
+const complete = (status = "completed", error = null) =>
   send({
     method: "turn/completed",
     params: {
       threadId: "review-thread",
-      turn: { id: "review-turn", status },
+      turn: { id: "review-turn", status, error },
     },
   });
 
@@ -37,6 +37,20 @@ for await (const line of createInterface({ input: process.stdin })) {
     assert.equal(message.params.apiKey, "synthetic-review-key");
     send({ id: message.id, result: { type: "apiKey" } });
   } else if (message.method === "thread/start") {
+    if (["request-error", "credential-error"].includes(scenario)) {
+      send({
+        id: message.id,
+        error: {
+          code: -32000,
+          message:
+            scenario === "credential-error"
+              ? "Authentication failed: Bearer synthetic-review-key"
+              : "Authentication required",
+          data: "Synthetic private response data",
+        },
+      });
+      continue;
+    }
     assert.equal(message.params.ephemeral, true);
     assert.equal(message.params.permissions, "codex_security_review");
     assert.equal(message.params.approvalPolicy, "on-request");
@@ -65,7 +79,11 @@ for await (const line of createInterface({ input: process.stdin })) {
     });
     send({ id: message.id, result: { turn: { id: "review-turn" } } });
     if (scenario === "exit") process.exit(1);
-    if (scenario === "text-only") {
+    if (scenario === "invalid-json") {
+      process.stdout.write("Synthetic private response data\n");
+    } else if (scenario === "invalid-submission") {
+      submit("invalid", { decision: "UNKNOWN" });
+    } else if (scenario === "text-only") {
       send({
         method: "item/completed",
         params: {
@@ -90,12 +108,17 @@ for await (const line of createInterface({ input: process.stdin })) {
   } else if (message.id === "invalid") {
     assert.equal(message.result.success, false);
     assert.match(message.result.contentItems[0].text, /Resubmit/);
-    submit("valid", { decision: "SAME" });
+    if (scenario === "invalid-submission") complete();
+    else submit("valid", { decision: "SAME" });
   } else if (message.id === "valid") {
     assert.equal(message.result.success, true);
     if (scenario === "failed-turn") {
       process.stderr.write("Synthetic provider failure with private details\n");
-      complete("failed");
+      complete("failed", {
+        message: "Rate limit exceeded",
+        codexErrorInfo: "usageLimitExceeded",
+        additionalDetails: "Synthetic private response data",
+      });
     } else complete();
   }
 }