The verb registry cannot release a name: add unregister(name) that also retracts the projected CLI command

[philcunliffe]

Root cause

registerVerb claims a name on two surfaces at once: byName + byTool in the verb registry, and a CLI command projected into the command registry on the spot. Neither registry supported removal, so a claimed name could never be given back.

That blocks the grep-search work. hypaware-server #364 registers its own archive-backed grep_search and displaces a kernel-shipped twin through verbs.unregister when the kernel offers it, re-checking getByTool rather than trusting the call. Without the affordance, once hyp query grep lands every server host boots a kernel twin holding the grep_search slot and answers from the local cache only: no org BlobStores, no export ledger, no archive fan-out (LLP 0264 §verb calls shipping the kernel half alone "a regression on every server host").

The fix

VerbRegistry.unregister(name) (src/core/registry/verbs.js)

• deletes from byNameandbyTool, so the tool slot is genuinely free and a re-register succeeds instead of throwing "already registered";
• retracts the projected CLI command, and only that one. register() skips projection when a command already occupies the name (commandAlreadyRegistered), so the retraction tests identity, not name: src/core/cli/verb_command.js records every command it projects in a module-level WeakSet<CommandRegistration> and exports isVerbProjection(command), which gates the removal. A per-registry projected ledger was the first shape and does not work: core verbs are projected before any registry-level ledger exists, so it reads empty for exactly the commands that need retracting. A pre-existing same-named command fails the identity test and is left alone;
• is by-name, idempotent, and a no-op on an unknown name, which is exactly the shape the server feature-detects. It runs at daemon boot, so a throw would take boot down.

CommandRegistry.unregister(name) (src/core/registry/commands.js) is the matching removal: it resolves what get resolves (primary name or alias) and clears everyaliasIndex entry pointing at the removed command, so the name and its aliases are claimable again and match(argv) no longer routes at a command that is gone. Verb projections carry no aliases today (verbToCommand emits none), but the registry-level contract has to stay correct for commands that do.

Both are declared on the VerbRegistry and CommandRegistry interfaces in hypaware-plugin-kernel-types.d.ts, so plugins and the server see them in the published types. A verb registry built over a command registry that predates unregister still releases both maps (the projected command is the only thing left behind), mirroring how commandAlreadyRegistered already tolerates a registry without has.

No policy is added: nothing here decides who wins a contested tool name. That stays the server's call in its LLP 0178.

The tests that prove it

12 new tests, all failing on master before the change (verbs.unregister is not a function / commands.unregister is not a function) and passing after.

test/core/verb-registry.test.js

• removal frees both the name map and the tool map, and drops out of list()
• a released name can be claimed again, and the replacement's command projects again
• unknown name is a no-op; a second removal of a real name is a no-op
• the projected CLI command is retracted (get, has, list all clear)
• a pre-existing same-named command the registration did not project survives
• a verb registry with no command registry unregisters cleanly

test/core/command-registry-unregister.test.js (new)

• get / has / list / size all clear
• alias-index cleanup, including match(['d']) no longer routing, and other commands' aliases untouched
• name and aliases re-claimable after removal
• removal by alias, exactly as get accepts one
• unknown name (and empty string) is a no-op
• hyp --help no longer lists the retracted command, driven through dispatch(['--help'], ...)

npm test is green (4495 pass, 1 skipped, 0 fail) and npm run typecheck is clean in the worktree.

Fixes#871

[philcunliffe]

Verdict

The affordance is the right shape and matches #871's contract exactly (by-name, idempotent, total on an unknown name, both maps freed, no policy). One high defect made the headline promise false on the only path that matters: the projected CLI command was not retracted for core verbs at daemon boot. Fixed and pushed as e3c9d3ac. Two low findings reviewed and deliberately not actioned, reasoning below.

Findings

HIGH - unregister never retracts the CLI command on the real boot path

src/core/registry/verbs.js:61 (pre-fix). Retraction fired only when the name was in the per-registry projected Set, and on the boot path no core verb ever lands in it:

1. src/core/cli/dispatch.js:212 calls registerCoreCommands;
2. src/core/cli/core_commands.js:80-81 pre-projects every CORE_VERBS command so hyp --help renders before the kernel boots;
3. src/core/runtime/boot.js:184-190 builds the runtime over that same command registry, registerCoreVerbs runs, commandAlreadyRegistered is true, projection is skipped, projected.add never fires.

Reproduced on f4e12334:

registerCoreCommands(registry); createKernelRuntime({ commandRegistry: registry })
runtime.verbs.unregister('query sql')
-> verbs.get / getByTool: undefined (correct)
-> registry.get('query sql'): STILL PRESENT
-> registry.match(['query','sql']): STILL routes at the removed verb

This is precisely the silent divergence the PR exists to prevent. Once hyp query grep ships as a CORE_VERB per LLP 0264 §verb, a server host that displaces it gets the archive-backed implementation on the MCP tool and the kernel's local-cache implementation on hyp query grep, with no error anywhere. Same root cause also broke the "runtime re-created over a shared command registry" case that register's own comment at src/core/registry/verbs.js:46-48 anticipates.

The tests missed it because test/core/verb-registry.test.js uses bare registries, and the one test that does call registerCoreCommands (test/core/command-registry-unregister.test.js:79) registers a non-core verb name, so it takes the projecting branch.

Fixed in e3c9d3ac: retraction is now identity-based rather than bookkeeping-based.

• src/core/cli/verb_command.js records every command verbToCommand builds in a module-level WeakSet and exports isVerbProjection(command).
• src/core/registry/verbs.js drops the projected Set; retractCommand retracts only when registry.get(name) is a verb projection.
• That covers the pre-boot projection and the shared-registry re-creation case, while a plugin's own same-named command still survives (it was never a projection), so the "and only that one" guarantee is unchanged.
• hypaware-plugin-kernel-types.d.ts doc comment updated to state the guarantee accurately.

Two regression tests, both red on f4e12334 and green after:

• test/core/command-registry-unregister.test.js - "unregister retracts a core verb command projected before the kernel booted": after registerCoreCommands + createKernelRuntime, verbs.unregister('query sql') clears registry.get, registry.has, and match(['query','sql']) falls back to the bare query group command.
• test/core/verb-registry.test.js - "unregister retracts a projection a different registry made over the same command registry".

LOW - unregister does not clear groups (not actioned)

src/core/registry/commands.js:93. A retracted command leaves any registerGroup prose behind. Reviewed and declined: groups and commands are independent registrations by design (LLP 0214 §d2), a group normally owns many commands, and dropping the group when one of them is retracted would be wrong. A group with no commands renders its own header, which is the documented behavior for a plugin namespace.

LOW - plugins reach the raw registries (not actioned)

src/core/runtime/activation.js:141,146 hand ctx.commands / ctx.verbs to every activated plugin, so unregister lets any plugin silently retract a core or foreign command with no error and no log. Real, but out of scope by #871's own constraint: "removal is an affordance, not a policy; nothing in this task decides who wins a contested tool name." Worth a follow-up issue if the fleet ever runs untrusted plugins.

Checked and clean

Alias sweep in commands.js:97-99 (Map deletion during iteration is safe in JS); alias-vs-primary resolution matches get; the byTool.get(verb.tool) === verb guard; the no-command-registry path; the retractCommand feature-detect against a registry that predates unregister; verbToCommand emits no aliases so the alias path is contract-only, as the PR body says; @ref LLP 0264#verb anchor exists and the gloss is honest.

Checks

npm test 4497 pass / 0 fail / 1 skipped, npm run typecheck clean, on e3c9d3ac.

[philcunliffe]

neutral review round: e3c9d3ac -> findings (2 actionable, 2 fixed)

Reviewed with /code-review in an isolated worktree at e3c9d3ac. Baseline on
the reviewed head: npm test 4497 pass / 0 fail / 1 skipped, npm run typecheck
clean, @ref LLP 0264#verb resolves.

What I checked and found correct

• Map deletion while iterating aliasIndex is safe in JS; the alias sweep is
  complete, and unregister resolves a name exactly the way get does, so
  retractCommand's get(name) and commands.unregister(name) can never
  disagree about which entry they mean.
• The alias-shadowing hazard is closed by the identity check: a verb name
  colliding with another command's alias makes register skip projection
  (has covers aliasIndex), and since verbToCommand never emits aliases the
  shadowing command is not in the WeakSet, so retraction correctly no-ops.
• registerCoreCommands runs only where dispatch builds its own registry
  (src/core/cli/dispatch.js:212), so a retracted core verb command cannot be
  silently re-projected later in the same process.
• Nothing caches verbs.list() or the command list (src/core/mcp/server.js,
  makeGroupCommand, listGroupChildren all read live), so help and the MCP
  tool surface both track retraction.
• ctx.verbs is runtime.verbs directly, so the plugin that needs
  unregister actually reaches it.

Findings

1. unregister declared required contradicts the kernel's own runtime
tolerance - low - FIXED

hypaware-plugin-kernel-types.d.ts:911 and :1724 added unregister as a
required member of CommandRegistry and VerbRegistry, while
src/core/registry/verbs.js:154 guards typeof registry.unregister !== 'function' and the stated consumer (hypaware-server) is supposed to
feature-detect verbs.unregister. Compiled against the published
declarations, typeof verbs.unregister === 'function' narrows to always-true
(strict lint configs flag it as an unnecessary condition), inviting removal of
the guard; installed against any kernel older than this change - legitimate,
since plugins declare a kernel semver range - the daemon then throws
verbs.unregister is not a function at boot, exactly the boot-down failure the
implementation comment says must not happen. Secondarily, any downstream that
implementsCommandRegistry (proxy, wrapper, test double) stopped
typechecking until it grew the member.

Fixed: both members are now unregister?(name: string): void with the reason
recorded in the doc comment. The concrete factories pin it as present so
in-repo callers stay unconditional -
src/core/registry/commands.d.ts (the hand-maintained declaration that shadows
commands.js, and which the PR had left without the member at all) and
createVerbRegistry's @returns {VerbRegistry & { unregister: ... }}. The one
call site typed through the VerbRegistry interface
(test/core/command-registry-unregister.test.js:147) now feature-detects, the
way a host must.

2. Silent fall-through hides a half retraction - low - FIXED

Both tolerated branches of retractCommand (src/core/registry/verbs.js:153-155)
returned silently: a registry predating unregister, and a command under the
name that is not a verb projection. In that state verbs.unregister still
clears byName/byTool, so the caller's prescribed check - re-reading
getByTool - reports success while hyp <verb> keeps routing at the released
verb's run closure. That is precisely the "answers from the local cache
only... a regression on every server host" outcome LLP 0264 verb warns about,
and nothing in the logs identified the broken step (contrary to the repo's
log-driven-development guidance).

Fixed: each branch emits getLogger('verb-registry').warn(...) with
hyp_operation, status: degraded, and a distinguishing error_kind
(registry_without_unregister / command_not_verb_projection) plus
verb_name. The "no command under that name" case stays silent, since that is
normal, not degraded. Added
test/core/verb-registry.test.js coverage for the registry-predates-unregister
branch, which had none.

Non-blocking note (no code change)

The PR description says the fix "tracks the names it actually projected in a
projected set" on the registry. The landed implementation is a module-level
WeakSet<CommandRegistration> in src/core/cli/verb_command.js keyed by
command identity, with isVerbProjection - changed by the second commit
precisely because a per-registry ledger is empty for the core verbs a host
wants to displace. Worth updating the description so later readers reason about
the mechanism that actually shipped.

Result

Pushed f380c7ef to fix/issue-871. npm test 4498 pass / 0 fail / 1 skipped;
npm run typecheck clean. Both fixes verified present in the committed tree
versus e3c9d3ac.

[philcunliffe]

Neutral triage at head f380c7ef: the review-round cap is exhausted; every residual finding was judged against the head code and classified non-blocking (no production-defect risk). Deferred to follow-up issue #908:

1. commands.unregister leaves registerGroup metadata behind - design-consistent per LLP 0214, cosmetic help output at worst.
2. Plugins receive the raw registries, so unregister is unguarded - trusted-plugin model, explicitly out of The verb registry cannot release a name: add unregister(name) that also retracts the projected CLI command #871's scope ("affordance, not a policy").
3. PR description still describes the superseded per-registry projected Set rather than the shipped WeakSet identity mechanism - docs drift only.

Both review rounds' actionable findings (the HIGH boot-path retraction miss, the required-vs-optional unregister contract, the silent degraded fall-through) are fixed and verified present at this head; reviewer-reported checks on f380c7ef: 4498 tests pass, typecheck clean.