refactor(members): retire the dead acp_backend factory kwarg (#8166 item 1)

[chenmingwei23]

What is the problem?

Issue #8166 recorded two accepted follow-ups from PR #8153's review. This PR takes item 1 only: create_provider_factory's _acp(acp_backend=...) kwarg and the explicit precedence arm of members.select_provider_backend that it feeds have no non-test consumers. They are an unconsumed seam sitting on the provider-construction path.

Item 2 (session-key prefix normalization at one boundary) is deliberately out of scope - see the last section.

Why this issue matters to the user

An unconsumed seam on a construction path is a maintenance and review cost with no user benefit: it is one more parameter every reader of the factory has to account for, one more arm the harness-parity review has to reason about, and a false signal that a per-slot backend picker is imminent. The repo's boundary/harness-parity gate already pushes toward exactly one selection gate on the kiro construction path with no branching of its own; a dead precedence arm is friction against that direction. Removing it is behavior-preserving for every user.

How our fix solves it (symptom -> root cause)

• Symptom: select_provider_backend takes a leading explicit: str | None and returns resolve_selected_backend(explicit) when it is truthy; _acp declares acp_backend: str | None = None and forwards it as that explicit argument.
• Root cause: the only live caller of the factory closure (config/loader.py) never passes acp_backend, and no per-slot backend picker consumes it anywhere in src/. I confirmed the absence with a control - the same grep method finds the live member-route caller (which passes session_key, not acp_backend) - so the instrument is not failing silent. The three tests that exercised the arm were its only exercisers, matching PR feat: crew member dispatch via per-session session_* mount on ACP backends #8153's review and dwu96's re-triage on the issue.
• Fix: drop the explicit param + its arm from select_provider_backend (precedence is now member-DM auto-route > configured default), and drop the acp_backend param from the _acp closure. The closure keeps **_kwargs, so a stray keyword is still absorbed harmlessly. The kiro construction path is unchanged (harness-parity H13: no conditional/argument/failure-mode added to it - a branch is removed), and the member/default arms are behaviorally identical.

What tests we did

• pytest -n0 test/test_member_dispatch_mount.py - 35 passed (deleted test_explicit_pick_wins, which only exercised the removed arm; updated the 3 surviving select_provider_backend calls to the 3-arg signature).
• pytest -n0 test/test_config_loader.py -k "factory or provider or backend" - 18 passed, including test_factory_resolves_canonical_crew_identity (exercises the _acp closure).
• black --check, isort --check-only, flake8, mypy on the three touched files - all clean. Brand-name gate clean on added prose.
• Enumerated every non-test acp_backend= site and confirmed each is an AcpClient/AcpProvider/AcpRuntime constructor (the instance attribute), not the factory kwarg - untouched.

Any other suggestions on the work

• Item 2 is out of scope and stays open. Session-key prefix normalization touches security-sensitive readers that branch on prefixes (mcp_core.py's startswith("channel:") guard, dashboard/token_auth.py, mcp_gateway/claim.py/stub.py, session_allocation.py) with no agreed canonical-key contract; history_search.py already carries idempotent "collapse stacked dashboard_ prefixes" logic and is_member_session_key accepts three spellings - both symptoms of keys travelling un-normalized. The issue text, CrysisDeu's triage, and dwu96's re-triage all call item 2 "larger than any one feature PR" / architecture. A mechanical sweep without that contract risks silently disarming a confinement check, so it wants its own design pass, not a rider on this cleanup.
• The either/or in item 1's text. The issue frames item 1 as "remove them OR land their first real consumer (a per-slot backend picker)." I searched every open PR/issue and found no picker planned or in flight, so this PR takes the remove branch. If a maintainer knows a per-slot picker is imminent, the seam can be re-added by that author - the point of removing it now is not to carry an unconsumed parameter indefinitely. Flagging so review can decide.

Refs #8166

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 7e096d2eb1f2b3419152b311f0a3ca24167ce030 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All factory call sites pass only agent/cwd/session_key; every acp_backend= usage is an AcpClient/AcpRuntime constructor, unrelated to the factory. The explicit arm has had no caller since it landed in #8153, the removal tightens the single-selection-gate design (H3/H13), and the docstring/comment updates travel in the same commit. No spec documents the retired arm, so no doc drift. Nothing survives the kill-filter.

Design-Verdict: PASS

Dead surface removed from the one backend-selection gate; no caller ever exercised the explicit arm, and the single-gate invariant gets strictly stronger.

[DESIGN-REVIEWED] 7e096d2

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 7e096d2eb1f2b3419152b311f0a3ca24167ce030 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All checks are complete. The change is a pure subtraction with verified zero live consumers of the removed surface, a declared and deferred sibling item, and no stale spec left behind. Final review:

First-Principles-Verdict: PASS

A pure subtraction of a zero-consumer seam, with the count verified in-repo and the deferred sibling declared — every item is the declared removal.

What this change ships

Intent: stop carrying an unconsumed per-call backend-pick seam on the provider-construction path. FIX (subtractive cleanup recorded in issue #8166).

1. The factory closure no longer accepts an acp_backend pick — justified: grepped acp_backend= in src/, all 10 hits are AcpClient/AcpProvider/AcpRuntime constructor sites, zero factory-closure callers.
2. select_provider_backend loses its explicit-pick arm; precedence is now member-route > default — justified: 1 non-test caller (config/loader.py:4303), which never passed it.
3. Comments/docstring rewritten to the two-arm precedence — rides with the fix, correct.
4. test_explicit_pick_wins deleted, 3 sibling tests moved to the 3-arg signature — declared; tests are not consumers.

No spec goes stale: grepped select_provider_backend and "explicit pick" across docs/ — zero hits, so no same-commit doc update is owed. Item 2 of #8166 (session-key normalization) is an accepted-and-deferred sibling, declared in the description with the reachable blockers named; leaving it out is the right scope call, since it crosses prefix-branching confinement readers this cleanup never touches.

Watch

• The closure's **_kwargs still swallows a stray acp_backend="kas" silently instead of erroring — pre-existing absorption, zero such callers today; only worth revisiting if a per-slot picker ever lands.

[FIRST-PRINCIPLES-REVIEWED] 7e096d2

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 7e096d2eb1f2b3419152b311f0a3ca24167ce030 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 7e096d2

Verdict parsed from the review's SHA-scoped output markers for commit 7e096d2eb1f2b3419152b311f0a3ca24167ce030.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 7e096d2eb1f2b3419152b311f0a3ca24167ce030: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 7e096d2eb1f2b3419152b311f0a3ca24167ce030 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 7e096d2

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 7e096d2eb1f2b3419152b311f0a3ca24167ce030: <one-sentence reason>

[iamwhatever]

Tier 1 auto-approve: refactor (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: removes the dead acp_backend kwarg from the ACP provider factory and select_provider_backend, dropping an explicit-pick arm no caller used -- narrows the selection surface, no behaviour change, tests updated in step.