From b5491a08bf1c338c3aa2a16e4ab7ff94fa6498c6 Mon Sep 17 00:00:00 2001
From: Peter Steinberger <steipete@gmail.com>
Date: Sun, 16 Aug 2026 06:35:35 -0700
Subject: [PATCH] fix(cli): keep approvals JSON failures parseable (#124585)

---
 ...exec-approvals-cli.pending-resolve.test.ts | 13 +++++++++
 src/cli/exec-approvals-cli.test.ts            |  9 ++++---
 src/cli/exec-approvals-cli.ts                 | 27 ++++++++++---------
 3 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/src/cli/exec-approvals-cli.pending-resolve.test.ts b/src/cli/exec-approvals-cli.pending-resolve.test.ts
index 144ce2a80267..02086858b19f 100644
--- a/src/cli/exec-approvals-cli.pending-resolve.test.ts
+++ b/src/cli/exec-approvals-cli.pending-resolve.test.ts
@@ -273,6 +273,19 @@ describe("exec approvals pending and resolve CLI", () => {
     });
   });
 
+  it("writes pending approval failures as JSON", async () => {
+    callGatewayFromCli.mockRejectedValue(new Error("gateway unavailable"));
+
+    await expect(runApprovalsCommand(["approvals", "pending", "--json"])).rejects.toThrow(
+      "__exit__:1",
+    );
+
+    expect(defaultRuntime.writeJson).toHaveBeenCalledOnce();
+    expect(defaultRuntime.writeJson).toHaveBeenCalledWith({ error: "gateway unavailable" }, 0);
+    expect(defaultRuntime.error).not.toHaveBeenCalled();
+    expect(defaultRuntime.exit).toHaveBeenCalledWith(1);
+  });
+
   it("preserves whitespace-bearing ids verbatim and keeps them distinct", async () => {
     const now = Date.now();
     callGatewayFromCli.mockImplementation(async (method: string) => {
diff --git a/src/cli/exec-approvals-cli.test.ts b/src/cli/exec-approvals-cli.test.ts
index 97001844beb9..96646a695a72 100644
--- a/src/cli/exec-approvals-cli.test.ts
+++ b/src/cli/exec-approvals-cli.test.ts
@@ -945,7 +945,8 @@ describe("exec approvals CLI", () => {
 
     await expect(runNativeApprovalsFileCommand(filePath)).rejects.toThrow("__exit__:1");
 
-    expect(runtimeErrors[0]).toContain("File exceeds 1048576 bytes");
+    expect(writtenJson().error).toContain("File exceeds 1048576 bytes");
+    expect(runtimeErrors).toHaveLength(0);
     expect(callGatewayFromCli).toHaveBeenCalledTimes(1);
   });
 
@@ -954,7 +955,8 @@ describe("exec approvals CLI", () => {
 
     await expect(runNativeApprovalsFileCommand(dir)).rejects.toThrow("__exit__:1");
 
-    expect(runtimeErrors[0]).toMatch(/EISDIR|directory/i);
+    expect(writtenJson().error).toMatch(/EISDIR|directory/i);
+    expect(runtimeErrors).toHaveLength(0);
     expect(callGatewayFromCli).toHaveBeenCalledTimes(1);
   });
 
@@ -990,7 +992,8 @@ describe("exec approvals CLI", () => {
       openSpy.mockRestore();
     }
 
-    expect(runtimeErrors[0]).toContain("File exceeds 1048576 bytes");
+    expect(writtenJson().error).toContain("File exceeds 1048576 bytes");
+    expect(runtimeErrors).toHaveLength(0);
     expect(callGatewayFromCli).toHaveBeenCalledTimes(1);
   });
 });
diff --git a/src/cli/exec-approvals-cli.ts b/src/cli/exec-approvals-cli.ts
index 9b0baf49204d..e3a481ba9877 100644
--- a/src/cli/exec-approvals-cli.ts
+++ b/src/cli/exec-approvals-cli.ts
@@ -302,8 +302,6 @@ async function loadSnapshotTarget(opts: ExecApprovalsCliOpts): Promise<{
 }
 
 function exitWithError(message: string): never {
-  defaultRuntime.error(message);
-  defaultRuntime.exit(1);
   throw new Error(message);
 }
 
@@ -382,6 +380,16 @@ function formatCliError(err: unknown): string {
   return safe.length > 300 ? `${truncateUtf16Safe(safe, 300)}...` : safe;
 }
 
+function failApprovalsCommand(err: unknown, opts: ExecApprovalsCliOpts): void {
+  const message = formatCliError(err);
+  if (opts.json) {
+    defaultRuntime.writeJson({ error: message }, 0);
+  } else {
+    defaultRuntime.error(message);
+  }
+  defaultRuntime.exit(1);
+}
+
 function isApprovalDecision(value: string): value is ApprovalDecision {
   return (APPROVAL_DECISIONS as readonly string[]).includes(value);
 }
@@ -1082,8 +1090,7 @@ async function runAllowlistMutation(
       targetLabel: context.targetLabel,
     });
   } catch (err) {
-    defaultRuntime.error(formatCliError(err));
-    defaultRuntime.exit(1);
+    failApprovalsCommand(err, opts);
   }
 }
 
@@ -1132,8 +1139,7 @@ export function registerExecApprovalsCli(program: Command) {
         }
         renderPendingApprovals(entries);
       } catch (err) {
-        defaultRuntime.error(formatCliError(err));
-        defaultRuntime.exit(1);
+        failApprovalsCommand(err, opts);
       }
     });
   nodesCallOpts(pendingCmd);
@@ -1146,8 +1152,7 @@ export function registerExecApprovalsCli(program: Command) {
       try {
         await resolvePendingApproval(id, decision, opts);
       } catch (err) {
-        defaultRuntime.error(formatCliError(err));
-        defaultRuntime.exit(1);
+        failApprovalsCommand(err, opts);
       }
     });
   nodesCallOpts(resolveCmd);
@@ -1187,8 +1192,7 @@ export function registerExecApprovalsCli(program: Command) {
         renderApprovalsSnapshot(snapshot, targetLabel);
         renderEffectivePolicy({ report: effectivePolicy });
       } catch (err) {
-        defaultRuntime.error(formatCliError(err));
-        defaultRuntime.exit(1);
+        failApprovalsCommand(err, opts);
       }
     });
   nodesCallOpts(getCmd, { timeoutMs: APPROVALS_GET_DEFAULT_TIMEOUT_MS });
@@ -1236,8 +1240,7 @@ export function registerExecApprovalsCli(program: Command) {
         file.version = 1;
         await saveSnapshotTargeted({ opts, source, nodeId, file, baseHash, targetLabel });
       } catch (err) {
-        defaultRuntime.error(formatCliError(err));
-        defaultRuntime.exit(1);
+        failApprovalsCommand(err, opts);
       }
     });
   nodesCallOpts(setCmd);