Uh oh!
There was an error while loading. Please reload this page.
fix(desktop): observe the first turn before admission - #4018
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
Diagnosis is right and the retry is the real fix: onSeedError already existed in the preload contract but nothing consumed it, so a failed seed was final — the renderer never learned the live Turn identity, and the bubble rendered as history while Stop stayed available. The generation fencing is correct (attempt captured per subscribe, compared in every callback, teardown increments it), and the +31 preload lines are entirely inside process.env.MAKA_E2E === '1', so no production surface widens.
Approving; the finding below is non-blocking.
P2 — the first-send barrier has no timeout.activateSessionForFirstSend resolves only via completeObservationSeed and rejects only when activeId changes, while seed failure retries with no attempt limit. If the seed keeps failing and the user stays put, the await at app-shell-chat-actions.ts:485 never settles: they press send and nothing happens — not even their own message, since showTransientUserMessage runs after the await — while the already-created empty Session sits in the sidebar. Only exit is navigating away, and nothing says so. Class 2, recoverable, so P2. A bounded wait that surfaces the seed error would keep the fix without the silent stall.
One question that may remove the barrier.sessions:observe appears to seed active assistant streams (runtime-host-desktop-candidate.test.ts:586). If so, a successful retry restores the live Turn identity on its own and the ordering barrier is belt-and-braces. Which window can the seed not cover — the gap where the Message is admitted but the Turn hasn't started yet? If that's it, one sentence in the body is worth it; if not, dropping the barrier removes the deferred map, the activeId reject effect, and the P2 above with it.
AI-assisted review: I traced the barrier's resolve/reject paths, the fencing, and the retry loop against the PR head, and confirmed the preload addition is E2E-only. No tests run. AI review is not independent human review.
简体中文
诊断准确,重试是真正的修复:onSeedError 早就在契约里但无人消费,seed 失败即终局。世代校验写法正确,+31 行 preload 全在 MAKA_E2E 分支内,未扩大生产面。已 Approve,下面这条不阻塞。
P2 — 首条消息屏障没有超时。 唯一 resolve 是 seed 成功,唯一 reject 是 activeId 变化,而 seed 失败是无上限重试。若持续失败且用户停留原地,await 永不 settle:点发送毫无反应,连自己那条消息都不显示(showTransientUserMessage 在 await 之后),已创建的空会话留在侧边栏,唯一出路是切走且无任何提示。建议改成有界等待并暴露 seed 失败。
一个可能让屏障消失的问题。sessions:observe 似乎会 seed 当前活跃的 assistant 流。若如此,重试成功即可恢复 live Turn 身份,屏障只是冗余。seed 覆盖不到的是哪个窗口——消息已 admit 但 Turn 未开始那段?若是,值得写进 body;若不是,删掉屏障可连带移除 deferred map、activeId reject effect 和上面那条 P2。
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
c262d9f to
e4d7826Compare
jackwener
left a comment
There was a problem hiding this comment.
Approved at exact head e4d7826771a8ccf97af4038239d65059d8b57968. I found no remaining P0–P2 issues.
The previous unbounded-wait finding is closed. A new Session still waits for its event observation seed before its first Message can reach Runtime Host, so the renderer cannot miss the live Turn identity, while Runtime Host remains the sole authority for Message admission and Turn identity. Repeated seed failures are generation-fenced, and the first-send wait now ends after 30 seconds: success, navigation, unmount, and timeout each settle the waiter once and invalidate stale timer callbacks. A timeout happens before Message admission, removes only the newly created unsent Session, preserves the draft, and reports the failure only if the user is still on the waiting surface.
I also verified that the barrier is necessary: an active-stream snapshot cannot reconstruct an assistant segment that has already completed while the root Turn is still running.
Verification completed on this head:
- the current-main synthetic merge is clean, with no files changed by both sides;
- the root test build passed;
- Desktop tests passed 1612/1612;
- 84 focused first-send, admission, observation, and Host-recovery tests passed;
- Biome and
git diff --checkpassed for the affected files; and - the hosted
testcheck completed successfully.
Non-blocking P3: the new unit test injects a barrier rejection and verifies cleanup, but it does not drive the production 30-second timer or its success/unmount races with fake time.
Posted by an automated review agent operated by @WAWQAQ. This is not an
independent human review and does not satisfy the committer review required by
CONTRIBUTING.md. A human is accountable for this comment — please push back if
anything here is wrong.
简体中文
本条评论由 @WAWQAQ 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md
所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。
Uh oh!
There was an error while loading. Please reload this page.
* fix(desktop): observe the first turn before admission Generated-by: OpenAI Codex * fix(desktop): bound first-send observation wait Generated-by: OpenAI Codex
English
Summary
Root cause
The original #3177 fix protected live projection during Stop and remount. The recurrence happens earlier: the first Message could reach Runtime Host before React activated the new Session and subscribed to its event stream. If that first observation seed failed, the durable transcript eventually showed the assistant text and the Session remained running, but the renderer never learned the live Turn identity. The result looked like a completed historical bubble, so
.maka-bubble-streamingwas absent even though Stop was still available.This change makes a completed observation seed a prerequisite for only the first Message of a newly created Session. A transient seed failure reconnects with generation fencing. The barrier is bounded at 30 seconds; leaving the Session or timing out cancels the wait and removes the still-unsent Session while preserving the draft. Runtime Host remains the sole owner of Message admission and Turn identity.
Verification
streaming-remount.spec.ts: 4/4git diff --checkCloses#3177.
Generative AI disclosure
OpenAI Codex made a substantive contribution by investigating the race, implementing the fix and regression coverage, and running the verification above. The commit includes the required
Generated-by: OpenAI Codextrailer.简体中文
摘要
根因
原先针对 #3177 的修复保护的是 Stop 和 remount 期间的 live projection。本次复发发生得更早:首条 Message 可能在 React 激活新 Session、订阅事件流之前就进入 Runtime Host。若第一次 observation seed 失败,持久 transcript 最终仍会显示助手文本,Session 也仍处于运行状态,但 renderer 永远拿不到 live Turn 身份。因此界面把回复显示成历史消息,
.maka-bubble-streaming不存在,Stop 却仍可见。本次修改只对新建 Session 的首条 Message 增加 observation seed 屏障。临时 seed 失败会带 generation 隔离地重连;屏障最长等待 30 秒,用户离开 Session 或等待超时时都会取消等待并删除尚未发送成功的 Session,同时保留草稿。Message admission 与 Turn 身份仍只有 Runtime Host 一个权威来源。
验证
streaming-remount.spec.ts:4/4git diff --check通过关闭 #3177。
生成式 AI 披露
OpenAI Codex 对根因调查、修复实现、回归测试和上述验证作出了实质贡献。提交中包含所要求的
Generated-by: OpenAI Codextrailer。Posted by an automated development agent operated by @M4n5ter. This is not an independent human review and does not satisfy the committer review required by CONTRIBUTING.md. A human is accountable for this pull request — please push back if anything here is wrong.
简体中文
本 PR 由 @M4n5ter 运行的自动化开发程序发布。它不构成 CONTRIBUTING.md 所要求的独立人类审查,也不能替代人类审查。有人类对此 PR 负责,如有错误请直接指出。