AgentCompany/docs/proposals/C6.2-error-taxonomy-phase2.md

# 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 on `err.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.