ci: retry windows-advisory unit step once on transient failure - #1047
Conversation
The windows-advisory workflow has shown ~40% red over the last ten runs on dev, driven by two transient failure modes that are not regressions from the merged PR: - Effect-TS sleep-based timing assertions in opencode session tests (e.g. SessionRunState defers disposeAll) that flake on the Windows runner's coarser wall clock. - Native Bun crashes on windows-latest (segfault in watcher.node frames in #1028's run), independent of test content. Linux ci is the load-bearing required gate; this workflow is advisory and non-blocking. Wrap each matrix shard's unit step in a process-level retry (max_attempts=2) so a single transient failure does not turn the advisory red. Keep the first-attempt failure visible in the step summary ("Windows unit attempt 1 failed (retrying)" / "Windows unit recovered on retry") so persistent regressions are not hidden by the retry. Each attempt runs in a subshell so cd packages/... in matrix.command does not leak across attempts. No third-party retry action — the repo pins all action uses by SHA and a small bash loop avoids an extra dependency.
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
Warning Review limit reached
More reviews will be available in 58 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ 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 |
Astro-Han
added
enhancement
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Address review feedback: the new retry semantics on the windows-advisory
unit step had no self-test fixation, so future changes to the retry
count, exported first-attempt exit code, or recovery-summary headers
could silently regress without breaking ci-workflow.test.ts.
Add a focused "retries the Windows unit step once on transient failure"
test that pins:
- attempts=2 (single retry budget)
- ( ${{ matrix.command }} ) subshell so cd does not leak between attempts
- first_exit_code GITHUB_OUTPUT export
- First-attempt failure summary header and recovery summary header
- Final exit uses the last attempt's status, not the first
github-actions
Bot
added
the
harness
Address review feedback: recovery from a retry currently only writes a step-summary header, so the first-attempt failure is easy to miss when scanning the PR checks list or the run's annotations panel — making the advisory's true flake rate hard to track. Emit a GitHub Actions workflow command on the two terminal outcomes of the retry loop: - `::notice title=Windows unit recovered on retry::...` when attempt 1 failed but attempt 2 succeeded. Surfaces in the run-level annotations panel and per-check annotation list — observable without expanding the step summary. - `::warning title=Windows unit failed advisory signal after retry::...` when both attempts failed. The advisory still goes red on this path, but the warning annotation makes the post-retry failure scannable alongside the run-level red icon. Update ci-workflow.test.ts to pin both annotation strings, so future edits to the retry contract cannot silently drop them.
1 participant
Summary
Wrap each matrix shard's
unitstep in.github/workflows/windows-advisory.ymlin a process-level retry (max_attempts=2). A first-attempt failure now writes a step-summary header (Windows unit attempt 1 failed (retrying)/Windows unit recovered on retry) and the job only turns red if both attempts fail. Each attempt runs in a subshell socd packages/opencode && ...in some matrix commands does not leak working directory across attempts.Why
The
windows-advisoryworkflow has been ~40% red over the last ten runs ondevdue to transient flake that does not come from the merged PRs:010c6d6) failed only onSessionRunState > defers disposeAll across all loaded directories when any directory has an active runinpackages/opencode/test/session/run-state.test.ts, which usesEffect.sleep("10/20 millis")timing assertions on Windows's coarser wall clock. Same-SHA rerun passed. PR diff was 5 files all underpackages/app/.05d8ba1) failed with a Bun 1.3.14 native segfault onwindows-latest(Ywatcher.nodestack frames), unrelated to PR content.Linux
ciis the load-bearing required gate;windows-advisoryis advisory and non-blocking. A single transient failure should not turn the advisory red, but persistent regressions still must be visible — hence retry with first-attempt failure surfaced in the step summary rather than silently swallowed.Related Issue
No issue; surfaced from CI history triage on 2026-06-01 after two recent
devmerges (#1043, #1037) showedwindows-advisoryfailure.Human Review Status
Pending
Review Focus
ci-workflow.test.tsself-test asserts:${{ matrix.command }},echo "exit_code=$status",### Windows unit diagnostic,failed advisory signal. Confirm none of these are accidentally broken.( ${{ matrix.command }} )per attempt — verify this is the right scoping for matrix shards that containcd packages/...(opencode-session, opencode-config-project, opencode-server-tools).forloop instead ofnick-fields/retry@v3— this matches the repo's SHA-pinned-actions discipline; if you prefer the third-party action anyway, say so.opencode-session, because the Bun-segfault failure class is process-level and shard-agnostic.Risk Notes
attempt 1 exit codewhenever retry recovers, so persistent flake remains visible to humans reviewing the workflow run. The required Linuxcigate is untouched, so genuine product regressions are still blocked from merging.Upload unit artifactsstep uploads the JUnit XML from whichever attempt wrote it last; on retry recovery the artifact reflects attempt 2. Acceptable for an advisory signal.Effect.sleep-based timing inrun-state.test.tsto deterministic primitives is a larger Effect-TS surgery; deferred until after the 2026-06-15windows-latest→windows-2025-vs2026runner migration, since flake patterns will shift.How To Verify
Screenshots or Recordings
N/A — CI workflow change, no UI surface.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.