mcpx testing: inventory every feature against its integration tests, and guard that each has one #1978

Relocated from dezren39/darwin-nix#132 on 2026-10-28 (placement audit): mcpx's single source lives in developing-today/code (pkgs/mcpx); re-home tracked at #309. Original body below; issue references inside it are historical (dezren39/nix numbering).

Parent: #97

Summary

Map every mcpx feature to the integration tests that exercise it, list the features with none, and then make the map enforce itself: a guard, in the style of TestEveryDeclaredRouteAnswersOverTheSocket, that fails CI when a new op, command or setting ships without an integration test. The opencode plugin is part of the inventory. It was item 6 in the brief and is folded in here because its first step is small: run the test file that already exists.

This is a review (audit what exists) followed by a design (the guard). Closing the gaps it finds is follow-up implement work. File that work as one issue per feature group, not per op.

Original notes

• testing
  • a general goal is to ensure all features have one or more integration tests
    • the mcp servers can be specialized mocks but the app entrypoint or binary is used and config is passed in or config path set to appropriate test
    • for both binary && app entrypoint (consider binary a superset)

For this issue, "integration test" means what the two child bullets say: the real binary or an app entrypoint (#133) drives mcpx, mock MCP servers are allowed, and config is passed in. A unit test of internal/<pkg> does not count, however thorough it is.

Current state (written on d0b7e01; counts re-checked at 05c78b2)

Surfaces, and whether a guard can enumerate them

Gaps found by a cheap grep

Method: search internal/e2e/*.go for the literal command name as the first argument of e.run/e.try/e.runStdin, and for the literal /v1 path prefix. The grep can mislead in both directions. A mention does not prove a test, and a missing literal does not prove missing coverage, because the CLI reaches /v1 routes indirectly. The inventory has to go feature by feature. This list only says where to look first.

• CLI commands never invoked by e2e: adapter, api, completion, daemon, help, init, man, openapi, registry, scripts, plus the aliases namespaces and logs. Some are partly covered elsewhere. internal/adapter, internal/openapi and internal/registry have unit tests. package.nix postInstall runs man and completion, but nothing checks their output.
• /v1 paths with no literal in e2e (26 of 53): /v1/ask, /v1/ask/{id}, /v1/ask/{id}/abandon, /v1/ask/{id}/answers, /v1/client.ts, /v1/elicit, /v1/elicit/{id}, /v1/elicit/{id}/{action}, /v1/globals.d.ts, /v1/intent, /v1/namespaces, /v1/prompt, /v1/prompts, /v1/recipes, /v1/recipes/{name}, /v1/recipes/{name}/run, /v1/refresh, /v1/resource, /v1/resource-templates, /v1/resources, /v1/restart, /v1/search, /v1/session/release, /v1/shutdown, /v1/status, /v1/types. Several of these are reached through the CLI (mcpx elicit, mcpx status, mcpx prompt). All non-streaming ones are hit by the route guard, but that guard only asserts "not 404 and not 5xx" (internal/e2e/v1ops_test.go:350-357). It proves the route is served, not that it is correct.

Feature by feature (starter table; the review completes it)

The existing "every X has Y" guards (the model)

None of them asserts that a feature has a behavioural test. They assert reachability or parity. That is the gap this issue closes. The "declared but never read" audit and guard landed in #171 (rows above). It covers "every setting is read" and should not be duplicated here; the stronger form -- reading a setting can change an observable outcome through an ordinary caller -- is #129's (#181 collision 15).

Approaches

A -- a hand-maintained inventory table in docs (least recommended)

Put the table above in docs/testing.md and ask contributors to update it. It costs nothing and helps humans. It also starts going stale the day it is written. Nothing fails when someone adds an op and skips the table, and the history (#74: flags accepted and ignored, found by accident) shows that unenforced lists do not hold. Use it only as the output of the review, not as the mechanism.

B -- a static name guard

A Go test enumerates api.Ops(), the command table and the settings registry, then greps the e2e sources for each name. Any name that never appears fails, unless it is on an exemption list with a reason. It is cheap, runs in milliseconds, needs no daemon, and can run in every shard. The weakness: a mention is not a test. A name in a comment or an unrelated string satisfies it, so it measures presence, not behaviour, and it pushes people toward adding the literal rather than the test. Aliases and paths with parameters need special-casing.

C -- a runtime coverage ledger (recommended)

The e2e harness records what each test actually exercised. e.run/e.try log the resolved command (after alias and global-flag handling). The socket/TCP client logs the op. The daemon already records every request in its log store (/v1/log), so op coverage can also be read back from each env's state dir before cleanup. A TestMain in internal/e2e writes the ledger. A guard then compares it against the three registries and fails on any entry that was neither exercised nor exempted. The exemption list is a checked-in file with one reason per entry, like the route guard's inline skip list (v1ops_test.go:294-298).

Recommended because it measures what actually ran, so gaming it takes a real call. It extends naturally to "every setting was set by some test" and to the plugin's hooks. And it composes with #133: line coverage from a -cover binary is complementary evidence, not a replacement, because a covered line is not an asserted behaviour.

Costs:

• It needs the full e2e run. Under sharding (Bump golang.org/x/crypto from 0.16.0 to 0.17.0 in /sources/launcher #95) each shard writes a partial ledger, and a final job merges them and runs the guard. The CI tree owns that job; this issue defines the ledger format (JSON lines: {test, kind: cli|op|mcp|setting, name}).
• The command table must be enumerable (see above).
• Tests that call /mcp must go through a harness helper so the ledger sees them.

Take B's static check as a cheap per-shard pre-check if the merged job comes later. Do not treat it as the end state.

The plugin (folded item 6)

1. Run daemon.test.ts in CI. It runs as bun test plugin/opencode/mcpx/daemon.test.ts (the file's header comment). It needs only bun. The CI tree (Bump golang.org/x/crypto from 0.16.0 to 0.17.0 in /sources/launcher #95) owns where the step goes; it is under a second of work.
2. A plugin harness against the real daemon (build(deps): bump actions/checkout from 3 to 4 #47/Update Rust crate anyhow to 1.0.75 #48 need this). Load the plugin module with a stub opencode PluginInput, point it at an isolated e2e-style daemon (Update dependency htmx.org to v1.9.12 #138), and assert on the hooks. shell.env should inject MCPX_DAEMON_ENDPOINT (mcpx-session.ts:696), a tool call should reach fakemcp, and the TUI should load. Write it against Update Rust crate anyhow to 1.0.75 #48's harness-free core API once that exists, so the build(deps): bump actions/checkout from 3 to 4 #47 v2 rewrite reuses the same harness and does not get its own. Before Update Rust crate anyhow to 1.0.75 #48, a thin harness over the v1 plugin is acceptable, on the understanding it will be rewritten.

Open questions

• What is a "feature" for the guard: an op, a command, a setting, or also named behaviours (the ladder rungs, inline mode, profiles, aliases)? Proposal: the three enumerable registries, plus a hand list of behaviour features in the same exemption/manifest file so they at least appear.
• Does reaching a command only through its alias count?
• Per setting: is TestEverySettingIsReachableOverV1 enough, or must each setting have a test proving it changes behaviour, as TestEveryExecFlagChangesSomething does for exec flags? The second is Add omasnap upstream repository to flake inputs #129's guard (Phase 3: sync, blob gate, and relays #181 collision 15, building on Phase 2: delegated Manage cannot add members #171's consumed_test.go), so decide it there, with Bump unsafe-libyaml from 0.2.8 to 0.2.10 in /sources/pavex/examples/realworld #99 (guard-per-bug-class policy).
• Should ops that have no CLI command (Update Rust crate clap to 4.5.4 - autoclosed #76) be flagged by the inventory as a separate class?
• Plugin tests are bun-only (Bun.serve). That is fine for opencode, which runs on bun. Does Update Rust crate anyhow to 1.0.75 #48's harness-free core need a node/deno run too?
• Should the behaviour-feature list become a real registry (a features.json the man page and the book also read), rather than a hand list inside the exemption file? It would pay for itself in the docs tree (Bump golang.org/x/crypto from 0.16.0 to 0.17.0 in /sources/import #96) (from Phase 2: user subject for tags needs an identity event #160).
• One exemption file, or one per registry? One is easier to review; per-registry is easier to own (from Phase 2: user subject for tags needs an identity event #160).

Ease · Impact · Related

Ease M (review S; ledger and guard M; closing the gaps is separate implement work) · Impact high
Depends on / related: #133 (exported command table, harness runner), #138 (isolation, needed before the plugin harness), #95 (merged-ledger job, bun step), #99 (guard policy), #93 (tests that randomise take the run seed), #47, #48, #76, #74 (closed, fixed in #75 -- the precedent), #171 (merged: the declared-but-never-read guard), #129 (the behaviour-changing guard, #181 collision 15), #137 (whether real-software-only coverage counts).

Security/auth deferred; see tracker.

Acceptance criteria

• A feature↔integration-test inventory exists (on this issue or in the docs tree and linked here) covering CLI commands/ops, settings, and the opencode plugin, naming the features with no integration test.
• A guard in the style of TestEveryDeclaredRouteAnswersOverTheSocket fails CI when a new op, command, or setting ships without an integration test, or the guard's design is pinned here with a follow-up implementation issue filed.
• Gaps the inventory finds are filed as follow-up issues, one per feature group (not per op), and linked here.
• The work described in the Summary/Scope above is delivered via a branch + PR (no direct push to main); verification runs in GitHub Actions CI, not on the sandbox.
• Evidence (CI links or inventory output as applicable) is recorded on this issue before it is closed.