# #490 Architecture Refactor — Plan (team-reviewed)

> Generated by a 12-agent review team (docs review → 7-dimension code inventory → lead-architect synthesis → clean-code-architect review → revision). Verified against HEAD `2164f64` on `release/0.9.0`. **Plan only — no code changes.** Architect verdict: **accept-with-revisions**.

> **STATUS — COMPLETE (2026-06-07).** All workstreams merged into `release/0.9.0` via serialized local merges with per-merge gates + reviews: WS1, WS2, WS3a, WS3b, WS4, WS5, WS6 (12 slices), WS7, WS8, WS9, WS10.
> Headline results: **cli.ts 4,589 → 620 LOC** (−87%; only `setup`/`health`/`help`/`completions`/`main` remain inline, the latter three as CLI-framework infra); 16 per-family command modules under `src/commands/*-cli.ts`; `defineJsonCommand` factory adopted for byte-identical JSON envelopes; `akm health` checks converted to an ordered `HealthCheck` registry; `migrate-storage` `ctx.recordStep` 54→1 + 3 copy helpers→1 `copyTree`, gated by a sha256+mode fixture-stash differential test. Every merge preserved the byte-identical 39-key CLI surface and JSON envelopes; final gate green (4,129 unit + 306 integration, 0 fail).

## Executive summary

Revised refactor plan for epic #490, verified against HEAD 2164f64 (release/0.9.0), incorporating every required revision from the clean-code review. The epic's headline numbers are largely STALE because quick wins (#491-#495) shipped: AkmAssetType is a literal union, shapeForCommand/formatPlain/PASSTHROUGH are registry-driven, serializeFrontmatter is consolidated, sandbox.ts is adopted by 92 test files (not 1). Re-measured at HEAD: cli.ts=4589 LOC/106 defineCommand; `new Database` already confined to EXACTLY 3 engine modules (4 call sites) so the Database-constructor battle is already won — the remaining leak is raw SQL (~68 sites outside DB modules) and hand-rolled open/try/finally lifecycle. getLockfilePath duplicated (lockfile.ts:44 vs paths.ts:267); the two SQLite migration runners differ by ONE line; the `task` registry/union drift is a real latent bug. CHANGES FROM REVIEW: (1) the LOW/MEDIUM-risk parameter-object work (PassContext, ImproveRunContext, AkmImproveOptions decomposition) is PROMOTED out of the optional XL god-function bucket into its own Phase-1 type-only workstream (WS10) so it ships independent of the dangerous akmConsolidate extraction. (2) WS3 is SPLIT into WS3a (shared SqliteMigrationRunner — high value, do now) and WS3b (migrate-storage MigrationStep registry — severable, lower priority, behind a fixture-stash differential test). (3) src/services/env-store.ts is REMOVED from WS6 — it violated the plan's own anti-speculative-layer guardrail; env/secret logic moves into env-cli.ts/secret-cli.ts + core/env-secret-ref.ts only. (4) WS5 gains an explicit connection-lifetime hazard rule (repo methods MUST fully materialize result sets before the with*Repo scope closes) plus a per-repo extraction threshold (>=2 callers OR >=3 raw-SQL sites) so low-traffic tables don't get ceremony repos. NEW FINDING confirmed at HEAD: the `existingDb?` caller-owns-vs-callee-owns flag exists in only 6 sites across 2 files (search.ts, show.ts) — far narrower than the review's cited 41, so the lifetime hazard is contained. (5) defineJsonCommand factory is now a TRACKED WS6 deliverable with a KPI (123 runWithJsonErrors sites verified at HEAD; defineJsonCommand does not yet exist). (6) WS6 gains a lower-bound anti-fragmentation guardrail (group by command family; no <100-LOC single-command files). Hard rule throughout: zero behaviour change, `bun run check` green every PR, one shippable concern per PR, byte-identical 41-key CLI surface.

## Delivery process — local merges, no PRs

This refactor is delivered **directly on `release/0.9.0` via local merges with local
reviews** — there are **no GitHub pull requests**. Wherever this document says
"PR", "green PR", "one PR per concern", etc., read it as **one local merge unit**:

1. **Branch** a workstream (or sub-slice) off the current `origin/release/0.9.0` tip.
2. **Implement** it, writing the required characterization tests first.
3. **Gate:** `bunx tsc --noEmit`, `bun run lint`, unit, and the relevant integration
   tests must all pass.
4. **Local clean-code review:** an architect-role reviewer reads the diff against this
   plan's contract for that workstream (zero behaviour change, KPI met, no regression,
   no over-engineering) and returns approve / revise.
5. **Merge** the approved branch into `release/0.9.0` **locally**, re-run the gate on the
   merged result, and **push `release/0.9.0`**. Merges are **serialized** (one at a time)
   so each workstream builds on the prior's merged tip — never push in parallel to the
   shared branch (stale-local-ref hazard).

The gate + the local review are the safety net; nothing merges red or unreviewed. Each
merge stays small and independently revertable.

## Re-baseline: stale #490 claims corrected at HEAD

The quick wins (#491–#495) already shipped, so most headline numbers are stale. Verified at HEAD:

- cli.ts LOC: issue header says 4,989; the QW comment says it dropped to 3,647; child #527 cites 4,724. ACTUAL at HEAD 2164f64 = 4,589 LOC (wc -l), 106 defineCommand matches (grep -c). So all three cited numbers are stale; cli.ts has grown back from the post-QW1 3,647 to 4,589. The 'drops cli.ts ~1,400 LOC' QW1 gain has largely eroded.
- Storage-leak counts are all lower at HEAD than the issue table: getDataDir/getStateDir/getCacheDir = 52 occurrences across 14 files (issue: 53 callers); state.db refs = 71 (issue: 94); index.db/getDbPath = 58 (issue: 102); workflow.db/withWorkflowDb = 15 (issue: 16, ~accurate); openDatabase/openExistingDatabase = 73 call sites (issue: 48 — this one is HIGHER now); files importing from bun:sqlite = 23 (issue cited 48 'call sites importing Database'). The 102 index.db figure that drives the #524 'touch 102 callers' narrative is now ~58.
- QW2 already shipped and changed the AkmAssetType story: it is NO LONGER `= string`. At HEAD src/core/common.ts:27 defines `type AkmAssetType = (typeof ASSET_TYPES)[number]` (literal union). The #490 smell #3 'AkmAssetType = string (alias!)' is stale. However ref:string signatures are now 197 (issue: 186) — that smell got slightly WORSE, not addressed.
- QW4 already shipped: shapeForCommand and formatPlain are now registry-dispatched, not giant switches. src/output/shapes.ts has 1 case arm (issue: 16); src/output/text.ts is a registry dispatcher (issue: 68 arms). src/output/shapes/ has 18 files, src/output/text/ has 29 files — the per-command registry already exists. PASSTHROUGH_COMMANDS moved to src/output/shapes/passthrough.ts. The #490 smell #4 and QW4 are fully landed.
- QW5 already shipped: serializeFrontmatter clone count dropped from 9+1 to 3 sites; the reflect-local clone is gone. Smell #6 is largely resolved.
- QW3/sandbox: tests/_helpers/sandbox.ts is now imported by 100 test files (issue: '1 of 230'). mkdtempSync still appears in 145 test files out of 271 total test files (issue: 154/230). Env pokes = 1,129 process.env.XDG_/AKM_ references under tests (issue header: 518; child #526 already updated this to ~1,119 — so #526's number is current, the epic header's 518 is stale). withIsolatedAkmStorage does NOT exist yet (#526 unstarted).
- God-module list in #490 is partly stale/wrong-pathed. ACTUAL modules >1,500 LOC at HEAD: cli.ts 4,589, improve.ts 3,396, setup/setup.ts 2,826, consolidate.ts 2,694, indexer/db.ts 2,243, commands/health.ts 1,992 (NOT in issue list), indexer.ts 1,922, distill.ts 1,639. The issue lists 'metadata, proposals, wiki, text, reflect, state-db, db' — metadata.ts/proposals.ts/wiki.ts do not exist at those names; text.ts is now <1,446 (registry-split); state-db.ts is 1,398 (<1,500); reflect.ts 1,484 (<1,500); db.ts is indexer/db.ts at 2,243. health.ts (1,992) is a newly-large module the issue never mentions.
- migrate-storage.ts: issue body cites both 1,093 LOC and (leak #5) 1,093; child #528 cites 1,259. ACTUAL = 1,259 (wc -l). The epic's 1,093 is stale; #528's 1,259 is current.
- getLockfilePath duplication (#529 / leak #7) is STILL valid: confirmed two defs at HEAD — src/integrations/lockfile.ts:44 (private) and src/core/paths.ts:267 (exported). Note the epic body cites paths.ts:320 for the exported one but it is actually at :267 (as #529 correctly states). #529 is accurate.
- AkmImproveOptions field count: issue says 28; ACTUAL ~47 fields (awk count of typed members in the interface at improve.ts:84). runImproveLoopStage: issue says 16 args; ACTUAL ~17 destructured arg fields (improve.ts:2183, single args object). Both long-parameter-list smells are present and slightly worse than cited.
- Codebase-change premises in the task brief verified true at HEAD: vault asset type is gone (no 'vault' literal in asset-types), src/core/asset-create.ts exists (added), and the *-cli.ts extraction precedent is real — src/commands/ now has add-cli.ts, config-cli.ts, db-cli.ts, extract-cli.ts, feedback-cli.ts, improve-cli.ts, registry-cli.ts, remember-cli.ts (8 files). This means #527's target dir is ambiguous: precedent is src/commands/<name>-cli.ts but the architecture wants src/cli/commands/<verb>.ts — #527 acknowledges both but the split direction is unsettled.
- storage-locations.md hard-codes DB_VERSION = 14 (line 25) and graph schema framing 'As of graph schema v3 / DB_VERSION = 14'; actual src/indexer/db.ts:54 is DB_VERSION = 17. Doc is 3 schema versions stale.
- architecture.md still lists `vault` as a built-in asset type (line 23) and maps it to `vaults/` (line 28), and akm-core-principles.md lists 'vaults' among assets — but env-asset-refactor-plan.md + the task brief state vault was deprecated/removed in 0.9.0 in favor of `env`. architecture.md's Asset Types section is stale w.r.t. the env migration and does not mention the `env`/`secret` types at all (whereas v1-spec §4.1 DOES list env/secret and calls vault 'deprecated predecessor of env, removed in 0.9.0').
- AGENTS.md line 12 and ci.yml docs-ignore reference CLAUDE.md as a live governance doc, but CLAUDE.md does not exist at repo root on release/0.9.0 (ls confirms 'No such file or directory'). The pointer is dangling.
- v1-spec §7 module-layout tree is partly aspirational/stale vs the real tree: it shows top-level providers/ (twice), asset-types/, renderers/, embedders/, ingest/ that do not exist as written; the real layout (src/indexer/, src/output/renderers.ts, src/sources/providers/, src/registry/providers/) matches architecture.md's Module Boundaries table instead. Treat architecture.md's table, not §7, as the as-built map.
- storage-locations.md documents the proposal queue in two contradictory ways within the same file: a state.db `proposals` table that 'Replaces per-uuid JSON directories' (line 256-271) AND the on-disk $STASH/.akm/proposals/<uuid>/proposal.json layout (lines 419-420, 542-543). At most one is current; the doc was not reconciled when proposals moved into state.db. #490 work touching proposals must verify which is live at HEAD before assuming.
- v1-spec §2.6/§2.7 still shows the OLD per-asset commit pseudocode inline (writeAssetToSource committing per kind==='git'), with the 0.9.0 #507 batch-at-boundary correction appended only as an amendment box. per-asset-commit-unification-plan.md is written as a future ('Target design (0.9.0)') plan, but architecture.md already describes the batch model as shipped — so the plan doc reads as not-yet-done while architecture.md reads as done. On release/0.9.0 the batch model is the live contract; the plan doc and §2.6 pseudocode are historical.
- STABILITY.md line ~17 still enumerates 11 asset types including `vault` as supported (deprecated, 'removed in 0.9.0') and does not list `env`/`secret` in that stable-ref list, lagging the env-refactor that the task brief says already landed.

## Workstreams

### WS1 — StorageLocations resolver (single path-resolution seam)

- **Maps to:** #524 · **Effort:** M · **Depends on:** none
- **Rationale:** src/core/paths.ts exports ~15 path getters, but the AKM_DATA_DIR/XDG/transient-stash override precedence is copy-pasted across getConfigDir/getCacheDir/getDataDir, and two getters are byte-identical duplicates (getStateDbPath in state-db.ts:71 == getStateDbPathInDataDir in paths.ts:257). No single object owns 'where akm stores things', which is the root cause that makes #489 (XDG move) shotgun surgery. Lowest-risk highest-leverage seam; unblocks #525/#526/#529.
- **Scope:** NEW src/storage/locations.ts: typed StorageLocations resolver returning {indexDb,stateDb,workflowDb,lockfile,lockfileSentinel,dataDir,cacheDir,configDir,stashDir} delegating to existing core/paths.ts getters (additive pass-through, zero behaviour change).; Absorbs storage-leak findings 'StorageLocations resolver absent' and 'Duplicate getStateDbPath vs getStateDbPathInDataDir'.; Verified: getDataDir defined paths.ts:206; state.db dup at state-db.ts:71 and paths.ts:257; getStateDir() does NOT exist (0 hits) — field dropped from resolver shape.
- **Design patterns:** Facade / single-source-of-truth resolver, Resolve-once-at-boot value object
- **Before→after:** Before: callers import 17 free getters from 2 modules; env-override precedence triplicated. After: callers import one StorageLocations; free getters become thin delegates during migration (deleted only when call-sites migrate, not in this PR).
- **Regression strategy:** Purely additive: new module delegates to existing getters; no getter bodies change, no call-sites move in this PR. Add a unit test asserting locations.indexDb===getDbPath(), locations.stateDb===getStateDbPath(), etc. under a sandboxed env. tsc+lint+test green by construction.
- **Measurable KPIs:** new src/storage/locations.ts <120 LOC · 1 duplicate getter pair identified for deletion in WS9 · getDataDir refs tracked (22 today) for later single-source migration

### WS2 — Collapse duplicate getLockfilePath + .lck sentinel

- **Maps to:** #529 · **Effort:** S · **Depends on:** none
- **Rationale:** Confirmed at HEAD: getLockfilePath defined twice — private src/integrations/lockfile.ts:44 and exported src/core/paths.ts:267 (LOCKFILE_NAME==='akm.lock', semantically identical); lockfile.ts:55 re-derives the .lck sentinel duplicating getLockfileLockPath (paths.ts:272). Smallest lowest-risk win; good-first-issue.
- **Scope:** src/integrations/lockfile.ts: delete private getLockfilePath (line 44) and inline sentinel join (line 55); import getLockfilePath/getLockfileLockPath from core/paths (or StorageLocations once WS1 lands).; Absorbs 'getLockfilePath defined twice' and 'Duplicate getLockfilePath + re-derived .lck sentinel'.; Note: a third copy lives in tests/lockfile.test.ts:19 — leave or point at the import; do not change test behaviour.
- **Design patterns:** Single source of truth / Inline-then-delete duplication
- **Before→after:** Before: akm.lock path materialized in 2 source modules + sentinel computed 2 ways. After: 1 definition in paths.ts, lockfile.ts imports it.
- **Regression strategy:** Bodies are semantically identical (verified), so swapping the private copy for the import is behaviour-preserving. Existing lockfile tests are the characterization net; run full check.
- **Measurable KPIs:** getLockfilePath source definitions 2->1 · re-derived .lck expressions 2->1 · files referencing akm.lock literal reduced

### WS3a — Shared SqliteMigrationRunner extraction (high-value, do now)

- **Maps to:** #528 · **Effort:** L · **Depends on:** none
- **Rationale:** SEVERED from the larger migrate-storage rewrite per review. Verified two near-byte-identical SQLite migration engines: state-db.ts (interface :133 / MIGRATIONS :144 / ensureMigrationsTable :493 / runMigrations :513) and workflows/db.ts (:126/:135/:195/:245), differing by ONE line (bootstrapPreVersioningDb). This is the high-value, low-risk-to-extract core; it also feeds the openStateDatabase/ensureSchema decomposition. Independently shippable and unblocks nothing else, so it carries no downstream risk.
- **Scope:** NEW src/storage/engines/sqlite-migrations.ts: shared {Migration{id,up}}, ensureMigrationsTable, runMigrations(db,migrations,opts?{bootstrap}). state-db.ts/workflows/db.ts keep only their MIGRATIONS arrays + call shared runner (workflows passes bootstrap hook).; Absorbs migration-monolith findings on the two runners + 'Migration{id,up} interface declared twice'.
- **Design patterns:** Extract shared engine, Template method (optional bootstrap hook)
- **Before→after:** Before: 2 runMigrations bodies + 2 ensureMigrationsTable + 2 Migration interfaces. After: 1 runner, 1 table-setup, 1 Migration interface; each DB module supplies only its MIGRATIONS array.
- **Regression strategy:** Extract-and-delegate: the diff between the two runners is one line, so the shared runner with an optional bootstrap hook is provably equivalent. MANDATORY migration-replay characterization test BEFORE extracting: apply MIGRATIONS to a fresh DB, assert schema_migrations rows + final schema (sqlite_master DDL) byte-identical for both state.db and workflow.db. Hold green at every step.
- **Measurable KPIs:** SQLite runner bodies 2->1 · ensureMigrationsTable 2->1 · Migration{id,up} interface decls 2->1

### WS3b — migrate-storage MigrationStep registry + copyTree unification (severable, lower priority)

- **Maps to:** #528 · **Effort:** L · **Depends on:** WS3a
- **Rationale:** SEVERED per review: this is high-effort on a 1259-LOC operational script that runs rarely, so it is explicitly lower-priority and severable from WS3a. scripts/migrate-storage.ts has 54 hand-rolled ctx.recordStep boilerplate sites and two near-identical recursive copy helpers. Worth doing but NOT on any critical path; may be deferred without blocking the epic.
- **Scope:** scripts/migrate-storage.ts: introduce MigrationStep{id,scope,appliesWhen?,up,down?} registry + single runStep() owning skip/copy/verify/dry-run/recordStep; unify copyDirRecursive+copyDirNoClobber into copyTree({clobber}); extract MIGRATED_MARKER constant shared with src/indexer/unmigrated-vaults-guard.ts.; Keep old migrate-storage.ts working in parallel during cutover (locked decision).; Absorbs the remaining migration-monolith findings (54 recordStep clones, 2 copy helpers, .migrated magic-string).
- **Design patterns:** Registry / declarative step list, Template method (runStep), Extract constant for magic marker
- **Before→after:** Before: 54 recordStep clones; 2 copy helpers; .migrated string in 2 places. After: ~7 step terminals; 1 copyTree; 1 MIGRATED_MARKER constant; ~150 LOC glue long-term.
- **Regression strategy:** REQUIRED CUTOVER GATE: a fixture-stash differential test that runs the OLD script and the NEW registry against the same seeded stash and asserts identical step outputs (files copied, skip decisions, recorded steps, dry-run report) BEFORE deleting the old path. Old script stays intact in parallel until the differential test is green.
- **Measurable KPIs:** ctx.recordStep sites 54->~7 · recursive copy helpers 2->1 · migrate-storage.ts 1259 LOC -> <1000 (long-term ~150 glue) · .migrated magic-string sites 2->1 constant

### WS4 — withIsolatedAkmStorage test helper + allowlist ratchet

- **Maps to:** #526 · **Effort:** L · **Depends on:** WS1
- **Rationale:** RE-BASELINE: epic claims stale — sandbox.ts imported by 92 files (not 1), env-var writes ~1008 across 93 files (not 518), mkdtempSync in 145/271 files. The helper does NOT exist yet but the infra (tests/_preload.ts + sandbox.ts 5-helper chain) does. Gap: a composite to collapse the 5-helper cleanup chain (54 files chain 2+, 9 chain all 5, ~150 raw sandboxXdg* call-lines) and burn down the 73-entry grandfather allowlist.
- **Scope:** tests/_helpers/sandbox.ts: add withIsolatedAkmStorage<T>(fn,overrides?) per locked signature (ctx {dataDir,cacheDir,configDir,stashDir}, single tmp root, single cleanup, env restore). Fix divergent stash-skeleton lists (sandbox.ts:77 5-dir vs :137 7-dir) into one STASH_SKELETON_SUBDIRS constant sourced from the akm-init skeleton.; scripts/lint-tests-isolation.ts: make the 73-entry allowlist a shrink-only ratchet (meta-test asserts length never grows); block raw mkdtempSync(...akm-test...) outside tests/_helpers/.; Migrate the 62 env-writing files that don't import sandbox onto the composite (incremental, batched).; Absorbs all 6 test-isolation findings.
- **Design patterns:** Composite/Facade fixture, Extract constant, Ratchet (monotonic burn-down)
- **Before→after:** Before: 5 chained .cleanup lines + module-level let cleanup/stashDir/chain per file. After: one const {stashDir}=withIsolatedAkmStorage(); ~150 sandboxXdg* call-lines and ~54 mutable-state decls removed over the migration.
- **Regression strategy:** Helper additive; migrate files one batch per PR, running the existing leak tripwire (tests/_preload.ts afterEach) which THROWS on leaked env — that is the regression net. No production code touched.
- **Measurable KPIs:** withIsolatedAkmStorage exists · stash-skeleton lists 2->1 constant · linter allowlist 73 -> ~5 (shrink-only) · non-adopting env-writing files 62 -> 0 · raw sandboxXdg* call-lines ~150 -> ~0

### WS5 — Repository layer: with*Repo lifecycle + pull raw SQL out (threshold-gated)

- **Maps to:** #525 · **Effort:** XL · **Depends on:** WS1
- **Rationale:** The Database CONSTRUCTOR is already encapsulated (new Database in exactly 3 engine modules / 4 call sites — verified). The real leaks: (a) ~68 raw db.(prepare|query|exec) sites outside DB modules, index.db table names leaking into ~10 modules; (b) 9 command files hand-roll open/try/finally/close around index.db while a withWorkflowDb(fn) helper already exists (runs.ts:34) with no index.db/state.db twin. NARROWED per review: the existingDb? caller-owns flag is confirmed at only 6 sites across 2 files (search.ts:286/302/346, show.ts:299/336/345) — NOT 41 — so the connection-lifetime hazard is small and auditable.
- **Scope:** NEW src/storage/repositories/: generalize withWorkflowDb into withIndexRepo(fn)/withStateRepo(fn); extract WorkflowRunsRepository first (owns workflow_runs/workflow_run_steps SQL from runs.ts, 18 prepares), then ONLY repos clearing the threshold below.; EXTRACTION THRESHOLD (per review): a table gets a repo only if it has >=2 distinct callers OR >=3 raw-SQL sites. Low-traffic single-caller tables (e.g. RegistryCache) stay inline or fold into an existing repo — no ceremony repos.; Candidate repos passing the threshold to be confirmed by re-count at extraction time: WorkflowRuns (18 prepares), IndexMaintenance (indexer.ts:830-851), UsageEvents, Events, Proposals, TaskHistory, Index/Graph. Each is one PR.; Move lessons-coverage raw SQL (cli.ts:3433 collectTagSetFromEntries, 2 db.prepare) into a lessons repo so cli.ts holds ZERO raw SQL.; Bind WorkflowRunsRepository via StorageLocations.workflowDb so the repo is DB-location-agnostic and #489 changes only locations.ts.; Absorbs storage-leak findings: no repository layer, hand-rolled lifecycle, scattered SQL literals, table-name leaks; and cli-decomposition 'Raw SQL leaks into cli.ts'.
- **Design patterns:** Repository pattern, Scoped-resource / loan pattern (with*Repo), Intention-revealing methods
- **Before→after:** Before: db.prepare('SELECT * FROM workflow_runs WHERE id=?').get(id) in command code; manual close; 6 existingDb? ownership flags. After: withWorkflowRepo(r=>r.getRun(id)); SQL + lifetime behind the repo; ownership flag eliminated for migrated callers.
- **Regression strategy:** One repo per PR, leaf-first, threshold-gated. EXPLICIT LIFETIME HAZARD + RULE (per review): replacing an existingDb? ownership flag (search.ts/show.ts, 6 sites) with a with*Repo scope changes WHEN the connection closes relative to lazy prepared-statement iteration. RULE: every repo method MUST fully materialize its result set (.all()/array copy) and return plain values BEFORE the scope closes — no returning live statement iterators or cursors across the scope boundary. For each repo: write a characterization test capturing current query results on a seeded DB, move SQL verbatim into the repo method, re-point the single caller, assert identical results AND that the connection is closed exactly once. Audit the 6 existingDb? sites individually for iteration-after-close. tsc proves no dangling Database imports; full check per PR.
- **Measurable KPIs:** raw db.(prepare|query|exec) outside DB modules 68 -> <20 · command files importing openExistingDatabase/closeDatabase 9 -> 0 · existingDb? ownership-flag sites 6 -> 0 · index.db table-name reference files 10 -> ~3 · cli.ts raw-SQL prepare sites 2 -> 0 · repos extracted only when threshold met (no single-caller ceremony repos)

### WS6 — Split cli.ts into per-command modules + composition root + defineJsonCommand factory

- **Maps to:** #527 · **Effort:** XL · **Depends on:** WS5
- **Rationale:** cli.ts=4589 LOC/106 defineCommand/64 imports — a God Module. The extraction pattern is PROVEN: 8 *-cli.ts files already exist imported straight into main.subCommands with no call-site changes. Handlers are thin (52 dynamic await import). Largest inline block is env+secret (926 LOC). Verified at HEAD: 123 runWithJsonErrors boilerplate sites and 0 defineJsonCommand factory — the factory is now a tracked deliverable, not an aside.
- **Scope:** Extract per command FAMILY into src/commands/<group>-cli.ts (precedent): env+secret -> env-cli.ts/secret-cli.ts (largest), proposal(515), workflow(333), wiki(321), graph(~155), tasks(190), completions/setup/init residue. ANTI-FRAGMENTATION RULE (per review): group related subcommands into one *-cli.ts; do NOT create <100-LOC single-command files.; Hoist parseEnvRef/findEnvSource/makeEnvRef/resolveEnvPath/parseSecretRef/resolveSecretPath into NEW src/core/env-secret-ref.ts; route the 5 inlined env-path traversal-guard blocks (cli.ts:2147/2455/2494/2565) through resolveEnvPath; extract readInputValue() for the 3 cloned stdin/byte-limit readers; extract assertValidEnvKeys. NOTE (per review): env/secret logic moves into env-cli.ts/secret-cli.ts + core/env-secret-ref.ts ONLY — NO src/services/env-store.ts (it had no non-CLI consumer and violated the anti-speculative-layer guardrail).; TRACKED: introduce defineJsonCommand factory wrapping the runWithJsonErrors+output boilerplate; migrate sites incrementally with a KPI on boilerplate reduction.; Move *_SUBCOMMAND_SET routing constants into their command modules (derive from Object.keys(subCommands)).; Final: cli.ts -> composition root (imports + main defineCommand + import.meta.main block).; Absorbs cli-decomposition (all 6) + duplication findings 'env path-block inlined 5x','parseEnvRef trapped in cli.ts','stdin reader cloned 3x','ENV_KEY_RE dup'. Target band: <800 LOC composition root (the '<=500' is aspirational given 41 subCommands wiring).
- **Design patterns:** Composition Root, Per-command-family module / Strangler Fig, Extract function + parameter object (readInputValue), Command factory (defineJsonCommand)
- **Before→after:** Before: 4589 LOC, 106 defineCommand, 64 imports, env/secret inline, 123 runWithJsonErrors clones. After: ~700-800 LOC composition root, 0 defineCommand in cli.ts, ~15 imports; logic in <450 LOC per-family files; ref-resolution importable; JSON boilerplate behind defineJsonCommand.
- **Regression strategy:** Strangler-Fig, one command-family per PR, leaf-first. main.subCommands keys stay byte-identical so the public CLI surface (41 keys) is unchanged — no call-site changes outside cli.ts. defineJsonCommand must produce byte-identical stdout/stderr/exit-code for a migrated command (snapshot the JSON envelope before/after). Each PR: cut defineCommand+helpers to module, replace with import, run check (biome+tsc+unit+integration). Independently revertable.
- **Measurable KPIs:** cli.ts 4589 -> ~750 LOC across ~9 green PRs · defineCommand in cli.ts 106 -> 0 · imports 64 -> ~15 · env traversal-guard copies 5 -> 1 · cloned stdin readers 3 -> 1 · defineJsonCommand exists; runWithJsonErrors hand-written sites 123 -> <40 · no new <100-LOC single-command file created · cli.ts raw-SQL 2 -> 0 (with WS5)

### WS7 — AssetRef value-object threading + asset-type registry single-source

- **Maps to:** NEW · **Effort:** L · **Depends on:** none
- **Rationale:** Measurable primitive-obsession smell aligned with v1-spec AssetRef intent. Verified 228 ref:string sigs (worse than issue's 186) vs 93 parseAssetRef/makeAssetRef calls — refs serialize to strings at boundaries and re-parse deep in call trees (8 inline parse-and-discard sites). Registry/union drift: ASSET_SPECS has `task`, ASSET_TYPES union omits it; base-linter.ts refToRelPath is a 10-arm switch that already DIVERGED (task->.md vs registry .yml — a latent correctness bug); 34 inline ${type}:${name} constructions; makeAssetRef takes type:string not AkmAssetType.
- **Scope:** src/core/common.ts/asset-spec.ts: derive ASSET_TYPES from Object.keys(ASSET_SPECS) (or add task) — kill union/registry drift.; src/commands/lint/base-linter.ts: delete refToRelPath 10-arm switch, route through ASSET_SPECS.toAssetPath; build REF_RE from ASSET_TYPES (fixes latent task path bug).; src/core/asset-ref.ts: add refToString(AssetRef); tighten makeAssetRef(type:AkmAssetType); thread AssetRef through hot-path helpers in improve.ts/reflect.ts/graph.ts/proposals.ts (parse once at boundary).; Rename src/output/text/vault.ts -> env.ts (registers only env-* formatters post vault removal).; Absorbs all 7 dispatch-typing findings.
- **Design patterns:** Value Object threading, Single source of truth (derive union from registry), Named predicates / Strategy via registry fields, Rename
- **Before→after:** Before: ref:string passed+reparsed; 2 type catalogs; 34 hand-built ref strings; 10-arm path switch. After: AssetRef threaded; 1 catalog; refToString() single formatter; registry-driven path resolution.
- **Regression strategy:** Each sub-change independently shippable and behaviour-preserving EXCEPT the union derivation: deriving ASSET_TYPES from keys ADDS `task`, so it is NOT behaviour-preserving — gate it behind an equality test asserting the current union, run it to surface the `task` delta as an INTENTIONAL separately-reviewed fix, and gate the base-linter switch fix with a test asserting the corrected task path (.yml). Threading AssetRef changes types only, not runtime values — tsc is the net. Rename is mechanical.
- **Measurable KPIs:** ref:string sigs 228 -> <120 (toward 93 genuine parse points) · inline parse-and-discard sites 8 -> 0 · ${type}:${name} constructions 34 -> 1 refToString · asset-type catalogs 2 -> 1 · base-linter switch arms 10 -> 0 · text/vault.ts renamed

### WS8 — Frontmatter/atomic-write duplication cleanup

- **Maps to:** NEW · **Effort:** M · **Depends on:** none
- **Rationale:** Cheap low-risk duplication kills not owned by a child. Verified: 5 residual yamlStringify(fm).trimEnd() sites bypass serializeFrontmatter (the very drift it was created to stop); 3 hand-rolled ---\n...\n--- template literals bypass assembleAsset; secret.ts inlines a 2nd atomic-write impl despite writeFileAtomic existing. The DB-lifecycle residue formerly in this WS is REMOVED — it is fully owned by WS5's with*Repo (build one abstraction, not two).
- **Scope:** Replace 5 yamlStringify(fm).trimEnd() (extract.ts:193, consolidate.ts:999/1956/2285/2669) with serializeFrontmatter()/assembleAsset.; Replace 3 hand-rolled frontmatter templates (distill.ts:834, wiki.ts:775, remember.ts:112) with assembleAsset.; secret.ts: use writeFileAtomic(Buffer,mode) instead of bespoke fd+rename.; Absorbs duplication findings: residual yaml sites, hand-rolled templates, secret atomic-write. (openExistingDatabase try/finally residue handed entirely to WS5.)
- **Design patterns:** Replace inline with existing helper
- **Before→after:** Before: 5 yaml clones + 3 template clones + 2 atomic-write impls. After: serializeFrontmatter/assembleAsset/writeFileAtomic single-sourced.
- **Regression strategy:** Each replacement must produce byte-identical output — add a snapshot test on the produced asset file string BEFORE swapping (serializeFrontmatter body IS yamlStringify(fm).trimEnd(), so equivalence is provable). secret.ts swap gated by a test asserting same bytes + same file mode (0600). Mechanical, per-site, revertable.
- **Measurable KPIs:** yamlStringify(fm).trimEnd() outside asset-serialize 5 -> 0 · hand-rolled frontmatter templates 3 -> 0 · atomic-write implementations 2 -> 1 · serializeFrontmatter referencing files 3 -> 8

### WS10 — Parameter-object consolidation (PassContext / ImproveRunContext / AkmImproveOptions) — type-only, Phase-1

- **Maps to:** NEW · **Effort:** M · **Depends on:** none
- **Rationale:** PROMOTED out of the optional XL WS9 per review: this is the cheapest leverage-per-risk in the god-module cluster and must NOT be hostage to the deferrable akmConsolidate extraction. Verified at HEAD: AkmImproveOptions is a 30-field interface (improve.ts:84); runImproveLoopStage takes a ~22-field inline arg struct (improve.ts:2183); the 3 indexer pass fns (runMemoryInferencePass memory-inference.ts:152, runGraphExtractionPass graph-extraction.ts:426, runStalenessDetectionPass staleness-detect.ts:109) share a repeated positional signature that is also mirrored as injectable fields in AkmImproveOptions. All of this is type-only / tsc-verifiable with NO control-flow change.
- **Scope:** NEW PassContext value object for the 3 indexer pass fns + the mirrored injectable fields in AkmImproveOptions — collapse the repeated 7-positional signature.; Split AkmImproveOptions (30 fields) into nested groups (knobs / ImproveSeams / sync) — type reshape only.; Introduce ImproveRunContext for the runImproveLoopStage 22-field arg struct — pure parameter-object wrap; the function body is unchanged.; Absorbs the param-object findings (runImproveLoopStage arg struct, AkmImproveOptions field count, pass-fn positional params). The god-FUNCTION extraction (akmConsolidate etc.) stays in WS9.
- **Design patterns:** Parameter Object (PassContext, ImproveRunContext), Group cohesive fields into nested types
- **Before→after:** Before: 30-field option bag; 22-field inline arg struct; 7-positional pass signature cloned 3x + mirrored in options. After: AkmImproveOptions ~12 fields + 2 nested groups; runImproveLoopStage takes one ImproveRunContext; 3 pass fns take one PassContext.
- **Regression strategy:** TYPE-ONLY, behaviour-preserving by construction: reshaping a parameter list into an object passes the SAME runtime values; no branch, no order, no lifecycle changes. tsc --noEmit is the primary net (every caller must compile against the new shape). Do PassContext FIRST as the smallest validation of the approach. Existing improve/indexer unit + integration tests confirm runtime parity. Each of the 3 reshapes is an independent revertable PR.
- **Measurable KPIs:** AkmImproveOptions top-level fields 30 -> ~12 + 2 nested groups · runImproveLoopStage positional/struct fields 22 -> 1 ImproveRunContext · pass-fn positional params 7 -> 1 PassContext (x3 fns) · zero control-flow diff (assert via tests + review)

### WS9 — God-function decomposition (consolidate/improve-stages/setup/health) — OPTIONAL/DEFERRABLE

- **Maps to:** NEW · **Effort:** XL · **Depends on:** WS5, WS10
- **Rationale:** Highest blast-radius, highest regression risk; explicitly NOT in #524-#529. The type-only param-object wins are now in WS10, so WS9 is ONLY the genuinely-risky control-flow extractions. akmConsolidate is the single largest function (~1086 LOC, 8 nesting levels, consolidate.ts:1048); akmDistill ~770; akmReflect ~687; improve.ts stages 267-617 LOC; setup.ts 2826 and health.ts 1992 are unlisted god-modules. Lands LAST, behind characterization tests, and may be deferred entirely if the risk budget or branch-coverage is insufficient.
- **Scope:** Extract-Function pipelines: akmConsolidate, akmDistill, akmReflect, improve.ts stages into src/commands/improve/*.ts sharing the ImproveRunContext from WS10 (sub-helpers already exist as siblings) — HIGH risk.; setup.ts -> src/setup/steps/* (registry-driven) — MEDIUM, simpler registry-extraction; treat as separable sub-PR with its OWN characterization plan (snapshot the generated setup artifacts + prompts), do NOT bundle its regression net with consolidate.; health.ts -> src/commands/health/checks/* with a registry driver — MEDIUM, same separable treatment (snapshot each check's pass/fail output independently).; Absorbs the remaining god-module findings (control-flow extraction only).
- **Design patterns:** Extract Function / Extract Class, Guard clauses to flatten nesting, Registry (health checks, setup steps)
- **Before→after:** Before: 1086-LOC akmConsolidate at 8 nesting levels; setup.ts 2826; health.ts 1992 monoliths. After: <80-LOC orchestrators + named <150-LOC phase fns; nesting <=3; setup/health as step/check registries.
- **Regression strategy:** MANDATORY characterization tests FIRST, and BRANCH coverage (not just happy path) is the gating question for the HIGH-risk items (akmConsolidate touches clustering/merge/dedup/LLM): capture end-to-end output (proposals emitted, files written, journal) for consolidate/distill/reflect/improve on seeded fixtures covering each major branch, pin as snapshots, then extract with snapshots held green at every step. setup.ts and health.ts get SEPARATE, simpler characterization nets (artifact/check-output snapshots) and ship as independent lower-risk sub-PRs ahead of akmConsolidate. EXPLICIT ESCAPE HATCH: if branch-coverage of akmConsolidate proves insufficient, DEFER that extraction rather than guess — WS9 must never block a release.
- **Measurable KPIs:** largest function 1086 -> <150 LOC · max nesting 8 -> <=3 levels · improve.ts 3396 -> ~6 files <600 LOC · no single function >250 LOC in improve.ts · setup.ts 2826 -> step registry + <200-LOC driver · health.ts 1992 -> check registry + <200-LOC driver

## Sequencing

1. Phase 0 (re-baseline, no code): publish verified-at-HEAD numbers into #490 and children; mark stale claims (cli.ts 4589 not 4989; Database already encapsulated to 3 modules/4 sites; sandbox.ts adopted by 92 files; env writes ~1008; ref:string 228; existingDb? only 6 sites not 41; runWithJsonErrors 123 sites; defineJsonCommand does not exist). Corrects mis-sized children BEFORE any PR.
2. Phase 1 (parallel, low-risk additive/type-only seams — no behaviour change, unblock everything): WS1 StorageLocations, WS2 getLockfilePath, WS3a SqliteMigrationRunner, WS4 withIsolatedAkmStorage, WS7 AssetRef/registry, WS8 frontmatter dup, and WS10 parameter-objects (PROMOTED here — type-only, tsc-verifiable). WS1/WS2/WS3a/WS7/WS8/WS10 have no inter-deps and run fully in parallel; WS4 depends only on WS1. Highest quality-per-risk; land first. WS3b (migrate-storage registry) trails WS3a, lower priority, can slip without blocking anything.
3. Phase 2 (the cutover that unblocks #489): WS5 repository layer, leaf-first one repo per PR (WorkflowRunsRepository first, threshold-gated, bound via StorageLocations so #489 is location-only). Depends on WS1. WS5's with*Repo SUPERSEDES any generic withDatabase — build the repo abstraction, not two. Audit the 6 existingDb? sites for iteration-after-close per the WS5 lifetime rule.
4. Phase 3 (mechanical wiring, depends on WS5 so cli.ts can drop raw SQL): WS6 split cli.ts + defineJsonCommand factory, one command-FAMILY per green PR (anti-fragmentation: group by family, no <100-LOC files), env+secret last/largest, NO services/ layer. WS8's env-ref extractions feed WS6.
5. Phase 4 (highest risk, OPTIONAL/deferrable, behind branch-covering characterization tests): WS9 god-function CONTROL-FLOW extraction only (param-objects already shipped in WS10). Do the simpler setup.ts/health.ts registry sub-PRs first with their own nets; defer akmConsolidate/improve extraction if branch coverage is insufficient. Depends on WS5 (SQL-free seams) and WS10 (ImproveRunContext).
6. Critical path to #489 unblock: WS1 -> WS5 (Phase-2 cutover). Everything else parallelizes around it. WS9 and WS3b are off the critical path and must never block a release.

## Repo-wide measurable targets

- cli.ts: 4589 LOC -> ~750 (composition root); 106 defineCommand -> 0; 64 imports -> ~15; 2 raw-SQL prepare sites -> 0; runWithJsonErrors hand-written 123 -> <40 via defineJsonCommand.
- Database engine boundary: keep `new Database` at exactly 3 modules/4 call sites (already true — guard against regression); raw db.(prepare|query|exec) outside DB modules 68 -> <20; index.db table-name reference files 10 -> ~3; existingDb? ownership flags 6 -> 0.
- Path resolution: 17 scattered getters -> 1 StorageLocations seam; getLockfilePath defs 2 -> 1; getStateDbPath duplicate pair -> 1.
- Migrations: SQLite runner bodies 2 -> 1 (WS3a); Migration{id,up} interfaces 2 -> 1; migrate-storage.ts 1259 -> <1000 long-term ~150 glue (WS3b, severable); ctx.recordStep 54 -> ~7; copy helpers 2 -> 1.
- Tests: withIsolatedAkmStorage exists; lint-isolation allowlist 73 -> ~5 (shrink-only ratchet); non-adopting env-writing files 62 -> 0; raw sandboxXdg* call-lines ~150 -> ~0.
- Typing/dup: ref:string sigs 228 -> <120; ${type}:${name} constructions 34 -> 1; asset-type catalogs 2 -> 1; yamlStringify(fm).trimEnd() clones 5 -> 0; hand-rolled frontmatter templates 3 -> 0; atomic-write impls 2 -> 1.
- Parameter objects (WS10, Phase-1): AkmImproveOptions 30 fields -> ~12 + 2 groups; runImproveLoopStage 22-field struct -> 1 ImproveRunContext; pass-fn positional params 7 -> 1 PassContext.
- God functions (WS9, if taken): largest function 1086 -> <150 LOC; max nesting 8 -> <=3; no improve.ts function >250 LOC; setup.ts 2826 / health.ts 1992 -> registry + <200-LOC driver each.
- Process: `bun run check` (biome + tsc --noEmit + unit + integration) green on 100% of PRs; public CLI surface (41 subCommands keys) byte-identical throughout.

## Guardrails

- ZERO behaviour change is the contract. Every local merge keeps `bun run check` green and the public CLI surface (41 subCommand keys) byte-identical. No merge may change on-disk storage locations (STABILITY.md 'storage consolidation' is under user review — WS1/WS5 are abstraction-only, paths unchanged).
- One shippable concern per local merge; each independently revertable. Never gate epic #490 as a whole — gate + locally review each merge unit only. No GitHub PRs; merges land directly on `release/0.9.0` after a local clean-code review.
- Build ONE abstraction per concern: with*Repo (WS5) supersedes a generic withDatabase; StorageLocations (WS1) is the only path seam. NO src/services layer unless a concrete NON-CLI consumer exists — env-store.ts is explicitly DROPPED (env/secret logic lives in env-cli.ts/secret-cli.ts + core/env-secret-ref.ts).
- WS5 connection-lifetime rule: every repo method MUST fully materialize result sets (.all()/array copy, plain return values) before the with*Repo scope closes — never return live statement iterators/cursors across the scope boundary. Audit the 6 existingDb? sites (search.ts/show.ts) individually before collapsing them.
- WS5 repo threshold: extract a repo only for tables with >=2 callers OR >=3 raw-SQL sites; low-traffic single-caller tables stay inline — no ceremony repos.
- WS6 anti-fragmentation: group related subcommands into one *-cli.ts family file; do NOT create <100-LOC single-command files. defineJsonCommand must emit byte-identical JSON envelopes (stdout/stderr/exit-code) for migrated commands.
- Additive-before-subtractive: new module delegates to existing code; delete old getters/helpers ONLY after every call-site is migrated and tsc proves no references remain. No big-bang deletions.
- Characterization tests MANDATORY before any non-mechanical change: WS3a migration-replay, WS3b old-vs-new fixture-stash differential (cutover gate), WS5 per-repo query-result + close-once, WS7 union-equality (surfacing the intentional `task` delta), WS8 byte-identical asset snapshot + 0600 mode, WS9 BRANCH-covering end-to-end snapshots. WS10 is type-only (tsc is the net). If coverage is insufficient, DEFER rather than guess — WS9 (and WS3b) are explicitly optional.
- Re-derive numbers at HEAD before sizing each child; epic headline figures are stale (quick wins shipped). Do not implement to stale targets.
- No new features, no scope creep into #489's actual XDG move, no speculative layers. Resist gold-plating the layered architecture beyond what a finding demands.
- DELETION SAFETY: deleting a duplicated function (getLockfilePath, copyDirNoClobber, refToRelPath switch) is git-tracked source removal and safe; NEVER run rm/git clean against gitignored or user-data paths (stash/vault/.env/secrets) during any of this work.

## Architect review (clean-code expert)

**Verdict:** accept-with-revisions

**Completeness.** Strong coverage. Every dimension in the inventory maps to a workstream, and the plan correctly absorbs the orphan smells the children (#524-#529) did not own: AssetRef threading + registry drift (WS7), frontmatter/atomic-write duplication (WS8), and the god-functions (WS9). The re-baseline (Phase 0) is the right first move and the stale-number corrections are accurate (verified: cli.ts=4589 not 4989; Database already confined to 3 modules/4 sites; getLockfilePath dup real; the two runMigrations differ by exactly one line; `task` drift real). 

Gaps in completeness: (1) The plan acknowledges but does not schedule the `runImproveLoopStage` 19-field arg struct / ImproveRunContext consolidation as a discrete deliverable separable from the high-risk akmConsolidate extraction — it is bundled into WS9 (XL, optional). The parameter-object work (PassContext, ImproveRunContext, AkmImproveOptions decomposition) is LOW/MEDIUM risk and type-only; burying it inside the deferrable XL god-function workstream risks it never shipping despite being the cheapest leverage in that cluster. (2) No workstream owns the `defineJsonCommand` factory / 102 runWithJsonErrors+output boilerplate sites — WS6 mentions it in passing but it is not a tracked deliverable with a KPI. (3) setup.ts (2826) and health.ts (1992) decomposition is folded into WS9 with no separate characterization-test plan distinct from the consolidate/improve work, even though they are simpler registry-extractions at lower risk than akmConsolidate.

**Robustness.** Mostly real, with two soft spots. The genuinely behaviour-preserving workstreams (WS1 additive delegate, WS2 semantically-identical body swap, WS3 one-line-divergent runner extraction with migration-replay test, WS7 type-only threading with tsc as net, WS8 byte-identical-output snapshot) have credible, specific regression nets — not hand-wavy. The 'derive ASSET_TYPES from Object.keys(ASSET_SPECS)' change is correctly flagged as NOT behaviour-preserving (it adds `task`) and gated behind an equality test that surfaces the delta as an intentional separately-reviewed fix — good discipline.

Soft spot 1 — WS5 (repository layer, XL): 'write a characterization test capturing current query results on a seeded DB, then move the SQL verbatim' is the right idea, but moving SQL 'verbatim' behind intention-revealing methods is exactly where subtle behaviour changes hide (transaction boundaries, prepared-statement caching/reuse, connection lifetime when the existing `existingDb?` ownership flag is replaced by a with*Repo scope). The 41 `existingDb?` sites encode caller-owns-vs-callee-owns close semantics; collapsing them into a loan-pattern scope can change WHEN a connection closes relative to lazy iteration of a prepared statement's results. The plan does not call out statement-iteration-vs-connection-close ordering as a specific hazard.

Soft spot 2 — WS9 characterization tests for akmConsolidate/akmDistill/akmReflect rely on snapshotting 'proposals emitted, files written, journal' on seeded fixtures. For a ~1086-LOC function at 8 nesting levels touching clustering/merge/dedup/LLM, snapshot coverage of the BRANCHES (not just the happy path) is the real question, and the plan's own escape hatch ('if coverage insufficient, defer') is honest but means the regression strategy is conditional, not guaranteed. Marking WS9 optional is the correct mitigation.

**Over-engineering risks flagged (and folded into the plan):**
- WS5 introduces 7 repositories (WorkflowRuns/IndexMaintenance/UsageEvents/Events/Proposals/TaskHistory/Index/Graph/RegistryCache). The inventory justifies repos for the high-leak modules (runs.ts 18 prepares, indexer.ts 13), but a repo for low-traffic tables (RegistryCache, single-caller tables) may be ceremony without payoff. The plan should require each repo to clear a threshold (e.g. >=2 callers OR >=3 raw-SQL sites) before extraction, not extract all 7 by default.
- NEW src/services/env-store.ts (WS6): the guardrails themselves warn 'do NOT pre-build src/services unless a concrete command needs one' — yet WS6 scope lists env-store.ts as a planned module. env/secret logic moving to a *-cli.ts file already removes it from cli.ts; a separate services/ layer is only justified if the logic is reused by a non-CLI caller. As written this is a speculative layer the plan's own guardrail forbids.
- WS3's aspiration to drive migrate-storage.ts to '~150 glue long-term' via a full MigrationStep registry is high-effort for a 1259-LOC script that runs rarely and is operational tooling. The shared SqliteMigrationRunner extraction (the byte-identical-minus-one-line runner) is the high-value part; the 54-recordStep registry rewrite is a larger, riskier secondary that should be explicitly severable and lower-priority.

**Gaps:**
- No explicit hazard called out for WS5: replacing `existingDb?` ownership flags with a with*Repo scope can change connection-close timing relative to lazy prepared-statement iteration — needs a stated rule that repo methods fully materialize result sets before the scope closes.
- Parameter-object work (PassContext, AkmImproveOptions decomposition, ImproveRunContext) is LOW/MEDIUM-risk and type-checkable but is trapped inside the optional XL WS9; it should be promoted to a Phase-1 standalone workstream so it ships independent of the high-risk god-function extractions.
- defineJsonCommand factory for the 102 runWithJsonErrors+output sites is mentioned but untracked — no KPI, no owner, could silently drop.
- WS6 KPI band: plan admits '<=500 aspirational, ~400 unrealistic' and targets ~750. Fine, but there is no stated lower bound below which further splitting is gold-plating — risk of over-fragmenting into too many tiny <100-LOC command files.
- No rollback/parallel-run validation specified for WS3's migrate-storage registry beyond 'keep old script working' — needs a fixture-stash differential test asserting old-vs-new step outputs match before cutover.

**Required revisions (applied in the final plan):**
- Promote the parameter-object subtasks (PassContext for the 3 indexer pass fns; AkmImproveOptions -> nested groups; ImproveRunContext for the improve stages) OUT of the optional XL WS9 into a standalone Phase-1 workstream. They are type-only / tsc-verifiable, LOW-MEDIUM risk, and are the highest leverage-per-risk in the god-module cluster; they must not be hostage to the deferrable akmConsolidate extraction.
- Remove src/services/env-store.ts from WS6 scope (or explicitly justify a non-CLI consumer). It violates the plan's own guardrail against speculative service layers; moving env/secret logic into env-cli.ts/secret-cli.ts plus core/env-secret-ref.ts already achieves the decomposition.
- WS5: add an explicit hazard + rule that repository methods must fully materialize (not lazily iterate) result sets within the with*Repo scope, and document the connection-close-timing change when replacing the 41 `existingDb?` ownership flags. Add a per-repo extraction threshold (>=2 callers or >=3 raw-SQL sites) so low-traffic tables don't get a repo by default.
- WS3: split into WS3a (shared SqliteMigrationRunner extraction — high value, S/L, do now) and WS3b (migrate-storage MigrationStep registry + copyTree unification — larger, severable, lower priority). Require a fixture-stash differential test (old script vs new registry produce identical step outputs) as the cutover gate.
- Make the defineJsonCommand factory a tracked WS6 deliverable with a KPI (boilerplate sites reduced) or explicitly drop it; do not leave it as an aside.
- Add a lower-bound guardrail to WS6 (e.g. group related subcommands into one *-cli.ts; do not create <100-LOC single-command files) to prevent over-fragmentation.

## Open questions for the maintainer

- WorkflowRunsRepository targets workflow.db today, but #489 may move it to state.db — #525 notes 'workflow.db (or state.db once #489 lands)'. The repo abstraction is supposed to make this transparent, but the children don't specify which DB the repo binds to at extraction time. Needs a decision so #525 doesn't bake in workflow.db.
- Ordering between #524 (StorageLocations) and #529 (collapse getLockfilePath): #529 says it can consume core/paths.ts:267 directly OR StorageLocations once #524 lands — 'can land before or after #524'. And #526/#525 both say they 'may be done first/separately'. The only hard sequencing is #525 Phase-2 cutover unblocks #489. Otherwise the children are loosely ordered — confirm intended parallelism.
- cli.ts target is stated inconsistently across the issue: '<=500 LOC' (architecture diagram + #527), '~400 LOC' (Phase 2 checklist), and 'drops cli.ts ~1,400 LOC' (QW1). With cli.ts at 4,589 LOC today, the realistic target band needs confirming.
- The 7 repositories map to 3 DBs (state.db, index.db, workflow.db). With #489 potentially consolidating DBs, is the repo-per-data-type split still correct, or should some repos merge? Not addressed by the children.
- AkmImproveOptions / runImproveLoopStage (long-parameter-list smell) and the improve.ts god module (3,396 LOC) are listed as smells in #490 but are NOT covered by any of #524-#529. Is improve.ts decomposition deferred, or should it be a new child?
- CLAUDE.md is referenced as authoritative by AGENTS.md (line 12: 'read CLAUDE.md and docs/technical/v1-architecture-spec.md first') and by ci.yml docs-ignore globs, but NO CLAUDE.md exists at repo root on release/0.9.0. Is it gone (and AGENTS.md is now the governance file), or expected to be regenerated? #490 child issues should not cite a missing file as the contract source.
- storage-locations.md states DB_VERSION = 14, but src/indexer/db.ts currently has DB_VERSION = 17. The doc is stale by 3 schema bumps. Does the repository-layer issue (#525) need to re-audit the actual schema/tables before wrapping them, since the authoritative storage doc is behind HEAD?
- The 'Storage layout consolidation' item in STABILITY.md 'On the horizon' is marked 'under user review. Will be announced before any move happens.' Does #490's storage encapsulation count as that consolidation (requiring announcement), or is it purely internal refactoring with zero on-disk location changes? This gates whether #524 may change any path.
- v1-spec §7 module layout shows an aspirational tree (providers/, sources/, asset-types/, renderers/, embedders/, ingest/, commands/) that does NOT match the actual src/ layout (src/indexer/, src/output/, src/integrations/, src/commands already exist but differ). Which layout is the refactor target — the §7 ideal or the as-shipped architecture.md Module Boundaries table (which reflects reality more closely)?
- The proposal queue has TWO documented storage models: v1-spec §11.1 + storage-locations.md asset-files section say directory-per-id JSON under $STASH/.akm/proposals/<uuid>/proposal.json, while storage-locations.md §state.db says a proposals TABLE in state.db 'Replaces per-uuid JSON directories'. Which is current at HEAD? A repository layer must target the live one.

