Files
msd-core/docs/adr/2346-command-dispatch-completion.md
Tom Boucher cf004df678 refactor(#2360): host dispatch table + state cutover pilot (ADR-2346 P1) (#2364)
* refactor(#2360): host dispatch table + state cutover pilot (ADR-2346 P1)

Pilot cutover for ADR-2346 Phase 1 (epic #2345). Introduces the Layer-2 host
dispatch table — dispatchHostCommand + HOST_COMMAND_ROUTERS, consulted in
runCommand's default case after capability/overlay dispatch, before the
unknown-command error. Migrates 'state' as the pilot: removes the hardcoded
case 'state': arm; state now dispatches default -> dispatchHostCommand ->
routeStateCommand, byte-identical to the old path (proven by the new
state-command-cutover equivalence test, 5-category template).

Host commands are NOT capabilities (core, non-toggleable, no tier/activationKey)
— the capability registry stays reserved for toggleable feature bundles per
ADR-959. This is the host-vs-capability distinction the merged ADR-2346 lacked;
the ADR is corrected here alongside the code that realizes it.

- gsd-core/bin/gsd-tools.cjs: HOST_COMMAND_ROUTERS + dispatchHostCommand
  (prototype-pollution-safe); wired into default case; case 'state': removed;
  dispatchHostCommand + HOST_COMMAND_ROUTERS exported for tests.
- tests/state-command-cutover.test.cjs: UNIT/DISPATCH/BEHAVIOR/REGISTRY
  equivalence (recording-mock + runGsdTools end-to-end + pollution guard).
- docs/adr/2346-*.md: refine Decision 1/2 to the host-table vs capability-
  registry model (correction that did not land in the merged #2355).

Behavior-preserving. Subsequent P1b/c PRs migrate phase/init/roadmap/validate/
verify using this proven template.

Closes #2360.

* test(#2360): regenerate golden fixtures + allowlist for state cutover

Bookkeeping for the gsd-tools.cjs change: npm run gen:golden regenerates the
install-parity fixtures (gsd-tools.cjs content hash changed), and the new
tests/state-command-cutover.test.cjs is added to the lint-test-file-count
allowlist under the 'state' prefix.

* refactor(#2360): migrate remaining Tier-1 routers (phase/init/roadmap/validate/verify)

Completes P1: all 6 Tier-1 host routers now dispatch via HOST_COMMAND_ROUTERS
(state landed in the pilot commit). init preserves its #1688 warnIfStaleBake
pre-hook; validate binds the output emitter. Cutover test extended to assert
all 6 are consumed + owned. Golden install-parity fixtures regenerated.
2026-07-17 09:19:28 -04:00

82 lines
8.2 KiB
Markdown

# ADR-2346: Command Dispatch Completion
- **Status:** Accepted
- **Date:** 2026-07-17
- **Issue:** [#2346](https://github.com/open-gsd/gsd-core/issues/2346)
- **Epic:** [#2345](https://github.com/open-gsd/gsd-core/issues/2345) (Command Dispatch Completion)
- **Builds on:** [ADR-959](959-capability-command-contribution.md) (Capability Command Contribution — graduated `Proposed → Accepted` by this ADR) · [ADR-0012](0012-command-routing-hub.md) / [ADR-0174](0174-retire-gsd-sdk-package-boundary.md) (CommandRoutingHub)
## Context
ADR-959 established that an in-tree command family is *"just a router, discovered via the registry instead of hardcoded"* into `runCommand`'s switch, and named `_dispatchNonFamily` as *"the deliberately-prepared seam for registry dispatch"* (today a dead shim that always returns `false`). Three first-party families (`graphify`/`audit`/`intel`) were cut over to `dispatchCapabilityCommand` in the `default` case.
But ADR-959's scope is **family discovery only** — it assumes the 73-case switch and the `route*Command` routers *persist*. It does **not** decide (a) dissolving the switch *entirely*, or (b) where single-purpose "leaf" verbs belong. As a result the switch was never dissolved, and `runCommand` remains the repo's largest structural liability:
- **#1 PageRank symbol** (most central),
- **#1 Tarjan articulation point** (removing it splits the call graph into 4 components),
- **#1 most complex function** (cognitive complexity 1927, cyclomatic 616, ~2,338 lines),
- with **4+ duplicated inline arg-parsers** (`capFlagValue`, `capRepeatedFlag`, `getFlagValue`, and bespoke per-arm consume-loops) and a 706-line `case 'capability':` arm nesting ~40 inline `cap*` helpers.
`runCommand`'s upstream blast radius is **LOW** — only `main()` calls it — so a dissolution is internally safe to execute phase by phase.
## Decision
Complete the ADR-959 cutover and dissolve the switch entirely into a **two-layer dispatch**, recording four decisions ADR-959 leaves open. Each was grilled to a shared understanding before this ADR landed.
### 1. Two-layer dispatch (end state)
`runCommand` collapses to a ~15-line dispatcher:
```
try capability registry (dispatchCapabilityCommand) // toggleable FEATURE capabilities — ADR-959, unchanged
→ try host dispatch table (dispatchHostCommand) // all non-capability commands — fills the prepared seam
→ unknown-command error
```
- **Layer 1 — capability registry (`commandFamilies`):** toggleable FEATURE capabilities only — `graphify`/`audit`/`intel` (+ genuine future features). Populated from `capability.json` `commands` arrays by `gen-capability-registry.cjs` per ADR-959. **Unchanged by this ADR.**
- **Layer 2 — host dispatch table (`dispatchHostCommand` + `HOST_COMMAND_ROUTERS`, consulted in the `default` case):** ALL non-capability commands. This fills the seam ADR-959 named (`_dispatchNonFamily`). It holds **host routers** (multi-subcommand core commands like `state`/`phase`/`capability`) AND **leaf verbs** (single-purpose commands like `generate-slug`), dispatched by a `{ command → handler }` table.
**Host commands are NOT declared as capabilities.** They are core, non-toggleable, carry no `tier`/`activationKey`/install-profile membership, and cannot be tier-gated or turned off — so the capability registry (whose model is "toggleable feature bundle") is the wrong vehicle for them. The capability-vs-host boundary is the load-bearing distinction this ADR adds over ADR-959: a single-purpose leaf is never perverted into a fake capability, AND a core host command is never perverted into a toggleable feature.
### 2. Host-router vs leaf classification rule
> Organize a non-capability command as a **host router module** when its cluster has **(a) ≥3 related subcommands**, **(b) a shared backing module**, and **(c) a shared parse/return shape**. Lone verbs or pairs stay **leaves** (two adapters over different modules ≠ one seam). Both host routers and leaves dispatch through the Layer-2 host table — the distinction is code organization (a router module vs a themed leaf module), not dispatch routing.
Applied: 9 host-router clusters result — `state`, `phase`, `init`, `roadmap`, `validate`/`verify`, `capability`, plus 4 promoted clusters (`config`, `research`, `resolve`, `git`). `worktree` + `workstream` stay leaves (2 verbs, different modules). ~40 remaining verbs rehome into ~4 themed leaf modules. **None of these are capability declarations** — they are host routers/leaves in the Layer-2 table. (The capability registry's feature families — graphify/audit/intel — are unaffected.)
### 3. Shared `parseFamilyArgs`
A single helper (in `cjs-command-router-adapter.cts`, beside `routeHubCommandFamily`) consumes `--flag value` pairs → `{ values, positionals, repeated }` and calls `error()` on missing values. It deletes the 4+ duplicated inline arg-parsers (`capFlagValue`/`capRepeatedFlag`/`getFlagValue` and the bespoke `resolve-*` loops). Value-validation (e.g. `--effort` boolean coercion) stays per-handler; file-reading helpers (`readRequired`/`readOptional`) stay with their handlers. Introduced with its **first real consumer** (the Phase-1 cutover), not as a zero-consumer "foundation" PR (one adapter = hypothetical seam).
### 4. Capability arm extraction shape
The 706-line `case 'capability':` arm becomes a thin `capability-command-router` (intel-shaped, using `routeHubCommandFamily`) plus a `capability-cli.cts` owning the CLI wiring (scope resolution, output formatting, reconcile sweep). Handler bodies stay thin (resolve → `lifecycle.X` → format) — the fat logic already lives in `capability-writer`/trust/consent modules and is *wired*, not moved. Duplicated probes are consolidated: `capHostVersion` reuses `readHostVersion()`; `capReadStrict` + drift-guard's copy collapse into one shared `readStrictKnownRegistries`.
### 5. Phasing (epic #2345)
Each phase is one approved sub-issue + one behavior-preserving PR targeting `next`, each proven equivalent by extending the `tests/audit-command-cutover.test.cjs` 5-category template (UNIT / DISPATCH / BEHAVIOR / JSON-ERRORS / REGISTRY):
| Phase | Content |
|---|---|
| P1 | `parseFamilyArgs` (first consumer) + Tier-1 family cutovers (`state`/`phase`/`init`/`roadmap`/`validate`/`verify`) |
| P2 | capability arm extraction + `readStrictKnownRegistries` consolidation |
| P3 | promote `config`/`research`/`resolve`/`git` clusters to families |
| P4 | leaf dispatch table (fills `_dispatchNonFamily`) + `runCommand` collapse to ~15 lines |
## Alternatives considered
1. **Amend ADR-959** to expand its scope to full dissolution — rejected: it would bloat a focused mechanism-ADR ("the `commands` field") into an execution-plan ADR. ADR-959 stays the mechanism; this ADR is the completion decision.
2. **Everything-is-a-registry-family** (even `generate-slug`) — rejected: the capability registry is for co-located feature *bundles*, not 3-line leaf verbs; it would manufacture ~60 tiny router files and 60 capability declarations for one-liners.
3. **One flat dispatch table, no registry** — rejected: abandons ADR-959's decided direction.
4. **Tier-1-only cutover** (pure ADR-959 completion, no dissolution) — rejected: the thin family arms aren't where the mass lives; `runCommand` would barely shrink and remain the #1 hotspot.
## Consequences
- **Positive:** the repo's #1 central/bridge/complexity hotspot is eliminated; locality (each family's parsing lives in its router) and leverage (one dispatch path, N families); the duplicated arg-parsers are killed once, everywhere; ADR-959 graduates `Proposed → Accepted` with working completion as its evidence.
- **Negative / cost:** a sequence of ~4 behavior-preserving cutover PRs; the two dispatch paths (registry + leaf table) coexist transiently until P4 collapses the switch; each cutover carries a cutover-equivalence test (real work, not a no-op).
- **Neutral:** every command keeps its exact name/output/exit-code/flags (behavior-preserving); unmigrated commands stay on their current path until their phase lands.
## Out of scope
Third-party / out-of-tree command modules (deferred per ADR-959 §5); the `runCommand` argument-resolution preamble (`--cwd`, `--json-errors`, workstream context) which stays in `main()`; any change to command *names* or *outputs*.