refactor: centralize command family facets

[thymikee]

Summary

Centralizes command family facets behind a single registry and moves capture to the command-level shape from the architecture review.

Before: command metadata, executable definitions, CLI readers, daemon writers, output formatters, and CLI overrides were stitched together across separate central registries.

After: each family exports one family facet, and capture composes per-command facets for snapshot, screenshot, diff, wait, alert, and settings. This removes the old client-command facet registry and narrows intermediate exports. Also fixes format scripts to invoke the installed oxfmt entrypoint directly, avoiding the flaky shim path.

Touched files: 29. Scope stayed within command surface/family architecture and formatter script wiring.

Validation

Verified with formatter, command-surface guard tests, capture command tests, CLI args tests, fallow, tooling/build, and the full unit/smoke aggregate.

• node ./node_modules/oxfmt/bin/oxfmt --check src test skills package.json tsconfig.json tsconfig.lib.json rslib.config.ts vitest.config.ts .github/actions/setup-node-pnpm/action.yml .oxlintrc.json .oxfmtrc.json '!test/skillgym/.skillgym-results/**'
• vitest run src/commands/capture/index.test.ts src/commands/capture/screenshot-options.test.ts src/commands/tests/command-surface-metadata.test.ts src/utils/tests/args.test.ts
• pnpm check:fallow --base origin/main
• pnpm check:tooling
• pnpm check:unit

[thymikee]

#849 is not review-ready yet: GitHub reports mergeable=CONFLICTING / mergeStateStatus=DIRTY against main.

Please rebase/update the branch on latest main and rerun CI. Once it is clean and the full checks run, it should get a normal review pass.

[thymikee]

Reviewed the family-facet centralization. The core idea — one CommandFamilyFacet per family composed from per-command facets, with a single family/registry.ts merging metadata/definitions/readers/writers/schemas/formatters — is a clean consolidation, and the new command-surface-metadata.test.ts coverage is a good addition. A few things to address before this is mergeable:

🔴 1. #848's batch-step validation appears to be reverted (blocking)

This branch was cut before #848 (fix: validate batch steps through command contracts) landed on main, and the merge conflict has effectively undone it. On main, prepareBatchDaemonCommandRequest validates each step through the command contract before projecting:

return writer(metadata.readInput(input) as CommandInput);
// ...wrapped with: `Batch step ${stepNumber} ${command} input is invalid: ${message}`

In this PR command-projection.ts collapses to return writer(input), drops the stepNumber parameter (also removed from batch/projection.ts's PrepareDaemonCommandRequest type), and batch/cli.test.ts deletes the guard test batch rejects invalid structured step input before daemon projection. The "structured interaction target" test was also edited to drop its platform/udid input and assertions — consistent with step input no longer flowing through readInput.

Net effect: invalid structured batch steps (e.g. {"command":"focus","input":{"x":10}} missing y) are no longer rejected with the friendly per-step error, and common-option normalization on step input is lost. Note this regression won't show in the GitHub diff until you rebase (the PR's merge base predates #848), which makes it easy to miss. Please rebase onto main and preserve #848 — ideally by relocating the readInput validation into the new facet path rather than dropping it.

🟠 2. Compile-time exhaustiveness traded for runtime checks (see inline comment)

Several registries moved from … satisfies Record<CommandName, …> to listCommandFamily…() as Record<CommandName, …> (cli-grammar/registry.ts, cli-output.ts, cli-command-overrides.ts, command-surface.ts, command-metadata.ts). The satisfies previously guaranteed at compile time that every command had a reader/writer/schema; the cast removes that, leaving only the new runtime tests + Missing … for command throws. Worth checking whether defineCommandFamilyFromFacets can restore a CommandName-keyed exhaustiveness check.

🟠 3. Runner command traits decentralized + tests removed

runner-command-traits.ts and its test are deleted, and the single satisfies Record<RunnerCommand['command'], RunnerCommandTraits> table is split into three independent inline definitions: isReadOnlyRunnerCommand (|| chain in runner-contract.ts), PREFLIGHT_SKIP_ELIGIBLE_RUNNER_COMMANDS (set in runner-session.ts), and isRunnerReadinessProbeCommand (inline in runner-session.ts). This is the opposite of the PR's "centralize" theme, and it removes both the exhaustiveness guarantee and the test that enumerated every command — on a correctness-sensitive path (readiness-preflight skipping). If the decentralization is intentional, please add a test asserting these three sets cover the full RunnerCommand['command'] union.

⚪ Minor

• mergeable_state is dirty / conflicting with main (per your own bot comment). Resolving #1 via rebase should also clear this.
• package.json: switching format/format:check to node ./node_modules/oxfmt/bin/oxfmt to avoid the flaky shim is reasonable, but it now hard-codes the package layout — fine, just flagging it's coupled to oxfmt's bin path.

Items 2 and 3 are judgment calls (the runtime tests do provide a safety net); item 1 is the one I'd consider blocking.

---

Generated by Claude Code

[thymikee]

Centralizing these maps is a nice cleanup, but note the type-safety tradeoff: the old definitions used satisfies Record<CommandName, CliReader>, which guaranteed at compile time that every command had a reader (same pattern in cli-output.ts, cli-command-overrides.ts, command-surface.ts, command-metadata.ts). The new listCommandFamilyCliReaders() as Record<CommandName, CliReader> cast erases that guarantee — a newly added command can now compile while missing a reader and only blow up at runtime via the Missing CLI reader for command throw you added below.

The new command-surface-metadata.test.ts coverage mitigates this, but consider whether defineCommandFamilyFromFacets can re-establish a compile-time CommandName-exhaustiveness check so this stays as safe as the satisfies it replaced.

---

Generated by Claude Code