test: add a repository-wide CLI consistency gate - #848
Conversation
The focused CLI suites each prove one mechanism, so nothing enforced agreement across the whole registered surface at once. Add a structural sweep that enumerates every visible core command and asserts, for all of them together: usage lines invoke the command they document, options are spelled the way the parsers accept, every subcommand hangs off a registered bare command or group, aliases resolve to their owner, verb projections keep the usage their schema generates, top-level help lists exactly the visible top-level tokens (hidden commands out, subcommands one level down), every group renders its registry-backed table, every leaf renders its own summary and usage, and every help-only group rejects an unknown subcommand with exit 2 while naming the real ones. An active-plugin fixture covers group, leaf, and verb-projected help through the same dispatch, plus a manifest/runtime comparison pinned in both directions of drift. The two destructive commands keep their exact warnings. The sweep runs against an isolated HYP_HOME with an injected kernel, so it never boots, binds a listener, touches real user state, or reaches a service manager: dispatch intercepts --help before any command body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neutral review of #848. The gate catches every drift class its author demonstrated (all six injected mutations reproduced), but four of its own claims were weaker than the file said. - The injected kernel had no `cacheRoot`, so `createKernelRuntime` fell back to `defaultCacheRoot()` and bound 13 of the 22 tests to the developer's real `~/.hyp` rather than the temp `HYP_HOME`. Verified by making `defaultCacheRoot` throw: 13 failures before, 0 after. - `resolveConfigPath` reads `HYP_CONFIG` ahead of `HYP_HOME`, and the harness spread `process.env` while overriding only `HYP_HOME`. The sibling help tests in `command-dispatch.test.js` already pass `HYP_CONFIG: ''`; match them. - "stay dispatchable" was asserted against `registry.match`, which cannot see a visibility guard added inside `dispatch`. Verified by injecting one: the old assertion stayed green, the new one fails. - The `@ref` on the `HYP_HOME` sweep claimed to test the pre-boot property, but the harness injects a kernel so `bootKernel` is never reached (confirmed: the gate is 22/22 with `bootKernel` throwing). That property is pinned in `command-dispatch.test.js`; the gloss now says what this test actually holds. Also: the option-spelling check rejected `--format=<fmt>`, a spelling `verb_codec` does parse; both `--help` sweeps are now guarded against going silently empty if `listGroupChildren` regresses; and the header no longer claims no command body executes, since the unknown-subcommand sweep runs the bare group commands on purpose. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neutral review of #848 at
|
| Mutation | Result |
|---|---|
core_commands.js:144 query maintain -> query compact, usage left stale |
not ok 2 - every usage line invokes the command it documents |
core_commands.js:146 --dry-run -> --dry_run |
not ok 4 - every option named in a usage line is spelled as an option the parsers accept |
collect list leaf inserted with no collect command or group |
not ok 5 - every subcommand hangs off a registered bare command or a registered group |
core_commands.js:458 "Prompts on a TTY" deleted from purge help |
not ok 22 - destructive commands keep their exact warnings |
group_help.js:94 renderGroupHelp stops printing its usage line |
not ok 14 - every visible group renders its registry-backed subcommand table |
group_help.js:40 listGroupChildren stops filtering hidden |
not ok 19 - an active plugin renders group, leaf, and verb-projected help ... |
dispatch.js:749 renderHelp stops filtering hidden |
not ok 9, not ok 12, not ok 19 (3 failures) |
The evidence table in the body is accurate. One wording nit: the listGroupChildren half of row 5 is caught by the fixture test, not by a sweep, so "2 sweeps fail" is really "1 sweep + 1 fixture".
I then probed the isolation claim directly. Ambient HYP_DEV_TELEMETRY=1, DEV_RUN_ID, and a HYP_CONFIG pointing at a plugin-enabling config each left the gate 22/22, so it is not env-sensitive in practice. Not vacuous.
Findings (all fixed in 9a656231)
1. medium - test/core/cli-consistency-gate.test.js:98: the injected kernel was bound to the developer's real ~/.hyp, not to the temp HYP_HOME.
createKernelRuntime({ commandRegistry }) passes no cacheRoot, so defaultCacheRoot() (src/core/runtime/activation.js:106) falls back to process.env.HYP_HOME || ~/.hyp - and the harness only ever put HYP_HOME into the env object handed to dispatch, never into process.env. Confirmed empirically: the kernel resolved to /home/<user>/.hyp/hypaware/cache, and the LLP 0070 usage-policy resolver to the sibling real state dir. This contradicts the header claim on line 14 ("no real user state is read or written"), and it makes the readdir(hypHome) guard on line 300 watch a directory nothing was ever pointed at. Harmless today because only --help argv is dispatched, but the guard cannot fail. Verified by mutation: making defaultCacheRoot() throw produced 13 failures at 611ab67e and 0 after the fix. Fixed by passing cacheRoot: path.join(hypHome, 'hypaware', 'cache').
2. low - :109: HYP_CONFIG was not neutralized.
resolveConfigPath (src/core/runtime/boot.js:466) honours env.HYP_CONFIG ahead of hypHome, and the harness spread process.env while overriding only HYP_HOME. The sibling help tests do this correctly - test/core/command-dispatch.test.js:183,225,240,348,390,1065 all pass HYP_CONFIG: '' alongside HYP_HOME. Latent only because workspaceDir is empty so no manifests exist to select against. Fixed by matching the sibling convention.
3. low - :293: the @ref overclaimed what the test holds.
@ref LLP 0009#top-level-help-lists-plugin-commands-without-booting [tests]: help touches no state reads as pinning the pre-boot property, but harness() always injects opts.kernel and dispatch skips bootKernel outright when it does. Verified: with bootKernel mutated to throw unconditionally, the gate is 22/22 green while test/core/command-dispatch.test.js takes 4 failures. So the pre-boot property is pinned there, not here. Per CLAUDE.md ("keep refs honest"; a ref must tell you something the code does not), the gloss now states what this test does hold - that no registration's help page seeds the install - and the comment points at the suite that owns the boot property.
4. low - :287: "stay dispatchable" was asserted against the registry, not through dispatch.
The test called registry.match(...) directly, a registry unit check that cannot see a guard added inside dispatch. Verified: injecting a visibility guard at src/core/cli/dispatch.js:325 (matched.command.hidden ? undefined : matched) left the gate 22/22 green at 611ab67e and now fails not ok 12. This mattered - hyp smoke <flow> is a hidden command, so the regression would have shipped. Fixed by dispatching each hidden command's --help through the harness.
5. low - :161: the option-spelling check rejected a spelling the parsers accept.
The split class [\s[\]<>|]+ omits =, and LONG_OPTION forbids it, so a usage line documenting [--format=<fmt>] would fail the gate as an invalid spelling even though src/core/cli/verb_codec.js:63,154 parses --flag=value. Latent (no current usage line uses the form). Fixed by splitting on = so the check applies to the option name. Re-verified the --dry_run mutation is still caught after the relaxation.
6. low - :15 and :305/:323: two accuracy/robustness nits.
(i) The header said "no command body executes", but hyp <group> zzz-not-a-subcommand does run makeGroupCommand's run - deliberately, and its body is only a registry read plus an error, so the behaviour is right and the prose was wrong. Reworded. (ii) The two --help sweeps skip on children.length === 0, so both go silently empty if listGroupChildren ever regresses to returning nothing - precisely the failure they exist to catch. Added assert.ok(checked > 0, ...) to both, matching the guard the help-only-group sweep already had at :351.
(b) Scope: Fixes #841 is correct as written - no change made
I checked each deferred criterion against the sibling issues' own acceptance criteria, and every one has a verbatim home on an open issue, so auto-closing #841 loses nothing:
| Deferred #841 criterion | Tracked on |
|---|---|
| "every visible core command rejects a representative unknown option with exit 2" | #836: "Add a parameterized test proving every visible core command rejects a representative unknown flag." |
| "Compare declared usage/options with parser-accepted positionals and flags" | #836: "Generate registered usage from the schema ... or add an automated usage/schema agreement check." |
| "Validate literal repair commands against the command registry" | #834: "Add or extend a test that executes/resolves the recommended command spelling against the core command registry." |
| active-plugin fixture over the real bundled plugins | #837: "Add a generic bundled-plugin contract test that compares every manifest-declared command name and summary with runtime registration ... Cover verb-projected commands and plugin group metadata." |
| selected-but-activation-failed behavior | #837: "Make dispatch misses distinguish at least inactive, selected-but-unavailable/activation-failed, and genuinely unknown commands." |
The last row is the thinnest link - #837 owns the behavior, and the gate-side coverage is implied rather than spelled out - but not enough to justify leaving #841 open indefinitely behind three in-flight PRs (#844, #849, #850). I did not edit the PR body and did not open a follow-up issue: a new issue here would duplicate criteria that already exist on #834/#836/#837, and duplicated backlog is its own defect. The body's own "Acceptance criteria deliberately not implemented here" section is an accurate record for whoever reads the closed issue later.
Other checks
- No LLP needed - agreed. The gate asserts invariants LLP 0009/0034/0181 already settled and decides nothing new; the three file-level
@refs are[tests]/[constrained-by]and now honest. - Tier placement correct per CLAUDE.md: deterministic registry/render logic, no daemon, no network, no service manager, so a traditional test rather than a smoke.
- Not slow, flaky, or order-dependent. Standalone 0.25s; full suite unchanged at ~16.0s; every dispatch gets its own
mkdtempHYP_HOME, and each of the 22 tests passes in isolation. The three long sweeps carrySWEEP_TIMEOUT_MS; I added it to the hidden-command test too, since it now dispatches in a loop. - Style clean: no semicolons, no em dashes, JSDoc types only.
npm test4279 pass / 0 fail / 1 skipped,npm run typecheckclean, both before and after the fix commit.
Nit, not fixed (non-blocking)
harness() creates two mkdtemp directories per call and removes neither, leaking ~24 empty temp dirs per run. Consistent with the rest of the suite, so left alone.
Post-push CI note: the one red check is not this PR's
Pushing 9a656231 turned the pull_request test job red on both Node 22 and 24, with exactly one failure: not ok 1960 - no tool transcript is tracked in the repo (test/core/repo-scratch-hygiene.test.js:80). It is not caused by this PR. The push job at the same SHA, which builds the branch alone, is green; only the merge-with-master job fails, because origin/master tracks x/npm-test.log and x/typecheck.log. I confirmed by checking out ec3361bb on its own: master fails that test with no PR merged in at all.
The timeline explains why 611ab67e looked green: those files landed on master in adb448ab (#785) at 19:24 UTC and the lint that forbids them landed one commit later in bf9e4773 (#796); this PR's earlier pull_request run was at 19:05 UTC, before either.
Already tracked - issue #852, with PR #853 ("Delete the two committed tool transcripts that hold master red") open to fix it. Nothing to do on #848; it just cannot show green until #853 lands.
I also merged current origin/master into the branch locally and ran the full suite: 4504 pass / 1 fail, the single failure being that same master-owned test, and the gate itself 22/22. So this branch is compatible with master as it stands today. The merge was local only and was not pushed; origin/fix/issue-841 is exactly 9a656231.
Summary
Fixed and pushed as 9a656231: findings 1-6, all in test/core/cli-consistency-gate.test.js. Open: nothing. Findings 1, 3, and 4 were each confirmed by a mutation that the gate at 611ab67e could not see and now can.
…g temp dirs Two isolation defects in test/core/cli-consistency-gate.test.js. `dispatch` calls `installObservability()` with no argument, and that reads the real `process.env`, not the `env` the harness injects. With `HYP_DEV_TELEMETRY=1` exported the JSONL exporters wrote `<real HYP_HOME>/hypaware/dev-telemetry/*.jsonl` for every help render while `rendering help writes nothing under HYP_HOME` stayed green, because it only reads the temp home. Reproduced: 22/22 pass with three telemetry files landing outside the temp dir. Clearing `HYP_DEV_TELEMETRY` and `OTEL_EXPORTER_OTLP_ENDPOINT` on `process.env` (and in the injected env) disarms both exporters. `harness()` minted two `mkdtemp` directories per call and removed neither: 26 directories left in `os.tmpdir()` per run. A file-level `after` hook now removes them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review: PR #848 (
|
|
Triage at head |
What this adds
test/core/cli-consistency-gate.test.js: a structural sweep over the wholeregistered CLI surface, picked up automatically by
npm test(the runnerwalks
test/**/*.test.js).The focused CLI suites each prove one mechanism against one hand-written
example. Nothing enforced agreement across every registration at once, which
is how the audit found drift while 57 focused tests were green. This gate
asserts, for every visible core command together:
Registration inventory (registry only, no dispatch)
run();usageinvokes the command it documents (hyp <name> ...), so a renamecannot leave an unrunnable usage line behind;
<required>/[optional]groups;(catches
--dry_run,--dryRun,-dry);so no leaf lands under a namespace nothing describes;
usageForVerbgenerates from its schema(LLP 0034: one declaration, no CLI/MCP drift).
Help surface (through real
dispatch)--helpnever reaches a command body (proved with a fixture whoserunthrows), pinning LLP 0009's central interception;
hyp --helplists exactly the visible top-level tokens, sorted, withsubcommands one level down and hidden commands absent but still
dispatchable;
-hrenders the same page as--help;HYP_HOME;its own usage line and every visible child;
hyp <name> - <summary>plususage: <usage>with an empty stderr;
the real ones; an unknown top-level command exits 2 and points at help;
Active-plugin fixture
Group, leaf, and verb-projected help all render through the same dispatch; a
namespace with no bare command earns one synthesized top-level row; an
internal (hidden) command stays out of both. A
manifestRuntimeDrifthelpercompares manifest-declared commands against runtime registrations and is
pinned on all three findings it must produce (manifest-only, runtime-only
public, summary drift), so the comparison is not a tautology.
Safety-critical prose
Deliberately few exact assertions, on the two destructive commands only:
hyp purgeandhyp report deletekeep their verbatim warnings.Isolation
Every dispatch runs with a fresh
HYP_HOME, an emptyworkspaceDir(so nobundled manifest is discovered), and an injected kernel, so nothing boots, no
listener binds, and no real user state is read or written. Only
--helpandunknown-subcommand argv are dispatched, so no command body runs and no
service manager is reachable (LLP 0181). The sweeps carry an explicit
timeout, so a regression in the help interception fails the gate instead of
hanging on a command body that waits for input.
Evidence it catches drift
Each mutation was applied to the tree, the gate run, and the tree restored:
query maintaintoquery compact, leavingusagestaleevery usage line invokes the command it documentsfails--dry-runmisspelled--dry_runin a usage lineevery option named in a usage line is spelled as an option the parsers acceptfailscollect listwith nocollectbare command or groupevery subcommand hangs off a registered bare command or a registered groupfailshyp purgehelpdestructive commands keep their exact warningsfailsrenderGroupHelpstops printing its usage line;listGroupChildrenstops filtering hiddenrenderHelpstops filtering hidden commandsClean tree: 22/22 pass. Full suite:
HYP_HOME=$(mktemp -d) npm testis4279 pass / 0 fail / 1 skipped.
npm run typecheckis clean.Acceptance criteria deliberately not implemented here
Four of the issue's criteria cannot be satisfied on
mastertoday becausethe drift they exist to catch is still present and is the subject of the
sibling issues in this batch. Landing them now would put a red gate on
master, and weakening them to pass would defeat the point, so they areleft to the issues that own the corresponding fix:
option with exit 2." Unknown-option handling is currently ad hoc:
hyp backfill zzzexits 1,version/daemon stop/daemon restartignorearguments entirely. That is Standardize core command argument validation and usage generation #836, whose own acceptance criteria already
call for exactly this parameterized test once the parsers are unified.
flags." The
parseCommandArgvschemas are inline in the command bodiesand not exported, so there is nothing repo-wide to compare against yet.
Standardize core command argument validation and usage generation #836 covers generating usage from the schema or adding the agreement
check. The verb-projected half of this, where the schema is reachable,
is included above.
possible."
src/core/commands/clients.jsand the ai-gateway sessioncommand still print
(hyp start), which the registry does not answer to.That is CLI repair paths recommend nonexistent hyp start #834.
bundled plugins, and "Cover selected-but-activation-failed plugin
behavior once Enforce plugin manifest and runtime command agreement #837 settles it."
@hypaware/context-graph-enrichhas livemanifest/runtime summary drift, and Enforce plugin manifest and runtime command agreement #837 explicitly owns the generic
bundled-plugin contract test. The fixture-level comparison here proves the
mechanism without asserting over the drifted tree.
The inactive-plugin fixture criterion is already met by the existing
test/core/dispatch-inactive-plugin.test.js, so it is not duplicated.Once #834, #836, and #837 land, extending this file with those assertions is
additive: the inventory helpers and the isolated harness are already here.
No LLP was minted. The gate tests invariants LLP 0009, 0034, and 0181 already
settled; it does not decide anything new.
Fixes #841