fix(slack): re-hydrate thread agent overrides after an inline reroute

[LuisBrel]

Problem / Motivation

A Slack turn in a thread that is linked to a dashboard session runs under the generic default agent instead of the agent that session is bound to. The binding is persisted correctly in the session's own conversation-log metadata; the turn just never reads it, so the user's chosen agent is silently ignored and they get the default persona's answer.

Why it matters

An agent binding is the whole point of binding a session to an agent. Silently substituting the default persona is worse than an error: the reply looks plausible, so the user has no signal that their configured agent — its tools, its instructions, its model — was never used. Anyone driving a bound dashboard session from Slack is hit on the first reroute in a gateway process.

What changed (motivation → approach → change)

Symptom — a rerouted Slack turn resolves to the default agent.

Root cause — the per-thread override maps are keyed BY SESSION. handle_message hydrates them once at entry, under the pre-link session key derived from the Slack thread timestamp. When the thread turns out to be owned by a linked dashboard session, the reroute reassigns session_key = thread_owner_key, and the agent re-resolution on the very next lines reads _thread_agents.get(session_key) under a key that was never hydrated. It misses, and falls through to channel_agent or _get_default_agent().

Change — re-hydrate for the new owner in the same breath as the reroute, which is exactly what the sibling transport path already does: transport_dispatch._resolve_thread_owner calls _hydrate_thread_overrides(session_key, conversation_log) immediately after its own session_key = owner, with a comment explaining why it belongs inside the reroute rather than beside it. Rather than invent a second mechanism, this mirrors that one call. _hydrate_thread_overrides guards repeated I/O per session, so it costs nothing when the key is already hydrated.

Tests

test/test_slack_handler.py::TestPerThreadAgent::test_reroute_to_a_linked_session_keeps_its_agent — drives the real handle_message with a thread index that resolves the thread to a dashboard session key, and a conversation log holding that session's persisted agent. It asserts the observable outcome, the agent the turn is acquired with, not the internals.

Written first and confirmed failing for the right reason on the unfixed code — the reroute assertion passed, so the test was exercising the intended branch, while the agent came back None:

E   AssertionError: rerouted turn ran under None — the linked session's persisted agent binding was discarded
E   assert None == 'sisyphus'
1 failed, 184 deselected, 1 warning in 3.22s

After the fix, 185 passed for that file, and 742 passed across the 14 files covering this path (test_slack_handler.py, test_slack_thread_session_binding.py, test_slack_transport_dispatch.py, the three handler-coverage files, test_thread_override_persistence.py, test_slack_agent_passthrough.py, test_slack_dashboard_live_sync.py, test_close_guard_parity.py, test_slack_options_lifecycle.py, test_slack_mirror_unlink.py, test_chat_slack.py, test_slack_golden_transcript.py).

scripts/check_black_formatting.py, isort --check-only, flake8 and mypy all pass on the two changed files.

Manual verification

N/A — unit coverage sufficient. The test drives the real handle_message through the real reroute branch, so the assertion sits on the same code path a live Slack reply takes; the only doubles are the session manager and the conversation log, which is how every existing test in this file exercises the handler.

Why no screenshot: this is one inserted call inside the Slack inbound handler plus a regression test. There is no UI surface in the diff — the only observable difference is which agent a rerouted turn runs under, which is precisely what the test asserts.

Related Issues

Refs #2795 — deliberately not Closes.

That issue bundles two defects and this PR addresses only the second: the agent binding lost on an inline-handler reroute. Left out on purpose:

• Defect A, the additive effective-session context block. It is an undecided design question — what the block asserts, whether it re-injects on every follow-up turn, and how it composes with the existing per-turn transport line. Three commenters have recommended splitting the issue; that split is a maintainer's call, so this PR neither pre-empts it nor closes the issue.
• The parallel privacy-flag miss at the same reroute: _hydrate_conv_flags also runs only under the entry key. The transport sibling re-runs both, but the inline path has its own flag-hydration ordering, and widening into it here would fold a second concern into a one-line fix.

Pattern harvest

Rule candidate: review-prompt
Pattern: a cache hydrated once at entry, then read after the key it is indexed by has been reassigned — the lookup misses silently and falls through to a default instead of failing.

Checklist

• At most two commits (one is the norm), 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, no doc describes this path
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

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

The fix and its claims verify cleanly against the base: entry hydration at handler.py:2668, the linked-thread reroute at ~3124 reading _thread_agents.get(session_key) under a never-hydrated key, and the transport sibling (transport_dispatch.py:197-206) already re-hydrating inside its reroute helper. One thing the PR doesn't name: the route_pinned branch immediately above performs the other inline session_key reassignment (session_key = asker_key) feeding the very same unhydrated agent lookup, and it gets no re-hydration.

Design-Verdict: CONCERNS

Sound minimal fix mirroring the transport sibling, but it patches one of two session_key reassignments feeding the same unhydrated lookup.

Watch

• The route_pinned branch a few lines up (if asker_key: session_key = asker_key) reroutes inline too, and the same _thread_agents.get(session_key) then reads an unhydrated asker key — a pinned answer from a cron/dashboard asker can hit the identical silent default-agent fallback. Either hydrate there too or state why that path can't miss.
• The deferred _hydrate_conv_flags miss at this same reroute persists an incognito/temporary linked session's turn to disk — a privacy-boundary harm. Deferring is fine, but confirm it's tracked as its own issue rather than living only in this PR's description.

Suggestions

• Transport solved this structurally ("keeping this inside the helper makes it structurally impossible to reroute without it"); folding reassignment + hydration into one small inline helper would close both sites and prevent recurrence.

[DESIGN-REVIEWED] c913797

[github-actions]

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

Premise-level review of c913797e830fd797e90a47e6cc0d384f5ff8b757 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 claims verified against the base. The fix site, the transport sibling it mirrors, and the entry hydration all read as described. I found one undeclared sibling the author didn't mention. Final review:

First-Principles-Verdict: CONCERNS

One reassign-then-read site is fixed; the pinned-asker reassignment two lines above has the identical shape and is neither fixed nor declared.

What this change ships

Intent: make a Slack turn rerouted into a linked dashboard session run under that session's bound agent instead of the default — a FIX.

1. A rerouted thread turn now uses the linked session's persisted agent — justified (reported defect, refs A Slack turn routed to a linked dashboard session loses the model's own accurate self-description of that session #2795).
2. A regression test pinning that outcome — rides with the fix, justified.

No new surface: no config key, flag, or export. The fix reuses the existing _hydrate_thread_overrides (handler.py:887) rather than minting a second spelling, matching transport_dispatch._resolve_thread_owner (transport_dispatch.py:205).

Watch

• Counted siblings: this function reassigns session_key after entry hydration at 2 sites (grep session_key = in handle_message), and both flow into _thread_agents.get(session_key) at handler.py:3133. The fix covers handler.py:3126 (thread_owner_key); handler.py:3119 (session_key = asker_key, the pinned path) still reads an unhydrated key and falls through to the channel/default agent — same defect, undeclared. Verify whether a pinned cron/native asker can carry a persisted binding; if so this is a point patch.
• The declared deferral of _hydrate_conv_flags at the same reroute leaves the privacy-flag map unpopulated for the new owner — the exact harm transport_dispatch.py:198–206 names (an incognito session's turn persisted to disk), which sits on a named security boundary. The transport sibling treats the two hydrations as inseparable; the stated reason for splitting them here ("its own flag-hydration ordering") is worth a human's confirmation.

[FIRST-PRINCIPLES-REVIEWED] c913797

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

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

Review details

No findings that block.

FINDING — src/kiro_crew/slack/handler.py:3126 — the reroute rehydrates only _hydrate_thread_overrides(session_key, conversation_log) for the new owner key, never _hydrate_conv_flags(sessions, session_key); when the process-local privacy LRUs are empty (gateway restart / LRU eviction) is_temporary/is_incognito/_is_slack_restricted are pure membership checks (privacy_mode.py:167,172,182) that return the permissive answer for the un-hydrated owner key, so a rerouted incognito/temporary dashboard session's turn is persisted to disk and temporary-mode memory reads are no longer blocked (consumers at 3228, 3801, 3938, 4055). The reroute reassignment pre-dates this PR — the diff neighbours the defect rather than causing it — so advisory, not blocking. → Fix: add _hydrate_conv_flags(sessions, session_key) immediately after the new _hydrate_thread_overrides call, matching entry (2668-2669) and transport_dispatch._resolve_thread_owner (205-206).

[OPUS-REVIEWED] c913797

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

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

Review details

No findings.
[GPT-REVIEWED] c913797