From f9055edb2b45c3fb07202b1c9ac7e79ef2049450 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 14 Oct 2026 14:17:59 -0700 Subject: [PATCH] fix(session): surface swallowed store/resume errors instead of silent defaults A torn permissions approvals file read back as an empty store and any subsequent grant write permanently discarded prior allow-always grants with nothing surfaced; reads now distinguish ENOENT from corruption and a corrupt file aborts before the read-modify-write can clobber. Stall-after-interrupt approvals collapsed every resume-path throw into a bare 'refused' that looked identical to a system decline; the refusal now carries a scrubbed cause and a structured warn. The seven silent swallow sites (aged image markers, dropped handoffs, compaction evidence, corrupt MCP oauth backup, evidence-archive skips, drain failures, stale model picker) each log with their identifier. --- src/auth/oauth-scope-check.test.ts | 18 +++++++ src/auth/oauth-scope-check.ts | 25 +++++++-- src/mcp/auth-store.test.ts | 25 +++++++++ src/mcp/auth-store.ts | 34 ++++++++++--- src/mcp/plugin.ts | 23 +++++++-- src/permission/store.test.ts | 58 ++++++++++++++++++++- src/permission/store.ts | 59 +++++++++++++++------- src/session/approval-resume.test.ts | 64 ++++++++++++++++++++++++ src/session/approval-resume.ts | 36 +++++++++++-- src/session/attachment-store.ts | 7 ++- src/session/compaction-handoff.ts | 12 ++++- src/session/redacted-correlation.test.ts | 21 +++++++- src/session/redacted-correlation.ts | 13 +++++ src/session/stream-journal.ts | 12 ++++- src/session/summarizer.ts | 4 +- src/tui/provider/discovery.ts | 16 ++++-- src/upgrade/index.test.ts | 4 ++ src/upgrade/index.ts | 11 +++- 18 files changed, 390 insertions(+), 52 deletions(-) diff --git a/src/auth/oauth-scope-check.test.ts b/src/auth/oauth-scope-check.test.ts index 7b6cab870..59476c748 100644 --- a/src/auth/oauth-scope-check.test.ts +++ b/src/auth/oauth-scope-check.test.ts @@ -181,6 +181,24 @@ describe("checkOAuthProviderScope", () => { commandName, ); expect(result.status).toBe("unavailable"); + // The inconclusive outcome names the inconclusive evidence: the HTTP + // status is the only thing this check ever surfaces about the body. + if (result.status === "unavailable") + expect(result.message).toContain("provider HTTP 500"); + }); + + test("a timed-out probe says timed out, not network down", async () => { + stubFetch(() => { + throw new DOMException("The operation timed out.", "TimeoutError"); + }); + const result = await checkOAuthProviderScope( + "codex", + codexTokens, + commandName, + ); + expect(result.status).toBe("unavailable"); + if (result.status === "unavailable") + expect(result.message).toContain("probe timed out"); }); test("xai: builds the probe from staged tokens", async () => { diff --git a/src/auth/oauth-scope-check.ts b/src/auth/oauth-scope-check.ts index 13d36319e..e4ad2bd92 100644 --- a/src/auth/oauth-scope-check.ts +++ b/src/auth/oauth-scope-check.ts @@ -46,13 +46,22 @@ function insufficientScope(providerLabel: string): OAuthScopeCheckResult { }; } -function unavailable(providerLabel: string): OAuthScopeCheckResult { +function unavailable( + providerLabel: string, + cause?: string, +): OAuthScopeCheckResult { + const why = cause === undefined ? "" : ` (${cause})`; return { status: "unavailable", - message: `Couldn't confirm ${providerLabel} API access right now — continuing without blocking setup.`, + message: `Couldn't confirm ${providerLabel} API access right now${why} — continuing without blocking setup.`, }; } +/** AbortSignal.timeout's rejection, so "network down" and "probe hung" differ. */ +function isTimeout(err: unknown): boolean { + return err instanceof Error && err.name === "TimeoutError"; +} + function invalidCredentials(providerLabel: string): OAuthScopeCheckResult { return { status: "blocked", @@ -94,7 +103,7 @@ function classifyStatus( providerLabel: string, ): OAuthScopeCheckResult { if (status === 401 || status === 403) return insufficientScope(providerLabel); - return unavailable(providerLabel); + return unavailable(providerLabel, `provider HTTP ${status}`); } async function checkCodexScope( @@ -117,7 +126,10 @@ async function checkCodexScope( } catch (err) { if (isDefinitiveRefreshAuthRejection(err)) return invalidCredentials(providerLabel); - return unavailable(providerLabel); + return unavailable( + providerLabel, + isTimeout(err) ? "probe timed out" : undefined, + ); } } @@ -138,7 +150,10 @@ async function checkXaiScope( } catch (err) { if (isDefinitiveRefreshAuthRejection(err)) return invalidCredentials(providerLabel); - return unavailable(providerLabel); + return unavailable( + providerLabel, + isTimeout(err) ? "probe timed out" : undefined, + ); } } diff --git a/src/mcp/auth-store.test.ts b/src/mcp/auth-store.test.ts index 7d28ea9b8..27141a516 100644 --- a/src/mcp/auth-store.test.ts +++ b/src/mcp/auth-store.test.ts @@ -321,4 +321,29 @@ describe("mcp auth-store", () => { final.tokens === undefined || final.tokens.access_token === "v1", ).toBe(true); }); + + test("a corrupt auth file is backed up before the next overwrite", async () => { + const home = await tempHome(); + const path = authFilePath(linear, home); + await mkdir(dirname(path), { recursive: true }); + await writeFile(path, "{corrupt", { mode: 0o600 }); + + // Corrupt state reads as absent (a fresh authorization is offered), but + // the corrupt bytes survive for diagnosis. + expect(await loadAuthState(linear, home)).toEqual({}); + expect(await readFile(`${path}.corrupt`, "utf8")).toBe("{corrupt"); + + // The overwrite behind a fresh authorization cannot consume the last copy. + await updateAuthState( + linear, + (state) => { + state.tokens = { access_token: "tok", token_type: "bearer" }; + }, + home, + ); + expect((await loadAuthState(linear, home)).tokens?.access_token).toBe( + "tok", + ); + expect(await readFile(`${path}.corrupt`, "utf8")).toBe("{corrupt"); + }); }); diff --git a/src/mcp/auth-store.ts b/src/mcp/auth-store.ts index a90b84831..16f9982c6 100644 --- a/src/mcp/auth-store.ts +++ b/src/mcp/auth-store.ts @@ -8,16 +8,20 @@ import { unlink, writeFile, } from "node:fs/promises"; -import { readFileSync } from "node:fs"; +import { readFileSync, writeFileSync } from "node:fs"; import { homedir } from "node:os"; import { dirname, join } from "node:path"; import { setTimeout as delay } from "node:timers/promises"; +import { getLogger } from "@intx/log"; import type { OAuthClientInformationFull, OAuthTokens, } from "@modelcontextprotocol/sdk/shared/auth.js"; +import { LOG_NAMESPACE_ROOT } from "../branding.js"; import { SETTINGS_DIR_NAME } from "../branding.js"; +const logger = getLogger([LOG_NAMESPACE_ROOT, "mcp", "auth-store"]); + // Per-server OAuth state. The durable pair — registered client info and // tokens — reaches disk; the PKCE verifier stays instance-local in the // provider. Legacy codeVerifier keys are stripped on write. @@ -74,20 +78,32 @@ export function isEnoent(err: unknown): boolean { ); } -function parseAuthState(raw: string): MCPAuthState | undefined { +// Corrupt credentials are backed up (never logged and never discarded with +// the next overwrite) so an operator can diagnose a half-written tokens file. +// The backup keeps owner-only mode like the auth file itself. +function preserveCorruptAuthFile(path: string, raw: string): void { + try { + writeFileSync(`${path}.corrupt`, raw, { mode: 0o600 }); + } catch { + // Backing up is best-effort; the fresh authorization proceeds either way. + } + logger.warn`corrupt MCP auth file read as no state path=${path}`; +} + +function parseAuthState(raw: string, path: string): MCPAuthState | undefined { try { const parsed: unknown = JSON.parse(raw); if (typeof parsed === "object" && parsed !== null) return parsed as MCPAuthState; } catch { - // Corrupt auth file: treat as no state; a fresh authorization overwrites it. + preserveCorruptAuthFile(path, raw); } return undefined; } -function stateFromRaw(raw: string | undefined): MCPAuthState { +function stateFromRaw(raw: string | undefined, path: string): MCPAuthState { if (raw === undefined) return {}; - return parseAuthState(raw) ?? {}; + return parseAuthState(raw, path) ?? {}; } function readAuthFileSync(path: string): string | undefined { @@ -112,7 +128,8 @@ export async function loadAuthState( identity: MCPAuthIdentity, home: string = homedir(), ): Promise { - return stateFromRaw(await readAuthFile(authFilePath(identity, home))); + const path = authFilePath(identity, home); + return stateFromRaw(await readAuthFile(path), path); } // Cache refresh: missing/unreadable/corrupt files return undefined so the @@ -121,14 +138,15 @@ export function tryLoadAuthStateSync( identity: MCPAuthIdentity, home: string = homedir(), ): MCPAuthState | undefined { + const path = authFilePath(identity, home); let raw: string | undefined; try { - raw = readAuthFileSync(authFilePath(identity, home)); + raw = readAuthFileSync(path); } catch { return undefined; } if (raw === undefined) return undefined; - return parseAuthState(raw); + return parseAuthState(raw, path); } function isEexist(err: unknown): boolean { diff --git a/src/mcp/plugin.ts b/src/mcp/plugin.ts index 00ea36bbf..2ecfe54fa 100644 --- a/src/mcp/plugin.ts +++ b/src/mcp/plugin.ts @@ -2,6 +2,7 @@ import type { AgentTool } from "@intx/agent"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; import type { PermissionGate } from "../permission/gate.js"; import { gateAgentTools } from "../plugins/permission-plugin.js"; +import { getLogger } from "@intx/log"; import { scrubSecretShapedContent, scrubSecretShapedValue, @@ -20,10 +21,13 @@ import { } from "./client.js"; import { applyMcpResultProjection } from "./result-projection.js"; import { mcpToolName } from "./tool-name.js"; +import { LOG_NAMESPACE_ROOT } from "../branding.js"; export const MCP_RECONNECTING_TOOL_ERROR = "MCP server is reconnecting; retry the call once it reports connected."; +const logger = getLogger([LOG_NAMESPACE_ROOT, "mcp", "plugin"]); + /** * Stable marker prefixing JSON-serialized `structuredContent` (alone or * supplementing an empty flatten); distinguishes server data from free text. @@ -156,8 +160,12 @@ export function mcpClientTools( provenance: "mcp:post-policy-pre-flatten", }); archivedFullEnvelope = true; - } catch { - // Archive write must not fail a successful tool result. + } catch (err) { + // Archive write must not fail a successful tool result; the + // skip still gets a receipt with the call it dropped. The + // result path is secret scrubbed throughout — the receipt is too. + const message = err instanceof Error ? err.message : String(err); + logger.warn`evidence archive skipped for successful MCP tool result call=${call.id} error=${scrubSecretShapedContent(message)}`; } } const projected = isError @@ -229,8 +237,15 @@ export function mcpClientTools( callId: call.id, provenance: "mcp:error", }); - } catch { - // Archive write must not fail a successful tool result. + } catch (archiveErr) { + // archiveErr here comes from the archive writer, never the + // payload, but the result path is secret scrubbed throughout — + // the receipt is too. + const message = + archiveErr instanceof Error + ? archiveErr.message + : String(archiveErr); + logger.warn`evidence archive skipped for failed MCP tool result call=${call.id} underlying=${scrubbed} error=${scrubSecretShapedContent(message)}`; } } return { diff --git a/src/permission/store.test.ts b/src/permission/store.test.ts index 975dd1625..5dc582ea3 100644 --- a/src/permission/store.test.ts +++ b/src/permission/store.test.ts @@ -1,7 +1,8 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { SETTINGS_DIR_NAME } from "../branding.js"; import { loadGlobalApprovals, loadPendingProjectApprovals, @@ -188,3 +189,58 @@ describe("provider-model store", () => { ]); }); }); + +// A torn (non-JSON) store file must surface, never silently report an empty +// store: the empty read is the first half of read-modify-write, so a swallowed +// corruption reconciles the file's grants away on the next write. Repairs +// always work because the write path refuses to touch the corrupt bytes. +describe("torn store recovery", () => { + const globalStorePathFor = (): string => + join(home, SETTINGS_DIR_NAME, "permissions.json"); + + test("a corrupt global store surfaces on load instead of reporting zero grants", async () => { + await saveGlobalApproval({ tool: "run_shell", pattern: "git *" }, home); + await writeFile(globalStorePathFor(), "{torn"); + await expect(loadGlobalApprovals(home)).rejects.toThrow( + /permissions store is corrupt/, + ); + }); + + test("a corrupt provider-model store surfaces on load instead of reporting zero grants", async () => { + await saveProviderModelApproval( + "openai:gpt-5", + { tool: "run_shell", pattern: "npm *" }, + home, + ); + await writeFile(globalStorePathFor(), "{torn"); + await expect(loadProviderModelApprovals(home)).rejects.toThrow( + /permissions store is corrupt/, + ); + }); + + test("a later grant write cannot clobber a torn store's prior grants", async () => { + await saveGlobalApproval({ tool: "run_shell", pattern: "git *" }, home); + await saveGlobalApproval({ tool: "run_shell", pattern: "npm *" }, home); + const path = globalStorePathFor(); + const original = await readFile(path, "utf-8"); + // Simulate a torn write: the bytes are a shortened prefix of the store. + const torn = original.slice(0, Math.max(original.length - 4, 0)); + await writeFile(path, torn); + + await expect( + saveGlobalApproval({ tool: "run_shell", pattern: "cargo *" }, home), + ).rejects.toThrow(/permissions store is corrupt/); + + // The failed write left the corrupt bytes untouched for an operator repair. + expect(await readFile(path, "utf-8")).toBe(torn); + + // Repair regains the prior grants; the deferred grant re-applies on retry. + await writeFile(path, original); + await saveGlobalApproval({ tool: "run_shell", pattern: "cargo *" }, home); + expect(await loadGlobalApprovals(home)).toEqual([ + { tool: "run_shell", pattern: "git *" }, + { tool: "run_shell", pattern: "npm *" }, + { tool: "run_shell", pattern: "cargo *" }, + ]); + }); +}); diff --git a/src/permission/store.ts b/src/permission/store.ts index d749a0f94..4d68b0440 100644 --- a/src/permission/store.ts +++ b/src/permission/store.ts @@ -45,6 +45,15 @@ function hasLiteralFloor(pattern: string): boolean { return pattern.replace(/[*?\s]/g, "").length > 0; } +function isEnoent(err: unknown): boolean { + return ( + typeof err === "object" && + err !== null && + "code" in err && + (err as { code?: unknown }).code === "ENOENT" + ); +} + const ApprovalSchema = type({ tool: "string", pattern: "string", @@ -89,14 +98,31 @@ async function readApprovalsField( } async function readObjectFile(path: string): Promise> { + let raw: string; + try { + raw = await readFile(path, "utf-8"); + } catch (err) { + if (isEnoent(err)) return {}; + throw err; + } + let parsed: unknown; try { - const parsed = JSON.parse(await readFile(path, "utf-8")) as unknown; - return typeof parsed === "object" && parsed !== null - ? (parsed as Record) - : {}; - } catch { - return {}; + parsed = JSON.parse(raw) as unknown; + } catch (err) { + // A torn store must never read as empty: this empty result is the first + // half of read-modify-write, so pretending the file was absent would let + // the next grant write permanently discard every prior Allow-Always + // grant. Surface the corruption; the write path refuses the clobber and + // an operator repair regains all grants. + throw new Error( + `permissions store is corrupt: ${path} (${ + err instanceof Error ? err.message : String(err) + })`, + ); } + return typeof parsed === "object" && parsed !== null + ? (parsed as Record) + : {}; } // Serialize read-modify-write per path so concurrent grants to one file (a @@ -240,21 +266,16 @@ export async function loadGlobalApprovals( export async function loadProviderModelApprovals( home: string = homedir(), ): Promise { - try { - const raw = await readFile(globalStorePath(home), "utf-8"); - const parsed = JSON.parse(raw) as Record; - const map = parsed?.providerModels; - if (typeof map !== "object" || map === null) return []; - const out: Approval[] = []; - for (const [key, list] of Object.entries(map as Record)) { - for (const approval of parseApprovalList(list)) { - out.push({ ...approval, providerModel: key }); - } + const parsed = await readObjectFile(globalStorePath(home)); + const map = parsed.providerModels; + if (typeof map !== "object" || map === null) return []; + const out: Approval[] = []; + for (const [key, list] of Object.entries(map as Record)) { + for (const approval of parseApprovalList(list)) { + out.push({ ...approval, providerModel: key }); } - return out; - } catch { - return []; } + return out; } export async function saveGlobalApproval( diff --git a/src/session/approval-resume.test.ts b/src/session/approval-resume.test.ts index a508aa566..1c5bce3da 100644 --- a/src/session/approval-resume.test.ts +++ b/src/session/approval-resume.test.ts @@ -24,6 +24,7 @@ import { } from "./approval-resume.js"; import { createSessionOperationQueue } from "../tui/delivery-queue.js"; import { defined } from "../../testkit/defined.js"; +import { redactedCorrelationId } from "./redacted-correlation.js"; import { approvalTimeoutTurn as timeoutTurn, assistantToolCallTurn as assistantTurn, @@ -720,6 +721,69 @@ describe("suspended approval recovery", () => { expect(mismatch.delivered).toEqual([]); }); + test("a failed resume attempt refuses with the underlying cause, not an opaque error", async () => { + const t = recovery({ gate: "allow" }); + const failing = createSuspendedApprovalRecovery({ + storage: () => storeWith([operation]), + resume: { + handle: async () => { + throw new Error("deliver failed: transport closed"); + }, + status: t.resume.status, + }, + }); + failing.capture(result, () => true); + + const outcome = await failing.tryResumeOnce(); + + expect(outcome.code).toBe("error"); + expect(outcome.detail).toContain("Error"); + expect(outcome.detail).toContain("deliver failed: transport closed"); + }); + + test("a storage failure during resume verification carries its cause", async () => { + const failing = createSuspendedApprovalRecovery({ + storage: () => ({ + load: async () => { + throw new Error("store read failed: disk I/O error"); + }, + }), + resume: recovery({ gate: "allow" }).resume, + }); + failing.capture(result, () => true); + + const outcome = await failing.tryResumeOnce(); + + expect(outcome.code).toBe("error"); + expect(outcome.detail).toContain("store read failed: disk I/O error"); + }); + + test("a failing attempt's cause carries neither a secret nor a correlation ID in full form", async () => { + // A storage failure whose message embeds a bearer-shaped token and the + // parked call's correlation ID: the detail must redact both — the + // correlation ID is a bearer capability too (approval-provenance). + const correlationId = "6f4f7c16-2e21-4a2d-9f4b-7c1e93a88b41"; + const failing = createSuspendedApprovalRecovery({ + storage: () => ({ + load: async () => { + throw new Error( + `gate dispatch failed: Bearer sk-live-abcdef0123456789ghij correlation=${correlationId}`, + ); + }, + }), + resume: recovery({ gate: "allow" }).resume, + }); + failing.capture(result, () => true); + + const outcome = await failing.tryResumeOnce(); + + expect(outcome.code).toBe("error"); + expect(outcome.detail).toContain("[redacted: looks like a credential]"); + expect(outcome.detail).toContain(redactedCorrelationId(correlationId)); + expect(outcome.detail).not.toContain("sk-live-abcdef0123456789ghij"); + expect(outcome.detail).not.toContain(correlationId); + }); + test("a stale generation or a cleared capture never presents", async () => { const stale = recovery({ gate: "allow" }); stale.recovery.capture(result, () => false); diff --git a/src/session/approval-resume.ts b/src/session/approval-resume.ts index 3fc0478ae..df51054d2 100644 --- a/src/session/approval-resume.ts +++ b/src/session/approval-resume.ts @@ -31,11 +31,15 @@ import { APPROVAL_TIMEOUT_RESULT_TEXT } from "../permission/decline-markers.js"; import { buildRequests } from "../permission/classify.js"; import type { PermissionGate } from "../permission/gate.js"; import type { PermissionRequest } from "../permission/types.js"; +import { scrubSecretShapedContent } from "../plugins/tool-result-secret-scrub.js"; import { LOCAL_APPROVAL_FROM, LOCAL_APPROVAL_MAILBOX, } from "./approval-provenance.js"; -import { redactedCorrelationId } from "./redacted-correlation.js"; +import { + redactedCorrelationId, + scrubCorrelationShapedIds, +} from "./redacted-correlation.js"; import { snapshotArgumentsMatch } from "./pending-snapshot-scrub.js"; const logger = getLogger([LOG_NAMESPACE_ROOT, "approval-resume"]); @@ -67,10 +71,14 @@ export interface ApprovalResume { /** * What a watchdog resume attempt did. `code` is content-free (a fixed * vocabulary, never approval or tool payload) so it can go into a turn marker. + * `detail` carries the failed attempt's cause — the error class plus a + * secret-scrubbed message — for warn logs and diagnostics; it stays out of + * turn markers, which inherit `code`'s content-free guarantee. */ export interface StallResumeResult { readonly handled: boolean; readonly code: string; + readonly detail?: string; } /** The `reactor.gate.blocked` payload fields recovery reads. */ @@ -113,9 +121,22 @@ export function createSuspendedApprovalRecovery(args: { ) => { candidate = { result, stillCurrent, attempted: false }; }; - const refused = (code: string): StallResumeResult => ({ + // A refused attempt's cause: the error class plus a secret-scrubbed, + // correlation-redacted message. Content is drained from thrown Errors, + // never from approval or tool payloads, so warn lines stay safe to persist + // unauthenticated: a bearer token or an embedded live correlation ID — + // both re-grant a parked call — never lands in full form. + const causeOf = (err: unknown): string => { + const cls = err instanceof Error ? err.name : typeof err; + const message = err instanceof Error ? err.message : String(err); + return `${cls}: ${scrubCorrelationShapedIds( + scrubSecretShapedContent(message), + )}`; + }; + const refused = (code: string, detail?: string): StallResumeResult => ({ handled: false, code, + ...(detail !== undefined ? { detail } : {}), }); return { capture, @@ -184,8 +205,15 @@ export function createSuspendedApprovalRecovery(args: { return refused("not-delivered"); candidate = undefined; return { handled: true, code: "resumed" }; - } catch { - return refused("error"); + } catch (err) { + // Opaque "error" hid the case where the approval was dropped by a + // real failure and looked system-declined. Keep the content-free + // code for the turn marker; the scrubbed cause goes to the warn + // log and the returned detail so diagnostics can tell a dropped + // approval from a refusal. + const detail = causeOf(err); + logger.warn`stall resume attempt failed correlation=${redactedCorrelationId(correlationId)} error=${detail}`; + return refused("error", detail); } }, }; diff --git a/src/session/attachment-store.ts b/src/session/attachment-store.ts index 600cf2a29..2b0b42a3a 100644 --- a/src/session/attachment-store.ts +++ b/src/session/attachment-store.ts @@ -6,6 +6,7 @@ import type { ConversationTurn, StrategyBlob, } from "@intx/types/runtime"; +import { getLogger } from "@intx/log"; import { attachmentIdFromBase64, attachmentUri, @@ -13,6 +14,9 @@ import { parseAgedImageMarker, } from "./attachment-uri.js"; import { type CompactionArchive } from "./compaction-archive.js"; +import { LOG_NAMESPACE_ROOT } from "../branding.js"; + +const logger = getLogger([LOG_NAMESPACE_ROOT, "session", "attachment"]); export interface AgeImageResult { turn: ConversationTurn; @@ -112,7 +116,8 @@ export async function rehydrateAttachmentImages( }, }); changed = true; - } catch { + } catch (err) { + logger.warn`aged image marker stays dead in context image=${marker.id} error=${err instanceof Error ? err.message : String(err)}`; content.push(block); } } diff --git a/src/session/compaction-handoff.ts b/src/session/compaction-handoff.ts index 2afba8894..a53e9db7d 100644 --- a/src/session/compaction-handoff.ts +++ b/src/session/compaction-handoff.ts @@ -17,7 +17,11 @@ import { ArkErrors, type } from "arktype"; import type { ConversationTurn, StrategyBlob } from "@intx/types/runtime"; +import { getLogger } from "@intx/log"; import { VERIFY_REPAIR_HEADING } from "./compaction-verify.js"; +import { LOG_NAMESPACE_ROOT } from "../branding.js"; + +const logger = getLogger([LOG_NAMESPACE_ROOT, "session", "compaction"]); // Canonical home of the fold marker. compactor.ts re-exports it; this module // owns the literal. @@ -875,7 +879,13 @@ export async function tryReadPriorHandoffFile( if (readBlob === undefined) return undefined; try { return new TextDecoder().decode(await readBlob(HANDOFF_LATEST_KEY)); - } catch { + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + // A miss ("Blob not found") is the first compaction of a session, not a + // dropped handoff; only a present file failing to read is worth a receipt. + if (!/not found/i.test(message)) { + logger.warn`prior compaction handoff dropped handoff=${HANDOFF_LATEST_KEY} error=${message}`; + } return undefined; } } diff --git a/src/session/redacted-correlation.test.ts b/src/session/redacted-correlation.test.ts index da5d97587..bd269b192 100644 --- a/src/session/redacted-correlation.test.ts +++ b/src/session/redacted-correlation.test.ts @@ -1,6 +1,9 @@ import { describe, expect, test } from "bun:test"; -import { redactedCorrelationId } from "./redacted-correlation.js"; +import { + redactedCorrelationId, + scrubCorrelationShapedIds, +} from "./redacted-correlation.js"; describe("redactedCorrelationId", () => { test("never exposes the full ID", () => { @@ -29,3 +32,19 @@ describe("redactedCorrelationId", () => { expect(redactedCorrelationId(a)).not.toBe(redactedCorrelationId(b)); }); }); + +describe("scrubCorrelationShapedIds", () => { + test("redacts every UUID-shaped id to its standard redacted form", () => { + const a = "6f4f7c16-2e21-4a2d-9f4b-7c1e93a88b41"; + const b = "5a5dfc8e-90cd-4f6a-b6d3-4e8a1a2b3c4d"; + const out = scrubCorrelationShapedIds(`drop ${a} then ${b}`); + expect(out).toBe( + `drop ${redactedCorrelationId(a)} then ${redactedCorrelationId(b)}`, + ); + }); + + test("leaves prose and non-correlation tokens intact", () => { + const text = "store read failed for path ~/.corbits/settings.json: EIO"; + expect(scrubCorrelationShapedIds(text)).toBe(text); + }); +}); diff --git a/src/session/redacted-correlation.ts b/src/session/redacted-correlation.ts index ba93f3bd8..3208a660c 100644 --- a/src/session/redacted-correlation.ts +++ b/src/session/redacted-correlation.ts @@ -20,3 +20,16 @@ export function redactedCorrelationId(correlationId: string): string { const digest = createHash("sha256").update(correlationId).digest("hex"); return `${correlationId.slice(0, PREFIX_CHARS)}#${digest.slice(0, HASH_CHARS)}`; } + +// Correlation IDs on the approval rail are UUIDv4-shaped; error text from the +// storage/resume path can embed one (a store lookup failure, a driver log +// echo), which would leak the full bearer capability into an unauthenticated +// warn line. Each match is replaced with the same redacted form used for +// declared correlation IDs, so a receipt trail stays cross-line consistent. +const CORRELATION_ID_SHAPE = + /\b[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}\b/g; + +/** Replaces correlation-ID-shaped substrings with their redacted form. */ +export function scrubCorrelationShapedIds(text: string): string { + return text.replace(CORRELATION_ID_SHAPE, (id) => redactedCorrelationId(id)); +} diff --git a/src/session/stream-journal.ts b/src/session/stream-journal.ts index 198fb7853..367d78e20 100644 --- a/src/session/stream-journal.ts +++ b/src/session/stream-journal.ts @@ -191,7 +191,17 @@ export function createCycleTextRecorder( const thinkingSnapshot = cycleThinkingText; cycleText = ""; cycleThinkingText = ""; - if (opts?.drain !== undefined) await opts.drain.catch(() => undefined); + if (opts?.drain !== undefined) { + await opts.drain.catch((err: unknown) => + getLogger([LOG_NAMESPACE_ROOT, "session", "partial"]).warn( + "stream journal drain failed on dispose: {error}", + { + reason, + error: err instanceof Error ? err.message : String(err), + }, + ), + ); + } await writeRecord(reason, snapshot, thinkingSnapshot); return snapshot; }; diff --git a/src/session/summarizer.ts b/src/session/summarizer.ts index 635a23fbf..f9ebe5dc1 100644 --- a/src/session/summarizer.ts +++ b/src/session/summarizer.ts @@ -91,10 +91,12 @@ export async function buildArchiveSummaryExcerpt( let body: string | undefined; if (occ.gap === true) { body = "(payload not stored)"; + logger.warn`compaction evidence gap for occurrence=${occ.occurrenceId} kind=${occ.kind}`; } else { try { body = await archive.readAuthorizedPayload(occ.occurrenceId); - } catch { + } catch (err) { + logger.warn`compactable evidence payload unreadable occurrence=${occ.occurrenceId} kind=${occ.kind} error=${err instanceof Error ? err.message : String(err)}`; omitted++; continue; } diff --git a/src/tui/provider/discovery.ts b/src/tui/provider/discovery.ts index 80653ec1f..33013f278 100644 --- a/src/tui/provider/discovery.ts +++ b/src/tui/provider/discovery.ts @@ -7,6 +7,7 @@ import { createOverlayList } from "../shell/overlay-list.js"; import type { CliRenderer } from "@opentui/core"; +import { getLogger } from "@intx/log"; import { modelChoiceRows, providerListHeight, @@ -19,6 +20,9 @@ import type { SetupState, Surface, } from "./types.js"; +import { LOG_NAMESPACE_ROOT } from "../../branding.js"; + +const logger = getLogger([LOG_NAMESPACE_ROOT, "tui", "provider-discovery"]); export function createDiscoveryFlows( state: SetupState, @@ -136,8 +140,10 @@ export function createDiscoveryFlows( }); surface.paint(); }) - .catch(() => { - // Seed list is already on screen; a failed prefetch must not surface. + .catch((err: unknown) => { + // Seed list is already on screen; a failed prefetch must not surface + // modally. The now-stale seeded picker gets a non-modal receipt. + logger.warn`model catalog prefetch failed provider=go error=${err instanceof Error ? err.message : String(err)}`; }); }; @@ -175,8 +181,10 @@ export function createDiscoveryFlows( }); surface.paint(); }) - .catch(() => { - // Seed list is already on screen; a failed prefetch must not surface. + .catch((err: unknown) => { + // Seed list is already on screen; a failed prefetch must not surface + // modally. The now-stale seeded picker gets a non-modal receipt. + logger.warn`model catalog prefetch failed provider=zen error=${err instanceof Error ? err.message : String(err)}`; }); }; diff --git a/src/upgrade/index.test.ts b/src/upgrade/index.test.ts index 581a15d02..5dfecde3e 100644 --- a/src/upgrade/index.test.ts +++ b/src/upgrade/index.test.ts @@ -243,6 +243,10 @@ describe("checkForUpgrade", () => { }, }); expect(result.kind).toBe("skipped"); + if (result.kind === "skipped") + // The generic reason must name the cause: an opaque "upgrade check + // failed" hides whether the probe was offline, rate-limited, or worse. + expect(result.reason).toContain("Error: offline"); }); test("detects method from probe when not forced", async () => { diff --git a/src/upgrade/index.ts b/src/upgrade/index.ts index 447b3f06e..4c258340c 100644 --- a/src/upgrade/index.ts +++ b/src/upgrade/index.ts @@ -274,8 +274,15 @@ export async function checkForUpgrade( kind: "available", notice: { current, latest, method, message }, }; - } catch { - return { kind: "skipped", reason: "upgrade check failed" }; + } catch (err) { + // The class + message make the skip diagnosable (offline, rate limit, + // TLS) instead of one opaque generic reason; nothing here is sensitive. + return { + kind: "skipped", + reason: `upgrade check failed: ${ + err instanceof Error ? err.name : typeof err + }: ${err instanceof Error ? err.message : String(err)}`, + }; } }