Skip to content

feat(session-control): session_send — deliver a message to another session as its next turn - #5650

Merged
iamwhatever merged 1 commit into
mainfrom
feat/session-send
Aug 25, 2026
Merged

feat(session-control): session_send — deliver a message to another session as its next turn#5650
iamwhatever merged 1 commit into
mainfrom
feat/session-send

Conversation

@iamwhatever

@iamwhatever iamwhatever commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

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_stop as 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-default authorize_target gate the other verbs share (same SEL-prewarm-then-config-warm ordering as stop_target, for the same no-suspension-before-the-gate reason), then delivers via enqueue_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 in run_background_turn: that cap only binds unattended (app-owned) slots, and authorize_target refuses every _app target, 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.
  • Provenance is mandatory. The delivered prompt is prefixed [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]).
  • Size gates live in the business layer too (empty / >50k chars → 400), not only in the tool schema, because the HTTP route is reachable without the MCP layer's validation.
  • Route POST /api/session-control/send registered in _STRICT_INTERNAL_API_PATHS (the router-derived strictness test covers it) with the same _require_internal re-assert as its siblings.
  • Tool surface: session_send added to SESSION_CONTROL_TOOLS (strict caller-identity resolution applies), schema in validation.py (target ≤500, messageMAX_LONG_STRING), definition + dispatch in mcp_dashboard.py. Falls in the session_* class the registration ratchet already admits.

Channel-agent containment. session_send is added to CHANNEL_AGENT_BLOCKED_TOOLS (src/kiro_crew/channel.py) alongside its three sibling verbs. Without it, a channel.trusted (or YOLO) channel agent driven by external content could call session_send_blocked_tool_named would 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 asserts SESSION_CONTROL_TOOLS is 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_outbound before it is delivered, on the same grounds the steer path does (chat_delivery sanitizes right before slot.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.

  • Spec + module docstring now match the shipped behaviour: the module's stated invariant was "nothing here writes into another session's conversation", which this PR reverses, so docs/system-specs/modules/session-control.md and the docstring name session_send as 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_CHARS is now an alias of validation.MAX_LONG_STRING rather 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 new api_session_control_send route had no test of its own -- and the sibling stop route 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, its message_required validation (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 in test_mcp_dashboard_registration.py and test_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, enable agent.session_control, then from that session: session_create a peer, session_send a 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.

@iamwhatever
iamwhatever requested a review from a team as a code owner August 24, 2026 17:10
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Aug 24, 2026
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 7ebb31b44f3b7f9cf3357bf0887b5792d1df3983 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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

  • Phantom description on resource bounding. The PR states the turn "is charged against the background-turn cap via run_background_turn … so fanning a message out to N targets cannot exceed the cap," but the diff passes _run_chat straight through: "_run_chat is passed straight through, NOT wrapped in state.run_background_turn: that cap is structurally unreachable here." The code's reasoning is sound (every authorizable target is attended, so the wrapper is inert), but the consequence is that nothing bounds sends: two sessions with session-control granted can session_send each other in a loop, each delivery starting a new turn with no human present, no depth counter, and no rate cap — burning tokens until someone notices. Fix the description, and decide explicitly whether repeated/reciprocal sends need a budget; the self-target guard alone does not close the two-node cycle.

Suggestions

  • Cheap loop damper: refuse (or count) a send whose body already begins with the _SEND_PROVENANCE envelope, so a relayed send can't ping-pong unbounded.
  • The new [sent by session … via session_send] envelope belongs in the injected-messages catalog (docs/system-specs/common/injected-messages.md) alongside [auto-nudge cycle N], in this same commit per the spec rule.

[DESIGN-REVIEWED] 7ebb31b

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ human override accepted

Human judgment by @iamwhatever overrides the GPT 5.6 finding for 7ebb31b44f3b7f9cf3357bf0887b5792d1df3983; the recorded reason is authoritative for this commit.

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:
/ai-review override gpt 7ebb31b44f3b7f9cf3357bf0887b5792d1df3983: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 24, 2026
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 7ebb31b44f3b7f9cf3357bf0887b5792d1df3983 — 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.

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 ships

Intent: 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").

  1. New session_send verb (MCP tool + /api/session-control/send) — justified
  2. Result distinguishes "started now" from "queued" — justified
  3. Delivered message carries a [sent by session …] tag in the target's transcript — justified (injected-messages convention)
  4. Body redacted through sanitize_outbound before persist/broadcast — justified (external-content boundary)
  5. Channel agents blocked from the verb — justified (ratchet + containment boundary)
  6. Empty/oversized gates in the business layer — justified
  7. Second empty-message check in the route with its own code — duplicate of item 6
  8. Two new stop-route tests — rides along, declared, coverage-gate mandated
  9. Spec now records the queued-drain no-recheck window (Queued prompts drain without re-validating the authorization that admitted them #5911) — undeclared in the description
  10. Delivered turns are NOT capped — contradicts the description

Watch

  • Description: "The turn is charged against the background-turn cap via run_background_turn … so fanning a message out to N targets cannot exceed the cap." Diff: "_run_chat is passed straight through, NOT wrapped in state.run_background_turn: that cap is structurally unreachable here." The code's reasoning is sound (every authorized target is attended, so the wrapper is inert), but the claimed bound does not exist: N sends to N idle targets start N concurrent turns. Fix the description, or accept that fan-out is uncapped by design.
  • The description never mentions the accepted second-authorization-moment window the spec edit documents (queued message drains with no re-check, Queued prompts drain without re-validating the authorization that admitted them #5911). Declared in the diff, invisible to a reader of the PR.

Subtractions

  • Drop the message_empty gate in send_to_target (session_control.py:903-904) — it has zero reachable consumers: the only caller is the route (grep sc.send_to_target: 1 call site), which already refuses a whitespace body with message_required (handlers/session_control.py:164-165). One condition, two error codes; keep the route's (its isinstance half is load-bearing) and the length gate.

[FIRST-PRINCIPLES-REVIEWED] 7ebb31b

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 7ebb31b44f3b7f9cf3357bf0887b5792d1df3983 — this comment is updated in place on each push.

Review details

The candidate describes the queued-send drain-gate window: a target that gains a channel mirror between enqueue_or_run_prompt and the drain would broadcast the delivered turn. Testing (a)/(b)/(c):

  • (c) reachability is a race, not a concrete condition. It requires a user action (linking/mirroring target B to a channel) to land in the narrow window between enqueue and drain. That is precisely the "if a caller were to / might" shape Step 1 requires me to drop.
  • The diff does not introduce it. The drain path (enqueue_or_run_prompt → next-turn drain) is pre-existing and identically ungated for a human-typed queued message. This PR adds a new caller into an existing ungated drain; it does not create the gap. authorize_target already refuses a mirrored/linked target at enqueue time (mirrored_target/linked_session_target), so send_to_target itself is guarded at the moment it acts.
  • The minimal fix lives in untouched code — a re-check at the generic drain (or refusing to drain a now-channel-backed slot), i.e. the drain machinery this PR did not touch. Per the Step 3 override that is FINDING at most, not BLOCKING, and it does not clear the 80+ bar to survive at all.

I re-derived the authorization chain (authorize_target with operation="send" — self/unattended/ephemeral/app-scoped/linked/mirrored/crew/workspace all denied), the sanitize-before-persist path (sanitize_outbound from chat_delivery, raw-body length gate, provenance prefix outside the sanitized span), the channel-agent block (session_send added to CHANNEL_AGENT_BLOCKED_TOOLS), the strict-internal route wiring, and the schema. No introduced crash, injection, removed guard, or data-loss path. The candidate dies under falsification and I found nothing new at the 80+ bar.

No findings.

[OPUS-REVIEWED] 7ebb31b

Verdict parsed from the review's SHA-scoped output markers for commit 7ebb31b44f3b7f9cf3357bf0887b5792d1df3983.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 7ebb31b44f3b7f9cf3357bf0887b5792d1df3983: <one-sentence reason>

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 24, 2026
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 25, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Local pre-push review round (Opus lane mirror, model claude-opus-4.8) on d19ed5f76. New head: 7316ef533.

  • BLOCKING — session_send missing from CHANNEL_AGENT_BLOCKED_TOOLS (src/kiro_crew/channel.py:54)fixed (7316ef533)

    A trusted/YOLO channel agent → session_send not matched by _blocked_tool_named (channel.py:710) → auto-approved → send_to_target runs an arbitrary turn in a private dashboard session; the three sibling session-control verbs are all blocked here, send (the only one that injects an executable instruction) is not.

    Legitimate, and confirmed mechanically rather than by reading: the repo already ships the exact guard for this class — test_channel_blocked_tools.py::test_every_session_control_tool_is_contained asserts SESSION_CONTROL_TOOLS ⊆ CHANNEL_AGENT_BLOCKED_TOOLS, pinned against the advertised set precisely so "a fourth verb fails here instead of shipping reachable from a channel agent". Stashing the fix and running that one test reproduces the finding verbatim:

    AssertionError: session-control tools reachable from a channel agent: ['session_send']
    

    So this was a CI-certain failure, not a judgement call — it had not surfaced yet only because the PR's checks last ran on a base 92 commits stale. "session_send" added to the list; the neighbouring comment now records why send is the sharpest of the four (stop cancels, read exfiltrates, send delivers text the target executes). 13/13 in that file, 220 across the four session-control/dashboard suites, and 92 passed / 2 skipped across the channel containment+trust selection.

    Reviewer's stated uncertainty ("could not confirm a test pins block-list ⊇ SESSION_CONTROL_TOOLS; the comment implies none exists") was the one inaccurate part of an otherwise correct finding — the test does exist, which makes the finding stronger, not weaker.

  • Candidate 2 — background-turn cap charged only on the immediate-start path, not the queued drain (session_control.py:895)rebutted, and noting the reviewer itself dropped this in stage 2 as below-bar ("consequence chain is speculative, mirrors the existing composer path").

    Agreed on the outcome for an additional reason worth recording: enqueue_or_run_prompt deliberately reuses the composer's own queue semantics, so charging the cap on drain would make session_send behave differently from a human-queued prompt in the same session. The docstring is the thing that overreaches, not the code — it is scoped to the idle path in this head.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 25, 2026
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Aug 25, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Disposition for the GPT 5.6 blocking finding on 7316ef5. New head: 06b440309.

  • BLOCKING — message bypasses required outbound redaction (src/kiro_crew/dashboard/session_control.py:897)fixed (06b440309)

    Credential-bearing session_send input -> enqueue_or_run_prompt -> target transcript persistence/display -> credential exposure. Fix: Validate the raw length, then pass body through sanitize_outbound before constructing prompt.

    Legitimate, and the repo already holds the precedent that settles it: the steer path — structurally identical, one session's text appended into another session's transcript — sanitizes immediately before slot.append, with the reason stated in-code (chat_delivery.py: "Store the sanitized form — raw content must never reach an external surface"), and the queue-push broadcast sanitizes too. session_send reaches the same surface via enqueue_or_run_promptslot.append("user", prompt, ...) (state.py:4132) but was not sanitized, so the two sibling deliveries into a transcript disagreed. Same file also already sanitizes the caller-supplied title and agent for exactly this reason.

    The prescribed ordering is followed exactly, and the "validate the raw length first" half is load-bearing rather than incidental: redaction can only shrink the text, so gating the raw body is the honest limit — sanitizing first would let 60K of credentials pass as a short message. The provenance tag is built by this function (not caller-supplied) and is therefore deliberately outside the sanitized span, so the [sent by session <caller>] marker cannot be forged or redacted away.

    Two tests pin it, both mutation-verified: test_the_sent_body_passes_through_the_outbound_guard (stubs sanitize_outbound and asserts both the delivered prompt and the persisted transcript row carry the sanitized form) and test_the_length_gate_measures_the_raw_body (a collapsing redactor must not smuggle an oversized body past message_too_long). Reverting the sanitize_outbound(body) call fails the first and leaves the second passing — confirmed by actually reverting it: 1 failed, 1 passed.

Also on this head, from the local Opus mirror in the previous round: session_send added to CHANNEL_AGENT_BLOCKED_TOOLS — see the earlier disposition comment. 107 tests green across the session-control and channel-containment suites; black / isort / flake8 / mypy / subprocess-encoding clean. Rebased onto current main (0cfb87cdd) so these checks run on a fresh merge ref.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 25, 2026
@github-actions github-actions Bot removed the readiness: action required A blocking check or review needs attention label Aug 25, 2026
@github-actions github-actions Bot added the readiness: action required A blocking check or review needs attention label Aug 25, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Disposition for the GPT 5.6 blocking finding on c82b179ebneeds-a-decision (maintainer ruling required; no code change made).

  • queued send bypasses target authorization at execution (session_control.py:947)needs-a-decision

    Busy target -> send queues -> target is channel-linked before drain -> prompt executes and mirrors externally without reauthorization. Fix: Refuse busy targets with 409 instead of queueing.

    The window is real. _has_channel_mirror's own docstring states that a dashboard-born slot can be given an OUTBOUND mirror link later, so a target that authorize_target cleared can be channel-linked between enqueue and drain. Not disputed.

    Two things make the prescribed fix the wrong call, and neither is a judgement I can make unilaterally:

    1. 409-on-busy deletes a designed capability of this PR. The queue path is not incidental — started exists precisely to tell a coordinating caller "it ran" from "it will run later", and the PR's stated purpose is a write verb usable across several sessions at once. Refusing every busy target converts session_send into "send only to idle sessions", which is a materially smaller feature than the one under review.
    2. The window is not created by this diff. A human-typed message into a busy session queues through the same enqueue_or_run_prompt and drains through the same generic path, so the identical exposure already exists for user-authored content. The narrow fix that actually closes it — tag the queued entry (queue_append already takes kind/meta, the repo's documented "classification by metadata" pattern) and re-validate authorization at drain — lands in chat_runner's generic drain, outside this PR's surface, and would close it for human-queued content too. That is the correct layer.

    This is the third blocking finding on this one span (local GPT mirror on d19ed5f76, server GPT on 06b440309, server GPT on c82b179eb). Per this repo's own stall guidance, a fourth point-fix on the same span is the wrong move, so I am stopping rather than patching: the block clears only by a maintainer decision — either accept the deferral via /ai-review override gpt c82b179eb1124ef7b0871f21acd5c809cab66894: <reason>, or rule that the drain-side re-validation is in scope for this PR (widening it into chat_runner), or accept the capability loss of 409-on-busy.

    Recorded here so the open finding is visible to a human rather than sitting unread while a loop looks healthy.

@iamwhatever

Copy link
Copy Markdown
Collaborator Author

/ai-review override gpt 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.

@github-actions

Copy link
Copy Markdown
Contributor

Human judgment recorded

@iamwhatever marked the gpt AI finding as false positive, not applicable, or explicitly accepted for c82b179eb1124ef7b0871f21acd5c809cab66894.

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.

This decision applies only to this commit. A new push requires a new judgment.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 25, 2026
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: passed Eligible automated validation passed for the current revision labels Aug 25, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator Author

Design Review CONCERNS on c82b179eb — dispositions. New head: 7ebb31b44.

  • Spec contradiction, same-commit MUST violatedfixed (7ebb31b44)

    docs/system-specs/modules/session-control.md and the module docstring still state "Nothing here writes into another session's conversation" … the spec now documents the opposite of the shipped behavior and the tool/route tables omit the sharpest verb.

    Correct and the cheapest kind of legitimate finding to have missed: the PR reversed the module's stated core invariant and left the spec asserting the old one. Both the spec's opening claim and the module docstring now name session_send as the one verb that writes into another session's conversation, and both register the [sent by session … via session_send] envelope alongside the redaction and the channel-agent block, so the three bounds on the sharpest verb are documented where a reader looks for them. The "No message delivery" non-goal is replaced by the accurate one — no delivery to a target outside the addressable set, naming each refusal the guard applies.

  • Queued delivery invalidates "authorized at the moment it acts"accepted-and-deferred, and now explicitly documented, which is the second of the two options this finding offered

    Either re-check (or resolve the slot's addressability) at dequeue, or explicitly document the one-turn window as accepted.

    Taking the documentation arm rather than the re-check arm, because the window is not this module's to close: a human-typed message into a busy session drains through the same ungated path, so a re-check scoped to session_send would leave the identical exposure for user-authored content. The spec now states plainly that delivery has two authorization moments and only the first is enforced, why the window is accepted rather than overlooked, and where it is being fixed — issue Queued prompts drain without re-validating the authorization that admitted them #5911, which puts the re-validation at the generic drain so it closes for every queued prompt. The blocking GPT finding on the same span was cleared by maintainer override citing that issue.

@iamwhatever

Copy link
Copy Markdown
Collaborator Author

First Principles CONCERNS on c82b179eb — dispositions. New head: 7ebb31b44.

  • The background-turn cap is unreachable; _capped_run is dead codefixed (7ebb31b44)

    run_background_turn is a pass-through for attended slots (state.py:5814), unattended requires _app (state.py:4452), and authorize_target denies every app-scoped target (session_control.py:634) — so 0 reachable targets are ever capped.

    Verified against all three cited lines rather than taken on trust, and the chain holds exactly as stated: run_background_turn opens if not slot.unattended: return await coro with a docstring that says in as many words "this wrapper is inert for" an attended slot; _ChatSlot.unattended is bool(self._app) and not self._human_seen; and authorize_target refuses any _app target with app_scoped_target. So every target this function can authorize is attended by construction and the cap can never bind. _capped_run — the wrapper, its TimeoutError arm and the no-permit card — is deleted, _run_chat is passed to enqueue_or_run_prompt directly (matching the composer's own queued path), and the docstring now states why the cap is deliberately absent instead of claiming it applies. The PR body's cap sentence went with it. Good catch: the copied Issue Radar pattern lost the app-owned-slot predicate that makes it live there.

  • MAX_SEND_MESSAGE_CHARS duplicates validation.MAX_LONG_STRINGfixed (7ebb31b44)

    its own comment says "Matches MAX_LONG_STRING"; two spellings of one 50k limit will diverge.

    Aliased to the imported constant rather than restating the number, so the two cannot drift. The name is kept because it reads at the call site as the cap on this payload, and the tests address it there.

  • session-control.md left saying "No message delivery"fixed (7ebb31b44); see the Design Review disposition for the full spec rewrite, which also retires the recorded two-authorization-moment requirement explicitly rather than silently.

  • Collapse the two empty-message gatesrebutted (no code change)

    the business layer's message_empty branch is unreachable through its only caller … keep one code, delete the other branch.

    Unreachable today, through today's only caller — but that is the same argument the PR already makes for re-asserting the length limit at the business layer, and for the same reason: the HTTP route is not the only way in, it is merely the only one that exists now, and the route's own validation does not run for an in-process caller. Deleting the business-layer guard would move the invariant from "enforced where it is owned" to "enforced by whoever remembers", which is the shape of defect this module's deny-by-default single-chokepoint design exists to avoid. The two are not redundant in the sense that matters: the handler's message_required is the route contract (and the code the new route test pins), the business layer's message_empty is the last line. Both are two lines and neither is dead in the "no non-test caller" sense the repo's dead-code rule is keyed on.

@iamwhatever

Copy link
Copy Markdown
Collaborator Author

/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.

@github-actions

Copy link
Copy Markdown
Contributor

Human judgment recorded

@iamwhatever marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 7ebb31b44f3b7f9cf3357bf0887b5792d1df3983.

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.

This decision applies only to this commit. A new push requires a new judgment.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 25, 2026
@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention labels Aug 25, 2026
@iamwhatever
iamwhatever merged commit f83f33a into main Aug 25, 2026
114 of 120 checks passed
@iamwhatever
iamwhatever deleted the feat/session-send branch August 25, 2026 20:47
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Aug 25, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator

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 #5819 is PARTIALLY_COVERED relative to this PR. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #5819: CONTINUE_DEVELOPMENT. Main covers the agent-facing half completely and with stronger containment, so the PR's messaging.py handler, its validation schema, and the agent-half assertions in test/test_send_to_session.py should be dropped in favour of session_send; the split-view UI, the grid flag gate, the Settings toggle and the config key remain uncovered work worth landing. Files: src/kiro_crew/mcp_tools/messaging.py, src/kiro_crew/validation.py, website/src/components/ChatPane.tsx.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants