test(opencode): bump prompt-effect busy/idle timeout for windows runner#543
Conversation
Astro-Han
added
flaky-test
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTest timeout for the "loop sets status to busy then idle" test case increased from 3000 to 5000 milliseconds to provide additional execution time. Test logic and assertions remain unchanged. ChangesTest Timeout Adjustment
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/opencode/test/session/prompt-effect.test.ts (1)
1204-1204: ⚡ Quick winUse platform-specific timeout to match established pattern.
The PR description indicates this is a Windows-specific timing issue, but the fix applies a 5s timeout universally. The file already establishes a pattern for Windows-sensitive timeouts at lines 223 and 1812 (
slowIOTimeoutandshellQueueTimeout), both usingprocess.platform === "win32" ? 10_000 : 3_000.Consider using a platform-specific timeout to avoid unnecessary delay on non-Windows platforms.
⚡ Proposed platform-specific timeout
- 5_000, + process.platform === "win32" ? 5_000 : 3_000,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/opencode/test/session/prompt-effect.test.ts` at line 1204, Replace the hardcoded 5_000 timeout with the established platform-specific pattern used elsewhere (see slowIOTimeout and shellQueueTimeout) by using process.platform === "win32" ? 10_000 : 3_000 so Windows gets the longer timeout and other platforms keep the shorter one; locate the numeric literal near the failing test in prompt-effect.test.ts and swap it for the platform-conditional expression to match existing timeouts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/opencode/test/session/prompt-effect.test.ts`:
- Line 1204: Replace the hardcoded 5_000 timeout with the established
platform-specific pattern used elsewhere (see slowIOTimeout and
shellQueueTimeout) by using process.platform === "win32" ? 10_000 : 3_000 so
Windows gets the longer timeout and other platforms keep the shorter one; locate
the numeric literal near the failing test in prompt-effect.test.ts and swap it
for the platform-conditional expression to match existing timeouts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 742b235f-5bbe-4fa0-b0f7-cb99b8958c54
📒 Files selected for processing (1)
packages/opencode/test/session/prompt-effect.test.ts
…656) Why windows-advisory has been flaking at ~55% on dev push (27 failures / 50 runs in the latest sample). bun's default 3s it.live timeout is consistently tight on the Windows runner for Effect-fiber + SQLite + tmpdir-server tests in packages/opencode/test/session/prompt-effect.test.ts. Four prior PRs (#543, a3b8e54, cf6d1cd, #579) each bumped one or two tests at a time to a slowIOTimeout constant without converging — today's failure on "cancel records MessageAbortedError on interrupted process" was the fifth instance of the same root cause. What Wrap testEffect's live runner via withDefaultLiveTimeout so every it.live in prompt-effect.test.ts picks up a Windows-aware default (10s Windows / 3s elsewhere). Remove 12 third-arg timeout literals (5 x 3_000, 5 x slowIOTimeout, 2 x shellQueueTimeout) and the two duplicated `const ... Timeout` definitions. The wrapper also covers .only and .skip for symmetry. Explicit non-default timeouts (5_000, 10_000, 30_000) still override. Out of scope bun 1.3.13 watcher.node segfault on Windows process exit, and transient actions/cache failures, are upstream / infra and intentionally left to fail "normally" per d6fa1e6. The advisory workflow is not in branch protection and existing if: always() artifact and summary uploads preserve the diagnostic signal. Verification - tsgo --noEmit: clean - bun test test/session/prompt-effect.test.ts --timeout 30000: 54 pass / 0 fail / 28.69s (macOS) - bun test test/github/ci-workflow.test.ts --timeout 30000: 9 pass / 0 fail - PR CI on 4d4792c: all green (typecheck, lint, unit-app, unit-desktop, unit-opencode, e2e-artifacts, smoke-macos-arm64, analyze-js-ts, CodeQL) Review follow-ups Gemini suggested also wrapping .only / .skip; applied and thread resolved. An external review flagged a P1 about .only not being wrapped; audited against HEAD and the claim referenced the pre-amend version (8ad2c1a), not the current 4d4792c — no change required. Risk Detection of a true hang on Windows live tests now takes up to 10s rather than 3s. Acceptable for an advisory signal. No production code paths touched. windows-advisory will still show occasional red runs from upstream / infra causes by design.
1 participant
Summary
Increase the timeout for the Windows-sensitive
loop sets status to busy then idleprompt-effect test from 3s to 5s.Why
The latest
devWindows advisory run failed inpackages/opencode/test/session/prompt-effect.test.tsbecause this test timed out after 3015ms against a 3000ms budget. This file already has a precedent for Windows runner timing headroom ina3b8e5456 test(windows): unblock 4 prompt-effect timeouts under runner load; this PR applies the same bounded fix to the remaining busy/idle status test.Related Issue
task #19 / Windows advisory CI failure on
devHuman Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
loop sets status to busy then idle.Risk Notes
Low risk. Test-only timeout headroom for a Windows advisory timing edge. No product behavior, runtime code, dependencies, or generated files changed.
How To Verify
Screenshots or Recordings
Not applicable. This is a test-only timeout adjustment.
Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit
Note: This release contains test infrastructure updates only. No user-facing changes.