feat(session-control): session_send — deliver a message to another session as its next turn - #5650
Conversation
Design Review (Fable 5) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Description claims the background-turn cap bounds fan-out; the code deliberately bypasses it — so agent-to-agent turn triggering is actually unbounded. Watch
Suggestions
[DESIGN-REVIEWED] 7ebb31b |
GPT 5.6 Review — ✅ human override acceptedHuman judgment by @iamwhatever overrides the GPT 5.6 finding for This comment is updated in place on each push. The model was not re-run because an authorized human decision supersedes it. False positive or not applicable? A repository writer can comment: |
First Principles Review (Fable 5) — 🟡 CONCERNSPremise-level review of First-Principles-Verdict: CONCERNS The description sells a fan-out cap ("charged against the background-turn cap") that the diff explicitly and deliberately does not ship. What this change shipsIntent: let one session deliver a message that another session runs as its next turn — an ADDITION (the write verb the session-control set lacked, which its own spec scoped "to its own change").
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] 7ebb31b |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsThe candidate describes the queued-send drain-gate window: a target that gains a channel mirror between
I re-derived the authorization chain ( No findings. [OPUS-REVIEWED] 7ebb31b Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
9baebec to
4c8c9f9
Compare
4c8c9f9 to
7316ef5
Compare
|
Local pre-push review round (Opus lane mirror, model claude-opus-4.8) on d19ed5f76. New head:
|
7316ef5 to
cd445b4
Compare
cd445b4 to
06b4403
Compare
|
Disposition for the GPT 5.6 blocking finding on 7316ef5. New head:
Also on this head, from the local Opus mirror in the previous round: |
06b4403 to
c82b179
Compare
|
Disposition for the GPT 5.6 blocking finding on
|
Human judgment recorded@iamwhatever marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
…ssion as its next turn
c82b179 to
7ebb31b
Compare
|
Design Review CONCERNS on
|
|
First Principles CONCERNS on
|
|
/ai-review override gpt 7ebb31b: same ruling as on c82b179 — the queued-send authorization window is not created by this diff (a human-typed message into a busy session drains through the same ungated path), so it is fixed at the generic drain in #5911 rather than by deleting this PR's queue capability with 409-on-busy; the spec now documents the accepted window explicitly. |
Human judgment recorded@iamwhatever marked the gpt AI finding as false positive, not applicable, or explicitly accepted for
This decision applies only to this commit. A new push requires a new judgment. |
Open PR relationship auditThis 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
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
What is the problem?
The session-control set shipped in #2435 can open a session (
session_create), stop one (session_stop), and read one (session_read_message) — but it has no write verb. A session created by an agent starts empty and stays empty until the person types into it, a peer session that asked a question cannot be answered, and a session working from a stale premise cannot be corrected without stopping it outright. The set can stand a workstream up and watch it, but not communicate with it.Why this issue matters to the user
The concrete workflow this blocks is a coordinator session that decomposes a goal, opens one session per work item, and drives them to completion: without a send verb, every seed prompt and every mid-flight correction is a manual copy-paste the user must perform per item per round. It also leaves
session_stopas the only intervention — destructive where a one-line steer would have done.How our fix solves it
One new verb, built on the guard and delivery paths that already exist:
send_to_target(dashboard/session_control.py) authorizes through the same deny-by-defaultauthorize_targetgate the other verbs share (same SEL-prewarm-then-config-warm ordering asstop_target, for the same no-suspension-before-the-gate reason), then delivers viaenqueue_or_run_prompt— the composer's own queue-vs-run decision. An idle target starts a turn immediately; a busy one queues. The result reports which happened (started), because "ran" and "will run later" are different answers to a caller coordinating several sessions. The turn is deliberately NOT wrapped inrun_background_turn: that cap only binds unattended (app-owned) slots, andauthorize_targetrefuses every_apptarget, so no target this verb can reach is ever capped — a wrapper would add only a never-taken timeout arm. Issue Radar's pattern is live there because its slots ARE app-owned; copying it here without that predicate produced dead code, which First Principles review caught and this head removes.[sent by session <caller> via session_send]— the target renders it as a user row, and without the tag it is indistinguishable from something the person typed (same convention as auto-nudge's[auto-nudge cycle N]).POST /api/session-control/sendregistered in_STRICT_INTERNAL_API_PATHS(the router-derived strictness test covers it) with the same_require_internalre-assert as its siblings.session_sendadded toSESSION_CONTROL_TOOLS(strict caller-identity resolution applies), schema invalidation.py(target≤500,message≤MAX_LONG_STRING), definition + dispatch inmcp_dashboard.py. Falls in thesession_*class the registration ratchet already admits.Channel-agent containment.
session_sendis added toCHANNEL_AGENT_BLOCKED_TOOLS(src/kiro_crew/channel.py) alongside its three sibling verbs. Without it, achannel.trusted(or YOLO) channel agent driven by external content could callsession_send—_blocked_tool_namedwould not match the name, the next branch auto-approves, and the message would run as a turn inside a private dashboard session. Send is the sharpest of the four verbs: stop only cancels and read only exfiltrates, but send delivers text the target session EXECUTES. The repo's own ratchet (test_channel_blocked_tools.py::test_every_session_control_tool_is_contained, which assertsSESSION_CONTROL_TOOLSis a subset of the block list) fails without this line — verified by stashing the fix:AssertionError: session-control tools reachable from a channel agent: ['session_send'].Outbound redaction. The body passes through
sanitize_outboundbefore it is delivered, on the same grounds the steer path does (chat_deliverysanitizes right beforeslot.append: "raw content must never reach an external surface"). It arrives from another session's model and is persisted into — and broadcast from — the target's transcript. The length gate deliberately runs FIRST, on the raw body: redaction can only shrink the text, so validating the raw form is the honest limit. The provenance tag is built here rather than supplied by the caller, so it sits outside the sanitized span and cannot be forged or redacted away.docs/system-specs/modules/session-control.mdand the docstring namesession_sendas the one delivering verb, register the[sent by session …]envelope beside the redaction and the channel-agent block, and replace the "No message delivery" non-goal with the accurate one. They also state explicitly that delivery has two authorization moments and only the first is enforced — the queued arm's window is accepted here and fixed at the generic drain in Queued prompts drain without re-validating the authorization that admitted them #5911, because a human-typed queued message shares it.MAX_SEND_MESSAGE_CHARSis now an alias ofvalidation.MAX_LONG_STRINGrather than a second spelling of 50k.What tests we did
Route-level coverage. The Coverage Gate failed at 76.5% on
src/kiro_crew/dashboard/handlers/session_control.py(floor 80%, not baselined) because the newapi_session_control_sendroute had no test of its own -- and the siblingstoproute only had its 403 arm covered. Six route tests were added rather than baselining the file, which is what the gate's own message asks for ("add tests, do not extend the baseline"): send's forbidden-without-secret arm, its delivery path, itsmessage_requiredvalidation (that check lives in the route, not the business layer, so a whitespace-only body must be refused there), its refusal-not-500 mapping, plus stop's reaching path and refusal mapping. The file now measures 89%.Four new tests in
test_session_control.py: idle target starts a turn and the delivered prompt carries the provenance prefix; busy target queues (started: false, message in_queue); out-of-bounds target refused by the shared guard; empty and oversized messages refused with their codes. Ratchet pins updated intest_mcp_dashboard_registration.pyandtest_mcp_dashboard_folders.py. Full targeted sweep green after rebase onto current main: 229 passed across the four session-control/dashboard suites; isort and flake8 clean.Manual verification
Grant an agent
@kirocrew-dashboard, enableagent.session_control, then from that session:session_createa peer,session_senda seed prompt into it (verify the tagged user row and the started turn in the peer's transcript), send again while it is mid-turn (verify queueing), and confirm an incognito target is refused.