Files
AgentCompany/docs/proposals/C6.2-error-taxonomy-phase2.md
T
ZacharyZhang-NY e530849b0f Review iteration: warn on gitSha read failure, WAL invariant comment, C6.2 wave-3 coordination gate
- backup.ts: log.warn when `git rev-parse HEAD` fails so sandbox/CI runs
  with no git show diagnosably instead of silently emitting gitSha:null
  (Karpathy review ask).
- backup.ts: expand comment on VACUUM INTO to pin the WAL-mode invariant
  — future readers must not "helpfully" add a wal_checkpoint call
  (Karpathy review ask).
- C6.2 proposal: add hard coordination gate before wave 3 because
  commit 0db6e42 already demonstrated the race hazard when two agents
  touch scripts/ / connector-definitions/ in the same window
  (Linus review ask).
2026-04-20 12:33:45 -04:00

9.7 KiB

Proposal C6.2: Error taxonomy phase 2 — migrate remaining 228 catch sites

Source: Roundtable 2026-04-20 (Linus + Karpathy + Steve Jobs); C6.1 executed tonight Status: proposed Prerequisite: C6.1 landed (src/main/errors/ module + 20 pilot migrations) Owner: TBD Estimated effort: 4-6 working days, split across three waves Blocking dependencies: none — but coordinate with any in-flight module refactors to avoid diff storms

Problem statement

C6.1 established the AppError taxonomy and migrated the 20 highest-signal catch sites. At the time of this proposal src/main/ still contains 228 catch blocks with zero classification. The breakdown by category:

Category Count Typical shape Risk if left unmigrated
} catch {} empty ~35 Silent fallthrough Fatal conditions (DB corruption, OOM) hidden indistinguishable from expected misses
} catch (e) { log.warn; return default } fail-open ~90 Already logs, but no code/kind Telemetry cannot bucket; ops cannot answer "how many upstream 5xx in the last hour"
} catch (e) { log.error; throw e } pass-through ~40 Correct but loses classification on re-throw Upstream handler re-swallows without context
} catch (e) { return null / false } silent fail-open ~30 No log at all Behaviour correct-ish, observability non-existent
Narrow instanceof guarded catches ~20 e.g. if (err.message.startsWith("X")) return null; else throw These already encode intent — migrate carefully to UserInputError subclass vs re-throw
Mixed / platform-specific ~13 Electron render-disposed, ENOENT-on-read, EPIPE Each needs a bespoke classifier

Total: 228 non-trivial catches (count reconfirmed via grep -rn "} catch " src/main/ | wc -l = 248, minus 20 already migrated in C6.1).

Why this matters (Linus)

The backup / migration work already landing in B2 wants to know whether a DB open failure is "file corrupt" (fatal) or "file missing, first boot" (recoverable). Today the catch that opens the DB returns the same log-and-rethrow shape for both. Until every DB-path catch carries a code, the migration precondition checker has to re-classify by string matching — exactly the kind of fragile guesswork we wanted to avoid.

Why this matters (Karpathy)

Tonight's harness (B5) is structural. Next-tier harnesses (B5.1 judge tier, future chaos-injection tier) will want to assert things like "when the Anthropic provider returns 529, the agent must log an upstream.anthropic.overloaded code and retry; a fatal.db.corrupt must abort the run". That kind of assertion is impossible until every catch site publishes a stable code.

Scope

Migrate all 228 remaining catch blocks in src/main/** (excluding tests, scripts, and the already-migrated 20). Pure-library files with no accessible logger (e.g. skill-keys.ts, skill-format.ts) are kept out of scope unless a logger is plumbed in first — a separate concern.

Non-goals

  • Renderer-side catches (src/renderer/) — renderer errors surface as red-screens today; a separate proposal should cover them.
  • Changing observable behaviour of any catch. This migration is pure classification: same return values, same throws, same logs (only the log prefix gains a [code]).
  • Refactoring the catches themselves (merging, splitting, removing).

Migration waves

Partition the 228 sites into three waves so each wave is one coherent module group and one reviewable PR.

Wave 1 — worker + orchestration (≈80 sites)

Buckets:

Module Approx sites Dominant kind Why first
src/main/worker/** ~35 upstream (pi-ai), recoverable (lease/db) Highest churn in production; biggest payoff for observability
src/main/orchestration/** ~20 recoverable (lookups), user_input (payload parse) Feeds into every run; code-path coverage bump is large
src/main/agent-runtime/** ~25 Mixed artifact-projector's fs-walk catches are intentional silent; document in-line rather than classify

Estimated effort: 2 days, one PR. Run pnpm test:eval and pnpm test:unit after each file. Commit per module.

Wave 2 — IPC surfaces + service layer (≈90 sites)

Buckets:

Module Approx sites Dominant kind Notes
src/main/ipc-*.ts (excl. already-done credentials + support) ~50 user_input (zod fails), upstream (downstream handler fails) Uniform shape — candidate for a registerHandle-level classifier wrapper
src/main/*-service.ts ~25 Mixed Each service owns one subsystem; classify per service
src/main/automation/** ~15 recoverable (rule eval), user_input (bad config) Small; can ride wave 2 PR as final commit

Estimated effort: 2-3 days, one PR with one commit per file group. Depends on Wave 1 only insofar as registerHandle wrapper lives in ipc-support.ts and was partially migrated in C6.1.

Wave 3 — database + credentials + browser + leftovers (≈58 sites)

⚠️ Coordination gate (hard): Wave 3 touches src/main/database/** (Linus's domain — backup hook, migrations, state-machine work) and src/main/connector-definitions/** (which was rehomed into a subdirectory during C6.1). Before starting Wave 3 the executor MUST:

  1. Check git log --oneline -20 for any pending database / backup / migration commits from other streams.
  2. Confirm B2 (tasks state-machine refactor) is either landed or pre-gated — migrating database catches while B2 is mid-flight produces diff storms that lose both sides' intent.
  3. Open a short coordination thread (issue or chat) with the database owner before the first file edit — this wave is the highest-risk of the three for scope conflicts.

Skipping this gate previously caused commit 093718d in the C6.1 wave to accidentally carry an M4 scripts/ reorg because two agents touched scripts/ in the same window. Do not repeat.

Buckets:

Module Approx sites Dominant kind Notes
src/main/database/** ~20 fatal (corruption), recoverable (busy/locked) Coordinate with M1 / B2 — these catches are the raw material for the backup/migration preconditions
src/main/browser/** + browser-manager.ts ~12 upstream (Electron session), user_input (blocked URL) Small surface
src/main/credentials/** ~8 upstream (vault decrypt), user_input (schema) Should flow through UpstreamError / UserInputError subclasses
src/main/skills/** ~8 recoverable (fs), upstream (GitHub)
Residue (top-level files not already done) ~10 Mixed Clean-up commit

Estimated effort: 2 days, one PR.

Decisions to settle before execution

Question Default Notes
Should empty } catch {} get a minimum classifyUnknownError call? Yes, when a logger is in scope — even a silent fallback becomes grep-able by code Skip only in pure-library files with no logger
Should we introduce a Result<T, AppError> envelope for IPC? No — out of scope for C6.2 Separate proposal if desired
Should AppError.toJSON() be wired into existing log transports? Yes, but in a follow-up — taxonomy migration first, transport swap second
Code naming convention module.subdomain.condition (lowercase, dots) Examples: db.lock.busy, worker.run_log.lease_renewal_lookup_failed, ipc.payload.invalid

Verification checklist per wave

  • pnpm typecheck clean
  • pnpm test:unit green
  • pnpm test:eval green (no fixture drift)
  • grep -rn "classifyUnknownError" src/main/ | wc -l monotonically increases
  • grep -rnE "^\s*\} catch \(?\w*\)? \{\s*$" src/main/ | wc -l monotonically decreases (the empty-catch count)

Rollback plan

Each wave is one PR with per-file commits. Revert at file granularity if a regression surfaces in the eval harness — no cross-file entanglement by construction (classifyUnknownError is a pure call with no shared state).

Out-of-band follow-ups (not blocking this proposal)

  1. Structured log transport: swap log.error(\[${code}] …`)for a JSON shipper that keys onerr.code` directly. Needed for real telemetry buckets.
  2. Renderer-side taxonomy: duplicate the module into src/renderer/ (or hoist to src/shared/) so IPC error payloads arrive as typed AppError envelopes.
  3. Lint rule: "no bare } catch (e) {} in src/main/". Enforceable via an eslint custom rule once coverage is > 95%.

Out of scope explicitly

  • Replacing throw new Error(...) call sites with throw new FatalError(...) — that is a separate, heavier surgical pass that would change type signatures of many functions. File a follow-up if wanted.
  • Removing the console.error / electron-log split — orthogonal.

Appendix A — file-level checklist (seed)

The following is a starter list of the 228 sites, extracted from the current grep. A reviewer should refresh this list before starting each wave since the codebase is actively evolving.

src/main/worker/entry/*.ts            (re-grep)
src/main/worker/runtime/*.ts          (re-grep)
src/main/worker/handlers/*.ts         (14 remain after C6.1)
src/main/worker/observability/*.ts    (re-grep)
src/main/orchestration/*.ts           (18 remain after C6.1)
src/main/agent-runtime/*.ts           (~25)
src/main/api-server/*.ts              (~15)
src/main/ipc-*.ts (non-credentials)   (~50)
src/main/automation/*.ts              (~15)
src/main/database/*.ts                (~20)
src/main/browser/*.ts                 (~12)
src/main/credentials/*.ts             (~8)
src/main/skills/*.ts                  (~8)
src/main/connector-*.ts               (~10)
src/main/message-*.ts                 (re-grep)
leftover top-level files              (~10)

Refresh with grep -rn "} catch " src/main/ | grep -v "errors/" before starting each wave.