Plugin-owned command groups render a header, and the gate now covers them (#1005) - #1222
Conversation
…them (#1005) `hyp client claude-desktop --help` and `hyp client claude-account --help` opened on a naked `usage:` line: neither group has a bare command, and neither plugin registered a `CommandGroupRegistration`, so `renderGroupHelp` skipped the header and the reader was never told what the group is for. Both plugins now register a group description (LLP 0214 #d2), so the header and paragraph render the way every core group's does. The durable half is the gate. `test/core/cli-consistency-gate.test.js` swept core group prefixes only, so it was green while these two were headerless. The sweep body moves into `assertGroupHeaders` and runs a second time over a registry built from core plus every bundled plugin's dry-run contributions, so the next plugin cannot repeat this. Building that registry needs the registration's aliases (`vector` is an alias of `query vector`, and without it the sweep would report a group that renders correctly), so the doctor's dry-run snapshot now carries them. Three test fixtures that hand-roll an activation context gained the `registerGroup` stub the two plugins now call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nd its group summary is platform-honest Three review findings on the gate extension, all in the sweep's fidelity to what a real install does. The merged replay skipped a command whenever its name OR any alias was already registered. That first-wins skip is right for the `session` trio (ai-gateway and opencode each guard with `ctx.commands.get`, and a per-plugin dry run cannot see the other's registry), but wrong for an alias collision: `CommandRegistry.register` throws on one, the loader records `activate_failed`, and every registration after it in that plugin's `activate()` is lost. Swallowing it here would leave the sweep green over a CLI half of which never registered. The skip now covers the name case only and asserts on the alias case, naming the plugin, the command, and the alias. `assert.ok(dry.ok, ...)` dropped `dry.error`, so an unrelated activation failure surfaced in the group-header test with no cause. It now carries the kind and message. The claude-desktop group summary said "on this Mac", rendered unconditionally on every platform. `install` and `verify` refuse anything but darwin, so the group help now says "on macOS" and is true wherever it renders. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Verdict: approve after fixes. 5 findings, all low. 3 fixed and pushed as
|
Verdict: approve. Round 2 found 0 actionable findings; nothing changed, nothing pushed.Round 2's brief was the safety net itself: round 1 edited the gate, so this round re-proves 1. The gate is still non-vacuous after round 1's editRound 1 replaced an unconditional The original defect, re-caught at this head. Deleted the whole Restored; back to 26/26. So the narrowing cost the gate nothing on the defect it was built for. The new alias assertion fires, with a message that names the culprit. The interesting Plugin, command, and the colliding token, all three. Restored. And the production consequence round 1 asserted is real. Confirmed at source, not 2. The name-only narrowing lost no coverageFirst-wins existed to absorb the duplicate Hit exactly three times, all And the replay can't be tightened further without breaking something else. I checked 3. Round 1's other two fixes landed correctly
Kind and message both present, with the stack as a bonus. Restored. "on macOS" is accurate. Verified what the group actually does rather than taking the
4. The two deliberate omissions are genuinely pre-existingBoth re-checked against
5.
|
| its finding | severity | where it lands here |
|---|---|---|
| name-collision skip cannot tell a guarded re-registration from an unguarded one | medium | O2, above |
[name, alias] both-collide case reports a collision production never has |
low | O1, above |
commandGroups drops help, so the sweep proves the header and never the paragraph |
low | section 4, second item |
the new descriptions are invisible in hyp client --help |
low | section 4, first item |
It rates the name-collision gap medium where I recorded it as informational, and its
reasoning is worth having on the record: a plugin claiming a name core already owns
activates cleanly in the dry run (which has no core commands), so claimed comes back as
exactly [command.name], the replay skips, and the sweep stays green while the real loader
throws and drops that plugin entirely. That is a sharper statement of the hazard than mine.
It does not change the verdict, for the reason given in O2: it is strictly pre-existing and
strictly improved by this PR (before round 1 the alias case was swallowed too), it is latent
rather than live (the only names skipped today are opencode's three genuinely guarded
session * commands, which we both verified independently), and a per-plugin dry run cannot
distinguish the two cases even in principle - a plugin's ctx.commands.get guard would not
fire in the dry run but would fire for real. Closing it needs a different mechanism, not a
tighter assertion.
So: no finding from either reviewer asks anything of this PR, and the marker stays clean.
Checks run in the worktree
npm test: 5796 pass / 0 fail / 1 skippednpm run typecheck: clean (exit 0)npm run smoke -- cli_bundled_plugins_activated: oktest/core/cli-consistency-gate.test.jsalone: 26/26 in 378ms (test 18 costs ~140ms)hyp dev plugin doctoronclaude-desktopandclaude-account: 0 errors, 0 warnings
No commit, no push: the head under review is unchanged at
0627adea174659f64c60ebc7b4ede2761de05741.
Ship risk:
|
1 participant
What was wrong
Verified on
origin/masteratc0daf4e2by running the real CLI against aninstall with both plugins active, not by reading code:
No header, no description.
hyp client claude-account --helpdid the same.Neither group has a bare command, and neither plugin registered a
CommandGroupRegistration, sorenderGroupHelpskipped the header line(LLP 0214 #d2) and the reader was never told what the group is for.
PR #1004 fixed the three core groups in this state and added a gate, but the
gate sweeps
coreRegistry()only, so it was green the whole time these twowere headerless.
What changed
1. The two groups get a voice.
@hypaware/claude-desktopand@hypaware/claude-accounteach register a group description.--helpnowopens with the header and paragraph every core group's does.
registerGroupis metadata only, so nothing new appears in
list()and no manifest entrychanges.
2. The gate covers plugin-contributed groups. Rather than a parallel
gate, the existing sweep body in
test/core/cli-consistency-gate.test.jsmoves into
assertGroupHeaders(registry, run)and runs a second time over aregistry built from core plus every bundled plugin's contributions. The
plugin half comes from
dryRunActivate(the doctor's own isolated pass:throwaway paths, inert source registry, nothing binds), which
test/plugins/bundled-command-manifest-agreement.test.jsalready runs overthis same workspace. The new sweep adds ~0.4s.
Building that merged registry needs each registration's aliases:
vectorisan alias of
query vector, sohyp vector --helprenders correctly today,and a sweep blind to aliases would report it as a fourth headerless group.
RegisteredSnapshot.commandDetailstherefore now carriesaliases, readstraight off the registration.
3. Three fixtures.
plugin-command-visibility,claude-desktop-install-helperandclaude-desktop-installhand-roll anactivation context; each gained the
registerGroupstub the plugins nowcall.
Failing before, passing after
Before (source reverted, test kept):
After:
Revert proof:
git stashof the two plugin files alone reproduces thefailure above;
git stash poprestores 26/26.npm test5796 pass / 0 fail,npm run typecheckclean,npm run smoke -- cli_bundled_plugins_activatedok.Contracts checked
docs/CLI_REFERENCE.mddocuments both command groups but does not quote the--helpheader, and nothing generates or diffs it against CLI output. No--jsonshape changes:registerGroupfeeds group help only.No LLP: this realizes LLP 0214 #d2 (Accepted) rather than changing it, and
the new
@refs point at it.Not addressed
hyp claude-desktop --help(the alias prefix) still exits 2 with "unknowncommand". Alias prefixes are not group-navigable anywhere in the CLI, core
included, and the core sweep enumerates canonical names for the same reason.
Out of scope here.
Fixes #1005