Skip to content

client_attach_idempotent: the capability gate reports the enablement layer (LLP 0174) - #654

Open
philcunliffe wants to merge 1 commit into
masterfrom
fix/issue-652
Open

client_attach_idempotent: the capability gate reports the enablement layer (LLP 0174)#654
philcunliffe wants to merge 1 commit into
masterfrom
fix/issue-652

Conversation

@philcunliffe

Copy link
Copy Markdown
Contributor

The release-battery smoke client_attach_idempotent fails on origin/master and passes at 1fbfa0d (v1.19.0). This updates the smoke, which had gone stale against a deliberate, documented design change.

Bisect

First bad commit: f17b091 "Manual attach prompts to enable the client adapter (LLP 0174/0178) (#585)".

Found with git bisect run over 1fbfa0d..7116f95 in a clean detached worktree, using npm run smoke -- client_attach_idempotent as the test (FAIL in output = bad). Four steps:

BAD  fb60a9f
BAD  f17b091
GOOD 92f1ec7
GOOD f2b8634
f17b0917030a7aae9a1186568098b157d042ed74 is the first bad commit

92f1ec7 is the LLP-only half of the same work (design doc accepted, no code), which is why it is good and its code sibling f17b091 is bad.

Diagnosis

The smoke's kernel #2 activates no plugins at all, then runs hyp attach --client claude and asserts the resulting stderr names @hypaware/ai-gateway, with a client.attach span carrying error_kind=cap_missing.

f17b091 rewrote exactly that gate in src/core/commands/clients.js. The !ctx.capabilities.has('hypaware.ai-gateway') check still fires first and still exits 1; what changed is that it now resolves why the capability is absent before choosing the wording:

  • name is contributed by a known plugin (claude) -> error: the claude adapter is not enabled on this install; enable it with 'hyp init', or add @hypaware/claude to <config> and run 'hyp daemon restart', then re-run attach, error_kind=adapter_not_enabled
  • name no plugin contributes -> unchanged attach requires the @hypaware/ai-gateway plugin to be installed and activated, error_kind=cap_missing

So the precedence did not move. The gate is the same gate, in the same place; only its message and error_kind became state-dependent.

Code or smoke?

The smoke. This is not a judgement call once the docs are read:

  • llp/0174-attach-prompts-to-enable.design.md is Active, and its #detection section settles the three-state split, naming the capability gate as one of the two failure sites that must report the enablement layer. Its #summary quotes the pre-change message verbatim as one of the two failure modes it exists to fix.
  • llp/0178-attach-prompts-to-enable.plan.md T3 spells out the new message string, the adapter_not_enabled error kind, and specifies the test case "zero gateway-using plugins active (cap_missing path lands on not_enabled)" - i.e. precisely the situation the smoke constructs.
  • test/core/attach-enablement-state.test.js already asserts both halves (capability gate: a catalog-known client reports not_enabled, not cap_missing and capability gate: a name no plugin contributes keeps the cap_missing wording).

Reverting the code would undo an Active LLP and break existing unit tests. The new error is also strictly more useful: @hypaware/ai-gateway is not activated is unactionable when the real problem is that @hypaware/claude is not in your config.

The change

Only hypaware-core/smoke/flows/client_attach_idempotent.js. Rather than just retargeting the stale assertion, the smoke now covers both halves of the split, so the smoke tier proves the branch and not only one leg of it:

  • --client claude with no gateway: exit 1, stderr names the adapter to enable, span has error_kind=adapter_not_enabled, status=failed, hyp_client=claude.
  • new: --client frobnicator with no gateway: exit 1, stderr still names @hypaware/ai-gateway, span has error_kind=cap_missing, status=failed, hyp_client=frobnicator.

Docblock and @ref LLP 0174#detection [tests] annotations updated to match. No production code touched.

Verification

  • npm run smoke -- client_attach_idempotent: FAIL before (attach without ai-gateway prints a clear error, observed the claude adapter is not enabled on this install), ok after.
  • node scripts/run-tests.js: 3574 pass, 0 fail, 1 skipped.
  • npx tsc -p tsconfig.json --noEmit: clean.
  • Neighbouring smokes gateway_claude_capture and cli_bundled_plugins_activated: both ok, so the gate precedence is unshifted.

Unrelated, pre-existing (not touched here)

While checking neighbours I found claude_attach_detach and client_attach_on_join also fail, but they fail identically at 1fbfa0d (v1.19.0) with my change stashed, so they are long-standing and predate this bisect window. Neither is in the CLAUDE.md release battery. Worth a separate issue.

Fixes #652

…layer (LLP 0174)

The release-battery smoke asserted that attaching with no ai-gateway
capability always prints a message naming @hypaware/ai-gateway, with a
client.attach span carrying error_kind=cap_missing.

LLP 0174 #detection deliberately split that single failure into two
states, and #585 (f17b091) implemented it: when the requested name is
contributed by a bundled plugin, the capability being absent means that
adapter is not enabled, not that the install lacks a gateway, so the gate
now reports 'the claude adapter is not enabled on this install' with
error_kind=adapter_not_enabled. Only a name no plugin contributes keeps
the old cap_missing wording. The behaviour is the settled design and is
covered by test/core/attach-enablement-state.test.js; the smoke's
expectation was the stale half.

Update the smoke to assert both halves of the split rather than just
retargeting the old one: `claude` takes the adapter_not_enabled path and
must name the adapter to enable, and a new dispatch of an uncontributed
name proves the cap_missing wording and span are still reachable.

Fixes #652
@philcunliffe

Copy link
Copy Markdown
Contributor Author

Neutral review round 1 - 6b6f8e0 - CLEAN

Reviewed with the bar set deliberately high, because "the test was wrong, not the code" is the most self-serving conclusion a fix worker can reach. It checks out, against both the doc record and the code at head.

The LLPs settle it. llp/0174-attach-prompts-to-enable.design.md is Active, and its #summary explicitly lists the old string as a defect it exists to fix, saying both old messages "read as 'HypAware does not know this client' when the actual state is 'the adapter exists but is not enabled on this install'". Its #non-interactive quotes the replacement verbatim. llp/0178 T3 names the exact gate, the exact template, the adapter_not_enabled kind, and specifies as a test case "zero gateway-using plugins active (cap_missing path lands on not_enabled)" - which is precisely the smoke's kernel #2. Settled, not undocumented.

The precedence claim holds. clients.js:152 still has the same capability gate before capabilities.require(...) at :203, still returning 1 at :198. Resolution happens strictly inside the gate, and enablement.state === 'unknown' falls back to the original wording and cap_missing. Nothing was reordered and no path that previously exited 1 now proceeds; the only new escape is the TTY prompt's activated, unreachable non-interactively. So a genuinely missing gateway is not masked by an adapter message.

The smoke got stronger, not weaker. Line by line: the exit-code check is unchanged; the old single-substring stderr assertion became a two-substring check; all three original span assertions are retained and retargeted, with a new parallel trio for adapter_not_enabled. The frobnicator case genuinely exercises cap_missing - confirmed no plugin contributes that name. Net 4 assertions to 9.

Bisect spot-checked: the smoke FAILs at f17b091 and passes at its parent f2b8634. Corroborating the diagnosis: f17b091 rewrote clients.js (+707) and added 11 test files while touching no file under hypaware-core/smoke/, so it landed the behaviour change without updating the battery smoke. That is the whole root cause.

Scope clean: one file, +67/-7, no production code.

client_attach_idempotent ok, gateway_claude_capture ok, cli_bundled_plugins_activated ok, npm test 3574 pass / 0 fail / 1 skip, tsc clean.

Noted, not a finding

For a catalog-known client the non-interactive remedy names only the adapter (add @hypaware/claude) even when the real gap is that @hypaware/ai-gateway itself is absent; the interactive prompt names both. That is literally the string LLP 0178 T3 mandates, so it is sanctioned rather than a regression, and pre-existing on master. A future UX nit against LLP 0174, not this PR's business. Also cosmetic: the new dispatch is inline rather than through the file's existing runAttach helper.

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.

Release-battery smoke client_attach_idempotent fails on master (passes at v1.19.0)

1 participant