Follow-up: deferred review findings from PR #844

[philcunliffe]

Two review findings from PR #844 (fix/issue-834, head d42449b) were classified at triage as non-blocking and deferred here. Both are test-quality items in test/core/repair-command-spelling.test.js; neither affects shipped behavior.

1. Repair-spelling guard resolves against a core-only registry (low)

coreRegistry() in test/core/repair-command-spelling.test.js:63-67 builds createCommandRegistry() + registerCoreCommands() only, but one of the two give-up messages under test lives in the ai-gateway plugin, which registers its own commands (session ignore / unignore / status, hypaware-core/plugins-workspace/ai-gateway/src/index.js:64). If either message is ever reworded to name a plugin command (for example hyp session ignore), the guard fails on a spelling hyp actually accepts.

This is a loud false failure, not a silent gap, and the file header (test/core/repair-command-spelling.test.js:28-31) already instructs the reader to extend the registry rather than loosen the assertion. The deferred work: give the test a way to register the relevant plugin commands (or a lightweight activation-context fixture) so the registry matches what the dispatcher really accepts.

2. Test scaffolding duplication (low)

makeBuf (test/core/repair-command-spelling.test.js:97), makeUnboundCtx (:116), and withTempHome (:145) are near-verbatim copies of makeBuf / makeCtx / withTempHome in test/core/attach-endpoint-fallback.test.js:23-85, and both files already share installFakeDaemonService from test/helpers/daemon_service_fixture.js. Per-file scaffolding is the prevailing convention (the review found makeBuf defined independently in 69 test files), so this was deferred rather than fixed piecemeal. The deferred work: if a shared attach-give-up fixture (or general test-helper consolidation sweep) happens, fold these two files into it.

Backlink: PR #844, review rounds e66261b0 (findings 3 and 5) and 1d910efe (both of that round's findings were fixed on the branch).

[philcunliffe]

neutral picked this up under neutral:fix and stopped before writing any code. Two independent blockers.

Blocker 1: the subject file is not on master yet

Both findings are scoped entirely to test/core/repair-command-spelling.test.js. That file does not exist on master:

$ git cat-file -p origin/master:test/core/repair-command-spelling.test.js
fatal: path 'test/core/repair-command-spelling.test.js' does not exist in 'origin/master'

It exists only on origin/fix/issue-834 (head d42449bf), the head branch of PR #844, which is still open (not draft, MERGEABLE, mergeStateStatus: CLEAN, all CI green as of 2026-08-19T01:47Z). It is waiting on a maintainer merge, nothing else.

Per neutral's follow-up rule this is a hard stop: a fix branched off master would have to re-add the parent PR's file, and a PR opened now would either conflict with #844 or silently duplicate it. neutral will not open a speculative PR against an unmerged parent.

The other file finding 2 names, test/core/attach-endpoint-fallback.test.js, is on master, but it is only the donor of the duplicated scaffolding, so it does not unblock anything on its own.

Blocker 2: neither finding has a reproducing failing test, and finding 2 is a convention decision

Even after #844 merges, neither item clears neutral's ground-truth gate as written.

Finding 1 (core-only registry) is hardening against a hypothetical, not a bug. The guard passes today, because both give-up messages under test name core commands only. It would only misfire if a message is later reworded to name a plugin command such as hyp session ignore. There is no test that fails on the current code, because the current code is correct for the current messages. The file header at test/core/repair-command-spelling.test.js:28-31 already states the intended response ("extend the registry rather than loosening the assertion if that day comes"), and the issue body itself calls it "a loud false failure, not a silent gap".

It is still implementable as preventative work, but the shape needs a human call, and the choice is not obvious:

• (a) Give the test a real activation-context fixture that boots the ai-gateway plugin and lets it register its commands, so the registry matches what the dispatcher actually accepts. Most faithful, but pulls plugin activation into a test whose subject is CLI string spellings, and pins the test to the plugin's registration order.
• (b) Register a small hand-listed set of plugin command names alongside registerCoreCommands(). Cheap, but the hand list is a second place to forget, which is the same class of drift the file exists to catch.
• (c) Leave it. Accept the loud false failure as the signal, since the header already tells the next reader exactly what to do, and the failure mode is a red test rather than a shipped bug.

Finding 2 (scaffolding duplication) is a repo-wide convention change, not a fix. The issue records that makeBuf is defined independently in 69 test files, so per-file scaffolding is the prevailing convention here. Deduplicating just these two files makes them the odd pair out without changing the convention, and changing the convention across 69 files is an architectural sweep that needs a maintainer's intent (and probably an LLP), not an autonomous fix PR. The issue text itself gates this on "if a shared attach-give-up fixture (or general test-helper consolidation sweep) happens".

What neutral needs

1. Merge PR fix: name hyp daemon start in CLI repair paths #844 (or say it is being abandoned or reworked). Nothing here can proceed until test/core/repair-command-spelling.test.js is on master.
2. For finding 1, pick (a), (b), or (c) above, or describe a fourth shape.
3. For finding 2, say whether the consolidation sweep is wanted at all, and if so whether it is these two files only or the wider makeBuf/withTempHome population. If it is the wider one, neutral suggests it wants an LLP first rather than a fix PR.

If the answer to both findings is "not worth doing", closing this issue is a legitimate outcome and neutral will not reopen it.

Reply with a comment on this issue and neutral will re-engage with your guidance on its next tick.