fix(integration): harden flaky interactive read-then-write test - #7105
Merged
Conversation
The interactive read-then-write test read version.txt once immediately
after the write/edit tool call was logged and asserted strict equality
with '1.0.1'. This was flaky in two ways: the model may append a
trailing newline ('1.0.1\n'), and the file may not yet reflect the new
version when the tool call is observed (e.g. a first edit whose
old_string did not match is still logged by name and retried within the
turn). Poll the file until it contains '1.0.1' instead, matching the
lenient assertion already used by the non-interactive sibling test.
doudouOUC
approved these changes
Jul 17, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at 4b0050a. Correct, well-scoped, test-only hardening that fixes a genuine race — LGTM.
Root cause is sound
waitForAnyToolCall only matches the tool name in telemetry, which is logged the moment the model issues the call — before the write has necessarily landed, and a first edit whose old_string doesn't match is still logged by name and retried within the same turn. So reading the file once right after was racy. Polling until the content matches absorbs both the trailing-newline and the in-turn-settling variants.
Checks I ran
rig.poll(...)→expect(result).toBe(true)matches the established pattern (e.g.ctrl-c-exit.test.ts).- Writes go through atomic rename, so
version.txtalways exists — thereadFileSyncin the poll predicate can't hit ENOENT here. - Worst-case failure path (~30s tool wait + 60s CI poll ≈ 90s) stays well within the 5-min integration
testTimeout, soprintDebugInfostill fires before any vitest timeout. - Test-only; no core paths touched.
Non-blocking nits
- PR description is now stale. It says the assertion becomes
readFile(...).includes('1.0.1')and "matches the lenienttoContain('1.0.1')" of the sibling test, but the code (after the tighten commit) is.trimEnd() === '1.0.1', which is actually stricter than the sibling. The code is fine — arguably better, since it asserts the file is exactly the new version — please just update the description to match. .trimEnd() === '1.0.1'is stricter than the sibling'stoContain. Fine for this trivial version file; just flagging the divergence in case exact-match ever reintroduces rare flakiness.- Optional: the poll predicate reads the file without a
try/catch, unlikewaitForTelemetryReady. Not reachable here (file is pre-created + atomic writes), so safe to leave as-is.
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: This addresses a real flaky test — the interactive read-then-write E2E test fails in CI due to trailing newlines and in-turn write settling. The two failure modes are well-described and consistent with known E2E timing patterns. Direction: Aligned. Test hardening for CI flakes is core maintenance. The polling approach matches the established Size: Not applicable — test-only change, no production code touched (0 production lines, +15/-2 test lines). Approach: Scope feels right — one file, one assertion replaced. Using Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个真实的 flaky 测试——交互式 read-then-write E2E 测试在 CI 中因尾部换行符和回合内写入时序问题而失败。两种失败模式描述清晰，与已知的 E2E 时序问题一致。 方向：对齐。测试加固是核心维护工作。轮询方案与 6 个其它集成测试（ 规模：不适用——仅测试改动，未触及生产代码（0 生产行，+15/-2 测试行）。 方案：范围合理——一个文件，一处断言替换。实际 diff 中使用 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Code review: Clean diff. The Before (main branch, old code)Old code passed this run (it's flaky, not deterministic — fails when the model appends After (PR code applied)PR code passes. The ~1s overhead is the polling settling time — expected and acceptable for a flake fix. The real value isn't visible in a single pass; it's the robustness against the two documented failure modes (trailing newline and in-turn edit retry). 中文说明代码审查： diff 干净。 测试验证： 旧代码（main 分支）本次通过（12.32s），但该测试本身是 flaky 的。应用 PR 后测试通过（13.22s），约 1 秒开销为轮询等待时间，对 flake 修复来说可以接受。真正的价值在于对两种已记录失败模式（尾部换行和回合内 edit 重试）的鲁棒性。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage, no reservations. This is exactly what a flaky-test fix should look like: minimal scope (one file, one assertion), established pattern ( 中文说明置信度：5/5 — 各阶段均无问题，无保留意见。 这是一个典型的 flaky 测试修复：范围最小（一个文件，一处断言），使用既有模式（ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
approved these changes
Jul 17, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Hardens the interactive
should perform a read-then-write sequence in interactive modeE2E test. Instead of readingversion.txtonce immediately after the write/edit tool call is observed and asserting strict equality with1.0.1, the test now polls the file until it contains1.0.1. The existing assertion confirming awrite_file/edittool call was made is kept.Why it's needed
This test was flaky in CI in two ways, observed across the run and its retry:
1.0.1\nwhile the test expected1.0.1undertoBe.1.0.0when checked. The tool call is logged by name as soon as the model issues it, but a firsteditwhoseold_stringdoes not match is still logged (andwaitForAnyToolCallonly checks the tool name) and may be retried within the same turn, so the file is not guaranteed to reflect the new version the instant the call appears.Polling until the file contains
1.0.1absorbs both the trailing newline and the in-turn settling, while still failing if the model never actually updates the file. This matches the lenienttoContain('1.0.1')assertion already used by the non-interactive sibling test incli/file-system.test.ts.Reviewer Test Plan
How to verify
This is a non-user-visible test-only change. Confirm the assertion now tolerates a trailing newline and waits for the write to land:
readFile+toBe('1.0.1')is replaced byrig.poll(() => rig.readFile(fileName).includes('1.0.1'), 15000, 200)followed by an assertion on the poll result.rig.pollusage matches the established pattern in other interactive tests (e.g.ctrl-c-exit.test.ts).Evidence (Before & After)
N/A (test-only change, no user-visible / TUI behavior).
Tested on
Environment (optional)
N/A — test-only change; relies on CI E2E.
Risk & Scope
includes('1.0.1')), consistent with the non-interactive sibling test; it still fails if the file is never updated to1.0.1.Linked Issues
None.
中文说明
本 PR 做了什么
加固交互式 E2E 测试
should perform a read-then-write sequence in interactive mode。原测试在观察到 write/edit 工具调用后立即读一次version.txt并用toBe严格断言等于1.0.1；现改为轮询该文件直到其包含1.0.1。原有的"确认发起了 write_file/edit 工具调用"的断言保留。为什么需要
该测试在 CI 中有两种 flaky 表现（分别出现在首次运行与重试中）：
1.0.1\n，而测试用toBe期望1.0.1。1.0.0。工具调用一旦由模型发起就会按名字记录，但首次edit若 old_string 未匹配也会被记录（且waitForAnyToolCall只校验工具名），并可能在同一回合内重试，因此调用出现的那一刻文件并不保证已反映新版本。轮询直到文件包含
1.0.1可同时吸收尾部换行与回合内的写入时序问题；若模型始终未把文件更新为1.0.1，测试仍会失败。这与非交互式同名测试cli/file-system.test.ts中已使用的宽松断言toContain('1.0.1')保持一致。评审验证计划
如何验证
这是仅改测试、对用户不可见的变更。确认新断言能容忍尾部换行并等待写入落盘：
toBe('1.0.1')"被替换为rig.poll(() => rig.readFile(fileName).includes('1.0.1'), 15000, 200)，随后对轮询结果做断言。rig.poll的用法与其它交互式测试（如ctrl-c-exit.test.ts）中的既有模式一致。前后对比证据
N/A（仅测试改动，无用户可见 / TUI 行为）。
测试环境
环境（可选）
N/A —— 仅测试改动，依赖 CI E2E。
风险与范围
includes('1.0.1')），与非交互式同名测试一致；若文件始终未更新为1.0.1仍会失败。关联 Issue
无。