Merge pull request #729 from corbitsdev/cl-7264-keep-approval-persistence-failures-non-fatal

Keep approval persistence failures from crashing the session

Author: TheGreatAxios

CHANGELOG.md

@@ -27,6 +27,9 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
   previous model no longer covers the same action, new grants store under the
   new pair, and Kimi/Moonshot sessions get non-recursive `present` schemas
   immediately (canonical schemas restore when switching away).
+- A failed write of a project, global, or provider-model approval no longer
+  crashes the session. The grant still applies in memory, the approved tool
+  call still completes, and the operator is told remember did not stick.
 
 ## [0.3.10] - 2026-08-30
 

docs/PRODUCT.md

@@ -87,7 +87,7 @@ recovery line instead of dumping the file path and parse details.
 
 ## Safety Model
 
-- **Tiered permission gate** — Read-only tools (`read_file`, `search_files`, `grep`, `list_dir`) run freely. Every consequential tool (`write_file`, `edit_file`, `run_shell`, …) is gated. The operator can Allow Once or Allow Always (scoped to a file, a directory, or a command shape); "Allow Always" choices persist per working directory so repeat actions don't interrupt flow.
+- **Tiered permission gate** — Read-only tools (`read_file`, `search_files`, `grep`, `list_dir`) run freely. Every consequential tool (`write_file`, `edit_file`, `run_shell`, …) is gated. The operator can Allow Once or Allow Always (scoped to a file, a directory, or a command shape). Allow Always applies for the rest of the session; Corbits also tries to remember it on disk so later sessions don't re-ask. If that write fails, the grant still holds this session and the operator is told remember did not stick.
 - **Secret guard** — Path-keyed tools (`read_file`, `write_file`, …) hard-deny sensitive files (`.env`, `id_rsa`, `*.pem`, `.aws/credentials`, `.ssh/*`, `.git-credentials`, and similar), even with approval, `--dangerously-skip-permissions`, or `/yolo`. Template files like `.env.example` are exempt. Shell commands that _reference_ those paths (e.g. `bun --env-file=.env.staging run …`, `cat .env`) require explicit operator approval and never auto-run in auto mode; once approved, they proceed. Tool-result scrubbing still redacts credential-shaped output that reaches the transcript.
 - **Catastrophic-command deny** — Destructive shell patterns that target system roots (`rm -rf /`, home, `/etc`, …), plus `mkfs`, `dd`, `sudo`, fork bombs, `curl | bash`, force-push, … are blocked before they run. Recursive delete of ordinary workspace paths is not hard-denied but requires operator approval (never auto in auto mode).
 - **Constrained auto mode** — Default is on (`auto = true`). Pass `--no-auto` to start in ask mode, or `--auto` to force it on; there is currently no in-session key to toggle it. Auto mode auto-approves workspace file writes/edits/deletes and unconstrained shell without per-action prompts, but it is not a free-for-all:

src/exec/runner.ts

@@ -376,7 +376,13 @@ export async function runExec(config: Config): Promise<ExecResult> {
       model: config.model,
       requestApproval: (request: PermissionRequest): Promise<ApprovalOutcome> =>
         promptPermission(request, interactive),
-      persist: createApprovalPersist(config.cwd, () => `${config.providerName}:${config.model}`),
+      persist: createApprovalPersist(
+        config.cwd,
+        () => `${config.providerName}:${config.model}`,
+        (text) => {
+          stderr.write(`${text}\n`);
+        },
+      ),
       approvalLog: createApprovalLog(sessionDir(config.cwd, sessionId)),
       interactive,
       skipPermissions: config.dangerouslySkipPermissions,

src/session/runtime-assembly.test.ts

@@ -1,10 +1,16 @@
-import { afterEach, describe, expect, mock, spyOn, test } from "bun:test";
+import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test";
 import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
+import { getLogger } from "@intx/log";
+import type { ToolCall } from "@intx/types/runtime";
 
+import { LOG_NAMESPACE_ROOT } from "../branding.js";
 import * as permissionStore from "../permission/store.js";
+import { createPermissionGate } from "../permission/gate.js";
+import type { GrantScope } from "../permission/types.js";
 import {
+  APPROVAL_PERSIST_FAILURE_NOTICE,
   buildSubAgentProvider,
   createApprovalPersist,
   createLiveSubAgentSources,
@@ -160,6 +166,12 @@ describe("loadSeededApprovals merge order", () => {
 });
 
 describe("createApprovalPersist", () => {
+  const persistLogger = getLogger([LOG_NAMESPACE_ROOT, "session", "approvals"]);
+
+  beforeEach(() => {
+    spyOn(persistLogger, "warn");
+  });
+
   afterEach(() => {
     mock.restore();
   });
@@ -203,6 +215,120 @@ describe("createApprovalPersist", () => {
     expect(providerModel).toHaveBeenNthCalledWith(1, "openai:gpt-5", approval);
     expect(providerModel).toHaveBeenNthCalledWith(2, "anthropic:claude-opus", approval);
   });
+
+  const persistedScopes: {
+    scope: Exclude<GrantScope, "session">;
+    reject: (message: string) => void;
+  }[] = [
+    {
+      scope: "project",
+      reject: (message) => {
+        spyOn(permissionStore, "saveProjectApproval").mockRejectedValue(new Error(message));
+      },
+    },
+    {
+      scope: "global",
+      reject: (message) => {
+        spyOn(permissionStore, "saveGlobalApproval").mockRejectedValue(new Error(message));
+      },
+    },
+    {
+      scope: "provider-model",
+      reject: (message) => {
+        spyOn(permissionStore, "saveProviderModelApproval").mockRejectedValue(new Error(message));
+      },
+    },
+  ];
+
+  const shellCall = (command: string): ToolCall => ({
+    id: "c",
+    name: "run_shell",
+    arguments: { command },
+  });
+
+  async function flushUnhandledRejections(): Promise<unknown> {
+    let unhandled: unknown = null;
+    const onUnhandled = (reason: unknown): void => {
+      unhandled = reason;
+    };
+    process.on("unhandledRejection", onUnhandled);
+    try {
+      await new Promise((resolve) => setTimeout(resolve, 0));
+    } finally {
+      process.off("unhandledRejection", onUnhandled);
+    }
+    return unhandled;
+  }
+
+  for (const { scope, reject } of persistedScopes) {
+    test(`a rejected ${scope} write is contained, logged, noticed, and never becomes an unhandled rejection`, async () => {
+      const message = `${scope} disk full`;
+      reject(message);
+      const notices: string[] = [];
+
+      const persist = createApprovalPersist(
+        "/tmp/proj",
+        () => "openai:gpt-5",
+        (text) => {
+          notices.push(text);
+        },
+      );
+      persist({ tool: "run_shell", pattern: "npm *" }, scope);
+
+      expect(await flushUnhandledRejections()).toBeNull();
+      expect(persistLogger.warn).toHaveBeenCalledTimes(1);
+      expect(persistLogger.warn).toHaveBeenCalledWith(
+        "Failed to persist {scope} approval: {error}",
+        {
+          scope,
+          error: message,
+        },
+      );
+      expect(notices).toEqual([APPROVAL_PERSIST_FAILURE_NOTICE]);
+    });
+
+    test(`a throwing ${scope} persist notice is contained and never becomes an unhandled rejection`, async () => {
+      reject(`${scope} EIO`);
+
+      const persist = createApprovalPersist(
+        "/tmp/proj",
+        () => "openai:gpt-5",
+        () => {
+          throw new Error("notice exploded");
+        },
+      );
+      persist({ tool: "run_shell", pattern: "npm *" }, scope);
+
+      expect(await flushUnhandledRejections()).toBeNull();
+    });
+
+    test(`an approved call still completes and the in-memory ${scope} grant still applies when persist rejects`, async () => {
+      reject(`${scope} EACCES`);
+      const persist = createApprovalPersist("/tmp/proj", () => "openai:gpt-5");
+      let asked = 0;
+      const gate = createPermissionGate({
+        approvals: [],
+        requestApproval: async () => {
+          asked++;
+          return {
+            allow: true,
+            persist: { id: scope, label: "", pattern: "npm *", grant: scope },
+          };
+        },
+        persist,
+        interactive: true,
+        skipPermissions: false,
+        providerName: "openai",
+        model: "gpt-5",
+      });
+
+      expect((await gate.evaluate(shellCall("npm test"))).allowed).toBe(true);
+      expect(asked).toBe(1);
+      expect(await flushUnhandledRejections()).toBeNull();
+      expect((await gate.evaluate(shellCall("npm run build"))).allowed).toBe(true);
+      expect(asked).toBe(1);
+    });
+  }
 });
 
 describe("skillDirsFromEnabledPlugins", () => {

src/session/runtime-assembly.ts

@@ -3,6 +3,7 @@
 // Only near-verbatim blocks live here. Gate / toolset / director construction
 // bind runner-specific state and stay in each runner.
 
+import { getLogger } from "@intx/log";
 import type { ConversationTurn, InferenceSource } from "@intx/types/runtime";
 import type { Compactor } from "@intx/types/runtime";
 
@@ -13,6 +14,7 @@ import {
   loadAgentContextExtensions,
   loadSystemPromptOverrides,
 } from "../agent/context-extensions.js";
+import { LOG_NAMESPACE_ROOT } from "../branding.js";
 import type { ProviderCatalogEntry } from "../config/index.js";
 import { buildMainSessionSources } from "../config/inference-sources.js";
 import type { SessionMode } from "../config/session-mode.js";
@@ -119,21 +121,57 @@ export async function loadSeededApprovals(
   return [...sessionApprovals, ...projectApprovals, ...globalApprovals, ...providerModelApprovals];
 }
 
+const persistLogger = getLogger([LOG_NAMESPACE_ROOT, "session", "approvals"]);
+
+/** Operator-facing copy when an Allow Always write fails. The in-session grant still holds. */
+export const APPROVAL_PERSIST_FAILURE_NOTICE =
+  "Allow Always applies this session, but remember did not stick.";
+
+// The persist callback is fire-and-forget from the gate. A rejected write must
+// not become an unhandledRejection (that path is fatal at process level); the
+// in-memory grant already applies, so the approved call still completes.
+function persistBestEffort(
+  scope: GrantScope,
+  write: Promise<void>,
+  onPersistFailure?: (text: string) => void,
+): void {
+  void write.catch((err: unknown) => {
+    persistLogger.warn("Failed to persist {scope} approval: {error}", {
+      scope,
+      error: err instanceof Error ? err.message : String(err),
+    });
+    try {
+      onPersistFailure?.(APPROVAL_PERSIST_FAILURE_NOTICE);
+    } catch {
+      // Notice is best-effort; never rethrow into an unhandledRejection.
+    }
+  });
+}
+
 /**
  * Route a gate-persisted grant to the store its scope selects.
  * Session grants never reach here — the gate keeps those in memory only.
  * `getActiveProviderModel` is read at persist time so a live model switch
  * stores new provider-model grants under the pair now in use.
+ * Disk failures are logged, surfaced to the operator when a notice hook is
+ * provided, and swallowed so they cannot crash the session.
  */
 export function createApprovalPersist(
   cwd: string,
   getActiveProviderModel: () => string,
+  onPersistFailure?: (text: string) => void,
 ): (approval: Approval, scope: GrantScope) => void {
   return (approval: Approval, scope: GrantScope) => {
-    if (scope === "project") void saveProjectApproval(cwd, approval);
-    else if (scope === "global") void saveGlobalApproval(approval);
+    if (scope === "project")
+      persistBestEffort(scope, saveProjectApproval(cwd, approval), onPersistFailure);
+    else if (scope === "global")
+      persistBestEffort(scope, saveGlobalApproval(approval), onPersistFailure);
     else if (scope === "provider-model") {
-      void saveProviderModelApproval(getActiveProviderModel(), approval);
+      persistBestEffort(
+        scope,
+        saveProviderModelApproval(getActiveProviderModel(), approval),
+        onPersistFailure,
+      );
     }
   };
 }

src/tui/runner.ts

@@ -857,6 +857,8 @@ export async function runTUI(initialConfig: Config): Promise<number> {
     const isXaiAuthError = (err: unknown): boolean =>
       err instanceof Error && err.name === "XaiAuthError";
 
+    const approvalPersistNotice: { notify?: (text: string) => void } = {};
+
     // Shared by the permission gate and every operator-gate emission site: an
     // unattended auto-continue run must not park on any gate forever, whichever
     // kind it is. No caller arms this today — the goal subsystem was the only
@@ -880,7 +882,11 @@ export async function runTUI(initialConfig: Config): Promise<number> {
         emitGate: (event) => emitter.emit("permission.gate", event),
         approvalTimeout,
       }),
-      persist: createApprovalPersist(config.cwd, () => `${config.providerName}:${config.model}`),
+      persist: createApprovalPersist(
+        config.cwd,
+        () => `${config.providerName}:${config.model}`,
+        (text) => approvalPersistNotice.notify?.(text),
+      ),
       approvalLog: createApprovalLog(sessionDir(config.cwd, sessionId)),
       interactive: true,
       skipPermissions: config.dangerouslySkipPermissions,
@@ -2125,6 +2131,7 @@ export async function runTUI(initialConfig: Config): Promise<number> {
     const systemNotice = (text: string): void => {
       surfaceSystemNotice(host.shell, text);
     };
+    approvalPersistNotice.notify = systemNotice;
 
     /** Settle the shell after a rejected send so the run does not look live. */
     const handleSendFailure = (err: unknown): void => {