fix(ci): stabilize windows advisory checks - #995
Conversation
Astro-Han
added
bug
|
Warning Review limit reached
More reviews will be available in 43 minutes and 25 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 (6)
✨ 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 |
github-actions
Bot
added
the
harness
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Suggested priority: P3 (only low-risk paths changed (.github/workflows/windows-advisory.yml, packages/opencode/test/github/bun-version-workflow.test.ts, packages/opencode/test/github/ci-workflow.test.ts, packages/opencode/test/session/processor-effect.test.ts, packages/opencode/test/session/run-observability.test.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces cross-platform path normalization for GitHub workflow assertions, increases a connection timeout in a processor-effect test, and adds a test verifying that disabled unknown tools are excluded from retry boundary proofs. It also attempts to resolve a prefix-matching issue in Windows CI shards by targeting a specific test file (test/git/git.test.ts). Feedback suggests using a trailing slash (test/git/) instead of a specific file path to ensure future tests in that directory are executed, and highlights a similar prefix-matching issue with test/file that should be resolved with a trailing slash.
Root cause: - PR #995 fixed the deterministic Windows advisory shard and path issues, but post-merge verification exposed two remaining Windows-sensitive test assumptions. - The SessionRunState force-dispose lifecycle test used a fixed 10ms sleep as a startup barrier before force-disposing the instance; on Windows the run could still be outside the active work section, causing the fiber await to hit the 30s test timeout. - The worktree rollback test used a pre-existing branch to force git worktree add -b to fail; a manual Windows advisory run showed that Git for Windows can hang on that failure path, masking the intended .gitignore rollback assertion. Change boundary: - Replace the SessionRunState fixed sleep with a Deferred signal from the run body, preserving the same interrupt-provenance assertion without scheduler timing dependence. - Replace the worktree test failure trigger with a non-empty target directory, preserving .gitignore rollback coverage while avoiding the hanging existing-branch failure path. - No production behavior, dependency, generated-file, docs, or UI changes. Verification: - cd packages/opencode && bun test test/session/run-state.test.ts -t "annotates runner interrupts caused by force instance disposal" -> 1 pass, 0 fail. - cd packages/opencode && bun test test/session/run-state.test.ts -> 22 pass, 0 fail. - cd packages/opencode && bun test test/project/worktree.test.ts -t "restores .gitignore when git worktree add fails" -> 1 pass, 0 fail. - cd packages/opencode && bun test test/project/worktree.test.ts -> 15 pass, 0 fail. - cd packages/opencode && bun test --timeout 30000 test/config test/project test/worktree test/file/ test/github test/settings test/settings.test.ts -> 456 pass, 3 skip, 0 fail. - PR #996 checks all green: ci, codeql, dependency-review, desktop-smoke, e2e-artifacts, dev-dep-audit, pr-title-lint, pr-triage, CodeRabbit, semantic title. - Manual windows-advisory on codex/fix-windows-run-state-dispose run 26679629780 attempt 2 -> success across all Windows shards. Follow-up: - After merge, verify dev post-merge ci and windows-advisory before final cleanup.
1 participant
Summary
Stabilize the merged-dev
windows-advisoryfailures introduced around PR #990. There is no dedicated issue; this follows up the redwindows-advisoryrun on dev SHA6eb8585008ceb95ec12cc3928c3f829d43862809.Why
The Windows advisory run failed in three opencode shards:
path.relative()outputtest/git,test/file, andtest/sharecould prefix-match sibling directories such astest/github,test/filesystem, andtest/sharedThe first PR CI pass also exposed a Linux
unit-opencodetiming fixture issue inglobal-session-activity-list.test.ts, where the fakeDate.now()could be earlier than the synthetic reminder timestamp.Related Issue
No issue. This is a narrow CI follow-up to the failed merged-dev
windows-advisoryrun after PR #990.Human Review Status
Pending
Review Focus
Please check the Windows advisory shard command change and the retry test boundary. The intent is to fix deterministic Windows test failures without changing production retry behavior.
Risk Notes
No visible UI or copy changed, so the UI check item is left unticked. Platform impact is limited to the Windows advisory workflow and Windows CI timing sensitivity. No dependency, docs, credential, deletion, or generated-file surface changed.
How To Verify
Screenshots or Recordings
Not applicable. No visible UI changed.
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.