Log durability failures instead of swallowing catches

CL-5351 slice A: surface saveState/post-run, settings write-back,
resume hydrate, and crash-finalize failures without clobbering files.

Author: TheGreatAxios

src/exec/runner.ts

@@ -95,6 +95,11 @@ import { getToolApprovalBudget } from "../tui/tool-execution-watchdog.js";
 
 const logger = getLogger([LOG_NAMESPACE_ROOT, "exec"]);
 
+/** Normalize unknown catch values for structured warn/error logs. */
+export function formatCaughtError(err: unknown): string {
+  return err instanceof Error ? err.message : String(err);
+}
+
 /** Content-less inbound used after compact so the reactor re-enters (matches TUI). */
 function buildCompactionContinuationMessage(): InboundMessage {
   return {
@@ -194,7 +199,15 @@ export async function runExec(config: Config): Promise<ExecResult> {
       mcpServers: connectedMcp,
       ...(status !== "running" ? { finishedAt: Date.now() } : {}),
       ...(extra?.error !== undefined ? { error: extra.error } : {}),
-    }).catch(() => undefined);
+    }).catch((err: unknown) => {
+      // Persistence failure must not fail the run, but dropping it silently
+      // hides disk/permission problems that leave run.json stale.
+      logger.warn("saveState failed for session {sessionId} status={status}: {error}", {
+        sessionId,
+        status,
+        error: formatCaughtError(err),
+      });
+    });
   };
 
 
@@ -630,7 +643,13 @@ export async function runExec(config: Config): Promise<ExecResult> {
       toolCallCount: runSink.getToolCallCount(),
       ...(runError !== undefined ? { error: runError } : {}),
     });
-    await hookManager.dispatchPostRun(runSummary).catch(() => undefined);
+    await hookManager.dispatchPostRun(runSummary).catch((err: unknown) => {
+      // Post-run hooks are best-effort; keep the exec exit path intact but
+      // surface the failure so operators can see hook/script problems.
+      const message = formatCaughtError(err);
+      logger.warn("dispatchPostRun failed: {error}", { error: message });
+      stderr.write(`Warning: post-run hook failed: ${message}\n`);
+    });
 
     if (!sendCompleted || runError !== undefined || summaryStatus === "failed") {
       const message =

src/tui/runner.tsx

@@ -134,7 +134,9 @@ import {
 } from "../session/runtime-assembly.js";
 import { createAttachmentRehydrateTransform } from "../session/attachment-store.js";
 import { createModelSummarizer } from "../session/summarizer.js";
-import { ID_PREFIX, LOG_NAMESPACE_ROOT } from "../branding.js";
+import { COMMAND_NAME, ID_PREFIX, LOG_NAMESPACE_ROOT } from "../branding.js";
+
+const tuiLogger = getLogger([LOG_NAMESPACE_ROOT, "tui"]);
 
 export function createTUIEventEmitter(): EventEmitter {
   return new EventEmitter();
@@ -156,6 +158,30 @@ export function resolveExitCode(args: ResolveExitCodeArgs): number {
   return 0;
 }
 
+/** One-line transcript block when resume history fails to load. */
+export function resumeTranscriptLoadErrorBlock(err: unknown): {
+  type: "error";
+  message: string;
+} {
+  const message = err instanceof Error ? err.message : String(err);
+  return { type: "error", message: `Could not load prior session transcript: ${message}` };
+}
+
+/**
+ * Resolve the base for a local-settings read-modify-write.
+ * Absent file → empty object; unreadable/invalid → null (caller must skip write).
+ */
+export async function loadLocalSettingsWriteBase(
+  path: string,
+  load: (path: string) => Promise<LocalSettings | null> = loadLocalSettings,
+): Promise<LocalSettings | null> {
+  try {
+    return (await load(path)) ?? {};
+  } catch {
+    return null;
+  }
+}
+
 function buildCompactionContinuationMessage(): InboundMessage {
   return {
     ref: { uid: 0, mailbox: "system" },
@@ -270,7 +296,13 @@ export async function runTUI(initialConfig: Config): Promise<number> {
   const finalizeOnCrash = async (err: unknown): Promise<void> => {
     if (finalized) return;
     finalized = true;
-    await flushPartialOnCrash().catch(() => undefined);
+    await flushPartialOnCrash().catch((flushErr: unknown) => {
+      // Best-effort only — still attempt saveState below. Log so a flush
+      // failure is not invisible when diagnosing a crash exit.
+      const flushMessage = flushErr instanceof Error ? flushErr.message : String(flushErr);
+      tuiLogger.warn("crash finalize: partial flush failed: {error}", { error: flushMessage });
+      process.stderr.write(`${COMMAND_NAME}: crash finalize partial flush failed: ${flushMessage}\n`);
+    });
     const message = err instanceof Error ? err.message : String(err);
     await saveState(config.cwd, sessionId, {
       status: "failed",
@@ -281,7 +313,16 @@ export async function runTUI(initialConfig: Config): Promise<number> {
       error: message,
       model: `${config.providerName}:${config.model}`,
       mcpServers: [],
-    }).catch(() => undefined);
+    }).catch((saveErr: unknown) => {
+      const saveMessage = saveErr instanceof Error ? saveErr.message : String(saveErr);
+      tuiLogger.warn(
+        "crash finalize: saveState failed for session {sessionId}: {error}",
+        { sessionId, error: saveMessage },
+      );
+      process.stderr.write(
+        `${COMMAND_NAME}: crash finalize saveState failed for ${sessionId}: ${saveMessage}\n`,
+      );
+    });
   };
 
   try {
@@ -411,8 +452,16 @@ export async function runTUI(initialConfig: Config): Promise<number> {
   let liveWebOverride: string | undefined = config.settings?.web;
   const livePluginPaths: string[] = [...(config.settings?.pluginPaths ?? [])];
   const persistPluginSettings = async (): Promise<void> => {
-    const current = await loadSettings(config.globalSettingsPath).catch(() => null);
-    const base: Settings = current ?? { providers: {} };
+    // Absent file → fresh base; unreadable/invalid → skip write so we never
+    // clobber a corrupt settings file by rewriting from a minimal shell.
+    const base = await loadGlobalSettingsWriteBase(config.globalSettingsPath);
+    if (base === null) {
+      tuiLogger.warn(
+        "Skipping plugin settings write: unreadable global settings at {path}",
+        { path: config.globalSettingsPath },
+      );
+      return;
+    }
     const next: Settings = { ...base, plugins: livePluginConfig };
     if (livePluginPaths.length > 0) next.pluginPaths = livePluginPaths;
     else delete next.pluginPaths;
@@ -637,7 +686,15 @@ export async function runTUI(initialConfig: Config): Promise<number> {
     const picked = await promptSessionModeIfUnset(config.globalSettingsPath);
     liveSessionMode = picked ?? "orchestrator";
     if (picked !== undefined) {
-      const refreshed = await loadSettings(config.globalSettingsPath).catch(() => null);
+      const refreshed = await loadSettings(config.globalSettingsPath).catch((err: unknown) => {
+        // loadSettings already maps ENOENT → null; a throw is a real I/O or
+        // schema failure. Keep the in-memory config rather than pretending
+        // settings are empty.
+        tuiLogger.warn("Failed to reload settings after session mode pick: {error}", {
+          error: err instanceof Error ? err.message : String(err),
+        });
+        return null;
+      });
       if (refreshed !== null) config = { ...config, settings: refreshed };
     }
   }
@@ -1405,8 +1462,14 @@ export async function runTUI(initialConfig: Config): Promise<number> {
       {...(config.settings !== undefined ? { initialSettings: config.settings } : {})}
       onChangeCompactionMode={async (mode) => {
         liveCompactionMode = mode;
-        const current = await loadSettings(config.globalSettingsPath).catch(() => null);
-        const base: Settings = current ?? { providers: {} };
+        const base = await loadGlobalSettingsWriteBase(config.globalSettingsPath);
+        if (base === null) {
+          tuiLogger.warn(
+            "Skipping compaction mode write: unreadable global settings at {path}",
+            { path: config.globalSettingsPath },
+          );
+          return;
+        }
         await saveGlobalSettings(config.globalSettingsPath, { ...base, compactionMode: mode });
       }}
       onChangeMaxConcurrentSubAgents={async (limit) => {
@@ -1438,12 +1501,25 @@ export async function runTUI(initialConfig: Config): Promise<number> {
       onChangeSessionMode={async (mode, scope) => {
         if (scope === "local") {
           const path = localSettingsPath(config.cwd);
-          const existing = (await loadLocalSettings(path).catch(() => null)) ?? {};
-          const next: LocalSettings = { ...existing, sessionMode: mode };
-          await saveLocalSettings(path, next);
+          // Absent → {}; unreadable/invalid → null so we never clobber.
+          const existing = await loadLocalSettingsWriteBase(path);
+          if (existing === null) {
+            tuiLogger.warn(
+              "Skipping local session mode write: unreadable settings at {path}",
+              { path },
+            );
+            return;
+          }
+          await saveLocalSettings(path, { ...existing, sessionMode: mode });
         } else {
-          const current = await loadSettings(config.globalSettingsPath).catch(() => null);
-          const base: Settings = current ?? { providers: {} };
+          const base = await loadGlobalSettingsWriteBase(config.globalSettingsPath);
+          if (base === null) {
+            tuiLogger.warn(
+              "Skipping global session mode write: unreadable settings at {path}",
+              { path: config.globalSettingsPath },
+            );
+            return;
+          }
           await saveGlobalSettings(config.globalSettingsPath, { ...base, sessionMode: mode });
         }
       }}
@@ -1496,7 +1572,17 @@ export async function runTUI(initialConfig: Config): Promise<number> {
       const blocks = turnsToContentBlocks(turns, { maxBlocks: RESUME_TRANSCRIPT_BLOCK_LIMIT });
       if (blocks.length > 0) emitter.emit("history.hydrate", blocks);
     })
-    .catch(() => undefined);
+    .catch((err: unknown) => {
+      // Resume still works without painted history, but a silent empty
+      // transcript looks like a brand-new session. Log and surface a one-line
+      // error block so the operator knows history failed to load.
+      const block = resumeTranscriptLoadErrorBlock(err);
+      tuiLogger.warn("Failed to load resume transcript from {workdir}: {error}", {
+        workdir,
+        error: err instanceof Error ? err.message : String(err),
+      });
+      emitter.emit("history.hydrate", [block]);
+    });
 
   // Connect MCP servers after the TUI is up so the UI is usable immediately and
   // any OAuth authorization is surfaced as a copyable link rather than a browser

tests/unit/exec/runner.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, test } from "bun:test";
 import type { Config } from "../../../src/config/index.js";
-import { runExec } from "../../../src/exec/runner.js";
+import { formatCaughtError, runExec } from "../../../src/exec/runner.js";
 
 function bareConfig(task: string): Config {
   // Minimal unconfigured-shaped object is not enough — runExec only needs
@@ -20,6 +20,14 @@ function bareConfig(task: string): Config {
   } as unknown as Config;
 }
 
+describe("formatCaughtError", () => {
+  test("prefers Error.message and stringifies other values", () => {
+    expect(formatCaughtError(new Error("disk full"))).toBe("disk full");
+    expect(formatCaughtError("plain")).toBe("plain");
+    expect(formatCaughtError(42)).toBe("42");
+  });
+});
+
 describe("runExec", () => {
   test("empty prompt exits 2 with stderr message without bootstrapping", async () => {
     const stderrChunks: string[] = [];

tests/unit/tui/runner.test.ts

@@ -1,6 +1,11 @@
 import { test, expect } from "bun:test";
 import { EventEmitter } from "node:events";
-import { createTUIEventEmitter, getTUIRunSummaryStatus } from "../../../src/tui/runner.js";
+import {
+  createTUIEventEmitter,
+  getTUIRunSummaryStatus,
+  loadLocalSettingsWriteBase,
+  resumeTranscriptLoadErrorBlock,
+} from "../../../src/tui/runner.js";
 import { createRunSink } from "../../../src/session/run-sink.js";
 
 test("createTUIEventEmitter returns an EventEmitter", () => {
@@ -22,6 +27,32 @@ test("getTUIRunSummaryStatus distinguishes done, failed, and cancelled runs", ()
   expect(getTUIRunSummaryStatus(false, undefined)).toBe("cancelled");
 });
 
+test("resumeTranscriptLoadErrorBlock surfaces a user-visible error block", () => {
+  expect(resumeTranscriptLoadErrorBlock(new Error("EACCES"))).toEqual({
+    type: "error",
+    message: "Could not load prior session transcript: EACCES",
+  });
+  expect(resumeTranscriptLoadErrorBlock("disk full").message).toContain("disk full");
+});
+
+test("loadLocalSettingsWriteBase distinguishes absent from unreadable", async () => {
+  // Absent → empty base (safe to write a single key).
+  expect(await loadLocalSettingsWriteBase("/nope", async () => null)).toEqual({});
+
+  // Readable → merge base.
+  expect(
+    await loadLocalSettingsWriteBase("/ok", async () => ({ sessionMode: "single" })),
+  ).toEqual({ sessionMode: "single" });
+
+  // Unreadable/invalid → null so the caller skips the write instead of
+  // overwriting the file with only sessionMode.
+  expect(
+    await loadLocalSettingsWriteBase("/bad", async () => {
+      throw new Error("invalid schema");
+    }),
+  ).toBeNull();
+});
+
 // Rotation behavioral tests — per-session store semantics without a real TUI or agent.
 
 // When buildAgent throws after the old agent is closed, fatalBuildError must