fix(desktop): route mid-turn sends through the steering queue (#1954) - #2222
fix(desktop): route mid-turn sends through the steering queue (#1954)#2222sunheyi6 wants to merge 34 commits into
Conversation
|
@Astro-Han cc |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Review — PR #2222: route mid-turn sends through the steering queue (#1954)
Thanks for this PR — I think the core approach is exactly right. Routing plain-text sends through the existing steering queue at the main-process choke point (instead of opening a parallel root turn) removes the second writer on the single mutable backend, which is the real root cause of #1954. Reusing the #1087/#1357 seam instead of rebuilding #629's injectGuidance was the right call, and the live-vs-persisted steer rendering is the two sides the projection architecture needs, not duplication. Suggestions below are roughly in priority order; I'd be comfortable with this landing once sections 1–2 are addressed.
1. Correctness
1.1 Steered text can silently disappear when the turn ends before the next step boundary. After "立即发送" fires, runtime.steer() returns {kind:'queued'} and the renderer clears the draft. But if the turn ends or is stopped before the next drain point (stepLimitReached, stop, ...), releaseSteeringTurn folds the steer into followup (runtime-kernel.ts:2531-2540) — and Desktop has no drainFollowup path (only the CLI wires it, packages/cli/src/session-driver.ts:325), and the renderer doesn't handle queue_update either. The user's text vanishes with no trace: sessions:send already returned {ok:true, steered:true}. I'd suggest draining followup when the next turn opens on desktop, or having the release path fall back to opening a new turn with the folded text when the fold would be terminal.
1.2 The same steer renders twice. A drained steer lands in the persisted timeline via buildTurnTimeline (materialize.ts:893-905) and in the live overlay via pendingSteers/step.steers (materialize.ts:435-452). Both share the same messageId, but nothing dedupes — the "引导已注入" badge appears twice for the whole turn (and after it, since terminal projections keep the steers). I'd suggest skipping live steers whose messageId already exists in the timeline, and clearing pendingSteers when the projection turns terminal.
1.3 The steered branch skips onSessionResolved (app-shell-chat-actions.ts:416-421), which start_task relies on to bind taskSessionId. A voice "start a task: X" on a busy session returns steered:true but no session binding, and subsequent steer_task calls fail with no_active_task. One line: call options.onSessionResolved?.(sessionId) before returning.
2. The composer queue strip — my main scope suggestion
This is the piece I'd push back on most, and I want to be clear it's a scope judgment, not a quality judgment: the queue strip is a product decision beyond what #1954 needs. The minimal fix — which the main-process routing already achieves — is "send while a turn runs → steer the running turn". The strip turns that into "defer until idle or explicit fire", and it introduces three reproducible issues:
- The queue is component state on
Composer, which mounts once per chat surface, not per session. Auto-drain only checks the currently viewed session's busy state, so text queued in session A fires into idle session B after a switch. - Auto-drain re-queues failed sends at the front with no cap or backoff — a persistently failing send retries forever, pops a failure toast each cycle, and even races a remove click.
- The busy intercept runs before the payload-complexity check, so a send with attachments/quotes while busy is silently queued as bare text (the chips stay staged for the next send).
My suggestion: cut the strip from this PR (~250 lines incl. helpers/tests) and keep main routing + rollback/refresh + steer rendering + the continuation pass — the fix stands on its own. If product wants the queue UX, I'd build it on the runtime's existing queue_update events, keyed per session, rather than a third queue in component state. If you keep it, I'd at least fix the three issues above.
3. Design-language alignment (mostly cheap fixes)
3.1 The 7 hard-coded Chinese strings (立即/编辑/删除/排队中的消息/编辑这条消息/删除这条消息/引导已注入) should go into conversation-copy.ts as zh+en pairs — composer.tsx currently has zero hard-coded labels, and en users would see Chinese mid-transcript (this repo even patches Astryx upstream to remove hard-coded English, see patches/README.md).
3.2 The steer badge bypasses the Marker recipe (primitives/chat.tsx) that every sibling status uses (aborted, automation-origin, failed-banner). I'd add a steer variant there (neutral, matching the existing markers) rather than a new .maka-steer-badge class; the current markup also carries four Tailwind-style utilities (inline-flex flex-col gap-1 self-start) that don't exist in this repo, and .maka-steer-marker has no rule at all.
3.3 --info-weak is an invented token (undefined repo-wide). The established weak-info idiom is oklch(from var(--info) l c h / 0.10) (chat-detail.css:401). Also, per DESIGN.md ("don't turn every status into a colored pill"), a neutral marker would fit a quiet transcript note better than a tinted pill.
3.4 Queue styles landed in packages/ui/src/styles.css, but all composer chrome lives in apps/desktop/src/renderer/styles/composer.css; and the row style (bordered + shadowed mini-card per row) contradicts the documented "not a heavy info card" pattern of .maka-composer-revision-notice. I'd move it next to the notice family.
3.5 role="status" wraps three focusable buttons per row — ARIA-wise a live region shouldn't contain interactive content; role="list" / role="group" would be safer.
3.6 The bubble re-skin via opacity: 0.9 + background: transparent overrides the user bubble from another package. I'd render the steered text in the standard user bubble, or add a variant at the bubble seam, rather than an opacity hack.
4. Tests — the three that would make me comfortable
4.1 The most valuable missing test is an E2E journey (apps/desktop/e2e/mid-turn-send.spec.ts): send FAKE_WAIT_FOR_STEERING_PROMPT (the seam already exists in fake-backend.ts), send a second message while streaming, assert the strip appears, click 立即, and assert exactly one run / no parallel turn plus the steered text in the transcript. That matches AGENTS.md's guidance for journeys crossing renderer/main.
4.2 Two renderer branches have zero coverage: the steered === true rollback (harness exists: app-shell-first-send-cleanup.test.ts) and steering_message → refreshMessages (harness exists: streaming-handoff.test.ts).
4.3 Runtime edges: a test for the MAX_STEERING_CONTINUATIONS bound, the ack on the continuation path, the tool_calls + pending steer mixed branch, and a kernel-level test that hasPendingSteering is scoped to the owning turn.
5. Nits
MAX_STEERING_CONTINUATIONS = 8is generous for a billed provider call per pass; 2 would deliver the same UX (steers arriving during one step are drained in a single pass anyway).- The bridge contract declares
steered?: boolean; turnId: stringon the ok branch, but the steered response has noturnId; a discriminated union would make that type-checked, and preload.ts's return type should match.
Thanks again for taking this on — the direction is right and the runtime reasoning is solid. Happy to help with the E2E or unit tests if useful.
中文对照（点击展开）
PR #2222 审查：mid-turn 发送路由到 steering 队列（#1954）
先感谢这个 PR——我认为核心方向完全正确。在主进程统一入口把纯文本发送路由进已有的 steering 队列（而不是开一个并行 root turn），是移除单实例 backend 上的第二个写入者，这正是 #1954 的真正根因；复用 #1087/#1357 的既有 seam、而不是重建 #629 的 injectGuidance，是正确的选择；live 与持久化两侧的 steer 渲染是投影架构需要的两个面，不是重复。以下建议按优先级排列，1–2 处理完后我就放心合了。
1. 正确性
1.1 steered 文本可能在下一个 step 边界之前 turn 结束时静默丢失。 点击「立即发送」后 runtime.steer() 返回 {kind:'queued'}，renderer 清空草稿。但如果 turn 在下一个 drain 点之前结束或被 Stop（如 stepLimitReached、stop），releaseSteeringTurn 会把 steer 折进 followup（runtime-kernel.ts:2531-2540）——而 Desktop 没有 drainFollowup 路径（只有 CLI 有，packages/cli/src/session-driver.ts:325），renderer 也不处理 queue_update。于是用户文本无影无踪：sessions:send 已经返回 {ok:true, steered:true}。建议：桌面侧在开新 turn 时 drain followup，或 release 时若 fold 是终态则回退为用该文本开新 turn。
1.2 同一 steer 渲染两次。 drain 的 steer 既进持久化 timeline（buildTurnTimeline，materialize.ts:893-905），又进 live overlay（pendingSteers/step.steers，materialize.ts:435-452）。两者 messageId 相同但无去重——「引导已注入」在整段 turn（及结束后，因为终态投影仍保留 steers）出现两次。建议：overlay 跳过已在 timeline 中的 messageId，投影 terminal 时清空 pendingSteers。
1.3 steered 分支跳过 onSessionResolved（app-shell-chat-actions.ts:416-421），start_task 依赖它绑定 taskSessionId。busy 会话上语音说「start a task: X」会返回 steered:true 但无绑定，后续 steer_task 报 no_active_task。一行修复：返回前调用 options.onSessionResolved?.(sessionId)。
2. composer 排队条——我最大的范围建议
这块我最有保留，但先说清楚这是范围判断、不是质量判断：排队条是超出 #1954 需求的产品决策。最小修法——你的 main 侧路由已经实现——是「turn 运行时发送 → steering 当前 turn」。排队条把它变成「先拦截，等空闲或手动触发」，并带来三个可复现问题：
- 队列挂在
Composer组件上（每个 chat surface 只 mount 一次，不是每会话）；auto-drain 只看当前查看会话的忙闲——A 会话排的队，切到空闲 B 会话会发进 B； - auto-drain 把失败发送插回队首且无上限/backoff——持续失败的发送会无限重试、每轮弹失败 toast，连删除都会竞态输给重新入队；
- 忙时拦截发生在负载复杂度判断之前——带附件/引用的发送会被静默吞成裸文本排队（chips 残留给下一次发送）。
建议：本 PR 先砍掉排队条（约 250 行，含 helpers 与测试），保留 main 路由 + 回滚/refresh + steer 渲染 + continuation pass——修复依然完整。若产品确实要排队体验，建议基于 runtime 已有的 queue_update 事件、按 session 维度实现，而不是在组件状态里维护第三套队列。若保留现状，至少修掉上面三个问题。
3. 设计语言对齐（大多是低成本修复）
3.1 7 处硬编码中文（立即/编辑/删除/排队中的消息/编辑这条消息/删除这条消息/引导已注入）应进 conversation-copy.ts 的 zh+en 双份——composer.tsx 基线是 0 处硬编码 label，en 用户会在 transcript 中看到中文（这个仓库甚至为消灭硬编码英文而 patch 了 Astryx 上游，见 patches/README.md）。
3.2 steer 标记绕过了所有同级状态都在用的 Marker 原语（primitives/chat.tsx：aborted、automation-origin、failed-banner）。建议加一个 steer variant（neutral，与既有 markers 一致）而不是新的 .maka-steer-badge 类；另外现有 markup 带着 4 个仓库里不存在的 Tailwind utility 类（inline-flex flex-col gap-1 self-start），.maka-steer-marker 本身也没有任何样式规则。
3.3 --info-weak 是自创 token（全仓库无定义）。既有弱化 idiom 是 oklch(from var(--info) l c h / 0.10)（chat-detail.css:401）。另外按 DESIGN.md（「不要把所有状态变成彩色 pill」），对安静的转录注记，neutral marker 比彩色 pill 更贴。
3.4 queue 样式落在 packages/ui/src/styles.css，而 composer 全部 chrome 都在 apps/desktop/src/renderer/styles/composer.css；且每行是带边框+阴影的 mini-card，与 .maka-composer-revision-notice 文档化的「不是重型信息卡」模式相反。建议挪到 notice 一族旁边。
3.5 role="status" 里每行包了三个可聚焦按钮——ARIA 上 live region 不应含交互内容，role="list" / role="group" 更安全。
3.6 用 opacity: 0.9 + background: transparent 从另一个包覆写用户气泡——建议 steered 文本用标准用户气泡，或在气泡 seam 上加 variant，而不是 opacity 技巧。
4. 测试——三处补上我就放心了
4.1 最缺的是一个 E2E 旅程（apps/desktop/e2e/mid-turn-send.spec.ts）：发 FAKE_WAIT_FOR_STEERING_PROMPT（fake-backend.ts 里现成的 seam），流式期间发第二条消息，断言条带出现、点「立即」，断言恰好一个 run / 无并行 turn、steer 出现在 transcript。符合 AGENTS.md 对跨 renderer/main 旅程的要求。
4.2 两个 renderer 分支零覆盖：steered === true 回滚（harness 现成：app-shell-first-send-cleanup.test.ts）和 steering_message → refreshMessages（harness 现成：streaming-handoff.test.ts）。
4.3 runtime 边界：MAX_STEERING_CONTINUATIONS 上限、continuation 路径的 ack、tool_calls + pending steer 混合分支、以及 kernel 级「hasPendingSteering 只归属当前 turn」的隔离测试。
5. 小项
MAX_STEERING_CONTINUATIONS = 8对每次都是计费 provider call 来说偏大方；2 就能达到同样体验（同一 step 内到达的 steer 反正会在一次 pass 里全部 drain）。- bridge contract 在 ok 分支声明
steered?: boolean; turnId: string，但 steered 响应没有turnId；改成 discriminated union 可让该不变量类型化，preload.ts 的返回类型也应同步。
再次感谢接手——方向正确，runtime 推理扎实。需要帮忙写 E2E 或单测随时说。
|
@Astro-Han cc |
|
Thanks for tracing the mid-turn race and adding coverage around the embedded Desktop path. After comparing this PR with the Runtime Host M5 path, I think the fix now sits on the wrong ownership boundary. P1: This adds a second message-submission authority on a path M5 removesThe new routing is explicitly limited to the embedded runtime: The new backend hook also says that hosted message authorities do not use it: Runtime Host already has the canonical operation for this decision: It accepts the full By comparison, this PR adds a Desktop-owned decision:
That is a second submit protocol with different payload and continuation semantics. This matters now because #2420 makes Runtime Host the only production owner and removes Runtime Host admission prevents the parallel-writer crash, but a normal mid-turn send will still become a busy/error outcome instead of steering the active turn. In other words, this PR fixes the path being retired without fixing the production path that replaces it. I suggest retargeting the work around the existing Host authority:
With M5 implemented and awaiting final review, merging another embedded submission path would add code that the cutover immediately deletes. I think the current PR should be superseded or reduced to a Runtime Host-backed Desktop/UI follow-up rather than merged in its present form. 简体中文感谢你定位 mid-turn 并发问题，并为 Embedded Desktop 路径补充测试。把当前实现与 Runtime Host M5 路线对照后，我认为这个修复落在了错误的职责边界上。 P1：PR 在 M5 即将删除的路径上建立了第二套消息提交权威新增路由明确只服务 Embedded Runtime： 新增的 backend hook 也明确说明 Hosted Message Authority 不使用它： Runtime Host 已经有处理该决策的 canonical operation： 它接收完整的 相比之下，本 PR 新增了一套由 Desktop 决定的语义：
这形成了第二套提交协议，而且 payload 与 continuation 语义都不同。 现在这个问题已经会影响交付路线。#2420 会让 Runtime Host 成为唯一 production owner，并删除 Runtime Host admission 会阻止并行 writer 崩溃，但普通 mid-turn send 仍会得到 busy/error，而不是 steering 当前 turn。也就是说，这个 PR 修复了即将退出的路径，却没有修复接替它的 production 路径。 建议把工作收敛到现有 Host authority：
M5 已经完成实现并等待最终审查。此时再合并一条 Embedded submission 路径，只会增加随后立即被 cutover 删除的代码。我建议当前 PR 由 Runtime Host-backed Desktop/UI follow-up 取代，或者缩减为该 follow-up，而不是按现状合并。 |
9ad99e4 to
30416a2
Compare
…bmit (apache#1954) Retargets the mid-turn steering fix onto the Runtime Host authority per review: Desktop sends whose payload fits canonical MessageContent route through turn.message.submit, so the Host atomically returns turn_started | steering | followup instead of Desktop opening a parallel root turn or re-deciding steering itself. - runtime-host-session-execution-ipc-main.ts: sessions:send submits MessageContent-capable payloads (text/displayText/attachments/quotes/ inlineReferences) with placement current_turn; skills and orchestration keep turn.start (the submit protocol cannot carry them). Returns the Host-authoritative turnId on turn_started. - bridge contract + preload: sessions.send result becomes a discriminated union on disposition instead of the private steered flag. - renderer: steering/followup dispositions undo new-turn bookkeeping and still fire onSessionResolved (start_task binding, review 1.3); turn_started uses the Host-returned turnId for the optimistic message and poll. - sessions-ipc-main.ts (embedded): reports disposition turn_started so the shared contract stays sound until the M5 cutover removes this path. - app-shell-session-events.ts: steering_message refreshes the transcript; the Host observer already emits these from its durable queue projection. Removes the embedded-only machinery from the earlier revision: the embedded steering router + followup drain, the runtime pending-steering peek and continuation pass, the composer queue strip, and the embedded-fake-backend e2e. Steer display on the Host path rides the observer's steering_message/queue_update events and the transcript's interjection projection. Rebased onto current main (branch was DIRTY).
…rge" This reverts commit 40a2283.
e239170 to
31e6a75
Compare
|
Thanks for moving this onto the existing Runtime Host and Astryx rendering paths. Since this changes a core mid-turn interaction, could you add a couple of screenshots—or ideally a short recording—showing the second message while the turn is running and again after it settles? I’d mainly like to confirm that it appears once in the expected position, remains visible, and does not open a parallel turn before approving. Would it also make sense to split the unrelated draft persistence, revision recovery, and Quote Companion drag-test stabilization into follow-ups? The Quote Companion adaptation to the shared |
|
…o feat/desktop-mid-turn-steer # Conflicts: # packages/ui/src/conversation-copy.ts # packages/ui/src/live-turn-projection.ts
…rn-steer # Conflicts: # apps/desktop/src/main/__tests__/runtime-host-session-observer.test.ts # apps/desktop/src/main/runtime-host-session-execution-ipc-main.ts # apps/desktop/src/main/runtime-host-session-observer.ts
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Codex automated review
I reviewed exact head 7923e8713c53eedbf56a98fbde8f2ed47e2bde3f against the earlier review ledger, following Host submit/steering authority through optimistic live state, session events, local queueing, attachments/quotes, auto-drain, and recovery. The original parallel-root ownership issue is fixed, but two current-head state/identity bugs remain; see the P2 inline findings. Checks are green, but the branch is currently conflicting.
The minimal #1954 fix is the canonical Desktop→Host submit/steering path. The local queue strip is a separate product slice and a third queue authority; it has already created the per-entry context bug below. I recommend splitting that queue UX (and its ~300 lines/tests/styles) into a follow-up built on Host queue events, leaving the Host steering fix independently reviewable. The unrelated revision/draft reapply changes can likewise land separately.
Disclosure: This is an automated review performed by Codex using delegated adversarial review passes and a final evidence check. It has not been independently verified by Astro-Han or another human reviewer, does not constitute human approval, and does not represent the final judgment of a human reviewer.
- P2-1: bind tray quotes/attachments to each queued entry at queue time; drains use their own snapshot, never the current tray; clear captured items by identity after a successful drain - P2-2: rebind the optimistic arm to the Host-named turn id so a no-content error/abort settles it instead of leaving the composer stuck in Stop/busy - retain steering guard in settleLiveTurnStep (terminal with pending steering must not collapse)
Resolves the PR conflict against the 247-commit mainline drift. Mainline has since shipped the canonical Host submit/steering authority (turn.message.submit via submitMessage, apache#2420) and a reworked steering projection model (pendingSteering/leadingSteering); the PR's desktop routing layer and steer rendering are superseded by it. This merge keeps only what remains PR-specific on top of mainline: - composer mid-turn queue strip (bound per-entry staged context, P2-1) - optimistic arm rebind to the Host-named turn id (P2-2) - optimistic steering rollback adapted to pendingSteering - queue copy keys ported into the mainline conversation-copy - compose with upstream's deep-path @maka/core imports
The merge landed the PR's optimistic steering work on mainline's reworked projection model (pendingSteering/leadingSteering). Adapt: - rollbackOptimisticSteering removes from pendingSteering, not the old steering field - streaming-handoff assertions use pendingSteering with the upstream content shape - composer-helpers imports QuoteRef/AttachmentRef via the deep @maka/core/events path (root barrel removed upstream)
The auto-merge dropped upstream's messageReadErrorMessage from app-shell-copy.ts while keeping messageRefreshErrorMessage; upstream's app-shell-effects imports it. Restore the export (same definition as upstream main).
The merge's auto-merge dropped uiLocale/setMessages/toastApi from the useActiveSessionEvents options (upstream interface requires them) and left the renderer tsconfig's PendingAttachment reference unresolved. Both restored to upstream shape.
# Conflicts: # apps/desktop/src/main/runtime-host-desktop-candidate.ts # apps/desktop/src/preload/preload.ts
Resolve conflicts from the transcript range-store refactor (apache#2937 and related) that removed the readMessages bridge and refreshMessagesUntilTurn polling: - runtime-host-session-execution-ipc-main.ts: keep both isClosedSteeringAdmission (mid-turn steer) and normalizeTranscriptRangeRequest (transcript range) helpers. - bridge-contract.d.ts: keep steer(messageId?) and drop the removed readMessages method. - app-shell-chat-actions.ts: drop the removed refreshMessagesUntilTurn settle poll (readMessages is gone); the transcript range controller now owns refresh. - app-shell-first-send-cleanup.test.ts: keep both imports, adapt send mocks to the disposition/turnId contract, and track transcript refresh through the range controller instead of readMessages.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Moving message admission into Runtime Host is the correct ownership direction, but this branch no longer solves the stated problem. Its normal composer path chooses next_turn, so a mid-turn race becomes a follow-up rather than steering; the new test codifies that mismatch. Current main has since solved #1954 at the closest seam: start normally, catch the authoritative session_busy race, then submit the same content with current_turn.
The simplest outcome is therefore to treat this PR as superseded rather than preserve a parallel send protocol. If any unique coverage remains, port only that coverage onto current main's single admission path.
Review performed with Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified both failures against the latest head and compared them with current main.
中文评论
把消息 admission 收回 Runtime Host 是正确的职责方向，但该分支已不再解决其声明的问题：普通 composer 路径选择 next_turn，mid-turn race 会变成 follow-up 而非 steering，新增测试还把这一不一致固化成预期。当前 main 已在最近的 seam 上解决 #1954：正常 start，捕获权威的 session_busy race，再以 current_turn 提交相同内容。
因此最简单的结果是把本 PR 视为已被 supersede，而不是保留并行 send protocol。若仍有独有测试价值，只把该覆盖迁移到当前 main 的单一 admission 路径。
本次审查使用了 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具；我已依据最新 head 和当前 main 复核两个失败路径。
| ...(command.quotes ? { quotes: command.quotes } : {}), | ||
| inlineReferences, | ||
| }, | ||
| placement: "next_turn", |
There was a problem hiding this comment.
P1 — This contradicts the PR's steering contract. When the renderer sees idle but another client starts a root turn before this IPC arrives, next_turn becomes a Host followup, so the user's message cannot steer the active turn. The added test now locks that behavior in despite the PR/issue specifying current_turn. Current main already uses the narrower solution: attempt turn.start, and only on the authoritative session_busy race submit the content with placement: 'current_turn'. Rebase onto that seam or close this as superseded.
| // mid-turn (a create/send race). Treat it like the existing-session | ||
| // queued branch: no optimistic user message, keep the session, refresh | ||
| // the transcript. | ||
| if (sendResult.disposition !== 'turn_started') { |
There was a problem hiding this comment.
P2 — The new-session queued race leaves the optimistic turn armed and invisible. This branch clears unsentSessionId but does not disarmTurnActive(session.id, turnId) or make the new session active; refreshMessages(session.id) can then no-op because activeIdRef is still empty. If another client starts the just-created session before send, the user remains on the new-chat surface while the accepted queued message is not shown and the optimistic busy/Stop state can remain. Clean up the arm and navigate to the resolved session, or remove this branch by adopting current main's narrower busy-race path.
|
/agentic_review |
Code Review by Qodo
1. Mid-turn sends become followups
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| ...(command.quotes ? { quotes: command.quotes } : {}), | ||
| inlineReferences, | ||
| }, | ||
| placement: "next_turn", |
There was a problem hiding this comment.
1. Mid-turn sends become followups 🐞 Bug ≡ Correctness
Fix-now: canonical ordinary composer input sent through sessions:send uses `placement: "next_turn", so a busy Host classifies it as a followup` and runs it only after the active turn instead of steering that turn as the PR title and stated behavior require. Because the Host derives steering only from current_turn, this preserves the reported #1954 regression in the primary mid-turn-send scenario.
Agent Prompt
## Issue description
Canonical ordinary `MessageContent`-capable `sessions:send` requests currently use `placement: "next_turn"`, which causes mid-turn sends to become follow-ups rather than steering the active turn.
## Issue Context
Reuse the existing Host submit authority; no new branch, state, configuration, protocol, or public surface is needed. `sessions:steer` already uses the same Host submit seam with `placement: "current_turn"`; reuse that placement for the `MessageContent` send branch so the Host atomically starts an idle turn or queues steering on a busy one, and update tests that currently encode `followup` for ordinary mid-turn sends.
## Fix Focus Areas
- apps/desktop/src/main/runtime-host-session-execution-ipc-main.ts[223-270]
- apps/desktop/src/main/__tests__/runtime-host-session-execution-ipc-main.test.ts[538-585]
- packages/runtime-host/src/server/message-coordinator.ts[595-615]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const messageContentCapable = | ||
| command.text.trim().length > 0 && | ||
| !new RegExp(SKILL_INVOCATION_TOKEN_SOURCE).test(command.text) && | ||
| (command.skillIds?.length ?? 0) === 0 && |
There was a problem hiding this comment.
2. Attachment-only sends bypass submission 🐞 Bug ≡ Correctness
Fix-now: messageContentCapable requires non-empty text even though canonical MessageContent permits empty text with attachments, quotes, or inline references. Such sends fall through to startTurn, bypassing the Host's atomic steering/follow-up admission and potentially opening a parallel root turn while a session is busy.
Agent Prompt
## Issue description
Attachment-, quote-, or reference-only canonical messages are incorrectly classified as unable to use `turn.message.submit`.
## Issue Context
Consolidate all canonical `MessageContent` payloads through the existing submit seam. No additive behavior is required; the capability check should exclude only payload fields the submit protocol cannot carry, such as skill invocation and orchestration.
## Fix Focus Areas
- apps/desktop/src/main/runtime-host-session-execution-ipc-main.ts[232-250]
- apps/desktop/src/main/__tests__/runtime-host-session-execution-ipc-main.test.ts[257-334]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (sendResult.disposition !== 'turn_started') { | ||
| unsentSessionId = undefined; | ||
| options.onSessionResolved?.(session.id); | ||
| await refreshMessages(session.id); |
There was a problem hiding this comment.
3. Queued first send stays armed 🐞 Bug ≡ Correctness
Fix-now: when a newly created session receives steering or followup due to the acknowledged create/send race, the new branch returns without disarming the optimistic turn armed before the send. The session can therefore remain in an unconfirmed waiting state and keep the composer showing a live/busy turn that never started.
Agent Prompt
## Issue description
The new-session queued-disposition branch leaves its optimistic turn active even though no turn started.
## Issue Context
Reuse the existing `disarmTurnActive` cleanup already used by the equivalent existing-session branch; no new state or public surface is necessary. Add a regression assertion for the create/send race branch.
## Fix Focus Areas
- apps/desktop/src/renderer/app-shell-chat-actions.ts[408-418]
- apps/desktop/src/main/__tests__/app-shell-first-send-cleanup.test.ts[517-570]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (disposition === 'commit') { | ||
| committedPointerId = pointerId; | ||
| if (completed.changed) actions.scheduleSettle(); |
There was a problem hiding this comment.
4. Stale pointer suppresses cancellation 🐞 Bug ≡ Correctness
Fix-now: a committed pointer ID is retained after pointerup, but finishGesture immediately removes the capture-loss listener that is supposed to consume it. When Chromium reuses that pointer ID, a genuine capture loss on the next gesture is mistaken for the prior automatic release, leaving the new gesture active and blocking subsequent selections.
Agent Prompt
## Issue description
The committed-pointer marker can outlive its capture listener and suppress cancellation of a later gesture that reuses the same pointer ID.
## Issue Context
Correct the existing gesture boundary rather than adding another registry or timer. Ensure the marker is consumed or cleared within the same capture lifetime, and test the real hook ordering where `pointerup` removes or retains the listener before automatic `lostpointercapture`.
## Fix Focus Areas
- packages/ui/src/use-message-selection-quote.ts[57-98]
- packages/ui/src/use-message-selection-quote.ts[204-274]
- packages/ui/src/__tests__/message-selection-quote-boundary.test.ts[86-98]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Thank you for the substantial work on the mid-turn send path. Current main now resolves #1954 through #3032 (commit 083b3e6) at the single Runtime Host-owned admission seam: Desktop starts normally, catches the authoritative session_busy race, and submits the same MessageContent as current_turn steering. This covers the stated bug without retaining the parallel send contract from this branch. This PR is now superseded and can be closed to keep one source of truth and a clear backlog. If any coverage here remains unique, please feel free to port only that focused coverage onto the current main path. We appreciate the investigation and iterations that helped establish the final ownership model. 中文感谢你在 mid-turn send 路径上投入的大量工作。当前 main 已通过 #3032（commit 083b3e6）在唯一的 Runtime Host-owned admission seam 上解决 #1954：Desktop 正常发起 start，捕获权威的 session_busy race，再把同一份 MessageContent 作为 current_turn steering 提交。它已经覆盖了本 PR 声明的问题，同时不需要保留该分支中的并行 send contract。 此 PR 现在已被 supersede，可以关闭，以维持单一真相源并保持 backlog 清晰。如果这里仍有 main 未覆盖的独立测试价值，欢迎只把那部分聚焦 coverage 迁移到最新 main。感谢这段调查与多轮迭代，它们帮助我们明确了最终的职责边界。 |
3 participants
Summary
Fixes #1954 (desktop: mid-turn send opens a parallel root turn — should route through the steering queue). Reworked per review to sit on the Runtime Host authority instead of the embedded runtime: the embedded path is retired by the M5 cutover (#2420), which deletes
sessions-ipc-main.tsandsession-send-resolve.ts; this PR fixes the path that replaces it.Problem. On the Runtime Host-backed facade,
sessions:sendcalledstartTurn()unconditionally: a mid-turn send either failed with a busy/error outcome or opened a second root turn instead of steering the running turn. Only the separatesessions:steerhandler used the canonicalturn.message.submit.Fix. Desktop sends whose payload fits canonical
MessageContentnow route throughturn.message.submit(placement: 'current_turn'), so the Host atomically decidesturn_started(idle session) orsteering(busy session) — one submit protocol, no Desktop-owned steering decision. The Host's durable queues and the observer'ssteering_message/queue_updateevents (already emitted from the Host queue projection) surface the injected text in the transcript; the existing interjection projection renders it.Changes
Runtime Host-backed facade
apps/desktop/src/main/runtime-host-session-execution-ipc-main.ts—sessions:sendnormalizes the command and routes MessageContent-capable payloads (text/displayText/attachments/quotes/inlineReferences) throughsubmitMessage; returns the Host's disposition and the Host-authoritativeturnIdonturn_started. Skill and orchestration payloads keepturn.start(the submit protocol cannot carry them).Contract
apps/desktop/src/preload/bridge-contract.d.ts+preload.ts—sessions.sendresult is a discriminated union ondisposition: 'turn_started' | 'steering' | 'followup'(replaces the privatesteeredflag).apps/desktop/src/main/sessions-ipc-main.ts— the embedded facade reportsdisposition: 'turn_started'so the shared contract stays sound until feat(runtime-host): complete M5 production cutover #2420 deletes it.Renderer
apps/desktop/src/renderer/app-shell-chat-actions.ts—steering/followupdispositions undo the new-turn bookkeeping (disarm, clear optimistic state) and still fireonSessionResolved(start_task binding, review 1.3);turn_starteduses the Host-returnedturnIdfor the optimistic message and the settle poll.apps/desktop/src/renderer/app-shell-session-events.ts—steering_messagerefreshes the transcript (events come from the Host observer's queue projection).Removed (embedded-only machinery from the earlier revision)
session-send-resolve.tshelpers) + followup drain insessions-ipc-main.ts.hasPendingSteeringpeek + same-turn continuation pass (backend-types/runtime-kernel/ai-sdk-backend/agent-flow/ai-sdk-flow/invocation-context/runtime-runner).mid-turn-send.spec.ts); the Host-mode E2E journey needs feat(runtime-host): complete M5 production cutover #2420's isolated entrypoint.Verification
turn.start), renderer queued-send rollback,steering_messagerefresh — all pass. The remaining suite failures on this machine are pre-existing environment issues (Windows temp-sqlite EBUSY and a storage-root markerinomismatch) reproduced on untouched code and the other local worktree.mergeable: true.Notes