Files
AgentCompany/docs/proposals/C6.2-error-taxonomy-phase2.md
T
ZacharyZhang-NY ded707d960 Document C1 codemod recipe + C6 wave boundaries + N-scan TODO deadlines (T2 iteration)
Address T2 review asks from Karpathy (request_changes) + Jobs
(approve with minor asks) without touching published commits:

- scripts/codemod/README.md (new): records remove-appdatabase-passthrough.mjs
  usage, the completeness proof for METHOD_TO_MODULE (typecheck-clean +
  524 tests green at 118e3d8 = no orphaned call sites), and the sequential
  revert recipe for C1 (3 -> 2 -> 1, not independent because 118e3d8
  bundled vault simplification).

- CONTRIBUTING.md (new): pass-through ban + codemod usage + sequential
  revert recipe + file-size ceiling + C6 wave cap. Whitelisted in
  .gitignore alongside the existing doc exceptions.

- docs/proposals/C6.2-error-taxonomy-phase2.md: wave 2 scope boundary
  names renderer-adjacent / api-server / browser / automation / skills
  explicitly and excludes src/main/database/** + src/main/worker/runtime/**
  (B2 / M10 high-churn zones). Wave 2 and wave 3 each capped at 30
  catch sites so C6 cannot expand without bound.

- src/main/agent-runtime/artifact-projector.ts, runtime-bridge.ts:
  N-scan TODOs now carry a death date ("TODO(M-followup by 2026-05-20)").
  If the date passes without decomposition, a follow-up session MUST
  revisit or justify the exception.

Karpathy T2 asks #1 and #2 (split part 3 / "independent revert" language)
are addressed in CONTRIBUTING + codemod README as documentation, not
history rewrites, because 118e3d8 is already published.

Typecheck clean. No structural code changes; pure docs + comment edits.
2026-04-20 13:48:25 -04:00

11 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 — renderer-adjacent + api-server + browser + automation + skills (≤30 catches)

Scope boundary (hard): Wave 2 covers src/main/{renderer-adjacent,api-server,browser,automation,skills}/* plus top-level ipc-*.ts and *-service.ts files. Wave 2 explicitly avoids src/main/database/** and src/main/worker/runtime/** — those are the high-churn B2 (tasks state-machine) and M10 (agent-runtime hooks) zones, and overlapping edits produce diff storms that lose both sides' intent. Database and worker/runtime catches are deferred to Wave 3 under the coordination gate below.

Scope cap (hard): Wave 2 migrates at most 30 catch sites. If the live count in the listed modules exceeds 30, stop at 30 and move the remainder to Wave 3. This prevents C6 from expanding into an unbounded sweep.

Buckets:

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

Estimated effort: 2 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 + worker/runtime + credentials + leftovers (≤30 catches)

Scope cap (hard): Wave 3 migrates at most 30 catch sites. Any residue beyond 30 gets punted to a follow-up proposal (C6.3) rather than expanding Wave 3. C6 does not get to grow without bound.

⚠️ 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.