Skip to content

ci: split Windows advisory checks - #409

Merged
Astro-Han merged 3 commits into
devfrom
chore/ci-advisory-split
May 3, 2026
Merged

ci: split Windows advisory checks#409
Astro-Han merged 3 commits into
devfrom
chore/ci-advisory-split

Conversation

@Astro-Han

@Astro-Han Astro-Han commented May 3, 2026

Copy link
Copy Markdown
Owner

Summary

Split Windows advisory unit checks out of the primary ci workflow into a dedicated windows-advisory workflow.

Why

The required PR checks have been stable, but the main ci workflow still waited on non-blocking Windows advisory matrix jobs. That made PR CI look slower and noisier even though those jobs do not participate in the merge gate. This keeps PR ci focused on blocking Linux checks while still running Windows advisory coverage after merges to dev or on manual dispatch.

Related Issue

No related issue. This is a small CI hygiene change from current CI success-rate review.

Human Review Status

Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.

Review Focus

Please check that Windows advisory coverage was moved without changing the matrix commands, and that the new workflow does not run on pull_request by default.

Risk Notes

Low. PRs no longer run Windows advisory checks by default, so Windows-only regressions remain advisory signals after merge to dev or manual dispatch. Required PR checks and the live dev ruleset are unchanged.

How To Verify

Ruby YAML parse: ok for ci.yml, desktop-smoke.yml, windows-advisory.yml
actionlint: ok for all three changed workflows
Diff check: no whitespace errors
Trigger check: windows-advisory has no pull_request trigger
CI check: ci.yml no longer contains unit-windows

Screenshots or Recordings

Not required. No visible UI changes.

Checklist

  • Human review status is stated above as pending, approved, or not required
  • I linked the related issue, or stated why there is no issue
  • This PR has type, scope, and priority labels, or I requested maintainer labeling
  • I described the review focus and any meaningful risks
  • I listed the relevant verification steps and the key result for each
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
  • I manually checked visible UI or copy changes when needed, with screenshots or recordings
  • I considered macOS and Windows impact for desktop, packaging, updater, signing, paths, shell, or permissions changes
  • I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
  • I reviewed the final diff for unrelated changes and suspicious dependency changes
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English

Summary by CodeRabbit

  • Tests

    • Moved Windows unit testing into a dedicated advisory workflow; expanded CI tests to validate its matrix, artifact naming, docs-only skips, and related behaviors
  • Chores

    • Simplified merge-blocking CI aggregation to exclude Windows results
    • Clarified CI/branch-protection guidance for adding new blocking jobs

@Astro-Han Astro-Han added ci Continuous integration / GitHub Actions windows Windows-specific P3 Low priority github_actions Pull requests that update GitHub Actions code labels May 3, 2026
@gemini-code-assist

Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@coderabbitai

coderabbitai Bot commented May 3, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 585cc9b1-4b0b-443e-a15b-57870c2dde7a

📥 Commits

Reviewing files that changed from the base of the PR and between dfe3700 and 3a6e58e.

📒 Files selected for processing (2)
  • .github/workflows/windows-advisory.yml
  • packages/opencode/test/github/ci-workflow.test.ts

📝 Walkthrough

Walkthrough

This PR removes the blocking unit-windows job from .github/workflows/ci.yml, updates the check aggregator to exclude Windows jobs from merge-blocking, introduces a new .github/workflows/windows-advisory.yml that runs sharded Windows unit tests (docs-only gating, per-shard caching, JUnit uploads), and updates tests and a workflow type to match.

Changes

Windows Unit Testing Migration & Tests

Layer / File(s) Summary
CI Aggregator Update
.github/workflows/ci.yml
Deleted the blocking unit-windows job and adjusted the check aggregator to validate only changes, typecheck, unit-app, unit-opencode, and unit-desktop. Added comments about branch-protection needs: requirements.
Advisory Workflow Foundation
.github/workflows/windows-advisory.yml
New windows-advisory workflow (triggers: push to dev, workflow_dispatch) with concurrency and permissions.contents: read.
Docs-only Diff Detection
.github/workflows/windows-advisory.yml
changes job checks out full history, computes BASE_SHA/HEAD_SHA (first-commit fallback), diffs with renames/copies, classifies docs-only via is_docs_path(), and exports docs_only.
Windows Test Execution
.github/workflows/windows-advisory.yml
unit-windows job runs when docs_only != 'true' on windows-latest with a matrix of package shards, fail-fast: false, Node/Bun setup, conditional Turbo cache restore, per-shard test commands, captured exit codes, and failure diagnostics appended to step summary.
Artifact Uploads
.github/workflows/windows-advisory.yml
Each matrix shard unconditionally uploads JUnit XML artifacts (missing files ignored) with 7-day retention; artifact name includes ${{ matrix.package }}-${{ github.sha }}-${{ github.run_attempt }}.
Desktop Smoke Comment
.github/workflows/desktop-smoke.yml
Updated explanatory comment above the check job to note compatibility role and that live ruleset enforces smoke-macos-arm64 directly.
CI Tests Updated / Added
packages/opencode/test/github/ci-workflow.test.ts
Tests now reference ci.yml and windows-advisory.yml; helpers accept optional workflowPath; assertions ensure Windows jobs are absent from ci.yml and validate the advisory workflow’s matrix, setup, cache behavior, artifact naming (includes ${{ github.sha }}), and docs-only gating.
Workflow Parser Type
packages/opencode/test/github/workflow-parser.ts
Workflow type updated: `concurrency["cancel-in-progress"]?: boolean

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

"I’m a rabbit in the CI glade,
Windows shards hop off the main parade,
Docs-only hops keep runs light,
Artifacts named and hopping right,
Advisory runs snug through the night." 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'ci: split Windows advisory checks' directly describes the main change—moving Windows advisory unit checks from ci.yml to a new windows-advisory.yml workflow.
Description check ✅ Passed The description follows the template with all major sections completed: summary, why, related issue, human review status, review focus, risk notes, verification steps, and a fully checked checklist.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/ci-advisory-split

Review rate limit: 9/10 reviews remaining, refill in 6 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/windows-advisory.yml:
- Around line 8-16: Remove the pull_request trigger block from the workflow so
the file only uses push (to dev) and workflow_dispatch as triggers; locate the
'pull_request' and 'workflow_dispatch' keys in the YAML and delete the entire
pull_request section, leaving push/branch triggers and workflow_dispatch intact
to enforce post-merge/manual advisory runs, and then update the test in
packages/opencode/test/github/ci-workflow.test.ts to stop asserting that
'pull_request' exists (adjust assertions to expect only push/workflow_dispatch
or to assert absence of pull_request).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 3ef398f9-cbb7-4827-ab81-e784ef2b1383

📥 Commits

Reviewing files that changed from the base of the PR and between a40e233 and dfe3700.

📒 Files selected for processing (3)
  • .github/workflows/windows-advisory.yml
  • packages/opencode/test/github/ci-workflow.test.ts
  • packages/opencode/test/github/workflow-parser.ts

Comment thread .github/workflows/windows-advisory.yml Outdated
@Astro-Han
Astro-Han merged commit d6fa1e6 into dev May 3, 2026
22 checks passed
@Astro-Han
Astro-Han deleted the chore/ci-advisory-split branch May 3, 2026 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Continuous integration / GitHub Actions github_actions Pull requests that update GitHub Actions code P3 Low priority windows Windows-specific

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant