From 0335dc0b07e3cde7b9c3b6c75d676f12aac4aaf4 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 30 Aug 2026 22:42:49 -0700 Subject: [PATCH 1/2] Keep approval persistence failures from crashing the session --- CHANGELOG.md | 3 + src/session/runtime-assembly.test.ts | 104 ++++++++++++++++++++++++++- src/session/runtime-assembly.ts | 23 +++++- 3 files changed, 126 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28c27795..295a5399 100644 --- a/CHANGELOG.md +++ b/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 a concise diagnostic is logged. ## [0.3.10] - 2026-08-30 diff --git a/src/session/runtime-assembly.test.ts b/src/session/runtime-assembly.test.ts index efb6dd0d..836c18e3 100644 --- a/src/session/runtime-assembly.test.ts +++ b/src/session/runtime-assembly.test.ts @@ -1,9 +1,14 @@ -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 { buildSubAgentProvider, createApprovalPersist, @@ -160,6 +165,12 @@ describe("loadSeededApprovals merge order", () => { }); describe("createApprovalPersist", () => { + const persistLogger = getLogger([LOG_NAMESPACE_ROOT, "session", "approvals"]); + + beforeEach(() => { + spyOn(persistLogger, "warn"); + }); + afterEach(() => { mock.restore(); }); @@ -203,6 +214,97 @@ describe("createApprovalPersist", () => { expect(providerModel).toHaveBeenNthCalledWith(1, "openai:gpt-5", approval); expect(providerModel).toHaveBeenNthCalledWith(2, "anthropic:claude-opus", approval); }); + + const persistedScopes: { + scope: Exclude; + 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 { + 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, and never becomes an unhandled rejection`, async () => { + const message = `${scope} disk full`; + reject(message); + + const persist = createApprovalPersist("/tmp/proj", () => "openai:gpt-5"); + 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, + }, + ); + }); + + 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", () => { diff --git a/src/session/runtime-assembly.ts b/src/session/runtime-assembly.ts index 9a91a848..a92a04d4 100644 --- a/src/session/runtime-assembly.ts +++ b/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,36 @@ export async function loadSeededApprovals( return [...sessionApprovals, ...projectApprovals, ...globalApprovals, ...providerModelApprovals]; } +const persistLogger = getLogger([LOG_NAMESPACE_ROOT, "session", "approvals"]); + +// 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 { + void write.catch((err: unknown) => { + persistLogger.warn("Failed to persist {scope} approval: {error}", { + scope, + error: err instanceof Error ? err.message : String(err), + }); + }); +} + /** * 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 and swallowed so they cannot crash the session. */ export function createApprovalPersist( cwd: string, getActiveProviderModel: () => string, ): (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)); + else if (scope === "global") persistBestEffort(scope, saveGlobalApproval(approval)); else if (scope === "provider-model") { - void saveProviderModelApproval(getActiveProviderModel(), approval); + persistBestEffort(scope, saveProviderModelApproval(getActiveProviderModel(), approval)); } }; } From 32431eee88aafdb9bd7227a63df710694e09202c Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Mon, 31 Aug 2026 07:45:12 -0700 Subject: [PATCH 2/2] Surface a notice when approval remember fails A rejected Allow Always write already applied in memory and was logged, but the operator had no on-screen signal that later sessions would re-ask. Persist now uses a live provider-model getter so a later merge with mid-session model switching cannot capture a stale key. --- CHANGELOG.md | 2 +- docs/PRODUCT.md | 2 +- src/exec/runner.ts | 8 ++++++- src/session/runtime-assembly.test.ts | 28 +++++++++++++++++++++++-- src/session/runtime-assembly.ts | 31 +++++++++++++++++++++++----- src/tui/runner.ts | 9 +++++++- 6 files changed, 69 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 295a5399..983ef252 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,7 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename 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 a concise diagnostic is logged. + call still completes, and the operator is told remember did not stick. ## [0.3.10] - 2026-08-30 diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index 9d5e6a80..707f3f87 100644 --- a/docs/PRODUCT.md +++ b/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: diff --git a/src/exec/runner.ts b/src/exec/runner.ts index 43127891..dc56d88e 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -376,7 +376,13 @@ export async function runExec(config: Config): Promise { model: config.model, requestApproval: (request: PermissionRequest): Promise => 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, diff --git a/src/session/runtime-assembly.test.ts b/src/session/runtime-assembly.test.ts index 836c18e3..479a9f2b 100644 --- a/src/session/runtime-assembly.test.ts +++ b/src/session/runtime-assembly.test.ts @@ -10,6 +10,7 @@ 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, @@ -260,11 +261,18 @@ describe("createApprovalPersist", () => { } for (const { scope, reject } of persistedScopes) { - test(`a rejected ${scope} write is contained, logged, and never becomes an unhandled rejection`, async () => { + 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"); + const persist = createApprovalPersist( + "/tmp/proj", + () => "openai:gpt-5", + (text) => { + notices.push(text); + }, + ); persist({ tool: "run_shell", pattern: "npm *" }, scope); expect(await flushUnhandledRejections()).toBeNull(); @@ -276,6 +284,22 @@ describe("createApprovalPersist", () => { 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 () => { diff --git a/src/session/runtime-assembly.ts b/src/session/runtime-assembly.ts index a92a04d4..56725b61 100644 --- a/src/session/runtime-assembly.ts +++ b/src/session/runtime-assembly.ts @@ -123,15 +123,28 @@ export async function loadSeededApprovals( 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 { +function persistBestEffort( + scope: GrantScope, + write: Promise, + 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. + } }); } @@ -140,17 +153,25 @@ function persistBestEffort(scope: GrantScope, write: Promise): void { * 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 and swallowed so they cannot crash the session. + * 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") persistBestEffort(scope, saveProjectApproval(cwd, approval)); - else if (scope === "global") persistBestEffort(scope, 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") { - persistBestEffort(scope, saveProviderModelApproval(getActiveProviderModel(), approval)); + persistBestEffort( + scope, + saveProviderModelApproval(getActiveProviderModel(), approval), + onPersistFailure, + ); } }; } diff --git a/src/tui/runner.ts b/src/tui/runner.ts index e09ab6af..f7e65189 100644 --- a/src/tui/runner.ts +++ b/src/tui/runner.ts @@ -857,6 +857,8 @@ export async function runTUI(initialConfig: Config): Promise { 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 { 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 { 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 => {