Synced from GitHub
test: remove the timing races behind recurring CI and release failures#22523
Release and CI kept failing on the same handful of tests that pass on rerun. Each one had a real race; this fixes the cause in each instead of raising timeouts. (The release runner split landed separately in #22524. This branch is merged with current main, including #22533.)
Web
The mouse leaked between tests. Playwright's mouse stays where the last test left it, so a component that mounts under it gets
pointerenterand:hoverbefore the test runs. That is theComposerBannerStackflake (about 14 failures today, #22324) and why #21901 had to park the pointer for one test.render()now parks the mouse off the page when its screen disposes, via aresetPointerbrowser command. It runs on dispose, not before mount: anawaitbefore the first mount broke 21 tests that set up state just beforerender(). The test-only workaround from #22508 is removed.Diff workers raced Pierre's 10s startup limit. Each worker loads Shiki on start, about 1.5s idle and over 10s on a loaded runner.
PullRequestDetailPanelnever needed real workers, so it now uses Pierre's main-thread fallback (first "Add to agent" on one CPU: 7–20s before, 4–6s after), and the 30s timeout from #22508 is gone.StyledDiffCodeViewraises the startup limit except in the test of that limit, and its queries stay in the test's own container, because a timed-out test keeps running and later tests clicked its leftover buttons.ProjectFaviconPickerDialog. The atom registry drops an unsubscribed atom on its next task. A row asks for its URL while rendering but subscribes after commit, so a URL published in between was lost. The fake asset atoms are nowkeepAlive. Under CPU load: 2/6 failed before, 10/10 passed after.MessagesTimelineMermaid copy. Only the first Mermaid test failed: it paid Mermaid's cold load (over 10s on the loaded release runner). The file now loads and renders Mermaid once before its tests.ProjectSettingsPanelscope. One test drove three menu passes through navigation and ran out of its 15s budget under load. It is now three tests that each mount at their own URL. This drops the check that the menu updates on navigation inside one mount.
Server
ClaudeAdapterV2steer hang (all 9 of today's 120s timeouts). A steer waits up to 5s for foreground subagents to move to the background, but that timer starts after the steer reads skill files from disk, so the test's clock move could land before the timer existed. #22533 made the test tick the clock up to 100 times instead, which still fails when the file read outlasts the ticks. The adapter now takes the deadline as the steer's first step, before any I/O, so one 5s clock move always covers it. The test starts the steer at once and awaits aDeferredfor the background request instead of polling, and keeps #22533'sDeferredfor the late answer. With a forced 50ms file-read delay plus 50 scheduler yields, all 6 steer cases still pass.transferBudgetV230s timeout. Shell streams group and skip unchanged shells, so a split turn may never send an item at the last sequence. The test waits for the final shell instead.OpenCodeServerLedger. The fixture read the process before the inner shell had started; it now waits for areadyline.killGroupignores onlyESRCHwhen the group is already gone.AcpAdapterV2detached PIDs. The mock agent exited before the detached fixture wrote its PID file; it now waits for apublishedline.
Verification
- Each race was reproduced first: forced delays gave the exact CI errors before each fix and passed after.
- Stress runs on one or two CPUs (
taskset) after the fixes:ClaudeAdapterV210/10,transferBudgetV220/20 (9/12 failed before),AcpAdapterV210/10 (8/10 failed before),OpenCodeServerLedger16/16, diff worker files 3/3 runs,ComposerBannerStack5/5. - Full web package: 546 files, 8509 tests passed. Touched server and provider files: 484 tests passed.
- Targeted lint, format, and typecheck are clean.
Made with Claude Opus 5.5 in Claude Code (T3 Code).
🤖 Generated with Claude Code
Checks
23 successful · 11 skipped
Added 1 commit
- test(web): park the pointer after each browser test; release runs web tests on their own runnerbace03d
Synced from GitHub
Approvability
Verdict: Approved at 917b698
Macroscope's review found this PR approvable — This is primarily a focused CI/test race-remediation change. Its sole production-path edit makes Claude steer timeout accounting bounded from request start, while the remaining edits are isolated test fixtures and browser-test infrastructure.
Notes:
- No code objects were reviewed. Approvability was decided on eligibility alone.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
Thread transfer impact
✅ Thread transfer remains within every enforced ceiling.
| Provider | Metric | Main baseline | This PR | Impact | PR ceiling | |
|---|---|---|---|---|---|---|
| Codex | Total thread wire | 5.0 KiB | 5.0 KiB | −7 B (−0.1%) | 6.8 KiB | ✅ |
| Codex | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Codex | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | −7 B (−0.6%) | 2.0 KiB | ✅ |
| Codex | Live turn WebSocket decoded | 20.9 KiB | 20.9 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Codex | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
| Claude | Total thread wire | 5.0 KiB | 5.0 KiB | 0 B (0.0%) | 6.8 KiB | ✅ |
| Claude | Thread snapshot wire | 3.8 KiB | 3.8 KiB | 0 B (0.0%) | 4.9 KiB | ✅ |
| Claude | Live turn WebSocket wire | 1.2 KiB | 1.2 KiB | 0 B (0.0%) | 2.0 KiB | ✅ |
| Claude | Live turn WebSocket decoded | 21.2 KiB | 21.2 KiB | 0 B (0.0%) | 29.3 KiB | ✅ |
| Claude | Live turn messages | 2 | 2 | 0 (0.0%) | 8 | ✅ |
Baseline: 5d27ecd · PR result: 917b698 · Source CI: success
Scenario and decoded snapshot size
10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
- Codex decoded thread snapshot: 108.5 KiB
- Claude decoded thread snapshot: 108.8 KiB
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.
Synced from GitHub
No actionable comments were generated in the recent review. 🎉
ℹ️ Recent review info
⚙️ Run configuration
- Configuration used: Path: .coderabbit.config.ts
- Review profile: CHILL
- Plan: Team
- Run ID:
e3823f7b-7db4-4978-bbcc-35e18c2cfc59
📥 Commits
Reviewing files that changed from the base of the PR and between 2212a3aa89c54c411d6a5b27a08e5c61f60a435e and 917b698d02a77cb746f6a96072bc9863513cb8c2.
📒 Files selected for processing: 3
apps/web/src/components/chat/MessagesTimeline.browser.test.tsxapps/web/src/components/settings/ProjectFaviconPickerDialog.browser.test.tsxapps/web/src/components/settings/ProjectSettingsPanel.browser.test.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
📝 Walkthrough
This change updates browser test isolation, WebSocket completion checks, process lifecycle synchronization, and Claude steering timing. Claude steering now applies a five-second background-transition deadline measured from the steer request.
Changes: Browser test cleanup
Layer / File(s) Summary Reset pointer during browser test cleanup
apps/web/test/render.tsx,apps/web/vite.config.tsThe browser test project defines resetPointer, and screen disposal calls it after cleanup.Scope diff view browser tests
apps/web/src/components/diffs/StyledDiffCodeView.browser.test.tsxDiff view tests use container-scoped queries and configure worker startup limits for test cases. Update component browser test setup
apps/web/src/components/chat/ComposerBannerStack.browser.test.tsx,apps/web/src/components/pullRequest/PullRequestDetailPanel.browser.test.tsxThe banner test checks initial visibility directly. Panel tests mock diff worker construction and remove a custom timeout from the action test. Stabilize browser test setup
apps/web/src/components/chat/MessagesTimeline.browser.test.tsx,apps/web/src/components/settings/ProjectFaviconPickerDialog.browser.test.tsx,apps/web/src/components/settings/ProjectSettingsPanel.browser.test.tsxBrowser tests warm up Mermaid, preserve asset results with keep-alive atoms, and separate project environment scope cases. Transfer stream completion checks
Layer / File(s) Summary Use stream-specific completion conditions
apps/server/integration/transferBudgetV2.integration.test.tsThread queues use the stored event sequence for completion. Shell queues wait for a matching final projected shell update. ACP mock process synchronization
Layer / File(s) Summary Wait for detached command publication
apps/server/scripts/acp-mock-agent.tsThe detached launcher signals when output arrives. The exit path waits for that signal before its delay and process exit. Claude steering deadline
Layer / File(s) Summary Apply the request-start background deadline
apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts,apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.tsSteering passes a deadline measured from the steer request to subagent backgrounding. Tests coordinate steering with background-task requests and advance time in five-second steps. OpenCode process group tests
Layer / File(s) Summary Synchronize process group test checks
packages/provider-opencode/src/server/OpenCodeServerLedger.test.tskillGroupattempts group termination directly and ignoresESRCH. The wrapper test waits for output from the inner shell before recording wrapper exit.Priority: ➖ Normal
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
Suggested reviewers:
juliusmarminge
Merge Risk: ⚪ Minimal · up to 917b6
No concrete merge-blocking risk was identified in the reviewed changes.
Pre-merge checks: Passed 3 · Failed 1
Failed Failed checks: 1 warning
Check name Status Explanation Resolution Description check Warning The description thoroughly explains the problems, changes, affected tests, and verification results. It does not provide the required Scope and approval information, including explicit maintainer appr… Add a Scope and approval section that links the triaged issues or approval discussion and identifies explicit maintainer approval. If this qualifies as an approval exemption, explain why the change is a focused fix and provide the required … Passed Passed checks: 3 passed
Check name Status Explanation Title check Passed The title clearly summarizes the primary change: removing timing races that caused recurring CI and release test failures. 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.
Full details: Description check
Explanation
The description thoroughly explains the problems, changes, affected tests, and verification results. It does not provide the required Scope and approval information, including explicit maintainer approval or an explanation for why approval was unnecessary.
Resolution
Add a Scope and approval section that links the triaged issues or approval discussion and identifies explicit maintainer approval. If this qualifies as an approval exemption, explain why the change is a focused fix and provide the required scope rationale.
✨ Finishing Touches
📝 Generate docstrings
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
- Commit to this branch
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Added 2 commits
Synced from GitHub
Added 1 commit
- Merge main into t3/stop-release-flakes2212a3a
Synced from GitHub
Added 1 commit
- test(web): remove three more browser test races917b698
Synced from GitHub