fix(chat): stop the session a linked slot's turn actually runs on

[leonlaiyc]

Problem / Motivation

Pressing Stop on a channel-linked chat slot (a Slack/Discord-linked session)
reports success and cancels nothing.

A channel-linked slot's turns run under its linked_session_key (slack:<ts>),
which is what chat_utils.effective_session_key resolves. All three stop paths in
chat_handlers.py instead addressed the session as _history_key_for(slot) —
dashboard:<slot> — a key no running session owns:

handler	path	key passed to stop_turn
api_chat_slot_stop	cooperative stop (first press)	_history_key_for(name)
api_chat_slot_stop	hard-kill escalation (second press)	_history_key_for(name)
api_chat_slot_interrupt	interrupt + run next queued	_history_key_for(name)

stop_turn therefore had nothing to cancel, while the handler still inserted the
stop_event card and returned {"ok": true}.

Why it matters

Stop is the control a user reaches for when something is going wrong, and this is
a silent failure: the UI shows a clean stop, so the user stops watching. The
blast radius is whatever the turn does next — file writes, pushes, tool calls. A
stop that errors is recoverable; one that lies is not.

The escalation path is affected too, which removes the natural fallback: a user
whose first press did nothing presses again, and the hard kill mis-addresses the
same way.

What changed (motivation → approach → change)

Root cause. The slot's TRANSCRIPT key was used where its SESSION key was
required. Those coincide for an ordinary dashboard slot and diverge for a linked
one, so the bug is invisible until a channel link exists.

Change. The three stop_turn calls now resolve the session through
effective_session_key(slot). This is not a new convention — it is the one
already written down:

• effective_session_key's own docstring: "Use this anywhere a slot's SESSION is
  addressed — resolving the session its turns run on, mirroring its links. For the
  slot's TRANSCRIPT use slot_history_key."
• api_chat_slot_continue, in this same module, already resolves its session with
  effective_session_key(slot).

The SEL records deliberately keep the slot-derived key. They answer which tab
the operator pressed, not which session was cancelled, and
api_chat_slot_continue already splits the two exactly this way
(effective_session_key for the session, _history_key_for(name) for the audit).
TestAuditKeepsTheSlotIdentity pins that split so a later cleanup does not
"tidy" the audit key along with the session key.

Unlinked slots are unaffected by construction: with no linked_session_key,
effective_session_key falls back to _history_key_for(slot.key) — byte for byte
the previous value.

Authorization: two conditions, because ownership is not enough

/stop and /interrupt never applied the App Kit §5.2 ownership check that
api_chat and api_chat_slot_continue apply. That gap is on main today, not
introduced here — the ownership tests below fail against pristine origin/main
as well. What resolving the real session key changes is exploitability: while
the handlers addressed dashboard:<slot>, a cross-tenant cancel hit a key no
session owned and did nothing. Reverting the key resolution would reopen #2462
and leave the gap in place, so the guard ships here.

Ownership alone is insufficient, and this is the substantive finding. An app
caller must satisfy both:

1. the app owns the slot, and
2. the effective session is still the slot's own dashboard session —
   effective_session_key(slot) == _history_key_for(slot.key).

Condition 2 is load-bearing because an app can legitimately come to own a
channel-linked slot. POST /api/chat passes a caller-supplied slot name
straight into get_or_create_slot(slot_name, app=request.get("app", "")), and
that function sets _app and then, for a name shaped like a channel session
stem, resolves the binding itself:

if is_channel_session_key(name):
    resolved = self.sessions.channel_key_for_stem(name)
    if isinstance(resolved, str) and is_channel_session_key(resolved):
        slot.linked_session_key = resolved

So an app that names a live Slack thread owns a slot bound to a conversation it
has no claim on, and an ownership-only guard authorizes cancelling that
channel's turn — a slot binding becomes capability escalation. A dashboard
caller has no app scope and may cancel either kind.

An earlier revision of this PR checked ownership only. The three
test_an_owned_*_channel_linked_slot_* cases fail against it.

Indistinguishable 404, actually indistinguishable. Refusals and genuinely
missing slots both return _slot_not_found() —
{"error": "not found", "code": "slot_not_found"}, matching
api_chat_slot_continue. Previously a refusal carried code and a missing slot
did not, so an app could tell "not mine" from "not there" and enumerate foreign
slot names. test_a_missing_slot_answers_exactly_like_a_refusal asserts the two
bodies are equal, and every denial test compares the whole body rather than the
status.

The guard runs before every side effect. The escalation branch clears the
queue and drops pending steers, and both handlers write a stop card and claim
_stop_state, all before stop_turn.

The running turn owns a stable session identity

Resolving the session at cancel time is not safe, because the field it resolves
from changes underneath a live turn:

• linked_session_key says where the slot routes a new turn, and it is
  mutable on a running one — inject_cron_result_to_dashboard
  (dashboard/cron_inject.py:29) binds an already-live slot to cron:<id> and
  does not gate on slot.running. api_cron_to_chat does the same on a click.
• _run_chat captures its key once, at the boundary below every
  local-command return, and uses that one key to acquire, audit and release for
  the whole turn.

So after a mid-turn rebind the two disagree, and a cancel that re-derives the key
stops a session this turn never ran on while the real turn keeps executing. On
origin/main the cancel used _history_key_for(name), which in that scenario
happens to be correct — so an earlier revision of this PR, by switching to
effective_session_key, regressed it. Fixing that is part of making this PR
correct, not a separate concern.

_ChatSlot._active_turn_session_key publishes the running turn's own identity:

• runtime only — never persisted, never serialized, empty after a restart;
• _run_chat is its sole lifecycle owner, installing it at the local-command
  boundary and retiring it in the same finally that releases the session;
• deliberately a plain field, not a property — it has exactly one writer.

Two placement details are load-bearing and each has a test:

• Not keyed on _prompt_depth == 0. /prompts get re-enters _run_chat at
  _prompt_depth=1, and the depth-0 invocation is a local wrapper that returns
  before the turn machinery. Keying on depth would put the identity on the
  wrapper; keying on the boundary puts it on the invocation that actually runs
  the turn.
• Retired inside the release finally, not at the end of teardown. The
  reset there can be cancelled and CancelledError derives from
  BaseException, so a clear placed after that block is skipped and the slot
  keeps advertising a turn that is gone. It also has to land before
  _start_next_queued_turn, which runs later in the same teardown and installs
  the successor's key — compare-and-clear so only the turn that installed a key
  may retire it.

Each cancel route now derives one target and hands the same value to both
authorization and stop_turn:

cancel_key = _cancel_target(slot)   # the running turn's key, else the routing
denied = _app_cancel_denied(request, slot, "chat_stop", cancel_key)

Authorization tests cancel_key rather than re-reading the slot. That is not
only a TOCTOU guard: for a turn that started on the app's own session and was
rebound mid-flight, re-reading would deny the app its own running turn,
because the routing now points somewhere it does not own.

Disposition of the automated review findings

Both came from GPT 5.6 on the fork pipeline and both were verified against the
production mutation path before being acted on.

• "Authorized session key is re-resolved after an await" — correct; the
  request-body await in /interrupt is a real window.
• "Escalation can kill a newly linked, unrelated session" — correct, and
  broader than stated. Its suggested fix, retaining the first soft stop's key
  for the escalation, is insufficient: a rebind can land before the first
  Stop, so that leaves a single press wrong, and it cannot help /interrupt
  either. The turn-owned identity fixes soft stop, hard escalation and interrupt
  uniformly, and needs no state carried between two requests.

The send/continue capability escalation that the ownership guard below refers to
is fixed at its own root in #2783 rather than by widening this PR.

Tests

New file test/test_stop_addresses_linked_session.py, asserting the key the
handler hands to stop_turn, because that is the entire defect — the card and
state bookkeeping around it were already correct.

26 tests in test/test_stop_addresses_linked_session.py, in five groups, plus 9 lifecycle tests in a new file. The
failure classes are deliberately distinguished, because they have different
provenance:

Class 1 — the #2462 session-key defect (5; fail on origin/main, pass from
this PR's first head onward):

• test_soft_stop_on_a_linked_slot_cancels_the_channel_session
• test_hard_kill_on_a_linked_slot_cancels_the_channel_session
• test_interrupt_on_a_linked_slot_cancels_the_channel_session
• test_dashboard_user_is_not_treated_as_an_app
• test_sel_record_is_keyed_on_the_slot_not_the_link — also pins the
  audit/session split going forward.

Class 2 — the authorization gap (fail on origin/main too, which is the
evidence that it pre-dates this PR):

• test_app_token_cannot_stop_a_dashboard_owned_linked_slot
• test_app_a_cannot_stop_app_bs_slot
• test_the_hard_kill_path_cannot_bypass_the_guard
• test_app_token_cannot_interrupt_a_foreign_slot

Class 3 — linked-session escalation and the existence oracle (fail against
the ownership-only revision of this PR):

• test_an_owned_but_channel_linked_slot_is_still_refused
• test_an_owned_channel_linked_slot_is_refused_on_interrupt_too
• test_an_owned_channel_linked_slot_is_refused_on_hard_kill_too
• test_a_missing_slot_answers_exactly_like_a_refusal

Each asserts the 404 body, that stop_turn was never awaited, and that no stop
card, _stop_state change, queue clear or steer drop occurred.

Class 4 — the running turn owns the target (TestTheRunningTurnOwnsTheTarget,
7; fail against the previous head f0704dc6d). Each starts a turn on one
session and rebinds the slot underneath it:

• test_soft_stop_after_a_mid_turn_rebind_stops_the_running_turn
• test_the_hard_escalation_follows_the_same_turn
• test_interrupt_after_a_mid_turn_rebind_stops_the_running_turn
• test_a_turn_that_started_on_the_link_is_still_stopped_there — the Stop on a channel-linked session addresses the wrong session key and cancels nothing #2462 fix
  restated on the stronger rule.
• test_an_app_may_still_stop_its_own_turn_after_a_rebind — mutable routing
  must not lock an app out of a turn it legitimately started.
• test_an_app_is_still_refused_a_turn_running_on_a_foreign_session — and the
  security boundary is unchanged.
• test_an_idle_slot_falls_back_to_its_routing

Lifecycle — new file test/test_active_turn_session_key.py (9), driving the
real _run_chat through the harness test_turn_teardown_release.py uses:
identity published for a plain turn, for a linked turn, and unmoved by a
mid-turn rebind; retired after normal completion, after a provider error, after
a cancellation delivered into the teardown, and when the session was never
acquired; the clear observed to land before _start_next_queued_turn and to
leave a successor's identity intact; and the /prompts get re-entry proving the
depth-1 invocation owns the key while its depth-0 wrapper owns nothing.

Preservation guards (pass throughout) — an unlinked slot's stop, hard kill
and interrupt still address dashboard:test-slot; a dashboard user is not
treated as an app; and the owning app may still cancel its own unlinked slot,
so the guard costs apps nothing they legitimately had.

Manual verification

• test_stop_addresses_linked_session.py, test_stop_handler_idempotent.py,
  test_chat_slot_interrupt.py, test_chat_slot_end_wait.py,
  test_dashboard_chat.py, test_dashboard_chat_pins.py,
  test_dashboard_chat_rewind.py, test_error_code_contract.py,
  test_active_turn_session_key.py, test_turn_teardown_release.py →
  734 passed, 1 skipped, 0 failed (Windows 11 26200 / py3.10.6).
• Fail-before for the turn-identity work: 13 failed, 22 passed against
  f0704dc6d; 35 passed after.
• pytest test/ -k "chat or slot or stop or interrupt or turn or cron or session or app"
  → 11012 passed, 212 skipped, 4 xfailed, 21 failed. All 21 are pre-existing
  on this machine and unrelated to the diff — symlink-creation tests hitting
  WinError 1314 without elevation, two CloudFormation-template assertions, two
  app-backend env-allowlist tests and a cookie-expiry parse test — and every one
  reproduces with the three changed source files reverted to the previous head.
• test_stop_kill_cancel.py fails 8 on the same machine
  (asyncio.start_unix_server, POSIX-only os attributes), likewise
  pre-existing and platform-only.
• Four fixtures in test_stop_handler_idempotent.py gained an explicit absent
  app header. A bare MagicMock answers .get("app") with a truthy mock, which
  the guard correctly reads as an app token; those cases are dashboard-user
  presses, so the fixture was modelling the request wrong. No assertion weakened.
• flake8, isort, mypy src/kiro_crew/dashboard/chat_handlers.py,
  git diff --check and BRAND_BASE_REF=origin/main scripts/check_brand_name.py
  all clean.
• No spec change: docs/system-specs/modules/session.md documents stop_turn(key, ...) as the shared orchestration layer without prescribing which key a caller
  passes, and this restores the contract effective_session_key's docstring
  already states.

error-code-baseline.json regenerated on the current base. This branch was
rebased onto 24a6f8ee5 after #2300 landed; the baseline was regenerated there
rather than carried across from the old base, so these numbers are against
current main:

field	main	this PR
_totals.missing_code	1409	1407
dashboard/chat_handlers.py missing_code	76	74
_compliant	706	721

The missing_code drop is this PR: the two 404s in these handlers now carry a
code, and test_baseline_is_not_stale requires an improved count to be
recorded. Regenerated with the documented python test/test_error_code_contract.py --update rather than hand-edited.

_compliant needs a word, because +15 looks larger than a two-response change.
Regenerating on a pristine main already yields 720, so 14 of that delta is
pre-existing drift in the committed file and only +1 is attributable to this
PR. _compliant is also the one field the contract tests do not assert — the
ratchet is missing_code per file, _totals matching the per-file map, and the
staleness check.

Scope

The three stop_turn call sites plus the ownership guard the cancel routes were
missing. Not touched: the SEL session_key
arguments (see above), the Slack-side stop paths in slack/interactions.py (they
already receive a real session key), and spec_builder's own stop, which is a
different surface with its own slot model.

Note for scheduling: #2435 is open against the same file. It is not an overlap
— that PR refuses channel-linked targets outright rather than reaching this path,
and its description says this was left deliberately for its own change and its own
verification. Whichever lands second needs a trivial rebase.

Related Issues

Fixes #2462

Checklist

• Single commit with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — N/A, see Manual verification
• No secrets, credentials, or internal references in the diff

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

The candidate concerns the SEL audit session_key changing from effective_session_key(slot) to _history_key_for(name) for the stop/interrupt log records.

Falsifying it:

• (a) is real: an operator pressing Stop on a channel-linked slot.
• (b) is real: _history_key_for(name) yields the dashboard-prefixed slot key rather than the channel session key.
• (c) is speculative: the only claimed harm is "forensic correlation by session_key no longer surfaces the stop event," which requires assuming a downstream SEL consumer that correlates stop events by session_key — something the candidate itself admits it could not confirm. That is an "if a caller were to" outcome.

Furthermore this is a deliberate, coherent design split: the functional cancel (stop_turn(cancel_key)) addresses the real running session, and the SEL record intentionally carries the tab identity (the slot the operator pressed), matching api_chat_slot_continue. The session_key for a plain slot is unchanged; only linked slots differ, by design. It is not a crash, data loss, corruption, removed guard, or security hole — it is an audit-metadata design choice, and even the advisory harm is unverifiable. It does not survive at 80+.

No self-originated finding grounds to the bar either — the core routing (_cancel_target), the App Kit §5.2 guard testing the resolved target_key, and the runtime-only lifecycle of _active_turn_session_key in _run_chat's finally are internally consistent and heavily tested.

No findings.

[OPUS-REVIEWED] 2b39d17

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Advisory design-level review of 2b39d1740af3cb8c52a4da8a874b1d54abd5531d via the fork AI-review pipeline — updated in place on each push; does not block merge.

All key claims verified against the base tree: effective_session_key's docstring contract (chat_utils.py:564-580), the cron injection binding a live slot with no running gate (cron_inject.py:95-96), and get_or_create_slot resolving a channel binding for a channel-shaped name (state.py:4211-4214). The diff matches the description bidirectionally, the fix is at the root cause (transcript key used where session key required), the turn-owned identity handles the mutable-routing case the simpler re-derive approach gets wrong, and the ownership guard protects a named boundary (app tokens vs. channel sessions) with deny semantics and an enumeration-proof 404 that mirror the existing api_chat_slot_continue pattern. The one bundled extra — the pre-existing authorization gap — is justified in place, since the key fix is what makes it exploitable. No design-level findings survive.

Design-Verdict: PASS

Root-cause fix at the documented seam, with a turn-owned identity that correctly survives mid-turn rebinds the naive re-resolve would mis-target.

[DESIGN-REVIEWED] 2b39d17

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

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

Review details

No findings.
[GPT-REVIEWED] 2b39d17

[leonlaiyc]

Both findings taken. The second one is correct and broader than stated, so the fix is broader too — eecb4f598.

Verified before acting. inject_cron_result_to_dashboard (dashboard/cron_inject.py:29) calls get_or_create_slot(name=...), which returns an existing slot, and assigns slot.linked_session_key = f"cron:{job.id}" with no slot.running gate. api_cron_to_chat does the same on a click. So the field is mutable under a live turn, while _run_chat captured its own key once at line 3549 and uses that one to acquire, audit and release for the whole turn. After a mid-turn rebind the two disagree — and origin/main's _history_key_for(name) happens to be correct in that scenario, so an earlier revision of this PR regressed it. Fixing it belongs here.

Why not the suggested fix. Retaining the first soft stop's key for the escalation is insufficient in two ways: a rebind can land before the first Stop, leaving a single press wrong, and it does nothing for /interrupt. It also requires carrying state between two requests.

What landed instead — the running turn owns a stable identity. _run_chat now publishes the key it captured as _ChatSlot._active_turn_session_key. Runtime only: never persisted, never serialized, empty after a restart, and _run_chat is its sole lifecycle owner. Two placement details are load-bearing:

• Installed at the local-command boundary, not on _prompt_depth == 0. /prompts get re-enters _run_chat at depth 1, and the depth-0 invocation is a wrapper that returns before the turn machinery — keying on depth would put the identity on the wrapper. expire_slack_options moved inside the try so a cancellation there cannot strand an installed key.
• Retired inside the same finally that releases the session. The reset there can be cancelled and CancelledError derives from BaseException, so a clear placed after that block is skipped — I hit exactly that while writing the test. It also has to land before _start_next_queued_turn installs a successor's key, hence compare-and-clear: only the turn that installed a key may retire it.

Each cancel route derives one target and passes the same value to authorization and to stop_turn. Authorization tests that key rather than re-reading the slot, which matters beyond TOCTOU: for a turn that started on the app's own session and was rebound mid-flight, re-reading would have denied the app its own running turn.

Tests — fail-before 13 failed / 22 passed against f0704dc6d, 35 passed after.

• TestTheRunningTurnOwnsTheTarget (7) — soft stop, hard escalation and interrupt after a mid-turn rebind all address the running turn; a turn that started on its link is still stopped there; an app may still stop its own rebound turn; an app is still refused a turn running on a foreign session; an idle slot falls back to its routing.
• test/test_active_turn_session_key.py (9) — lifecycle against the real _run_chat, using the harness from test_turn_teardown_release.py: published for plain/linked turns and unmoved by a rebind; retired after normal completion, a provider error, a cancellation delivered into the teardown, and a never-acquired session; the clear observed at the _start_next_queued_turn dispatch point in both directions; and the /prompts get re-entry.

Focused suites 734 passed / 1 skipped. The wider -k "chat or slot or stop or interrupt or turn or cron or session or app" sweep: 11012 passed, 21 failed — all pre-existing Windows-only (symlink WinError 1314, CloudFormation templates, env allowlist, a cookie-expiry parse), each reproduced with the three changed files reverted.

One adjacent thing I checked and am not touching here: DashboardState.running_session_keys() derives from the same mutable routing, so it has the same conceptual discrepancy. Its only consumer is session_storage.py:99, whose own docstring says the set "is only ever used to EXPLAIN a refusal, never to grant one" — active_sids decides every refusal. No reachable harm, so it stays out of this PR.

[leonlaiyc]

The shard-4 red on eecb4f598 was mine, not environment noise. Fixed in 80018c9f5.

Exact node: test/test_slack_options_lifecycle.py::TestControlPostedAfterTheWindowIsSpent::test_a_local_dashboard_command_does_not_spend_the_control.

It is a source-inspection test — it reads inspect.getsource(_run_chat) and asserts positional ordering — which is why it failed identically on 3.10, 3.12 and Windows rather than looking like a platform problem. It pins two things: every local-command return must sit above the expire_slack_options call, and that call must sit above _acquired = False. Publishing the turn identity, I had moved the expiry inside the try to keep it inside the identity's lifetime, which put it below that marker.

Found by reproducing the CI shape rather than guessing. Local pytest -n auto --splits 4 --group 4 with the workflow's BACKEND_DESELECTS, run against 80018c9f5's parent and against the exact base 24a6f8ee5, then diffed the two failure sets. This machine produces a large constant of Windows-only failures in that shard (58 on the base, 42 on the head — symlink tests needing elevation, etc.), so the counts are not comparable to CI; the set difference is, and it was exactly one node. After the fix the difference is empty.

The fix keeps both invariants rather than trading one for the other: the expiry goes back to its pinned position, and the identity is installed immediately after it. That await is the only thing between the local-command boundary and the try, so installing below it means every exit that can occur while the identity is live still reaches the teardown that retires it — only plain assignments separate the install from the try. The chat_runner.py diff is now purely additive: no moved lines.

Nothing else changed. _active_turn_session_key keeps its lifecycle — installed at the boundary (not on _prompt_depth), retired inside the same finally that releases the session, compare-and-clear ahead of _start_next_queued_turn — and the cancel routes still derive one cancel_key for both authorization and stop_turn.

Verification: focused suites (now including test_slack_options_lifecycle.py) 827 passed / 1 skipped; test_active_turn_session_key.py + test_stop_addresses_linked_session.py 35 passed; shard 4 head-vs-base delta empty; isort re-run after the final edit, flake8 clean, git diff --check clean, mypy reports nothing in the three changed source files.

[leonlaiyc]

Rebased onto current main (2b39d1740). The rebase was not mechanical — main has since absorbed part of this PR, so the scope changed and the body needs re-reading.

What main already landed. api_chat_slot_stop now does session_key = effective_session_key(slot) itself, carrying this PR's own rationale verbatim ("_history_key_for(name) is wrong for every slot that carries a linked_session_key… stop_turn finds nothing and returns idle… a silent no-op that reports success"). So the headline defect is fixed upstream and this PR is no longer the thing that fixes it.

What is still only here.

1. _cancel_target(slot) — prefers the turn's own recorded _active_turn_session_key and falls back to effective_session_key. linked_session_key is mutable on a live slot (inject_cron_result_to_dashboard rebinds without a running gate), so re-deriving at cancel time stops whatever the slot routes next, not the turn the operator is stopping. Main's version re-derives.
2. The App Kit §5.2 cancel guard (_app_cancel_denied) on /stop and /interrupt.
3. The audit/session split — stop_turn gets cancel_key, the SEL record keeps _history_key_for(name) (the tab the operator pressed).

Two semantic collisions the test suite caught, which a clean textual rebase would have shipped silently:

• Main's new session_key was feeding the five SEL calls, so the audit record started keying on the link instead of the slot — caught by test_sel_record_is_keyed_on_the_slot_not_the_link (assert 'slack:1730000000.123456' == 'dashboard:test-slot'). Restored to _history_key_for(name).
• Main's new TestStopCancelsTheSessionTheTurnRunsOn builds a bare MagicMock request, whose .get("app") returns a truthy mock that this PR's §5.2 guard reads as an app token — so all three of its cases denied and never reached stop_turn. Fixed the same way this branch already fixes four sibling classes: request.get = lambda key, default="": default.

After that, session_key was dead in both handlers (_cancel_target subsumes it), so it is removed and main's failure narrative is folded into the _cancel_target docstring rather than dropped.

57 passed across test_stop_addresses_linked_session, test_active_turn_session_key, test_stop_handler_idempotent, test_error_code_contract; 190 passed across four adjacent chat-handler suites. Baseline regenerated on the new base (pristine main regenerates to compliant 761 while its committed file says 758, so main's baseline is 3 stale — this PR's own delta is −2 missing_code / +1 compliant). flake8 and isort clean. Still one commit.

Happy to retitle/re-body this around items 1–3 if you'd prefer it framed as a follow-up to what already landed.

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Advisory premise-level review of 2b39d1740af3cb8c52a4da8a874b1d54abd5531d 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; does not block merge.

First-Principles-Verdict: CONCERNS

The titled fix already sits on main (f9db38a, #3074); the real delta is an auth guard, a turn-identity snapshot, and an undeclared audit-key reversal.

What this change ships

Intent: make Stop on a channel-linked tab actually cancel the running turn — framed as a FIX.

1. App token gets 404 stopping/interrupting a slot it doesn't own — justified (App Kit §5.2 boundary, mirrors api_chat_slot_continue)
2. App token refused even on its own slot when linked to a foreign session — justified (binding via state.py:4211 is real)
3. Missing slot on /stop and /interrupt now returns code: slot_not_found — justified (AGENTS.md code invariant + anti-enumeration)
4. Stop after a mid-turn rebind cancels the turn's original session, not the new routing — justified, mechanism-level (cron_inject.py:96 has no running gate)
5. New runtime field _active_turn_session_key with _run_chat lifecycle — one consumer (_cancel_target), justified
6. SEL stop/interrupt rows now record dashboard:<slot> instead of the session addressed — undeclared reversal of fix(dashboard): stop the session the turn runs on, not dashboard:<slot> #3074's audit key
7. The linked-session key resolution itself — duplicate of chat_handlers.py:1438,1909 already on the trusted base
8. error-code-baseline counts updated — mechanical

Watch

• Framing is stale against the base: "All three stop paths … addressed the session as _history_key_for(slot)" and "On origin/main the cancel used _history_key_for(name)" are both false — base lines 1438 and 1909 already read session_key = effective_session_key(slot), and the diff's own removal hunks delete fix(dashboard): stop the session the turn runs on, not dashboard:<slot> #3074's comment. Item 7 ships nothing new; items 1–6 are the PR.
• Item 6 is described as "The SEL records deliberately keep the slot-derived key", but the base logs the effective session key (fix(dashboard): stop the session the turn runs on, not dashboard:<slot> #3074 changed exactly that); this PR reverts it. Its only support is symmetry with api_chat_slot_continue (chat_handlers.py:1723) — analogy, and the row loses which session was actually cancelled.
• Unfixed sibling of the root cause (transcript key handed to stop_turn): 1 — spec_builder/backend/routes.py:1832 (grep stop_turn(); benign today only because spec slots are never linked.
• Item 5's subtractive alternative goes unexamined: gating the live rebind (if not slot.running at cron_inject.py:96) would remove the divergence without a new field, at the cost of deferring the cron bind — worth a human's judgment.

Subtractions

• Shrink item 6: pass cancel_key to the five SEL calls instead of switching them to _history_key_for(name), preserving the base's recorded identity; the slot is already in metadata={"slot": name}.
• Shrink the description: drop the Problem table and the "on origin/main" claims that restate the already-merged fix(dashboard): stop the session the turn runs on, not dashboard:<slot> #3074, and state the actual delta (guard + turn identity + 404 shape).

[FIRST-PRINCIPLES-REVIEWED] 2b39d17

[iamwhatever]

Tier 1 auto-approve: small-fix (7 files, fork). Criteria: no conflict, no requested changes, no security surface, AI reviewers green. Category: stop the session a linked slot's turn runs on — ensures correct session lifecycle teardown for linked chat slots.