feat(acp): select claude models from the provider's advertised list

[iamwhatever]

Problem / Motivation

Selecting Opus 4.8 sometimes left a session pinned to the 200K context window instead of the model's 1M window — silently, with no UX signal. Two independent, intermittent causes:

• (A) Allowlist sibling collision. The per-session settings.local.json allowlist carried both the versioned ...claude-opus-4-8[1m] id and its bare 200K base sibling ...claude-opus-4-8. The claude-agent-acp adapter dedups availableModels by base name and could resolve to the 200K spelling.
• (B) Stale warm-pool seed. A pooled runtime seeds settings.local.json at spawn with the pool's default model. On claim, set_model switched the model on the wire but never re-seeded the file, so the spawn-time window stuck.

Why it matters

The context window is the single most consequential per-session setting. A silent halving to 200K truncates long sessions with no indication the selected model's full window was lost — the user picks 4.8 (1M) and unknowingly runs at a fifth of the capacity.

What changed (motivation → approach → change)

Root cause was that model selection was reconstructed from the static registry rather than reconciled with what the provider actually advertises, and the warm-pool re-apply was claude-special-cased at the call site.

• model_registry — add _dedup_window_siblings: drop a base-window id when a 1M sibling with the same advertised key is present (order-preserving; also drops exact dupes). seed_available_models routes both the advertised (warm) list and the static (cold) fallback through it, so the seeded allowlist can no longer carry both spellings (fixes A).
• acp_backends — introduce two opt-in capability frozensets, ACP_BACKENDS_ADVERTISED_MODEL_SELECTION (feed the advertised cache + fold the wire id at spawn and on set_model) and ACP_BACKENDS_SEED_LOCAL_SETTINGS (re-seed settings.local.json on a warm claim, fixing B), plus model_registry_namespace(backend) mapping every known backend to its registry index key. Claude is the sole member today; a future provider opts in with one edit rather than a not is_claude inference (harness-parity H5/H6).
• acp/client — set_model folds the bare id onto the advertised spelling and re-seeds the settings file (both gated on membership); _capture_available_models, the _spawn fold, and _write_claude_local_settings read the capability sets + _model_registry_namespace instead of _is_claude/"claude_code".
• acp/types — re-export the new names for existing importers.

Net effect for non-claude backends is zero behavior change (the added init-path call early-returns unless the membership-gated capture flagged a change).

Tests

• test_model_registry — dedup drops the base-window sibling of a 1M id, dedups the advertised list too, preserves order, drops exact dupes, keeps distinct base models; cold-cache fallback compares against the deduped list.
• test_acp_client_more_coverage — set_model folds the bare id onto the advertised spelling, re-seeds settings on claude, and a non-member backend (codex) neither folds nor re-seeds.
• test_acp_capability_sets_leaf — pins the two new members and the model_registry_namespace map for every known backend, so opting a harness in stays a visible, deliberate edit.
• test_acp_session_mcp — updated the settings-seed assertion to compare against the deduped seed_available_models (the source the seed now uses), locking in that the written allowlist no longer carries the 200K base sibling.

Manual verification

N/A — unit coverage sufficient. The seed/fold/re-seed paths are asserted directly (settings file contents, wire id folding, membership gating); no integration or external service is involved.

Related Issues

N/A — no tracked issue; found and fixed while unifying provider-advertised model selection.

Pattern harvest

Pattern: a silent capability downgrade from a stored-vs-served spelling gap (a versioned [1m] id and its bare base sibling both present, dedup-by-base picking the weaker one) combined with a spawn-time seed never re-applied on mutation. Rule candidate (review-prompt / agents-md): a per-session settings artifact seeded at spawn must be re-seeded on any wire-level mutation of the same field, and an allowlist must not carry two spellings that a downstream dedup can collapse to the weaker one.

Checklist

• At most two commits (one here), Conventional Commits title
• Existing tests pass and new tests added
• Self-review completed; follows project style
• Documentation updated (if applicable) — N/A
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

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

Design-Verdict: CONCERNS

Sound root-cause fix, but the "one-edit opt-in" claim is half-true and the spec this PR obsoletes ships unchanged.

Watch

• The spawn-path fold is gated on _is_claude, not the new capability set: resolve_wire_model_id at spawn sits inside the pre-existing if self._is_claude: block, so a future ACP_BACKENDS_ADVERTISED_MODEL_SELECTION member gets the fold on set_model but silently not at spawn — reproducing exactly the stale-seed class of bug (B) for the next harness, and contradicting the description's "the _spawn fold … read[s] the capability sets … instead of _is_claude". Hoist the fold behind _uses_advertised_model_selection.
• Documented behavior changed with no spec update in the same commit: docs/system-specs/features/claude-code-provider.md:230 still says the seed takes "availableModels from the registry", and the new persisted sidecar provider_models.json (a new data-home artifact with a schema and staleness semantics) appears nowhere in the acp-client/providers docs. The checklist marks docs "N/A"; AGENTS.md's same-commit rule says otherwise.

Suggestions

• Persist-if-changed is invoked at two call sites gated on the same membership the capture already checked — fold the persist trigger into one place next to the capture so a third session/new path can't forget it.

[DESIGN-REVIEWED] 57a5a18

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 57a5a183c6856cfa8ba47470c1f4e9d2139278ab — 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 counts are run; I have what I need. Final review follows.

First-Principles-Verdict: CONCERNS

The fix is real and cause-level, but the "one-edit opt-in" story the two new capability sets sell is contradicted by the diff's own spawn path.

What this change ships

Intent: FIX — picking Opus 4.8 sometimes silently ran at a 200K window instead of 1M.

1. An Opus 4.8 pick reliably gets the 1M window; the seeded allowlist drops the 200K sibling — justified, cause-level
2. Switching model on a warm-pool claim rewrites the session settings file — justified, cause-level
3. A picked id is rewritten onto the provider's advertised spelling at spawn and on switch — justified
4. Advertised models now persist to a new data-home file provider_models.json, loaded at import — undeclared persisted state
5. Seed allowlist sourced from what the provider served, static registry as cold fallback — justified (sibling precedent: _KIRO_WINDOWS, same module)
6. Two new single-member capability frozensets gate the claude-only behavior — declared; generality claim half-true (see Watch)
7. A backend→registry-namespace map for all four backends — one live key, generalized
8. Empty-allowlist warning reworded — rides along, trivial

Watch

• Description: "a future provider opts in with one edit rather than a not is_claude inference." The diff keeps the spawn-time fold AND the initial seed under if self._is_claude: (client.py:3581), and the re-seed target _write_claude_local_settings is claude-hardcoded — so one edit to ACP_BACKENDS_SEED_LOCAL_SETTINGS gives a future member the re-seed but never the initial seed, and membership in ADVERTISED_MODEL_SELECTION folds on set_model but not at spawn. Grepped all consumers: 3 membership call sites (client.py:3196, 3219, 3283) vs 2 spawn-side _is_claude sites for the same capabilities. Positive _is_claude satisfies harness-parity as written ("identity is positive… or membership").
• The persisted sidecar provider_models.json (import-time load, atomic write) is a permanent on-disk format the description's What-changed list never names; the two reported defects are fixed without it (dedup + re-seed work on the static list). Its own harm — a served-but-unregistered model collapsing to the base window — is real and derived (registry drift, same cause _KIRO_WINDOWS exists for), but a human should see the new file.

Subtractions

• _MODEL_REGISTRY_NAMESPACE_BY_BACKEND (acp_backends.py:273): drop the KIRO/KAS/CODEX rows — each restates the .get(..., "acp") default, and every live caller passes claude (counted: 4 call sites, all claude-gated). A one-entry dict plus the default is byte-identical behavior; trim the test pin with it.
• ACP_BACKENDS_SEED_LOCAL_SETTINGS + _seeds_local_settings: 1 consumer (client.py:3219), gating a call into claude-hardcoded _write_claude_local_settings whose spawn-time twin is gated on _is_claude. Use the same _is_claude and delete the set until a second seeding backend makes the generality real.

[FIRST-PRINCIPLES-REVIEWED] 57a5a18

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 57a5a183c6856cfa8ba47470c1f4e9d2139278ab — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 57a5a18

Verdict parsed from the review's SHA-scoped output markers for commit 57a5a183c6856cfa8ba47470c1f4e9d2139278ab.

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 57a5a183c6856cfa8ba47470c1f4e9d2139278ab and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 57a5a18

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

[iamwhatever]

**rebutted** — span=0dfda262228b (`src/kiro_crew/acp/client.py` — adapter persistence on the Kiro construction path)

The awaited persist is already gated: on ed885107 both init-path call sites read if self._uses_advertised_model_selection: before await self._persist_advertised_models_if_changed(), so for kiro/KAS/codex the awaited step never runs — the construction path is behaviorally identical to before this PR.

The remaining objection is to the presence of the membership if-branch on the shared path. That does not meet WHAT BLOCKS under the GPT contract:

• it is not a residual-class defect (no security hole, no crash/data-loss/corruption, no removed guard — the guard is added), and
• it is not a blocking: true AUTOSDE rule violation: the repo's Harness Parity Gate (check_harness_parity.py) is green on this head, and an opt-in frozenset-membership branch is the exact pattern every other backend capability here uses (ACP_BACKENDS_COMPACT, _STEER, _SESSION_SHARING).

Removing the branch entirely would require extracting the shared new-session/resume block into a member-only override — a redesign wider than the problem, for zero behavioral change. Per the two-question rule this is legitimate-but-disproportional, so the code stays as-is.

Adapter persistence changes the Kiro construction path

[iamwhatever]

**fixed** — span=e713faf8c271 (`src/kiro_crew/model_registry.py` — cold-cache fallback comment contradicted the dedup)

The docstring claimed the cold-cache fallback returns "the same list shipped before this cache existed, so behaviour is unchanged", which contradicts the _dedup_window_siblings pass applied right below it.

"the same list shipped before this cache existed" contradicts the subsequent deduplication

Fixed in ca45ac36b: the comment now says the fallback is the static registry list "which is then window-deduplicated below just like the warm list", matching the actual code path.

[chenmingwei23]

Approved: PR Readiness green on this head, all check-runs pass on best-per-name.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #6777 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6777: MERGE_DISCUSSION. Not a competing design so much as a style the landed seam has already moved to: PR #6777's (self._is_kiro or self._is_opencode) guards should become membership in the capability sets main already ships (ACP_BACKENDS_MODEL_VIA_CONFIG_OPTION and friends) that PR #8257 extends further. Files: src/kiro_crew/acp/client.py.
• PR #8255 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8255: KEEP. 8255's new added-line gate rule would fail 8257's two membership reads verbatim, so merge order decides whether a green PR turns red; the two capability registers need one agreed answer. Files: src/kiro_crew/acp_backends.py, src/kiro_crew/acp/types.py, src/kiro_crew/acp/client.py.
• This PR is OVERLAPPING with PR #7096. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8257: MERGE_DISCUSSION. Complementary halves of the same warm-pool reconciliation pattern with no file or line overlap; both can land independently. Files: src/kiro_crew/session_allocation.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.