fix(dashboard): settle orphaned stop tasks

[Premshay]

Problem / Motivation

A stopped dashboard turn can retain an orphaned task after the normal task reference has been cleared. The next stop action then has no task to cancel, leaving the turn lifecycle incomplete.

Why it matters

Users can see a turn remain stuck after requesting stop, and follow-on turn cleanup may not run against the orphaned task.

What changed (motivation → approach → change)

Stop and interrupt capture the slot runner before asking the provider to stop. On terminal outcomes they settle that captured task, never a successor that took ownership while the stop was in flight. An idle interrupt then dispatches its preserved queue explicitly, because no provider turn remains to run the normal queue-drain finally path. The force/second-press path follows the same captured-runner settlement and every idle terminal outcome releases stop state and refreshes the slot view.

Tests

• Added stop-lifecycle coverage for an idle slot with an orphaned task.
• python -m pytest -q test/test_dashboard_chat.py -k 'stop or task' — 52 passed.

Manual verification

N/A — the task ownership and cancellation transitions are covered at the dashboard lifecycle boundary.

Related Issues

N/A

Pattern harvest

Rule candidate: review-prompt
Pattern: cleanup paths must reconcile durable task ownership when the fast-path reference is absent.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

N/A — no CLA wording has been supplied by the project.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of dece26522856e6111711756a355836febfe6d316 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

Cancelling on "hard" races a runner that settles naturally, injecting CancelledError into the very finally-block cleanup this PR exists to guarantee.

Watch

• On "hard", reset(key) kills the provider, so the runner's await raises and it unwinds its finally on its own — which contains unshielded awaits (Slack mirror calls, _consume_pending_reset, the queue drain at chat_runner.py:12583; the except Exception guards there do not catch CancelledError). The added await _cancel_terminal_stop_task(task_at_stop) on outcome in {"hard", "idle"} can therefore land mid-finally and skip _requeue_unconsumed_steers, the queue drain, and _finish_queue_cycle — stranding the queue this path calls preserve_queue=True for, the same harm the PR fixes for idle. All new tests cover only "idle".
  Clears when: the cancel is gated to "idle" (or becomes settle-then-cancel: shield-wait first, cancel only on timeout), or a test pins that a hard-outcome runner mid-finally still completes its queue drain.
• Description ↔ diff: "An idle interrupt then dispatches its preserved queue explicitly" has no backing code — the mechanism is the opposite (the cancelled runner's finally drains it, and handler_dispatch.assert_not_awaited() pins that the handler does not dispatch).
  Clears when: the description states the actual mechanism (cancellation drives the runner's queue-drain finally).

Suggestions

• The session.md hunk documents the cancel but not the queue-handoff consequence; since the drain-by-cancellation is now load-bearing for interrupt, state it there in the same commit.

[DESIGN-REVIEWED] dece265

[github-actions]

First Principles Review (Fable 5, fork) — ✅ PASS

Premise-level review of dece26522856e6111711756a355836febfe6d316 via the fork AI-review pipeline — 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 lenses run: the defect has provenance (the added test_stop_turn_idle_cancels_orphaned_task fails on base, where an idle outcome never touches slot.task), the cancel-and-shield pattern already exists at chat_handlers.py:4891 and spec_builder runtime.py:1506 (2 sites) so the new helper is the third instance of a real dashboard idiom rather than a duplicate mechanism, the dashboard siblings that hold a runner task (slot close, spec_builder halt, openai_compat ×2) already settle it themselves, messaging-channel stop_turn callers hold no runner task, and the helper has 3 counted call sites. The one gap: the description's interrupt sentence claims an explicit queue dispatch the diff never ships — the new test pins the opposite (handler_dispatch.assert_not_awaited(); the cancelled runner's finally does the handoff).

First-Principles-Verdict: PASS

Verify one thing: the description's "dispatches its preserved queue explicitly" never shipped — the cancelled runner's finally does the handoff, and the new test pins exactly that.

The description sentence "An idle interrupt then dispatches its preserved queue explicitly, because no provider turn remains to run the normal queue-drain finally path" is contradicted by the shipped mechanism: cancelling the captured runner is precisely what runs that finally path, and test_interrupt_idle_runner_finally_starts_only_one_queued_turn asserts handler_dispatch.assert_not_awaited(). Confirm the runner-finally handoff is the intended design and correct the sentence; nothing in the code needs to change for it.

What this change ships

Intent: make the dashboard Stop/Interrupt buttons actually end a turn whose runner keeps running after the provider reports no active turn — a FIX.

Inventory (7 items)

1. Stop on an idle-reporting provider now cancels the still-running turn — justified
2. A stop settles only the runner captured at press time, never a successor turn — justified
3. Hard and force/second-press outcomes settle the captured runner the same way — justified
4. Idle interrupt cancels the runner, whose teardown dispatches the next queued prompt — justified
5. Idle outcome always releases stop state and refreshes the slot view, card or not — justified
6. A runner unsettled after two seconds logs a warning instead of blocking the request — justified
7. session.md documents the captured-runner settlement — justified

[FIRST-PRINCIPLES-REVIEWED] dece265

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

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

Review details

FINDING -- src/kiro_crew/dashboard/chat_handlers.py:4222 -- cancelling an idle _stage_loop skips its queue handoff, so Interrupt returns success while the queued message remains stranded -> Fix: dispatch the queue after cancellation only when no successor owns the slot.
[GPT-REVIEWED] dece265

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

No findings.

[OPUS-REVIEWED] dece265

[bolichen97]

@Premshay Thanks for this one. A repo-wide audit of the open PRs looked at it, and the relationship side is clean: nothing on main implements this fix and no other open PR duplicates it, so the whole scope is still yours to land. _cancel_terminal_stop_task and task_at_stop do not exist on origin/main, and the merged commits that touched your three files since the merge base are all unrelated (#8982, #9076, #9151, #9084, #7096, #9123, #9113). Your own #9115 edits the same files with zero line overlap.

What holds it up is the diff itself, and there are three things to settle:

1. The blocking review finding looks correct. Cancelling the captured _stage_loop task on an idle interrupt trips the not _cancelled gate at chat_orchestrator.py:1146, so _start_next_queued_turn just below is skipped and a promoted successor turn is stranded. The interrupt path needs to dispatch the queued turn after it resets stop state.

2. Two changes ride along undeclared in src/kiro_crew/dashboard/chat_handlers.py: the force/second-press path now also settles the captured runner (line 3638), and _stop_state = "idle" plus push_slots_update() now run even when no stop card is open, because the and slot._stop_event_id guard was split in both handlers. Please declare them in the body or revert them.

3. The body says the fallback fires when the primary task reference is absent, but _cancel_terminal_stop_task returns early when task_at_stop is None. The real trigger is a terminal outcome with a live task reference. The docs/system-specs/modules/session.md wording is already right.

The tests read well, including the successor-not-cancelled guard. Could you also rebase? The branch is 107 commits behind main.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.