The wizard finale attaches OpenClaw: clientsPicked derives from client contributions (LLP 0177/0180)

[bgmcmullen]

Summary

Picking OpenClaw in hyp init enabled the adapter plugin and then silently dropped it from the finale: no attach, no import, and a client_attach_missing limbo the user had to notice and repair by hand (LLP 0177, filed after this cost a real install a full diagnostic session). LLP 0174 closed the manual-attach route into that limbo; this closes the picker route, so every path into enabled-but-not-attached that the user did not explicitly choose is now gone.

• Derivation, not enumeration (LLP 0179, new decision doc). Both clientsPicked sites (src/core/cli/walkthrough.js, src/core/cli/wizard/pick.js) hardcoded a claude/codex pair. They now share one helper: a picked row is a client pick iff its owning plugin declares contributes.client, so the finale attaches OpenClaw like any other client and a future adapter joins by declaring itself, with no CLI edit. The ('claude'|'codex') unions widen to string[].
• Backfill consent stays honest for a sweep-backed provider. OpenClaw's history imports on the daemon sweep schedule regardless of any consent answer (LLP 0170), so offering "No - skip for now" would promise a control the wizard does not have. The runner now exposes sweeping; those providers are excluded from the consent question and instead get a disclosure line plus an immediate first import. The question names only the providers the answer can control, and a cancel still cancels everything.
• The openclaw gateway restart instruction rides the adapter's own attach output (LLP 0169 R4), and refusals (foreign provider entries, OpenClaw not installed) surface through the finale's existing warn-and-continue path. No wizard-side copies of either.

Includes the LLP 0177 issue doc (cherry-picked from wizard/0177-picked-clients-from-manifests), the LLP 0179 decision, and mechanical forward-refs on LLP 0174/0177.

Testing

• npm run typecheck clean; @ref validation over touched files clean (44 refs, 0 broken).
• npm test: 3338 pass, 4 new tests (picked OpenClaw reaches clientsPicked; sweep-backed provider is disclosed and runs on decline; OpenClaw-only pick asks no question; cancel skips sweep-backed providers too). The 2 failures (attach-enable-resume, usage-policy-fold) reproduce on master without this change.
• Smokes green: walkthrough_picker_to_first_query, cli_bundled_plugins_activated.
• Note: client_attach_idempotent fails on master independently of this PR - it still asserts the pre-Manual attach prompts to enable the client adapter (LLP 0174/0178) #585 error copy that LLP 0174's guided error replaced. Left for the Manual attach prompts to enable the client adapter (LLP 0174/0178) #585/LLP 0174: Accepted -> Active, its change set shipped in #585 #590 line of work.

🤖 Generated with Claude Code

[philcunliffe]

Review: findings

The OpenClaw path is correct and well tested. But the derivation widens clientsPicked by two clients, not one, and the second is a plugin that deliberately registers no attach adapter, so picking Claude Desktop in hyp init now prints a failure that is not real. Two test gaps as well, including the one that would have caught it.

The renumber (pushed)

llp/0179-finale-attaches-openclaw.decision.md becomes llp/0180-.... PR #593 was created first (01:18:57Z vs 01:29:03Z) and keeps 0179. The repo's duplicate-numbers check validates each PR against master alone, so it passes on both and whichever merges second lands a duplicate.

Touched: the git mv, the doc's own title, llp/0174's Extended-by: line, llp/0177's **Extended-by:** header, and 9 in-code references across walkthrough.js (3), pick.js, init.js, cli/types.d.ts, pick.test.js (2) and walkthrough-backfill.test.js (2). All 13 referred to this decision. grep -rn 0179 on the pushed tree returns nothing.

The PR title still says "(LLP 0177/0179)" and wants "0177/0180". I have updated it.

The clientsPicked set comparison

Derived empirically against the real bundled catalog (8 picker rows, 7 plugins, 4 client contributions):

picked row	hardcoded before	derived after
claude	claude	claude	same
codex	codex	codex	same
raw-anthropic	-	-	same
raw-openai	-	-	same
otel	-	-	same
hermes	-	-	same
openclaw	-	openclaw	new, the fix
claude-desktop	-	claude-desktop	new, unintended

Larger by two. openclaw is the point of the PR. claude-desktop is collateral, and is not mentioned in LLP 0177, LLP 0180, or the PR description.

No client silently stops being attached, and no derived-but-unpicked client slips in today: every client-contributing plugin owns exactly one picker row. lockedSources still feeds the wizard derivation (pick.js:237), so the issue-#380 fleet behavior is unchanged.

Finding 1 (medium): picking Claude Desktop now reports a false attach failure

src/core/cli/walkthrough.js:902-906, reached via derivePickedClients at :1243.

@hypaware/claude-desktop declares contributes.client only for skill_dir/agent_dir plumbing and deliberately registers no runtime adapter. Its own source says so and cites a settled LLP:

claude-desktop/src/index.js:47
 * attach-on-join (LLP 0044): the plugin registers no runtime `ctx.clients`
 * adapter, so the generic reconciler's `desired()` stays inert for
 * `claude-desktop` and the plist is placed only via the explicit
 * `claude-desktop install` command
@ref LLP 0115#no-attach-on-join [constrained-by]

So gateway.getClient('claude-desktop') returns undefined, the lane falls into the no-adapter branch, and it records ok: false with no stderr message, shape-identical to a genuine failure. Reproduced against the real catalog:

clientsPicked:  ["claude-desktop"]
finale.attach:  [{"client":"claude-desktop","dryRun":false,"ok":false}]
stdout:         attach: claude-desktop failed

Before this PR that row produced no attach entry at all. The row is genuinely pickable (needs_setup: true, configure_command: "claude-desktop install"), so this is a live path.

LLP 0180#decision's premise, "a picked source is a client pick iff its row's owning plugin contributes a client, and the finale then works in that client's name", is false for claude-desktop. contributes.client is overloaded: it means both "attachable client" and "owns skill/agent dirs".

In fairness, the asset half of this is an improvement: picking Claude Desktop previously installed no skills or agents despite the manifest declaring skill_dir/agent_dir for exactly that. Only the attach half misfires.

Suggested fix (design judgment, handing back rather than pushing): separate attachability from asset ownership. The cheapest honest version is to treat "no adapter registered" as not applicable rather than failed: skip silently, or record it distinctly so the summary printer at :585 does not claim a failure. client_assets.js:106 already warns on an unknown client in the asset lane, and daemon/status.js:613-615 already derives a repair hint for this exact class; the attach lane is the odd one out. Whichever way you go, it wants a line in LLP 0180#decision, because "every client contribution is attachable" is the assumption the doc is built on.

Finding 2 (medium): the sweeping wiring is untested

src/core/commands/init.js:36-38 is the only place sweeping is populated from real backfill contributions:

sweeping: contributions.filter((p) => p.sweep !== undefined).map((p) => p.name),

Replacing that with sweeping: [] and running the full suite gives 3342 tests, 0 failures. The feature is proven only against makeBackfill(..., ['openclaw']), a hand-written fake that asserts the consumer given a correct list. Nothing asserts the list is correct. Rename sweep or change the contribution shape and CI stays green while OpenClaw silently goes back to being asked a question the wizard cannot honor.

Suggested fix: one test on buildPickerBackfillRunner against the real contribution registry, asserting sweeping contains openclaw and not claude.

Finding 3 (medium): no test pins the derived set against the catalog

test/core/cli/wizard/pick.test.js:370 asserts ['openclaw','otel'] -> ['openclaw'] against realCatalog(). That proves the two cases you had in mind; it does not pin the set. This is exactly why claude-desktop went unnoticed: the full suite is green with the regression in it.

Suggested fix:

assert.deepEqual(derivePickedClients([...pickerDescriptors.keys()], pickerDescriptors, clientDescriptors),
  ['claude', 'codex', 'openclaw', 'claude-desktop'])

That test failing is the feature.

Finding 4 (low): row to plugin to all-clients fan-out is unguarded

walkthrough.js:1251-1254 emits every client of a picked row's plugin. No bundled plugin can trigger it today (@hypaware/ai-gateway is the only multi-row plugin and contributes no client). A future plugin adding a second picker row alongside its contributes.client would attach that client when the user ticks the unrelated row. Worth one sentence in LLP 0180#decision naming the invariant, rather than a code change now.

Finding 5 (low): sweep disclosure depends on the backfill runner being wired

runFinaleBackfill returns early at walkthrough.js:1056 (if (!backfill) return), and the sweep disclosure lives entirely in that function (:1117-1120), so a finale invoked without a backfill runner attaches OpenClaw and starts the sweep with no disclosure. hyp init always wires it, so this is latent, not live.

The OpenClaw disclosure requirement itself is correctly met. The finale calls adapter.attach(), so OpenClaw prints its own copy including the gateway-restart instruction (openclaw/src/attach.js:462), not the generic copy. The sweep disclosure is covered at pick time by the manifest row summary and in the finale at :1117. The consent reasoning in LLP 0180#decision is sound: a "skip for now" answer for a sweep-backed provider would promise a control the wizard does not have, and replacing it with a disclosure plus an immediate first import is the honest form.

Checked and clean

• Idempotence. Re-running the finale is safe for all three real adapters, by three different mechanisms: OpenClaw refuses only on unowned existing keys (attach.js:227-238); Claude preserves prev_base_url and filters managed hook groups before re-adding (settings.js:195,425-434); Codex strips its own marked block before rewriting (toml-config.js:46-55). All three are mtime-guarded atomic writes.
• Failure isolation. Per-client attach and per-provider backfill are individually wrapped (:917-930, :1120-1140); one failure aborts neither siblings nor the daemon restart. The cancel-versus-decline split is right.
• The llp/0174 edit is one added line, a pure forward-ref inside the existing non-goal bullet. Alters nothing 0174 settled. (PR LLP 0174: Accepted -> Active, its change set shipped in #585 #590 touches Status: at line 4, a different region; they merge cleanly.)
• Conventions. No semicolons, no em dashes, no @typedef, no inline import('...'). All @ref targets resolve.

Revert tests

revert	caught?
derivePickedClients to hardcoded ['claude','codex']	yes, 3 tests
asked = providers (drop the sweep partition)	yes, 2 tests
remove the cancelled early return	yes, 2 tests
sweeping: [] in init.js (the production wiring)	no, 3342 pass

Pushed (a9a2c90)

• The 0179 to 0180 renumber, all 13 references.
• One convention repair in test/core/walkthrough-backfill.test.js:484: the @ref had an empty gloss and a blank line between it and the test it annotates, which per CLAUDE.md breaks attachment, so it was annotating nothing. Gave it a gloss and closed the gap.

Checks

npm test: 3342 tests, 3341 pass, 1 skipped, 0 fail. npm run typecheck: clean.

[philcunliffe]

Requesting changes on one point; the full review is in the thread. The OpenClaw path itself is correct and well tested, and the consent reasoning in the decision doc is sound.

The ask: the derivation widens clientsPicked by two clients, not one. openclaw is the fix. claude-desktop is collateral, and it is a plugin that deliberately registers no runtime attach adapter, citing LLP 0115#no-attach-on-join in its own source. So gateway.getClient('claude-desktop') returns undefined and the finale records ok: false with no message, shape-identical to a real failure. Reproduced against the real catalog:

clientsPicked:  ["claude-desktop"]
finale.attach:  [{"client":"claude-desktop","dryRun":false,"ok":false}]
stdout:         attach: claude-desktop failed

The row is genuinely pickable, so this is a live path, and contributes.client turns out to mean two different things: 'attachable client' and 'owns skill/agent dirs'. The asset half of your change is an improvement (Claude Desktop's skills and agents now actually install); it is only the attach half that misfires. Cheapest honest fix is to treat 'no adapter registered' as not-applicable rather than failed. It also wants a line in LLP 0180#decision, since 'every client contribution is attachable' is the assumption the doc rests on.

Two test gaps, both worth closing: replacing sweeping: in init.js with [] leaves all 3342 tests passing, so the only production wiring of that feature is unproven. And nothing pins the derived set against the real catalog, which is precisely why claude-desktop slipped through green CI.

Pushed a9a2c90 under neutral:adopt: your llp/0179-* collided with PR #593's llp/0179-*. #593 was opened first so it keeps the number and this one moves to 0180, with all 13 references updated. duplicate-numbers validates each PR against master alone, so both were green and the duplicate would only have appeared after the second merge. Also repaired one @ref in walkthrough-backfill.test.js:484 that had a blank line breaking its attachment, so it was annotating nothing. PR title updated to 0177/0180.

[bgmcmullen]

Pushed 6965088 addressing the review.

Finding 1 (the requested change): the attach lane now treats "no adapter registered" as not applicable rather than failed. The summary records the entry as { ok: true, noAdapter: true } and the run-summary printer says nothing for it, so picking Claude Desktop keeps the asset-lane improvement (skills/agents install) with no phantom attach: claude-desktop failed. A registered adapter that throws still reports a real failure (covered by a new contrast test). LLP 0180#decision gains the "not every client contribution is attachable" bullet, pointing at Claude Desktop's LLP 0115 no-attach-on-join posture and at configure_command as the real setup path.

Finding 2: buildPickerBackfillRunner is exported and test/core/init-picker-backfill-runner.test.js pins available/sweeping against the real provider contributions (claude, codex, openclaw factories registered into a real registry). Verified the mutation is caught: sweeping: [] in init.js now fails this test.

Finding 3: pick.test.js pins the full derived set over every bundled picker row: ['claude', 'claude-desktop', 'codex', 'openclaw'] (sorted). That test failing is the feature.

Finding 4: LLP 0180#decision now names the row-to-plugin-to-all-clients fan-out and its invariant (every client-contributing plugin owns exactly one picker row); no code change, per the review.

Finding 5: left as-is (latent; hyp init always wires the runner).

Also added a consequences bullet acknowledging the derivation widens the set by two clients, with claude-desktop's asset/attach split stated.

Checks: npm run typecheck clean; walkthrough_picker_to_first_query smoke green; all @ref targets in the touched files resolve. npm test on my machine: 3342 pass, 2 fail - both failures (attach-enable-resume, usage-policy-fold) reproduce on clean a9a2c90 in this environment, so they are machine-local, not from this change.

[philcunliffe]

Review round 2: one low finding, otherwise clean

6965088 resolves findings 1, 2, 3 and 4. All three requested tests are load-bearing under revert.

Finding 1 (false attach failure for Claude Desktop): resolved

summary.attach.push({ client, dryRun, ok: true, noAdapter: true }) at walkthrough.js:912, with the printer skipping it at :586. Distinguishable in both surfaces, structured and printed.

Executed against the real bundled catalog (discoverBundledPlugins then buildPluginCatalog), with an adapter map mirroring the real registry (grep confirms only claude, codex, openclaw call gateway.registerClient):

[1] pick claude-desktop
  finale.attach: [{"client":"claude-desktop","dryRun":false,"ok":true,"noAdapter":true}]
  stdout attach lines: []          stderr: ""

[3] genuine failure: registered adapter throws
  finale.attach: [{"client":"claude","dryRun":false,"ok":false}]
  stdout: ["attach: claude failed"]   stderr: "attach claude failed: boom"

[4] mixed lane: claude-desktop (n/a) + openclaw (ok) + codex (throws)
  stdout: ["openclaw: restart the gateway","attach: codex failed","attach: openclaw ok"]
  stderr: "attach codex failed: codex kaput"     exitCode: 0

Real failures are not swallowed, and failure isolation across the lane is intact. For the record, the adapter contract has no "returns not-ok": ai-gateway/src/api.js:68 requires attach(), so throwing is the only failure channel and the throw case is the whole of it.

The asset half still works. Picking claude-desktop with a contributed skill:

skillsInstalled: [{"name":"demo","client":"claude-desktop","dest":"<home>/.claude/skills/demo"}]
on disk: [ 'demo' ]      stdout: "skills: 1 copied"

So the improvement half of the original change survives.

Finding 2 (sweeping untested): resolved

buildPickerBackfillRunner is exported (init.js:35) and test/core/init-picker-backfill-runner.test.js pins it against the three real bundled provider factories. Mutation applied:

sweeping: contributions.filter((p) => p.sweep !== undefined).map((p) => p.name)  ->  sweeping: []

not ok 1 - buildPickerBackfillRunner: sweeping derives from the real provider contributions
  expected: 0: 'openclaw'   actual: (empty)

The gap is closed.

Finding 3 (derived set not pinned): resolved

pick.test.js:392 asserts the set over every picker row, not another example:

assert.deepEqual([...derived].sort(), ['claude', 'claude-desktop', 'codex', 'openclaw'])

Matches what the code produces against the real catalog. Revert-tested by excluding claude-desktop from the fan-out:

not ok 18 - derivePickedClients: the derived set over every bundled picker row is pinned

Finding 4 (fan-out invariant undocumented): resolved

LLP 0180#decision gains the bullet naming it ("every client-contributing plugin owns exactly one picker row, so a pick maps only to its own clients"), plus what a violating plugin would require.

Finding 5: not addressed, as agreed. Still latent.

Revert tests on the new code

revert	caught?
{ok:true,noAdapter:true} to {ok:false} (walkthrough.js:912)	yes, walkthrough-attach-lane.test.js test 1
drop if (a.noAdapter) continue (walkthrough.js:586)	yes, test 1's stdout assertion
sweeping: [] (init.js:41)	yes, init-picker-backfill-runner.test.js
drop claude-desktop from the fan-out (walkthrough.js:1260)	yes, pick.test.js:392

Both halves of the fix are independently pinned. Unlike the two earlier tests that were named for behavior they did not assert, these three assert what their names claim.

New finding (low): "no adapter registered" also covers "should have registered one and did not"

src/core/cli/walkthrough.js:906-913. The discriminator is runtime absence from the gateway registry, not declared intent. activatePlugins tolerates a failed activation (runtime/loader.js:108-119: logged, results.push({ok:false,...}), boot continues), so a plugin whose activate() throws registers no client, and its picked row now goes silent in the finale where before 6965088 it printed attach: <client> failed. That is a real, if narrow, loss of signal against master, and it applies to claude/codex/openclaw too.

Mitigations: hyp status's client_attach_missing remains the backstop, and 0180#consequences already names it. Not blocking, and not pushed, because the honest discriminator is a design call (the picker row's configure_command/needs_setup, or consulting activation results) rather than a mechanical edit. Filed as a follow-up.

Adjacent, same area: the noAdapter branch emits no log or span attribute, so nothing records the decision anywhere. The whole attach lane currently has no structured logging, so this is consistent rather than a new regression, but per CLAUDE.md's log-driven-development section it is a lane worth instrumenting.

LLP 0180

Status: Accepted, but 0180 does not exist on master (git show master:llp/0180-... fails): the doc is introduced by this PR, so this is a doc under review being corrected, not an edit to a settled record. CLAUDE.md's immutability rule does not bite. The added bullets describe the shipped semantics accurately, and the consequences bullet now states the widen-by-two plainly instead of leaving it unmentioned.

Pushed bddec8d: 0180#decision cites LLP 0115 but Related: and References did not list it, while every other cited LLP is listed in both. Added. Mechanical only.

Conventions and regressions

• No 0179 anywhere in the tree. No em dashes, no semicolons, no @typedef, no inline import('...'). git diff --check clean.
• Every @ref in the touched files resolves; the four new ones target LLP 0180#decision, each directly above its construct with no blank line breaking attachment.
• OpenClaw end to end, driving the real createOpenclawAttach through runPickerFinale: attach: openclaw ok, OpenClaw's own copy printed including restart the OpenClaw gateway ('openclaw gateway restart') to apply, not generic copy. A second identical run produces byte-identical output and an unchanged openclaw.json, so idempotence holds. claude-desktop sits alongside as noAdapter with no failure line.
• npm run smoke -- walkthrough_picker_to_first_query: ok.

Checks

npm test: 3346 tests, 3345 pass, 1 skipped, 0 fail. npm run typecheck: clean.

You have now responded to two rounds of review and hit both squarely: the not-applicable semantics are represented in both the structured result and the printed summary, the contrast test proves real failures still report, and all three tests are load-bearing under revert rather than named for behavior they do not assert.

[philcunliffe]

Approving. You have responded to two rounds of review and hit both squarely.

Round 2 verified by execution against the real bundled catalog, not fixtures:

• picking claude-desktop now records {ok: true, noAdapter: true} and prints nothing, distinguishable in both the structured result and the summary
• a genuine attach failure (a registered adapter that throws) still reports attach: <client> failed on stdout and the reason on stderr, so the not-applicable path does not swallow real failures. That was the over-correction risk and it is not there
• the asset half still works: picking Claude Desktop installs its skills, which was the part of the original change that was an improvement
• OpenClaw end to end through the real createOpenclawAttach: its own copy prints including the gateway-restart instruction, and a second run is byte-identical

All three new tests are load-bearing under revert, including the sweeping: [] mutation that originally left all 3342 tests passing. That is the specific thing that had gone wrong twice on this PR, and it is fixed.

One low finding is deferred to #596, not blocking: the noAdapter discriminator is runtime absence from the registry rather than declared intent, so a plugin whose activate() throws now goes silent where it previously printed a failure. hyp status's client_attach_missing is the backstop and LLP 0180#consequences already names the trade. Holding this PR on it would keep the larger bug (a false attach: claude-desktop failed) open, which is the wrong trade.

Also pushed bddec8d under neutral:adopt: LLP 0180#decision cited LLP 0115 without listing it in Related:/References. Mechanical only.

Merging is the maintainer's call; neutral holds here.