test(desktop): fence active turn state in streaming-remount e2e

[rookepoole]

What changed

• Re-ordered composer readiness check (getByRole('button', { name: '发送' }).toBeEnabled()) to precede composer.fill(...) in apps/desktop/e2e/streaming-remount.spec.ts so React state transitions after an interrupted turn settle before filling text.
• Added an expect.poll fence verifying runningTurnIds.length > 0 on the session before expecting the Stop button to be rendered, ensuring active Turn state is fenced in durable session state.

Addresses #3177

---

Generated from a bounded immutable repository snapshot by PARS-Agent using Gemini 3.6 Flash. Tests listed above are recommendations unless GitHub checks report otherwise.

[coderabbitai]

📝 Walkthrough

Summary

• Moves the composer readiness check before composer.fill(...).
• Adds an expect.poll fence that waits for runningTurnIds.length > 0 before checking the Stop button.
• This prevents the test from checking UI state before React settles after an interrupted turn.
• The change extends the existing end-to-end test path. It does not create a parallel path.
• The diff is a small, coherent synchronization fix. The added waits are necessary to reduce timing-dependent failures.
• No code or test can be safely deleted based on the current diff without weakening regression coverage.

Validation

• The diff changes only apps/desktop/e2e/streaming-remount.spec.ts.
• No public or exported entities change.
• Required check results are unverified because no direct check output is available.

Review-relevant risks

No protected-area effect was identified in the current diff. The person performing the merge reviews the final diff, and a maintainer makes the final determination.

Walkthrough

The interrupted-conversation end-to-end test now checks send-button readiness before submission and confirms that the original session has an active running turn afterward.

Changes

Streaming remount test

Layer / File(s)	Summary
Synchronize prompt submission and active turn apps/desktop/e2e/streaming-remount.spec.ts	The test waits for the send button before submitting the large-response prompt. It then polls for an active running turn in the original session.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: 🔵 Low · up to 60c4e

The change only affects end-to-end synchronization, but the new check may proceed before a genuinely active turn is established, reducing the test’s ability to catch remount timing regressions. The PR is mergeable with owner awareness or a follow-up to align the predicate with the renderer’s active-turn condition.

Suggested reviewers: m4n5ter, astro-han

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the code changes and references issue #3177, but it omits the required Verification, AI use, and Checklist sections.	Add the required Verification, AI use, and Checklist sections, including actual test results and the applicable AI-use selection.
Ai Use Disclosure	⚠️ Warning	The PR description selects neither AI-use declaration, but its sole introduced commit contains Generated-by: gemini-3.6-flash.	Select exactly one declaration, name gemini-3.6-flash and its scope if applicable, and refer to Human ownership and AI attribution in CONTRIBUTING.md; preserve the trailer through squash or amend.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change to fence active turn state in the desktop streaming-remount end-to-end test.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-code-review]

PR Summary by Qodo

Fence active-turn state in streaming-remount E2E

🧪 Tests 🐞 Bug fix 🕐 Less than 10 minutes

AI Description

• Waits for composer readiness before entering the post-interruption prompt.
• Polls durable session state for an active turn before asserting Stop controls.
• Prevents React transition timing from making streaming-remount E2E coverage flaky.

Diagram

sequenceDiagram
  participant T as E2E Test
  participant S as Session API
  participant C as Composer UI
  participant R as Turn State
  participant A as Action Controls
  T->>S: Confirm no running turn
  T->>A: Wait for Send enabled
  T->>C: Fill and submit prompt
  C->>S: Start turn
  S->>R: Persist running turn ID
  T->>S: Poll for running turn
  S-->>T: Active turn confirmed
  R-->>A: Render Stop button
  T->>A: Assert Stop visible

Loading

High-Level Assessment

The PR’s explicit state fences are the appropriate approach because they synchronize against observable readiness and durable turn state rather than timing assumptions. Waiting only for the Stop button or adding fixed delays would preserve the race and make the test more load-dependent.

Files changed (1) +8 / -1

Tests (1) +8 / -1

streaming-remount.spec.tsFence prompt submission and Stop assertion on settled state +8/-1

Fence prompt submission and Stop assertion on settled state

• Moves the Send-button readiness check before filling the post-interruption prompt so pending React transitions can settle. Polls the session’s durable 'runningTurnIds' until a turn is active before asserting that the Stop button appears.

apps/desktop/e2e/streaming-remount.spec.ts

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 403ccef5-6b66-451c-8860-da2ae2653cf3

📥 Commits

Reviewing files that changed from the base of the PR and between be3fdd1 and 60c4edd.

📒 Files selected for processing (1)

• apps/desktop/e2e/streaming-remount.spec.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Align the poll with the active-turn predicate.

The runningTurnIds.length > 0 check can pass when the only ID is armedTurnId. The renderer treats that state as inactive when turnPhase is undefined. Assert a running turn different from armedTurnId, or require a new running-turn ID compared with the pre-submit state.

As per path instructions, this check targets a concrete synchronization-contract failure.

Source: Path instructions

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Astro-Han]

Thanks for the attempt — but this change makes the test deterministically fail, the opposite of the flake fix you're going for. Please hold on merge until verified.

Conclusion: FAIL — P1 + P2, both need resolution before merge.

P1 — the reordering inverts the assertion: the send button is disabled on an empty draft, so expect(发送).toBeEnabled() now times out every run. The spec now asserts the button before composer.fill(...) (diff at spec ~128-131). At that point the composer is guaranteed empty: sendCurrent clears the input unconditionally after a successful submit (packages/ui/src/composer.tsx:1078-1080 textPort.setValue('')), and the first hold-open turn has already been submitted in this test — nothing refills the draft (draft persistence only restores new-task:-prefixed keys). The send button's enabled state depends on a non-empty draft (composer.tsx:1220-1224 sendDisabled = ... || !text.trim() || ...), and after the interrupted turn streaming=false with empty text, the slot renders a disabled 发送 (composer.tsx:1796-1816). So this turns an occasional flake into a guaranteed 20s timeout on every CI run. The original order (fill → assert enabled) was correct — the fence must evaluate after the text exists. Quick check: run the single spec (playwright test streaming-remount.spec.ts -g 'keeps a completed reply') on this branch — it should time out deterministically.

P2 — the new durable-poll fence likely won't fix #3177's failing case and is unverified as a fence. The poll is honest (it reads the kernel's live runningTurnIds via listSessions, re-queried per call, so it can't hide a bad renderer — the later 停止 assertion still catches renderer failures). But in the #3177 failure shape — user message visible, 停止 not appearing — the user message only renders after the host accepted the turn, at which point durable runningTurnIds is already >0, so the poll passes immediately and the 停止 assertion still times out. It sharpens the failure boundary but doesn't address the renderer subscription/projection lag the issue itself suspects. The change as merged has no demonstrated effect on the flake: the PR body states tests are "recommendations unless GitHub checks report otherwise," and gh pr checks shows no test run (only CodeRabbit). Requirement: revert/fix the reorder (P1), run the spec locally multiple times on this branch to show the flake is actually gone (or that the failure mode is now correctly reported), and state what the fence is expected to prove (host-side start vs renderer-side projection are two different witnesses — this fence proves the former only).

P3 (optional): the new poll is a structural twin of the existing runningTurnIds === 0 poll in the same file (same evaluate shape, same 20s timeout) — a small runningTurnIdsOf(id) helper would dedupe; no behavioral impact.

---

AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash). The subagent traced the composer's send/disabled contract and the diff statically; the P1 timeout is a prediction from that contract (the spec was not executed under read-only constraints), so please confirm with a single-spec run. Please weigh these findings with your own judgment.

中文摘要（AI 辅助审查）

结论：FAIL（P1 + P2，合并前需解决）。P1：重排让断言反转——发送按钮在空草稿时恒为 disabled，现在 expect(发送).toBeEnabled() 在 composer.fill() 之前断言，必然每次运行 20s 超时（从偶发 flake 变成确定性红）。原因：sendCurrent 提交成功后无条件清空输入（composer.tsx:1078-1080），测试里第一轮 hold-open turn 已提交、draft 无回填机制；发送按钮 enabled 依赖草稿非空（composer.tsx:1220-1224），中断轮结束后 streaming=false 且 text 为空，渲染为 disabled 的发送按钮。原顺序（fill 后再查 enabled）才是对的——fence 必须在文本存在之后求值。验证方式：单跑该 spec 应确定性超时。P2：新增的 durable 轮询大概率治不了 #3177 的失败类别——用户消息可见意味着 host 已接受 turn、runningTurnIds 早已 >0，轮询立即通过，随后 停止 断言照样 20s 超时；轮询只证明 host 侧状态，不能证明 renderer 订阅/投影通道健康（#3177 疑点所在）。且 PR 无实际测试运行记录（gh pr checks 只有 CodeRabbit），不能声称 fixes #3177。要求：回退/修正重排（P1）、本地连续多轮运行该 spec 证明 flake 真的消失、说明 fence 证明的是 host 侧启动而非 renderer 侧投影。P3：新轮询与文件内既有轮询是结构孪生，可抽 runningTurnIdsOf(id) 助手去重。

[M4n5ter]

Thanks for taking the time to investigate #3177 and propose a synchronization fence.

We ultimately traced the intermittent failure to a production renderer race rather than a missing readiness check in the test: session settlement could combine an older catalog result with a newer live-Turn projection and incorrectly clear the Stop witness.

#3189 has now been merged with the root-cause fix. It binds settlement to the accepted authoritative list read, uses a per-projection compare-and-swap, projects observer-owned running Turn identities, and adds deterministic regression coverage.

This supersedes the test-only approach here, so we can close #3178 without asking you to make further changes. The current ordering would also check whether Send is enabled before filling the empty composer, as noted above, but there is no need to spend time revising it now unless an independent test improvement remains.

Thanks again for helping investigate the failure.