fix: carry the backend's model-withhold verdict in the slots payload

[chenmingwei23]

What is the problem?

The composer's model chip decided "is this pin usable?" by looking for the slot's
pinned model in the picker's list (GET /api/models). If it was not on that
list, the chip showed auto. The backend already computes the authoritative
answer per slot -- at spawn, against the live session's advertised list -- but
that verdict was never propagated, so the frontend re-derived it from a weaker
signal.

That conflated "not entitled" with "not listed for any reason". api_models
narrows its list for reasons that have nothing to do with entitlement (it drops
deprecated ids before the entitlement narrowing), so every one of those filters
silently became an entitlement signal and a filtered-but-runnable pin displayed
as auto.

The same predicate also had three expressions that had to stay in lockstep
across two languages by convention alone: the backend narrowing
(_entitled_kiro_models), the frontend display (displayModel plus the
modelsDegraded trust gate), and the mirrored normalizeModelKey.

Why this issue matters to the user

The chip names the model a turn will run on. When it is wrong, the user is told
their session runs on a model it does not, and the only recourse is to spot the
mismatch themselves. Today the wrong label is cosmetic and narrow. The real cost
is that it widens with no test tripping, because the heuristic cannot tell why
a row vanished from the picker: any future catalog filter (curation, dedup, a new
deprecation) mislabels every slot pinned to a model it removes.

How our fix solves it

Symptom: the chip reads auto for a pin the account can run (or names a pin no
turn will use). Cause: the frontend answers an entitlement question from list
membership, which is not an entitlement signal. Root cause: the one component
that knows the answer -- the session spawn -- never published it.

• _pinned_model_verdict (chat_runner) replaces _pinned_model_withheld and
  returns a tri-state: True withheld, False runnable, None unknown.
  Unknown is now its own answer: an empty advertised set used to fold into
  "runnable" through model_is_unusable, which is correct for the wire and wrong
  for a displayed verdict.
• The spawn records BOTH answers on the slot, and SlotProjection.to_dict
  carries it as model_withheld (true / false / null).
• displayModel reads it: true -> auto; false -> the pin (the list's
  spelling when the list has a row for it, so the picker still highlights);
  null/absent -> the existing list-membership heuristic with its degraded
  gate. Unknown fails open, as the issue requires.
• The verdict is stored against the model id it was computed for, so a re-pin
  invalidates it with no work from any of slot.model's writers (picker, bulk
  pick, pick rollback, two restore paths, canonical backfill). A verdict
  outliving a re-pin would label the new model with the old model's entitlement.
• A session teardown drops the verdict, because the verdict describes the session
  that advertised the list. There is no single chokepoint for that:
  _reset_slot_session is the funnel for the switch handlers (agent, workspace,
  model), but three other sites replace a slot's session directly -- the deferred
  project-change reset and the deferred conversation discard in
  _consume_pending_teardowns, and the post-turn agent-switch reset/discard in
  the runner's finally. Routing those through the funnel would also cancel any
  pending question card, which is not this change's business, so each drops the
  verdict itself and a test pins the invariant: every
  state.sessions.reset( / discard_conversation( site in chat_runner.py and
  chat_handlers.py must carry a record_model_withheld(None) beside it.
• Display stays firewalled from writes: the pin is still KEPT when withheld, and
  the pin-to-agent row still writes slot.model, never the displayed label.

Deliberately unchanged: a slot that has never spawned a session has no verdict,
so its display still falls back to list membership. The withhold is only knowable
once a session advertises a list, and collapsing unknown into "show the pin"
would re-show an unentitled pin in that window -- the symptom #1611 fixed. The
issue's named deprecated-pin instance lives in exactly that pre-first-turn
window and is separately self-correcting: _normalize_model rewrites such a pin
to its replacement at turn start, so from the first turn on the slot carries an
id the picker does list. What the verdict fixes is the class -- wherever a
verdict exists, no /api/models filter can relabel a runnable pin, and a
genuinely withheld pin reads auto even when the list is wider or degraded.

What tests we did

Targeted only (no full suite locally).

• Backend: test/test_dashboard_chat.py -k PinnedModelWithheld 20 passed;
  test/test_chat_runner_coverage.py + test_dashboard_approval.py +
  test_eager_spawn.py 398 passed; test_chat_slot_reset_conversation.py +
  test_chat_slot_project.py + test_reset_conversation_directive.py +
  test_mcp_core_set_project.py 81 passed;
  test/test_ask_question_roundtrip.py (the reset funnel's own tests) 63 passed;
  test_dashboard_chat_handlers_coverage.py + test_agent_default_model.py +
  test_resume_publishes_hydrated_slot.py 184 passed.
• Frontend: vitest run src/test/model.displayModel.test.ts 23 passed;
  tsc -b clean; eslint 0 errors on the five touched files (8 pre-existing
  warnings in ChatPage.tsx, none on changed lines).
• New coverage: a tri-state assertion for every unknown branch (no pin, auto,
  claude_code, claude backend, no getter, getter raises, empty advertised set);
  the payload carries both answers; the verdict is not reported after a re-pin
  and is reported again when the same pin returns; a cleared pin reports nothing;
  teardown forgets it; the reset funnel forgets it; the source-level ratchet over
  every teardown call site; and two end-to-end _run_chat tests -- a withheld
  pin still gets its notice card and now reports true, and a deprecated pin
  (rewritten to its replacement at turn start) reports false with no notice.
• Mutation-verified with the tests kept: with state.py, slot_projection.py
  and chat_handlers.py at base, the 7 payload/identity/teardown tests fail;
  with chat_runner.py at base, both end-to-end recording tests fail; with the
  reset-conversation route's drop removed, the ratchet fails naming
  chat_handlers:3658.
• Gates: flake8 and isort clean; mypy reports no error in the four touched
  dashboard modules (its 2 errors are pre-existing in src/kiro_crew/transcribe.py).
  black --check flags apps/builtins/auto_research/handlers.py; verified
  pre-existing by running the same check on that file at base -- my change there
  is one word inside a comment.

Why no screenshot: the chip's label does change, but only for a slot whose
live session withholds its pinned model (or whose pin an /api/models filter
removed while the session still advertises it). Neither state can be staged in a
pod without faking the backend's advertised list, so the delta is covered by the
displayModel unit tests and the two end-to-end _run_chat tests instead.

Any other suggestions on the work

• The pre-first-turn window could be closed properly by carrying the EFFECTIVE
  model id (the issue's other option) rather than a boolean. The chip could then
  name a deprecated pin's replacement instead of the deprecated spelling, and
  normalizeModelKey's mirror of _normalize_model_key could go away. It is a
  wider change (a second field, and the write firewall has to be re-argued for an
  id-shaped field), so it is not folded in here. Tracked as Carry the slot's effective model id so the last picker-membership inference can go #7575.
• Consequently displayModel's membership fallback is unreachable for any slot
  that has run a turn, but cannot be deleted until that unknown state is closed.
  The two spellings of "can this account run it" still coexist; the verdict just
  wins wherever it exists.
• The four teardown sites want one chokepoint. The blocker is layering:
  SessionManager is keyed by session key and knows nothing about slots, so it
  cannot drop slot-side state itself. A teardown callback on the manager (or
  moving the deferred-teardown consume behind the funnel once the pending-question
  interaction is settled) would collapse all four -- worth doing when something
  else slot-side needs the same hook.

Pattern harvest

Rule candidate: a cached verdict must be keyed to the input it judged, not to
the object that holds it. The defect class is a stored judgment outliving the
value it was computed for -- here slot.model has six writers, so an explicit
reset at each one would eventually be missed. This repo already learned the
same lesson for session resets (_reset_slot_session exists because "six call
sites each having to remember an extra line is how one of them gets missed"),
so the convention is worth making mechanical: a stored verdict is spelled as the
pair (_x, _x_for) and read only through a property that compares _x_for
against the live input. A lint rule can check the shape -- any slot named
_<name> that is published in a payload and written by a record_* method
should have a sibling _<name>_for -- which is cheaper than trying to detect
staleness itself. Where the input is NOT a field the holder owns (a live session,
here), the fallback is the source-level ratchet this PR adds: enumerate the sites
that invalidate, and fail the build when a new one appears without the drop.

Closes #1819

[github-actions]

UX Review (Fable 5) — ✅ PASS

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

The diff's UX surface is confined to the composer's model chip label logic — no new strings, no layout, no screenshots. The change makes the chip tell the truth in cases where it previously lied (auto for a runnable pin), the withheld notice card copy is untouched and already asserts state plus an action, unknown fails open to prior behavior, and the pin-to-agent row's disabled state stays consistent with the shown label in both ChatPane and ChatPage. The one residual oddity — a runnable-but-unlisted pin shows in the chip with no highlightable picker row — is inherent to a filtered list, acknowledged in the code, and tracked in #7575.

UX-Verdict: PASS

The chip now names the model a turn will actually use, unknown states fail open to prior behavior, and no new copy or flow is introduced.

[UX-REVIEWED] 5f2cae1

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Backend-owned verdict replaces a cross-language heuristic at the right layer; the field is additive, fail-open on unknown, and self-invalidating on re-pin.

The residual risks are already contained: the stronger alternative (carrying the effective model id) is named and tracked (#7575), the distributed teardown drops are pinned by a source-level ratchet test with the chokepoint deferred for a stated layering reason, and any reset site the ratchet's two-file scope misses yields only a bounded stale label that the next spawn's unconditional re-record overwrites.

[DESIGN-REVIEWED] 5f2cae1

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 5f2cae1e8041caf9bae160f7847457249081847e and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/dashboard/state.py:4181 -- return None preserves A’s verdict across A→B→A, so a successful live switch back to A can display stale auto -> Fix: permanently invalidate the cached verdict when the model changes.
[GPT-REVIEWED] 5f2cae1

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

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 5f2cae1e8041caf9bae160f7847457249081847e — 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 verification checks are done. The new field has two counted frontend consumers, no existing mechanism carries an effective-model answer in the slots payload, the teardown sites outside the two ratcheted files reset ephemeral non-slot sessions (hooks, refine, channels, taskrunner) rather than dashboard chat slots, and the deferred alternative is explicitly tracked. Final review:

First-Principles-Verdict: PASS

The backend publishes the entitlement verdict it already computes, replacing a frontend heuristic that turned every catalog filter into an entitlement signal — cause-level, every item counted.

What this change ships

Intent: stop the composer chip mislabeling a slot's pinned model by carrying the backend's own withhold verdict instead of inferring it from picker-list membership — a FIX.

1. Chip now names a runnable pin the picker list filtered out, instead of auto — justified (the reported defect)
2. Chip reads auto for a withheld pin even when the list still shows it or is degraded — justified, declared
3. Slots payload gains tri-state model_withheld — justified; 2 consumers (ChatPage.tsx:5404, ChatPane.tsx:258)
4. An empty advertised list now displays as "unknown", not "runnable" — justified, declared
5. Session teardown/agent-switch resets the verdict to unknown — justified; 5 sites, pinned by a ratchet test
6. Re-pinning the model silently invalidates the old verdict — justified (stale verdict would label the new pin)
7. Two builtin-app docstrings updated for the renamed helper — rides along, entailed by the rename

Watch

• The boolean is interim surface: the author's own tracked follow-up (Carry the slot's effective model id so the last picker-membership inference can go #7575, carry the effective model id) would supersede model_withheld, yet the field lands in the public slots payload and ChatSlot type where it must be honored after Carry the slot's effective model id so the last picker-membership inference can go #7575 ships. Considered and deferred by the author, but a human sequencing decision — land Carry the slot's effective model id so the last picker-membership inference can go #7575 first and this field never exists.
• displayModel now holds two answers to one question; the membership heuristic is unreachable for any slot that has run a turn (author's words) and is kept only for the pre-first-turn window. Delete it when Carry the slot's effective model id so the last picker-membership inference can go #7575 closes that window.

[FIRST-PRINCIPLES-REVIEWED] 5f2cae1

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 5f2cae1e8041caf9bae160f7847457249081847e — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5f2cae1

Verdict parsed from the review's SHA-scoped output markers for commit 5f2cae1e8041caf9bae160f7847457249081847e.

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

[chenmingwei23]

Disposition: First Principles CONCERNS on 26a0e92

Watch item -- "single reset funnel" premise has counted bypasses: FIXED, in
1b452b9.

Verified the finding and then widened it: grepping
state.sessions.reset( and state.sessions.discard_conversation( across
src/kiro_crew/dashboard/ found four slot-side teardowns, not two --

site	trigger
chat_handlers.py _reset_slot_session	agent / workspace / model / effort switches, reload
chat_runner.py deferred project-change reset	project change consumed at a turn boundary
chat_runner.py deferred conversation discard	/reset deferred past an in-flight turn
chat_runner.py post-turn finally	agent-switch reset, poisoned-conversation discard

chat_handlers.py's POST .../reset-conversation route was the fourth, and the
one the two-site count would have left behind.

Each now drops the verdict itself. I did not route the runner's sites through
_reset_slot_session as the finding suggested: that funnel also runs
_unblock_pending_waits, so calling it at a turn boundary would cancel a
pending question card -- a behaviour change outside this PR's scope. Drop timing
differs by path, deliberately: before the await for the resets (a failed teardown
leaves a session this slot cannot vouch for, and None is the fail-open
direction), after it for the deferred discard (a refusal is a normal outcome
there -- turn in flight, flag stays armed, session untouched -- so dropping first
would forget a verdict that is still accurate).

Four sites is the drift risk the property-based invalidation exists to avoid, so
it is pinned mechanically rather than by convention:
test_every_session_teardown_drops_the_verdict walks the source of both modules
and fails if any teardown call site has no record_model_withheld(None) beside
it. Mutation-verified -- removing the route's drop reds it naming
chat_handlers:3658.

Also updated the PR body, which carried the same over-broad "single funnel"
claim, and recorded in Other suggestions what a real chokepoint would need
(SessionManager is keyed by session key and knows nothing about slots, so
collapsing all four wants a teardown callback on the manager).

Items 1-7 of the "What this change ships" list need no action -- they read the
change as intended.

[chenmingwei23]

Disposition: GPT 5.6 blocking findings on d8bbde6

Both real, both FIXED in 5e57ae0, both mutation-verified (each new test
fails against d8bbde6 and passes after).

1. chat_handlers.py:2415 -- a declined reload erased a live session's
verdict. Correct, and the failure direction is the one this PR exists to
remove: the reload route resets with skip_if_busy=True, which returns False and
touches nothing while a turn is in flight, so clearing beforehand republished a
runnable pin as auto for a session that never changed. The drop is now gated on
the reset having happened, with the raise/cancel path kept explicit -- a teardown
that exploded leaves a session this slot cannot vouch for, so that path still
drops to unknown. My original "drop before the await" reasoning conflated those
two outcomes: it holds for a partial teardown, not for a clean decline.

New tests: test_a_declined_reset_keeps_the_live_session_s_verdict (False ->
verdict survives) and test_a_reset_that_raises_drops_the_verdict.

2. chat_runner.py:5469 -- an unknown replacement session retained the
previous verdict. Correct. A replacement can arrive without a teardown this
slot observed (a reaped session, a provider that re-spawned) and can advertise
nothing at all, so skipping the None write left the previous session's answer
published for a session that never took that entitlement snapshot. Recording is
now unconditional on the new/resumed branch, which is your suggested fix.

The skip was justified in-comment as "unknown must fail open, and overwriting a
known verdict with a guess is worse than carrying nothing" -- wrong on the second
half: None is not a guess, it is the honest state, and
record_model_withheld(None) stores exactly that. Failing open still happens,
just at read time in displayModel where it belongs.

New test: test_a_replacement_session_that_advertises_nothing_publishes_unknown
drives _run_chat with a pre-recorded verdict and a client advertising [], and
asserts the payload publishes null.

Gates after the fix: 23 in TestPinnedModelWithheld, 467 across
test_ask_question_roundtrip / test_chat_slot_reset_conversation /
test_dashboard_chat_handlers_coverage / test_chat_runner_coverage, 108 across
the mode / effort / project / default-model suites, flake8 and black clean on the
touched files.

Unrelated CI note for whoever reads this next: the red
test_security_posture.py::TestGateSideLogRedactorSpelling
("dashboard/handlers/memory.py: 2 sites, census says 0") is not from this
branch -- those files are byte-identical to main here, and main's own CI run
33480742836 on 5603ae7 fails the same test with the same message. The main-side
fix is open as #7554.

[chenmingwei23]

Disposition: First Principles CONCERNS on 5e57ae0

Watch 2 (deferred follow-up should be tracked): DONE -- filed as #7575,
carrying the constraints verbatim (write firewall re-argued for an id-shaped
field, pin kept when withheld, unknown stays its own state). Linked from the PR
body's Other suggestions on the next push.

Subtraction (delete the four non-spawn drops, the BaseException handler and
the ratchet): REBUTTED. Two reasons, and one concession about my own wording.

The window is not symmetric. The subtraction rests on "the window's fallback is
no better than the stale verdict it replaces". It is better, and the clearest case
is the agent switch -- one of the switches that resets a session. slot.model
survives an agent switch, so with the drops removed a verdict computed against
the OLD agent's advertised list is published for the NEW session. When the new
agent runs claude_code, _pinned_model_verdict cannot even judge that pin
(canonical key vs bare advertised ids -- it returns None by design), so a
carried-over true states "this account cannot run this model" about a session
whose namespace was never compared. The membership fallback in that window reads
GET /api/models for the current provider, entitlement-narrowed; it is
imperfect in one known direction (the deprecation filter, tracked in #7575), not
arbitrary.

A payload field is a wire claim, not a chip input. model_withheld is part of
SlotProjection.to_dict -- the slots payload, read today by two frontend call
sites and tomorrow by whatever else. Publishing a definite true/false for a
session that no longer exists is a false statement on the wire. null is not a
degraded version of that claim; it is the accurate one, and the frontend's
documented fail-open is where the display decision belongs. Those two wrongs are
not interchangeable.

Conceded wording. The skip_if_busy comment does invite the reading quoted --
that falling back to the heuristic is itself "the defect the verdict is carried to
remove". The defect was never the heuristic; it was inferring entitlement from
the heuristic while an authoritative answer existed. The distinction the code
actually turns on is whether the session the verdict describes still exists: a
DECLINED reset leaves it alive, so the verdict is still true and must be kept; a
completed teardown ends it, so the verdict is false and must go. I will tighten
that comment on the next push rather than force a re-roll of the clean lanes now
(this head is blocked on an unrelated main-owned red -- see below -- so a rebase
push is already coming).

On the two rounds of this lane. Round 1 on 26a0e92 found the funnel claim
over-broad and named the fix: "routing those two through the funnel (or dropping
the verdict there) is the general fix the funnel comment argues for." The drops
now proposed for deletion are that fix, and the ratchet exists because the round-1
count (two sites) was short by two -- the reset-conversation route and the
deferred discard. Since this comment is rewritten in place, a reader arriving now
cannot see that history, so it is stated here rather than assumed.

If a maintainer prefers the smaller surface, the subtraction is a one-commit
change and I will make it -- with the idle-window staleness stated plainly in the
PR body rather than left implicit.

Unrelated red, for the next reader:
test_security_posture.py::TestGateSideLogRedactorSpelling
("dashboard/handlers/memory.py: 2 sites, census says 0") is not from this branch.
Those files are byte-identical to main here, and main's own CI run 33480742836
on 5603ae7 fails the same test with the same message. The main-side fix is open
as #7554; this PR rebases onto it once it lands.

[bolichen97]

Reviewed via parallel subagent audit: diff matches description, CI fully green, no blocking findings, no unresolved threads.

[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 #7834 is PARTIALLY_COVERED relative to this PR. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #7834: CONTINUE_DEVELOPMENT. The merged predecessor covers only the verdict-known half of issue Issue #7575; the primary's pre-first-turn deprecated-alias label and the removal of the membership heuristic exist nowhere in current main, so this is a genuine follow-up, not a re-do. It is not mergeable as written because it silently invalidates the contract tests PR #7546 shipped. Files: src/kiro_crew/dashboard/state.py, website/src/lib/model.ts.

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