fix(desktop): keep Side Conversation follow-ups visible - #4901
fix(desktop): keep Side Conversation follow-ups visible#4901liuxiaocs7 wants to merge 6 commits into
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed this PR at head 19d0d8ef (18 files, +1133/−119) against base 90014db9.
What the change does: Side Conversation follow-ups now behave like the main conversation while a Turn is running: Enter queues a next-turn follow-up that renders immediately, Shift+Enter steers the active turn; the Host queue is projected into the transcript with the existing promote/edit/reorder/retract controls; settled replies survive Turn handoffs; cancelled optimistic messages are retired after reconnect/reseed. The queue projection shared with the main surface is extracted into deriveMessageQueueProjection (message-queue-projection.ts), and the cancellation-proof batching moves from the renderer into the IPC layer.
Design: The problem is real and the fix aligns the side chat with the main conversation's established contract (requestedMode routing in app-shell.tsx:1917; the Composer queue controls pre-exist in packages/ui). No new authority is introduced: the Host queue (queue_update from the session projector) remains the single source of truth; the renderer only projects it.
Function (production path): Enter/Shift+Enter routing (quote-companion-context-compaction.ts:55-61) → submitFollowUp port → submitMessage(sessionId, placement, ...) (create-workbar-services.ts:50-66) → IPC → Host. next_turn submissions do not occupy the single in-flight admission slot, so several follow-ups can queue; a queued outcome releases the slot and a started outcome adopts the Host-named Turn (use-quote-companion.ts). The three race branches are covered by tests.
Complexity: The extraction is faithful — I compared it line-by-line with the previous inline logic in app-shell-session-events.ts; the extra state === 'queued' filter on follow-up entries is a no-op because the protocol types follow-up entries as always-queued (QueuedMessageSnapshot in packages/runtime-host/src/protocol/message.ts). The renderer-side pagination loop in session-workspace-actions.ts is deleted and consolidated into the IPC handler.
Tests: 8 new hook-level tests drive the production useQuoteCompanion with mocked ports; each fails on the old behavior (no queue concept, single admission slot blocking multiple queues, settled messages dropped on Turn handoff). IPC tests cover batching, duplicate rejection, and invalid input. CI test is green on this exact head.
P3 (recorded, non-blocking): the 4096-entry cap on the cancellation-proof query is 64× the queue cap; only a constructed over-limit pending set would hit it, and the failure degrades to stale optimistic rows until the canonical echo (pathological, not reachable in normal use).
Not verified: I did not run the suites locally (CI is green on the exact head); the queue UI interactions in a real Electron shell (drag-reorder etc.) are not covered by automated tests — a manual pass (Enter to queue, Shift+Enter to steer, reorder/delete) is worth doing.
No P0–P2 findings. This is a behavior-changing fix; I am not approving — the merge decision belongs to humans.
Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the fix. I rechecked head 19d0d8efe701fd226470a88d6c6d8504deab08eb and found one recoverable follow-up/reconnect race, detailed inline. The Host remains the right admission authority; the issue is how the renderer reconciles a late admission receipt with the Turn's current state.
The exact-head CI check is green. This finding was traced through the production retry, durable-receipt, and renderer event paths; I did not run a live Desktop reproduction.
AI review disclosure: reviewer agents assisted with this review, and the coordinating agent cross-checked the finding against the source. This is not independent human verification.
中文
感谢修复。本轮复核发现一处可恢复的追问/重连竞态,已放在行内。Host 仍然是正确的准入权威;问题在于 renderer 将晚到的准入收据当作当前仍在执行的证明。当前 head 的 CI 已通过;本次依据真实重试、持久收据和事件消费路径进行源码核对,未运行实时 Desktop 复现。本评审由 AI reviewer 协助,并经主代理复核,不代表独立人工验收。
There was a problem hiding this comment.
AI review disclosure: this correction was made by the coordinating AI agent after checking the independent reviewer’s storage-path evidence.
Withdrawn — the normal queued handoff does publish a durable admission.
Thanks, and apologies for the incorrect finding. A further source check found the missing link in my analysis: sqlite-session-metadata-store.ts:2410-2419 adds steeringEventId: steeringProof?.eventId ?? messageId when committing the follow-up handoff. noteDurableTranscriptMessages() can therefore emit message_admission, and the hook adopts that Turn. The normal-path P1 above does not hold and requires no change.
The earlier late-started P2 is separate: a receipt can arrive after its Turn has already settled. I have also corrected that comment so it does not rely on the incorrect generalization about root admissions.
中文
撤回:正常排队交接会发布持久化准入事件。
抱歉,这条意见有误。再次核对发现,我漏查了存储写入点:sqlite-session-metadata-store.ts:2410-2419 在提交 follow-up handoff 时补充 steeringEventId: steeringProof?.eventId ?? messageId,因此 projector 可以产生 message_admission,hook 也会接管对应 Turn。上述普通路径 P1 不成立,无需为它修改代码。
此前“迟到 started”的 P2 是另一个问题:收据可能在 Turn 已经收束后才到达。我也已修正那条评论的描述,去掉关于 root admission 的错误泛化。
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks. After checking the storage-written admission and the observer replacement path separately, one additional P2 remains specifically on reconnect to an already-terminal successor; details are inline. The earlier normal-online P1 has been withdrawn and resolved. The two remaining P2 cases concern recovery/late-response reconciliation, not a missing admission on every queued handoff.
Reviewed head 19d0d8efe701fd226470a88d6c6d8504deab08eb. This is a source-traced finding; I did not run a live Desktop reproduction. AI reviewer agents assisted, and the coordinating agent checked the producer/consumer paths.
中文
感谢。分别核对存储层补写的 admission 与 observer replacement 后,确认重连至已完成后继 Turn 时还有一条 P2,详见行内。此前正常在线路径的 P1 已撤回并关闭。当前两条 P2 都涉及恢复/迟到响应的对齐,而非所有正常排队交接都缺少 admission。
本次核对相同 head,依据源码生产链路,未运行实时 Desktop 复现。由 AI reviewer 协助并经主代理核对生产者与消费者。
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for bringing Side Conversation follow-ups onto the shared queue projection. Two P2 recovery issues remain on this head: a late started receipt can re-arm an already settled turn, and a terminal successor can lose its admission on reseed, hiding its completed reply. Please address both before approval, preserving canonical Host ownership and covering the two event orderings in regressions. The withdrawn normal-handoff P1 requires no action.
This is an AI-assisted review follow-up, not independently human-verified.
中文
感谢让侧边对话复用队列投影。当前 head 仍有两处 P2:迟到 started 收据会重新激活已结束 Turn;重建观察时终态后继 Turn 的准入可能丢失,使已完成回复不可见。请在批准前修复两处问题,保持 Host 权威,并用回归覆盖对应事件顺序。已撤回的正常交接 P1 无需处理。这是 AI 辅助跟进,未经独立人工验证。
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the follow-up visibility work. The combined correctness and simplification audit found one remaining transient-state lifecycle issue; details are inline. AI-assisted review by Codex.
中文
本轮结合正确性与简化审查,发现一处临时状态未退休的问题,详见行内评论。|
Thanks for bringing Side Conversation follow-ups onto the existing Host queue. After tracing both chat surfaces at The Host still owns admission, queue ordering, consumption, retraction and durable execution. The concern is a second renderer implementation of the same transient-message lifecycle:
Could we reuse the existing transient projection/reconciliation functions from both production callers, and consolidate any additional common reconciliation rule at that same seam instead of introducing another side-chat controller or a new general-purpose framework? If feature-layer import rules require moving those functions to the application contracts seam, move the existing implementation and update both callers, rather than keeping another copy or a forwarding facade. Each Session can keep its own projection instance; the transition rules should have one implementation. Side chat should pass its actually renderable, own-Turn-filtered durable messages so unknown-admission visibility is preserved. The whole main-chat workspace controller is not a drop-in dependency: it also owns active-session selection, transcript paging and workspace UI. Keep those concerns separate, and keep the side-specific fork creation, quote handling, fork cleanup and visible-history boundary local. Those differences do not require duplicating queue merge or durable retirement. For reconnect ownership, consume the already existing This refines the fix direction for the existing P2 comments; it is not a claim that this PR adds a second Host scheduler, and it is not a request to rebuild all of Side Conversation. Shared-rule tests plus a small set of side-panel integration cases should then cover the behavior without parallel state-machine test suites. AI-assisted review by Codex; the main/side production callers and existing shared helpers were checked directly. 中文侧边对话需要消费 Host 的队列能力,不需要另一套队列生命周期实现。Host 执行权威仍在;问题在 renderer:主对话已有 queue merge 和 durable retirement 的共享函数,侧边却重写了数组合并,并把真正退休误写成仅过滤显示,已产生 P2 行为漂移。只共享 entry 转换还不够。 建议让两个生产调用方复用现有 transient projection/reconciliation 接缝,共同规则只实现一次,每个 Session 保留自己的投影实例;侧边传入自己已经实际可显示的 durable 消息,保留 unknown-admission 保护。主对话整个 workspace controller 绑定选中 Session、分页和外壳状态,不能直接搬过去;fork 创建、引用、清理和历史显示边界仍留在侧边。重连归属使用已有 queryMessageExecutions,不继续堆最新 root 特判。这个意见是现有 P2 的设计修法,不是要求重做整个侧边对话,也不是宣称新增了 Host 调度器。 |
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
Generated-by: Codex
7a2a0b3 to
7e21149
Compare
Summary
Make Side Conversation follow-ups behave like the main conversation while a Turn is running:
Fixes #4900
Verification
npm --workspace @maka/desktop run build:workspace-depsnpm --workspace @maka/desktop run build:testnode --test apps/desktop/dist/main/__tests__/quote-companion-retry.test.js apps/desktop/dist/main/__tests__/transient-message-projection.test.js apps/desktop/dist/main/__tests__/workbar-services-adapter.test.js— 60/60 passnpm --workspace @maka/desktop run typechecknpm --workspace @maka/desktop run check:architecture— 101/101 passnpx knip --workspace apps/desktopnpx biome check --diagnostic-level=error <changed files>git diff --check origin/main...HEADThe focused tests exercise the user-visible optimistic/queued projection, Enter vs Shift+Enter routing, multiple queued follow-ups, queue retraction, Turn handoff, and reconnect cancellation cleanup.
AI use
Select exactly one:
Tool(s) and scope: OpenAI Codex diagnosed the state/admission races, implemented the Desktop renderer and adapter changes, added regression coverage, and prepared the issue/PR text. The commits retain
Generated-by: Codextrailers.Checklist
Does this PR entail a change in behavior?