fix(session): refuse stale sessions at claim time and surface refused clears

[rnoack1]

Problem / Motivation

This description previously claimed the change adds an atomic skip_if_busy guard to the queued project-change reset. That claim was false against this base and has been rewritten. Verified at merge base 9f3c07dc78b7: chat_runner.py:3879 already reads reset_ok = await state.sessions.reset(pending_key, skip_if_busy=True), and effective_session_key(slot) is already resolved at the switch handlers (7 sites in chat_handlers.py). This diff adds neither — 0 added lines matching either. The one-line guard the old title named has landed upstream independently, so the red-then-green verification the old text described could not occur against this base.

What remains is larger than that headline and is stated below on its own terms.

A slot's project can change while a session bound to the OLD directory is still reachable. Three windows stay open once the queued reset is allowed to decline:

• A claim arriving with no stated cwd was reused on directory match alone, so a channel turn (which states no cwd) could be served a session bound to the pre-change project.
• An agent switch that keeps slot.project is invisible to a directory test, so the switched-away agent could serve the next turn.
• A cold start already in flight holds no registry entry, so neither reset nor has_session can see it, and a probe-then-act sequence cannot protect against it.

Separately, api_channel_clear_context was destructive on a busy member, and the earlier wording here got that backwards. It claimed the endpoint "reported success for a clear it did not perform"; verified at the merge base, the handler called state.sessions.reset(agent.session_key) with the manager default skip_if_busy=False, so the clear was performed — it tore down a session whose reply was still streaming. The reported-success half was real but narrower: a member holding a key with no live session counted as cleared. Both are fixed here; the defect to audit is the destructive one.

Why it matters

A turn served by a session bound to the wrong directory writes its relative paths into the wrong project — silent, and not visible in the transcript. The clear-context case is worse than silent: the endpoint answered ok while the agent kept its full history.

What changed

Four distinct pieces. They are listed separately because they are separable, and the third and fourth are contract changes rather than internal fixes.

1. Claim-time cwd validation (session_allocation.py, _reacquire_and_validate) — the reuse decision now validates the bound directory at the moment of the claim rather than trusting the caller to state one.

2. A retirement-arm subsystem (new: mark_retire_on_next_claim, note_project_change, retire_pending_agent, spend_retire_arm, discard_all_retire_arms, CWD_CLEARED, plus a per-key generation counter — all 0 occurrences at base). This replaces the base's documented leave-armed-and-retry policy at the consume site, where the base comment argues the opposite ("leaving it armed is always safe"). Two producers exist and are not interchangeable; the distinction is enforced by source-text census tests.

3. A _queue_held drain-gate unification (state.py, chat_runner.py) — one predicate replaces per-cause booleans across four drain gates. Renames _last_turn_auth_required.

4. A channel API contract change — api_channel_clear_context moves from unconditional clear to skip-busy, adding a 409 turn_in_flight, and the total-refusal case returns before the shared message buffers are wiped or saved. The refusal surfaces as an in-page ErrorNotice above the composer; it carries no askAgent hand-off, deliberately, because navigating away would destroy an unsent draft in the composer directly beneath it.

The unconditional clear was also the only in-band recovery for a wedged member turn, so what bounds such a turn matters. It is bounded, and not by this diff: every ACP prompt resolves its wait from agent.chat_turn_timeout_secs — default 7200s, clamped to 300s..86400s — applied in acp/session_handle.py via _effective_prompt_timeout_async and enforced by the transport wait on _turn_done. Channel members are dispatched by handlers_channel.py's _spawn_agent_task, a bare create_task that adds no ceiling of its own, so that prompt timeout is the whole bound. retry when idle therefore names an event that arrives. No force parameter is added: it could only make the refusal disappear by discarding the state of a turn that is still running.

• Switching channels now also clears the page-level error notice, not just a stale clear-context refusal: a failure raised against channel A named channel A's roles and would otherwise read as live for whichever channel the composer now sends to.

Also in this commit, declared

Three changes ride along that the four pieces above do not imply. They are named here so a
reviewer does not meet them as unexplained scope.

• A channel log lock (channel.py) — post and api_channel_clear_context now share one
  _log_lock, and the thread-parent resolution happens under it rather than before it. Derived
  from piece 4: making the clear skip busy members is what put awaits between the decision to
  wipe and the wipe. Without the lock a concurrent post is acknowledged and then wiped away by
  _save(), and a threaded reply can name a parent the wipe already removed.
• An ErrorNotice warn severity axis — one consumer, ChannelPage. A withheld clear is
  not a failure, so it needed chrome that is not the danger variant.
• A capture scene and its frames — website/capture/clear-context-busy-refusal.html,
  website/capture/clear-context-busy-refusal.tsx,
  website/scripts/capture-clear-context-busy-refusal.mjs and the ten
  temp-screenshots/clear-context-busy-refusal/*.png frames. These are evidence artifacts,
  not shipped surface: nothing in the app imports them, and each frame is written only if its
  own assertions hold. They are listed here because the four pieces above do not imply them.

Out of scope, stated explicitly

The arm settle is probe-then-act over a linked_session_key that is written outside
slot._lock, so a rebind landing after the last pass still leaves the arm on the abandoned
key. Closing that at cause means serializing every writer of that field, and there are eight
assignment sites across five modules -- dashboard/handlers/cron.py:1413,
dashboard/chat_persistence.py:1057/:1563/:1571, dashboard/state.py:6516/:6534,
dashboard/workflow_inject.py:156, dashboard/cron_inject.py:425 -- several of them inside
persistence loads that hold no slot lock. That is a cross-module change this PR does not
attempt. The residual window is bounded by the settle pass count, and the arm it can strand
is dropped by the next mint or teardown on that key.

Wire-contract changes a caller must know about

Called out explicitly because these are observable at the boundary rather than internal:

Change	Wire shape	What a caller must do
Clear-context skips busy members	409 {"error", "code": "turn_in_flight", "busy": [...]} when nothing cleared	Treat 409 as retryable once the named roles finish; it is not a failure to clear
A partial clear is now marked	200 {"ok": false, "cleared": [...], "busy": [...]}	Read ok, not the status: 200 with ok: false means some roles were kept
Agent and project switches can report an unavailable workspace	503 {"error", "code": "workspace_unavailable"}	Retry; the request changed nothing, and the slot is left exactly as it was found
A slot whose project was explicitly CLEARED no longer keeps its persisted resume SID	No response change; the next turn cold-starts instead of resuming	Nothing, unless a caller relied on a project-less slot resuming its old conversation. Scope note: the guard is slot.claim_cwd, which states nothing for a project that was never set, so this covers ONLY a project that was explicitly cleared
A slot whose project was explicitly CLEARED bypasses the warm pool	No response change; the claim cold-starts through the factory instead of taking a warm child	Nothing. Stated because the latency is intended, and it is paid only by a slot whose project was explicitly cleared: the per-session default is a directory no pooled child is in, so a warm hit would serve the stale binding this change refuses. The bypass is recorded as bypass_cwd in the claim's pool decision.
The queue hold now covers a deferred reset	No response change; a queued prompt stays queued rather than draining behind the turn that deferred	Nothing; the prompt remains visible and individually cancellable

Tests

Each new assertion was mutation-verified: the unmutated test passes first, then the guard is removed and the test is confirmed to fail on its own assertion rather than on an error.

• Total refusal must not destroy the shared channel log — reverting the 409 to run after the buffer wipe fails on "the shared message log must SURVIVE a total refusal ... got 0".

• The refusal carries a machine-readable code, satisfying the repo's error-code contract ratchet.

• clearContextBusyMessage (4 cases): names every refusing role, empty when nothing refused or the field is absent, and ignores a non-array value rather than rendering [object Object].
  Two pre-existing drain leaks, repaired deliberately. Unifying the four drain gates on _queue_held also closes two leaks that predate this change: the synthesis dispatch and the stage handoff never consulted the old _last_turn_auth_required flag, so both could drain a queue a gate had withheld. They are named here rather than left implicit because the unification is required by the core fix and reverting only these two sites would leave the new gate deliberately inconsistent across the four.

• Arm/generation behaviour, the producer census, and the consume-site census. Every session-ending teardown is also enumerated from the module and must declare itself spend-side or keep-side, so a new teardown that classifies itself nowhere fails the guard rather than passing unnoticed.

Reachability of the remaining CI failures, classified rather than dismissed — none is called flaky:

• Backend Lint & Type Check (3.12) read cancelled, not failed: the job hit its 15-minute cap while Check formatting (black, baselined) was still running, and that step passed. flake8 then ran 87s and emitted nothing, and mypy was skipped. The gate's workload is a fixed ("src", "test") scan of 3,394 files, independent of this diff — measured cold-cache locally at 30s at this head versus 29s at the previous one — and the job has been at 13m50s / 14m24s / 15m17s across three consecutive shas. Named for a maintainer rather than worked around, since the cap lives in .github/ which a fork PR may not touch.
• src/test/AppSdkSharedModulesCov80.test.ts (2) and src/test/ContextBreakdownPanel.test.tsx (2) fail on an unmodified upstream/main checkout on this host — npx vitest run src/test/AppSdkSharedModulesCov80.test.ts src/test/ContextBreakdownPanel.test.tsx → Tests 4 failed | 16 passed with none of this diff present. They also passed in CI's shards 1 and 3 at this PR's own head, so the difference is host-side and upstream of this change.
• test_dashboard_state_ws.py::TestSlotsBroadcastCarriesFolders and test_remote_crew_execution.py::...interrupted_row remain as previously classified: not attributable, with the residual uncertainty stated — neither was reproduced under CI's shard ordering or on a Windows host.

Pattern harvest

Rule candidate: a probe followed by a separate act cannot protect a resource whose state can change between the two — so the guard must be the decision itself, taken atomically with the mutation, or a flag the claimant must satisfy. The in-flight cold start is what proves it here: it holds no registry entry, so both probes read it as absent and no amount of probing closes the window.

Rule candidate: an endpoint that reports on work it also performs must answer a refusal BEFORE it mutates shared state. Ordered the other way, the report contradicts the effect — and when the mutation is persisted, the contradiction is irreversible.

Screenshots

Two error states appear below, and they lead differently on purpose: a partial clear leads Context partially cleared, a total refusal leads Not cleared in warn chrome, because a withheld clear is not a failure. Failed to clear context is reserved for a genuine error such as a 500. Leading a partial with the failure title contradicted its own body, which ends by naming what was cleared.

The clear-context controls surface a refusal that was previously swallowed, and no longer destroy a streaming reply to do it. A partial refusal answers 200 with the refusing roles in busy — a field no caller read, so a "cleared" click silently cleared nothing for those roles. A total refusal answers 409 turn_in_flight before any shared state is touched.

The banner states what was kept and what was cleared, in the page's own vocabulary. Naming only the kept roles let a partial refusal read as a total one, sending the user back through the confirm dialog to re-clear what had already cleared. It also renders in its OWN notice, without the agent hand-off: a clear-context refusal can sit above an unsent composer draft, and the hand-off unmounts the page and destroys it -- the same reason the composer's own failure notice omits it. The channel page's busy badge already says working, so the copy says "still working" rather than introducing "a turn is in flight" for the same state, and {{roles}} is never the subject of a verb — a comma-joined list of two roles has to read as well as one, which a singular verb cannot do in the translated catalogs. Both the 200 and the 409 render the same catalog string: the 409 is recognised by its code, not its prose, so the backend's English never lands in a localized page. The surface is an ErrorNotice, not a native dialog, and because the per-agent button lives in the agents side panel while the banner sits above the composer, the notice is scrolled into view when it appears.

Partial clear-all, two of three roles mid-turn:

The per-agent control, its addressed role refusing:

The 409 total refusal — same localized string, none of the backend's English:

The generic failure path, which already rendered inline before this change and now carries no agent hand-off:

Contrast — nothing refused, so no banner is owed:

Light-theme frames for all five are in the same directory (*-light.png). Each frame is written only if its own assertions hold: the banner carries every refusing role, the kept claim, the cause and the retry; the 409 frame additionally asserts the backend's prose did not leak; and every scene asserts zero dialogs.

Reproduce:

npx vite --host 127.0.0.1 --port 6841 --strictPort
node scripts/capture-clear-context-busy-refusal.mjs http://127.0.0.1:6841 ../temp-screenshots/clear-context-busy-refusal

[iamwhatever]

👋 Hi! This PR is currently in draft status. Workflow runs won't be auto-approved until it's marked as ready for review.

When you're ready, click "Ready for review" and the workflows will be approved on the next cycle automatically.

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed b19ee702ee4c413f8cdbc838e62a9a7eba8561c5 via the fork AI-review pipeline; updated in place on each push.

1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands.

BLOCKING -- src/kiro_crew/dashboard/state.py:4142 -- A stale clear marker overrides a newly assigned project
if getattr(self, "project_cleared", False): return CWD_CLEARED
Clear project → reopen named slot → create handler assigns a default/folder project without clearing the marker → next turn binds the fallback workspace instead.
Anchor: residual/crash-data-loss-corruption
Fix: Set project_cleared = False whenever the create/reopen path assigns a non-empty project.
[BLOCK-MERGE] b19ee70
[GPT-REVIEWED] b19ee70

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

The adjudicable block is empty (0 findings); only fenced finding F1 to rule on.

I opened src/kiro_crew/dashboard/state.py claim_cwd (diff lines 2187–2204): if getattr(self, "project_cleared", False): return CWD_CLEARED — the marker is read before self.project, so a stale marker wins over a real assigned project. I opened the create handler chat_handlers.py:2737-2758: it assigns slot.project (folder_project / cfg default / default_project_dir(workspace)) gated only on not slot.project, and does NOT reset project_cleared. The rehydrate/surface sites in the diff (channel_slots.py:872, chat_persistence.py:1543/1484) carry project_cleared=True forward onto the reopened slot. So a cleared→reopened→re-defaulted slot binds the per-session default workspace instead of the newly assigned project — silent wrong-directory binding, the unbounded residual/data-misplacement class this module exists to prevent.

The finding's condition combination (clear a project, reopen the named slot, let it acquire a default/folder project) is an ordinary user flow, not an extreme or self-contradicting one, and I opened no reset that closes it. I cannot construct a rarity argument that a human should accept the residual risk, so no FLAG record can be completed.

[ADJUDICATION] b19ee70 total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] b19ee70

[ADJUDICATION-FENCED] b19ee70 fenced=1 flagged=0
UPHOLD-FENCED F1 src/kiro_crew/dashboard/state.py:4142 -- Stale project_cleared marker read before slot.project makes a re-defaulted slot silently bind the default workspace over its assigned project via an ordinary clear→reopen flow; unbounded silent-misplacement harm, no extremeness/rarity record completable.
[GPT-ADJUDICATED-FENCED] b19ee70

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 2907bc9bea282c96b5bacdf01c332e72db2d0057 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound fixes to real, silent harms — but delivered as a large compensating subsystem with a self-admitted residual race and a deferred root-cause fix.

Watch

The retirement-arm store is a compensation for cold starts being invisible to the registry ("a cold start holds no registry entry until it finishes"). The spec names the fix that would obsolete most of the six-verb arm/generation machinery — registering a start before its provider binds — but never costs it, so future maintainers inherit the arm store, its census tests, and its per-key generation residency as permanent architecture rather than as a bridge.
Clears when: a maintainer explicitly accepts the arm store over registry pre-registration, or the spec's own revisit trigger is given an owner/issue.

The arm-address settle is probe-then-act over unlocked linked_session_key (_ARM_KEY_SETTLE_PASSES), the exact shape the PR's own harvested rule rejects; a rebind after the last pass strands an arm, and the eight writers that would fix it at cause are deferred. The spec's "any new writer must take the slot lock or be added to this list" trigger is enforced by nothing.
Clears when: the owed writer-serialization lands, or a test/gate ratchets new linked_session_key writers the way the spend/keep census does teardowns.

Suggestions

Piece 3 (_queue_held unification, fixing two pre-existing drain leaks) shares no mechanism with the arm subsystem and would revert independently — worth its own PR next time even under the two-commit budget.

[DESIGN-REVIEWED] 2907bc9

[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 #4118 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 #4118: KEEP. Independent goals in adjacent code; neither subsumes the other and no conflict is expected. Files: src/kiro_crew/dashboard/chat_runner.py.
• PR #7709 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 #7709: KEEP. Same expression, orthogonal conditions — coexist as two conjuncts; only a textual merge-order conflict. Files: src/kiro_crew/dashboard/chat_runner.py.

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

[bolichen97]

The core fix is right and minimal: _consume_pending_reset now passes skip_if_busy=True so the busy check and the teardown are one step under the session lock, matching the sibling discard_conversation call in the same function, and the ambiguous reset return (session is not None) is correctly disambiguated with a has_session presence probe rather than has_active_turn — presence is the right signal since a turn holding the semaphore with no prompt in flight is invisible to the latter. mark_retire_on_next_claim keying on the string rather than the session object is the load-bearing detail: a cold start holds no registry entry, so an object pin would miss exactly the provider already en route to the pre-change directory. The stale-cwd check is placed correctly in _reacquire_and_validate (semaphore held, so eviction cannot land under a streaming reply) instead of the pre-semaphore reuse decision, it normalizes both sides through Path so /p/a vs \\p\\a doesn't evict every warm session on Windows, and it guards on both sides being non-empty so a provider reporting no binding fails open rather than churning. The cwd_moved teardown ordering (pop under the permit, release in finally) is deliberate and argued, and the idempotence note makes the interrupted case safe. Collapsing _last_turn_auth_required into _queue_held is justified by the measurement that no site distinguished the two, and it fixes the real pre-existing leak where a signed-out CLI held the tail drain but not _run_pending_synthesis, which drains the queue too. Grepped the head: no stale _last_turn_auth_required references remain in chat_runner/chat_orchestrator/state.

[github-actions]

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of 2907bc9bea282c96b5bacdf01c332e72db2d0057 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have everything I need. The frontend surface is well-crafted (honest partial/total refusal copy, warn chrome distinct from failure, retry with a destructive re-confirm, localized role lists), but this lane cannot open the ten added screenshots and no blind read ran, so the new controls are unverified by any first-time reader. Composing the review.

UX-Verdict: CONCERNS

Solid, honest refusal UX — but every new control is unseen: the fork's screenshots aren't materialized here and no blind read ran.

Watch

• The warn notice uses an Info glyph, but the product's warn convention is AlertTriangle + text-warn at 20+ sites (CrashReportNotice.tsx:106, MigrationBanner.tsx:24, SecurityPanel.tsx et al.). A habituated user reads amber-triangle as "warning" everywhere else and meets an info circle here. Frequency: every refusal; impact: mild misread of severity; persistent. The in-file rationale (danger already owns the triangle in ErrorNotice) is real — a human should weigh which convention loses.

Evidence gaps

• All ten temp-screenshots/clear-context-busy-refusal/*.png frames are PR-added binaries (fork head never checked out) — the partial/total/per-agent refusal banners, retry button, and generic-failure state are unverified by any cold reader; push the branch to this repo for the blind read.
• "Context cleared." success status (clear-context-done, 12px muted, 4s auto-dismiss) appears in no capture scene — the clean frame asserts no banner, not this text; add a frame showing it.
• Both native confirm dialogs — the rewritten clear-all copy ("…deletes the channel's messages…") and the new retry_clear_all_confirm — appear in no artifact; every scene asserts zero dialogs.
• The clearing/disabled (aria-busy) state of the header Clear Context button and the per-agent row button is shown in no frame.

[UX-REVIEWED] 2907bc9