Skip to content

fix: name hyp daemon start in CLI repair paths - #844

Closed
philcunliffe wants to merge 4 commits into
masterfrom
fix/issue-834
Closed

fix: name hyp daemon start in CLI repair paths#844
philcunliffe wants to merge 4 commits into
masterfrom
fix/issue-834

Conversation

@philcunliffe

@philcunliffe philcunliffe commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Problem

Two live failure paths printed hyp start as the repair, but no such command is registered. Following the printed advice produced a second failure:

hyp: unknown command 'start'

The registered lifecycle command is hyp daemon start.

Change

  • src/core/commands/clients.js: the attach endpoint give-up message for an installed but unreachable daemon now says (hyp daemon start). The no-daemon-installed branch is unchanged (it already named hyp daemon install / hyp daemon start).
  • hypaware-core/plugins-workspace/ai-gateway/src/session_command.js: resolveGatewayEndpointForCli's error now says `hyp daemon start`.
  • Comments in src/core/commands/clients.js and src/core/cli/remote_commands.js that described the old repair follow the wording.
  • test/core/attach-endpoint-fallback.test.js: the case that preserved the bad spelling now requires the registered one.

No top-level start alias is added, per the acceptance criteria.

Regression test

test/core/repair-command-spelling.test.js extracts every quoted or parenthesized hyp ... mention from the two give-up messages and resolves it against a real core command registry (createCommandRegistry + registerCoreCommands) using the dispatcher's own longest-registered-prefix rule, plus a group check so an unknown subcommand under a group command (hyp daemon <x>) also fails.

Before the fix, 2 of its 3 cases failed:

hyp attach (daemon installed) recommends 'hyp start', which is not a registered command
hyp session endpoint resolution recommends 'hyp start', which is not a registered command

After the fix all 3 pass. Locally: npm test 4260 pass / 0 fail, npm run typecheck clean.

Fixes #834

test and others added 2 commits August 18, 2026 18:45
Two live failure paths told users to run `hyp start`, a command the
registry never had: the attach endpoint give-up message for an installed
but unreachable daemon, and the ai-gateway session endpoint resolution
error. Following either printed repair produced a second failure,
`hyp: unknown command 'start'`. The registered lifecycle command is
`hyp daemon start`.

Both messages now name the registered spelling, and the comments that
described the old repair follow. A new test resolves every `hyp ...`
spelling those messages print against the real core command registry,
using the dispatcher's own longest-prefix rule, so a repair line can no
longer name a command the dispatcher would reject. No top-level `start`
alias is added.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…any cast

The mention pattern matched only a run of lowercase words closed
immediately by a delimiter, so any repair carrying a flag or a
placeholder matched nothing and was skipped in silence. On the
no-daemon-installed message, whose two mentions include one that stays
well-formed, that is a green suite over an unrunnable repair: reword it
to `hyp start --foreground` and the guard never sees it. Capture the
body up to the closing delimiter instead and split it into argv, which
is what `registry.match` already consumes; the dispatcher stops at the
first flag on its own, so flags need no special case beyond skipping a
mention that is only a flag on the binary (`hyp --help`).

The group check now spares a help flag, matching `makeGroupCommand`'s
own rule via the exported `isHelpFlag`, so `hyp daemon --help` is not
read as an unknown subcommand.

`registerCoreCommands` takes exactly `ReturnType<typeof
createCommandRegistry>`, so the `any` cast on the registry bought
nothing and hid the argument from the typechecker. Every other caller
in the repo passes the registry directly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral review round: e66261b0

Verdict: approve the change, with two test-quality fixes pushed on top and three findings left open.

The production change is correct and complete. daemon start is genuinely registered (src/core/cli/core_commands.js:507), and after this change a repo-wide scan of every quoted hyp ... mention in src/ and hypaware-core/plugins-workspace/ against a real createCommandRegistry() + registerCoreCommands() registry finds no remaining core-command mention the dispatcher would reject. The only surviving hyp start strings are the two test comments that name it as the bug, and LLP 0178's plan text (see finding 4). npm test 4260 pass / 0 fail and npm run typecheck clean at both e66261b0 and the pushed head.

I also mutation-checked the new regression test at e66261b0: reverting either give-up string to hyp start turns 2 of its 3 cases red, so it does guard the thing it claims to.

Fixed and pushed: 1c39e250

1. test/core/repair-command-spelling.test.js:28 - medium (silent skip in the guard).
HYP_MENTION matched only a run of lowercase words closed immediately by `/'/). Any mention carrying a flag or a placeholder matched nothing at all and was dropped in silence. Because assertAllRegistered only requires names.length > 0 per message, the two-mention no-daemon-installed message hides this completely:

"...Run 'hyp daemon install' then 'hyp start --foreground' so it can attach clients."
old pattern -> ["daemon install"]          # green suite, unrunnable repair
new pattern -> ["daemon install", "start --foreground"]   # fails, correctly

The fix captures the body up to the closing delimiter and splits it into argv, which is what registry.match already consumes. No flag special-casing is needed beyond skipping a mention that is only a flag on the binary itself (hyp --help): registry.match already stops at the first - token, exactly like the dispatcher. The group check now spares a help flag via the exported isHelpFlag, mirroring makeGroupCommand's own rule, so hyp daemon --help is not misread as an unknown subcommand. Verified by mutation: (hyp start --foreground) in the installed-daemon message now fails the suite; before the fix that exact reword passed on the two-mention message.

2. test/core/repair-command-spelling.test.js:40 - low (dead any cast).
registerCoreCommands(/** @type {any} */ (registry)) cast away the argument for no reason: the parameter type is CommandRegistryExtended, which is defined as ReturnType<typeof createCommandRegistry> (src/core/cli/types.d.ts:356) - precisely what createCommandRegistry() returns. Every other caller in the repo (test/core/ignore-local-only-command.test.js:49, test/core/policy-command.test.js:58, ten smoke flows) passes it directly. Cast removed; typecheck stays clean.

I also added a short note to the file header recording that both messages under test name core commands only, so a future reword to a plugin command (hyp session ignore) is understood as "extend the registry", not "loosen the assertion" - see finding 3.

Open findings (not fixed here)

3. Core-only registry is a latent false-failure - low.
coreRegistry() holds no plugin commands, but one of the two messages under test lives in the ai-gateway plugin, which registers session ignore / unignore / status (hypaware-core/plugins-workspace/ai-gateway/src/index.js:64). If that give-up message is ever reworded to name one of its own verbs, this test fails on a spelling hyp actually accepts. Not fixed: registering plugin commands needs a real activation context, which is a materially heavier fixture than this test warrants. Documented in the header comment instead, so the next reader reaches for the registry rather than the assertion.

4. llp/0178-attach-prompts-to-enable.plan.md:121 - low (stale guidance).
T7 still reads: "Keep the existing wording and hyp start mention for the case where a daemon service is installed but not currently reachable." That is the instruction this PR reverses, and an implementer re-reading the plan could restore the bug from it. Left open deliberately: LLP 0178 is Status: Active, and CLAUDE.md's rule is that Active docs are records whose settled text is not edited. The sanctioned repair is a forward-ref, but the convention names an LLP number and this fix has none, so the shape of the correction is a maintainer call rather than a review-round edit. Note LLP 0174 #bootstrap-floor itself is not stale - it only ever constrained the no-daemon-installed branch - so both reworded @ref glosses in the test files are honest.

5. Test scaffolding duplication - low (quality).
makeBuf, withTempHome, and makeUnboundCtx in the new file are near-verbatim copies of makeBuf / withTempHome / makeCtx in test/core/attach-endpoint-fallback.test.js:22-80, and both files already share installFakeDaemonService from test/helpers/. Left open: per-file scaffolding is the prevailing convention here (makeBuf is defined independently in 69 test files, withTempHome in 10), so extracting only this pair would be inconsistent churn. Worth a separate sweep if the team wants a shared attach-give-up fixture.

Considered and rejected

assertAllRegistered only rejects a leftover token under a group command, so hyp sync bogus would pass. This is correct as written, not a gap: on a leaf command a leftover token genuinely is an argument (hyp attach claude, hyp query sql ...), and rejecting leftovers there would fail on valid mentions. The JSDoc above the function states this restriction accurately.

test and others added 2 commits August 19, 2026 00:19
The guard only saw `hyp ...` mentions wrapped in backticks, quotes, or
parens, and its "named no hyp command to check" floor is per message, not
per mention. In the no-daemon-installed message, which names two commands,
an undelimited `hyp start` was skipped silently while the other mention
kept the floor satisfied: all three cases stayed green with the exact
"hyp: unknown command 'start'" dead end back in the tree. Verified by
mutation before and after.

Also: forward-correct LLP 0178's T7 task line, which still instructs a
future implementer to keep the `hyp start` mention, and make the
attach-endpoint-fallback @ref gloss say what the test actually pins.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Review: findings (2, both low; both fixed)

Reviewed head 1d910efe. Fixes pushed as d42449bf.

The core change is right and the premise checks out. No start command or alias is registered anywhere (src/core/cli/core_commands.js registers daemon start; the repo's only alias is unattach -> detach), so hyp start really was an unrunnable repair. Both reworded strings, the two comments, and the retargeted attach-endpoint-fallback case are correct, and LLP 0174 #bootstrap-floor never fixed the spelling, so the reword does not edit a settled decision.

Verification performed

  • Mutation-tested the new guard so it is not accidentally self-agreeing. Reverting (hyp daemon start) -> (hyp start) in src/core/commands/clients.js:419 fails case 1; changing 'hyp daemon install' -> 'hyp bogus' fails case 2. Both attach branches are genuinely reached.
  • Swept every hyp <cmd> mention across src/, hypaware-core/plugins-workspace/*/src/, bin/, test/, docs/, README.md: no other runtime string names an unregistered command, and no hyp start remains outside comments and the LLP plan doc addressed below.
  • npm test: 4486 pass / 0 fail / 1 skipped. npm run typecheck: clean. Both green on the reviewed head as well, so nothing was pre-broken.

Finding 1 (low, fixed) - test/core/repair-command-spelling.test.js:41: the guard's regex could not see an undelimited mention, so the exact regression could walk back in with the suite green

HYP_MENTION required an opening `, ', or (, and the names.length > 0 floor in assertAllRegistered is per message, not per mention. In the no-daemon-installed message, which names two commands, an undelimited mention is skipped silently while the other one keeps the floor satisfied.

Evidenced by mutation, not inspection. Rewording src/core/commands/clients.js:423 from

`'hyp daemon start' so it can attach clients, or set 'listen' in the ` +

to

`hyp start so it can attach clients, or set 'listen' in the ` +

left all three cases in repair-command-spelling.test.js passing, with hyp: unknown command 'start' back in the shipped message. (The single-mention daemon-installed message was already safe: dropping its delimiters trips the named no hyp command to check assertion. And attach-endpoint-fallback.test.js does catch this specific mutation, which is why this is low and not high, but the file whose stated job is to be the general net was not being one.)

Fixed by dropping the required delimiter:

const HYP_MENTION = /(?<![\w-])hyp ([^`'")\n]+)/g

The lookbehind keeps it off HypAware and word-internal hits, the body still runs to the first delimiter so flag and placeholder mentions (hyp start --foreground) are still resolved, and an undelimited mention now drags the following prose into the argv and fails to resolve. That is the right answer: a repair a reader cannot tell apart from the sentence around it is not one they can copy into a shell either. The doc comment above the constant now explains this instead of the delimiter rationale it replaced. Re-ran the same mutation against the hardened guard: case 2 now fails with recommends 'hyp start so it can attach clients, or set', which is not a registered command. Unmutated, all three still pass.

Finding 2 (low, fixed) - llp/0178-attach-prompts-to-enable.plan.md:121: stale task text instructs a future implementer to keep the broken spelling

T7 still reads "Keep the existing wording and hyp start mention for the case where a daemon service is installed but not currently reachable", which is now the opposite of what ships. The PR updated the @ref gloss in attach-endpoint-fallback.test.js but not this line, and CLAUDE.md's living-docs rule asks not to leave guidance that would steer the next agent back into the bug.

The doc is Status: Active, so the original sentence is left intact as the record and a bracketed forward correction is appended after it, naming issue #834 and stating explicitly that what T7 settled (the installed-but-unreachable case names the start command only, never hyp daemon install) is unchanged. No decision was rewritten.

Also changed

test/core/attach-endpoint-fallback.test.js:152: the reworded @ref gloss called the message "the restart-only one", but the message names hyp daemon start, not a restart. Reworded to say what the test actually pins, since CLAUDE.md requires refs to stay honest.

Reviewed and deliberately left alone

  • assertAllRegistered resolves against a core-only registry, so a future give-up message naming a plugin command (hyp session ignore) would fail spuriously. The file's header comment already calls this out and says to extend the registry rather than loosen the assertion, so it is a documented tradeoff, not a defect.
  • String.prototype.matchAll clones the regex, so the module-level /g HYP_MENTION carries no shared-lastIndex hazard across the three cases.
  • The decision not to add a top-level start alias matches the acceptance criteria on CLI repair paths recommend nonexistent hyp start #834 and is not second-guessed here.

@philcunliffe

Copy link
Copy Markdown
Contributor Author

Triage at head d42449bf: every residual review finding is non-blocking, so this PR can merge as is.

  • Round e66261b0 finding 3 (core-only registry in the repair-spelling guard) and finding 5 (test scaffolding duplication) are test-quality items with no shipped-behavior impact; both verified against the code at head, and both are deferred to Follow-up: deferred review findings from PR #844 #888.
  • Round 1d910efe fixed both of its findings on the branch (pushed as d42449bf); nothing from that round remains open.
  • Verified at head: test/core/repair-command-spelling.test.js and test/core/attach-endpoint-fallback.test.js both green (9/9).

@philcunliffe philcunliffe added the neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030) label Aug 19, 2026
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #857. The task-oriented CLI rollover shipped the hyp daemon start repair spelling and its direct endpoint-fallback coverage on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

neutral:approved neutral reviewed this and holds it for a maintainer merge (own or adopted PR; LLP 0025/0030)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI repair paths recommend nonexistent hyp start

1 participant