Fix #380: derive clientsPicked from pre-filter selection so fleet-managed machines install skills/agents

[philcunliffe]

Root cause

src/core/cli/wizard/pick.js correctly filters org-locked sources out of the local-layer config composition (const sources = rawSources.filter((id) => !lockedSet.has(id))) because locked clients already live in the central layer. But clientsPicked was then derived from that already-filtered sources list. On a fleet-managed machine where the org locks both claude and codex, sources is empty, so clientsPicked was [].

The finale (runPickerFinale in walkthrough.js) gates client attach, skills install, and agents install on clientsPicked.length > 0. Those three steps are per-machine LOCAL work (client settings, ~/.claude skills/agents) that is NOT in the central layer, so they must still run for org-locked clients. With clientsPicked empty, all three silently no-op'd on every fleet-managed machine.

LLP 0135 #finale is explicit that skills/agents install iterates clientsPicked and is untouched by the managed/join pathway; only daemon-install/attach are skipped when the join lane already did them. So the locked filter belongs to config composition, not to clientsPicked.

Fix

Derive clientsPicked from the pre-filter selection (rawSources ∪ lockedSources), keeping const sources = rawSources.filter(...) untouched for config-layer composition. Using the union (rather than just rawSources) is robust even if a prompt does not echo back a disabled+checked locked row.

Regression test

test/core/cli/wizard/pick.test.js:

• New: runWizardPick: a fully fleet-managed machine still reports its locked clients as picked so the finale installs skills/agents - both clients locked; asserts sourcesPicked === [] and no adapter re-composed into the local config, but clientsPicked === ['claude', 'codex']. Fails before the fix (clientsPicked was []), passes after.
• Updated: the existing single-locked-client test's clientsPicked assertion, which had codified the buggy derivation (['codex'] -> ['claude', 'codex']).

Full npm test suite: only 7 pre-existing failures in test/core/leave-command.test.js remain (confirmed failing on untouched origin/master, unrelated to this change); all wizard/pick tests pass.

Fixes #380

[philcunliffe]

neutral-reconcile REVIEW — PR #382 (head fbb9cc4)

Verdict: CLEAN. No actionable findings. Nothing pushed; PR left held for the reconcilePR spine.

What the change does

src/core/cli/wizard/pick.js now derives clientsPicked from the pre-filter selection new Set([...rawSources, ...lockedSources]) instead of the composition-filtered sources. The locked filter (sources = rawSources.filter(id => !lockedSet.has(id)), pick.js:155) is retained and still scopes only the config-layer composition.

Correctness (verified)

• Fix is real. Reverting only the source (parent fbb9cc4^) while keeping the tests: 2 tests fail (the new fully-managed test Feature: iceberg #8 and the updated single-locked test Feature: github-install #6). Post-fix: all 11 pass. So the regression genuinely guards the behaviour.
• Managed clients now install skills/agents. On a fully fleet-managed machine (locked: [claude,codex]) the test asserts sourcesPicked === [] and no @hypaware/claude/@hypaware/codex plugin is composed into the local layer, while clientsPicked === [claude,codex]. That matches the finale contract in LLP 0135#finale: "Skills/agents install is untouched: it already iterates clientsPicked"; only daemon-install/attach are gated on joinedAlready. So including locked clients in clientsPicked is correct and does not re-compose them into config.
• @ref honesty: the new @ref LLP 0135#finale [implements] and the test [tests] ref both accurately describe the finale iterating clientsPicked over org-locked clients. The retained @ref LLP 0129#join-before-picker on the composition filter (pick.js:154) still applies.

Edge cases checked

• Neither locked: lockedSources empty, candidate set == rawSources, behaviour identical to pre-fix. OK.
• One locked (claude) + user picks codex: candidates {claude,codex}, clientsPicked === [claude,codex] (updated test Feature: github-install #6). OK.
• Unknown ids: both rawSources and lockedSources are pre-filtered by descriptors.has(...), so unknown ids cannot leak into candidates. OK.
• Non-client locked source ids: only claude/codex are extracted from the candidate set, so other locked sources are ignored. OK.

Gates

• npm run typecheck (tsc): passes.
• node --test test/core/cli/wizard/pick.test.js: 11/11 pass.
• Style: no semicolons, no em dashes, JSDoc types preserved.

Head reviewed: fbb9cc4. No fixes pushed; head unchanged.

[philcunliffe]

Re-review after head moved (merge resolution of #381 conflict) — CLEAN

Verdict: CLEAN, nothing actionable. Head reviewed: 6dd301c (merge of origin/master #381 into the #382 fix fbb9cc4). No changes pushed.

Merge resolution is correct

• git diff origin/master 6dd301c touches only src/core/cli/wizard/pick.js (+14) and test/core/cli/wizard/pick.test.js (+35), and the delta is exactly Fix #380: derive clientsPicked from pre-filter selection so fleet-managed machines install skills/agents #382's fix. Every other file Wizard orchestration (T11): fork-join-pick-configure-finale state machine (LLP 0135) #381 brought in (walkthrough.js, wizard/index.js, wizard/fork.js, wizard/types.d.ts, init.js, wizard/index.test.js, wizard/join.test.js, compose-picker-config.test.js, etc.) is byte-identical to master — no Wizard orchestration (T11): fork-join-pick-configure-finale state machine (LLP 0135) #381 line dropped, no test lost, no buildPickOption / · stays on this machine / opts.managed label logic reverted.
• No conflict-marker leftovers (<<<<<<</=======/>>>>>>>) anywhere in src/ or test/. No duplicated blocks or broken braces. The only conflicted file was the add/add in pick.test.js, resolved keeping both Wizard orchestration (T11): fork-join-pick-configure-finale state machine (LLP 0135) #381's tests and Fix #380: derive clientsPicked from pre-filter selection so fleet-managed machines install skills/agents #382's new test.

#382 fix intact and correct against the post-#381 structure

src/core/cli/wizard/pick.js:

• L228: const clientCandidates = new Set([...rawSources, ...lockedSources]) — clientsPicked derived from the pre-filter selection, locked rows included (L231-232). So org-locked clients are still reported picked and the finale runs their local work.
• L164: const sources = rawSources.filter((id) => !lockedSet.has(id)) — composition still excludes locked ids (they live in the central layer). Correctly untouched.
• The referenced variables (rawSources L123/130/146, lockedSources L90, lockedSet L91) all exist in the merged structure; the fix wires into them cleanly.
• clientsPicked reaches the finale via the active wizard path: wizard/index.js runWizardFinale -> runPickerFinale({ clientsPicked: picked.clientsPicked, ... }) (index.js L260); runPickerFinale gates skills/agents install and attach on clientsPicked (walkthrough.js L872/910/958). Path is real.

Regression test genuinely gates the fix

• With the fix reverted in-tree, test/core/cli/wizard/pick.test.js fails 2 tests (the amended locked source is filtered... assertion and the new fully fleet-managed machine... test); with the fix in place, all 13 pass. The new test drives the real runWizardPick with locked: ['claude','codex'] and asserts sourcesPicked: [] + no @hypaware/* plugins composed and clientsPicked: ['claude','codex'] — exercises the exact Wizard finale installs no skills/agents on fleet-managed machines (empty clientsPicked when org locks the clients) #380 path.

Style / refs

• No em dashes (U+2014), no semicolons in the added lines.
• @ref LLP 0135#finale is honest: anchor {#finale} exists (0135 L442) and L451-452 state "Skills/agents install ... already iterates clientsPicked" — the annotation adds non-obvious intent.

Test results

• test/core/cli/wizard/pick.test.js: 13/13 pass.
• wizard/index.test.js and wizard/join.test.js error at load with ERR_MODULE_NOT_FOUND: 'squirreling' from src/core/query/sql.js — an environmental missing-dependency issue: node_modules/squirreling is absent, sql.js is not touched by this PR, and the same failure reproduces at master HEAD (Wizard orchestration (T11): fork-join-pick-configure-finale state machine (LLP 0135) #381, 1c7d24e). Treated as pre-existing/unrelated (same class as the noted leave-command failures), not this PR's.

Non-blocking observation (not this PR's scope)

The legacy runInitWalkthrough in src/core/cli/walkthrough.js L531-533 still derives its own clientsPicked from picks.sources (no locked union). It is identical to master and superseded by the wizard path for managed hyp init, so it is out of scope here — flagging only in case a future change re-routes managed installs through it.