Repository navigation
ci: parallelize build and test, upgrade actions and pixi - #475
Conversation
Split the serial build→test pipeline into per-configuration workflows (native-test.yml, cross-test.yml) so each test only waits for its own build instead of all builds completing first. Also upgrade action versions to fix Node.js 20 deprecation warnings: - prefix-dev/setup-pixi v0.9.3 → v0.9.6, pixi v0.67.0 → v0.71.1 - dorny/paths-filter v3 → v4 - Swatinem/rust-cache v2 → v2.9.1 - re-actors/alls-green release/v1 → v1.2.2 - svenstaro/upload-release-action v2 → 2.11.5 - softprops/action-gh-release v2 → v3 - huacnlee/autocorrect-action v2 → v2.5.4
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the old test workflow with reusable native, cross, and editor workflows, updates ChangesCI Workflow Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/publish-vscode.yml (1)
31-38: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix
tag_nameto usegithub.ref_nameinstead ofgithub.ref.
github.refcontains the full ref path (refs/tags/v1.0.0), butsoftprops/action-gh-releaseexpectstag_nameto be just the tag name (v1.0.0). Usinggithub.ref_nameprevents the action from failing or creating malformed release metadata.tag_name: ${{ github.ref }} + tag_name: ${{ github.ref_name }}🤖 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 @.github/workflows/publish-vscode.yml around lines 31 - 38, The Release upload step is passing the full ref path into softprops/action-gh-release via tag_name, which should be the short tag value instead. Update the Upload .vsix to Release step to use github.ref_name for tag_name so the action receives the actual tag name, and keep the rest of the release upload configuration unchanged..github/workflows/main.yml (1)
49-61: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBroaden the
cmakefilter to cover the new CI control-plane files.Right now a PR that only changes
.github/workflows/main.ymlor.github/actions/setup-pixi/action.ymlcan skip every native/cross/editor job, even though those files now directly control how the CMake pipeline runs.Suggested fix
cmake: + - '.github/workflows/main.yml' + - '.github/actions/setup-pixi/action.yml' - 'CMakeLists.txt' - 'src/**' - 'include/**' - 'tests/**' - 'config/**' - 'cmake/**' - 'editors/nvim/**' - 'editors/vscode/**' - 'pixi.toml' - 'pixi.lock' - '.github/workflows/native-test.yml' - '.github/workflows/cross-test.yml'🤖 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 @.github/workflows/main.yml around lines 49 - 61, The cmake path filter in the workflow is too narrow and can miss changes to CI control-plane files that affect the native pipeline. Update the filter in the main workflow so it also includes `.github/workflows/main.yml` and `.github/actions/setup-pixi/action.yml`, alongside the existing cmake-related paths, to ensure changes to those controls trigger the native/cross/editor jobs.
🧹 Nitpick comments (1)
.github/workflows/main.yml (1)
117-124: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the new CI jobs to read-only token permissions.
These jobs only need repository read access, but without
permissionsthey inherit the repo defaultGITHUB_TOKENscope. Addpermissions: { contents: read }to the newzed,native-*,cross-*, andeditorjobs so untrusted PR code cannot accidentally run with broader write scopes.Also applies to: 125-244
🤖 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 @.github/workflows/main.yml around lines 117 - 124, The new GitHub Actions jobs are inheriting the default GITHUB_TOKEN scope instead of using read-only access. Update each of the added job definitions in the workflow, including zed, native-*, cross-*, and editor, to explicitly set permissions to contents: read so the build steps remain read-only for untrusted PRs.Source: Linters/SAST tools
🤖 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.
Inline comments:
In @.github/workflows/cross-test.yml:
- Around line 61-66: The cross-test workflow’s Build step is hardcoding the CI
environment flag to OFF, so it configures CMake differently from the native CI
path. Update the cmake-config invocation in the Build job to pass through the CI
flag the same way as the pixi build task used in native-test.yml, using the
existing inputs and cmake-config/cmake-build step names to locate the change.
- Around line 68-76: The cross artifact upload step is missing the hidden
build-tree .llvm payload, so the binaries copied by actions/upload-artifact can
fail on the other runner when they rely on LLVM libs via RPATH. Update the
Upload binaries step in cross-test.yml to include the build/${{
inputs.build_type }}/.llvm path alongside bin/ and lib/, keeping the existing
cross-build-${{ inputs.target_triple }} artifact setup intact.
---
Outside diff comments:
In @.github/workflows/main.yml:
- Around line 49-61: The cmake path filter in the workflow is too narrow and can
miss changes to CI control-plane files that affect the native pipeline. Update
the filter in the main workflow so it also includes `.github/workflows/main.yml`
and `.github/actions/setup-pixi/action.yml`, alongside the existing
cmake-related paths, to ensure changes to those controls trigger the
native/cross/editor jobs.
In @.github/workflows/publish-vscode.yml:
- Around line 31-38: The Release upload step is passing the full ref path into
softprops/action-gh-release via tag_name, which should be the short tag value
instead. Update the Upload .vsix to Release step to use github.ref_name for
tag_name so the action receives the actual tag name, and keep the rest of the
release upload configuration unchanged.
---
Nitpick comments:
In @.github/workflows/main.yml:
- Around line 117-124: The new GitHub Actions jobs are inheriting the default
GITHUB_TOKEN scope instead of using read-only access. Update each of the added
job definitions in the workflow, including zed, native-*, cross-*, and editor,
to explicitly set permissions to contents: read so the build steps remain
read-only for untrusted PRs.
🪄 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
Run ID: 622b32b9-fd5d-455b-b81a-4231ab786740
📒 Files selected for processing (9)
.github/actions/setup-pixi/action.yml.github/workflows/build.yml.github/workflows/check-format.yml.github/workflows/cross-test.yml.github/workflows/main.yml.github/workflows/native-test.yml.github/workflows/publish-clice.yml.github/workflows/publish-vscode.yml.github/workflows/test.yml
💤 Files with no reviewable changes (2)
- .github/workflows/test.yml
- .github/workflows/build.yml
- native-test.yml: build+test in single matrix job (same runner), each config tests immediately after its own build - cross-test.yml: self-contained build/test matrix (different runners) - editor-test.yml: extracted to separate reusable workflow - main.yml: 3 clean workflow calls instead of 8 per-config calls
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8725447165
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
activate-environment: true fails when multiple environments are installed; pass the explicit env name instead.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/main.yml (1)
49-62: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInclude the CI bootstrap files in the
cmakefilter.The new gate still misses
.github/workflows/main.ymland.github/actions/setup-pixi/action.yml. A PR that only changes the dispatcher or the shared Pixi bootstrap will leaveneeds.changes.outputs.cmake == 'false', sonative-test/cross-test/editor-testare skipped even though their execution path changed.Suggested patch
cmake: - 'CMakeLists.txt' - 'src/**' - 'include/**' - 'tests/**' - 'config/**' - 'cmake/**' - 'editors/nvim/**' - 'editors/vscode/**' - 'pixi.toml' - 'pixi.lock' + - '.github/workflows/main.yml' + - '.github/actions/setup-pixi/action.yml' - '.github/workflows/native-test.yml' - '.github/workflows/cross-test.yml' - '.github/workflows/editor-test.yml'🤖 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 @.github/workflows/main.yml around lines 49 - 62, The cmake change filter in the workflow currently omits the CI bootstrap files, so changes to the dispatcher or shared Pixi setup can be missed. Update the cmake paths list in the main workflow to include .github/workflows/main.yml and .github/actions/setup-pixi/action.yml, keeping the existing needs.changes.outputs.cmake gating logic intact so native-test, cross-test, and editor-test still run when those bootstrap files change.
🤖 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.
Inline comments:
In @.github/workflows/cross-test.yml:
- Around line 90-105: The test job in cross-test.yml uses needs: build, which
makes every matrix entry wait for the entire build matrix instead of its
matching target. Update the workflow structure around the build and test jobs so
each target can proceed independently, using the existing matrix fields like os,
build_type, and target_triple in the build/test job definitions to pair the
right build with the right test.
In @.github/workflows/native-test.yml:
- Around line 16-31: The native-test matrix is coupling unrelated platforms, so
the editor gate is waiting for every OS cell instead of only the Ubuntu
RelWithDebInfo build. Split the Ubuntu release build used by editor-test into
its own job or reusable workflow boundary, and keep the broader native matrix in
build-and-test; use the existing native-build-ubuntu-24.04-RelWithDebInfo target
as the dedicated dependency for editor gating.
---
Outside diff comments:
In @.github/workflows/main.yml:
- Around line 49-62: The cmake change filter in the workflow currently omits the
CI bootstrap files, so changes to the dispatcher or shared Pixi setup can be
missed. Update the cmake paths list in the main workflow to include
.github/workflows/main.yml and .github/actions/setup-pixi/action.yml, keeping
the existing needs.changes.outputs.cmake gating logic intact so native-test,
cross-test, and editor-test still run when those bootstrap files change.
🪄 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
Run ID: f0e2f594-12c0-4a92-a890-a43effffb52e
⛔ Files ignored due to path filters (1)
pixi.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/cross-test.yml.github/workflows/editor-test.yml.github/workflows/main.yml.github/workflows/native-test.yml
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/setup-pixi/action.yml (1)
18-18: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPin
prefix-dev/setup-pixito the commit SHA instead ofv0.9.6.
v0.9.6is a mutable tag; using the current commit SHA keeps this workflow from changing if the tag moves.Suggested change
- uses: prefix-dev/setup-pixi@v0.9.6 + uses: prefix-dev/setup-pixi@5185adfbffb4bd703da3010310260805d89ebb11 # v0.9.6🤖 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 @.github/actions/setup-pixi/action.yml at line 18, The GitHub Action reference for setup-pixi is using a mutable tag, so update the uses entry in the workflow/action definition to pin prefix-dev/setup-pixi to its commit SHA instead of v0.9.6. Locate the existing setup-pixi reference in the action configuration and replace the version tag with the exact SHA so the workflow remains stable if the tag changes.
🤖 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.
Outside diff comments:
In @.github/actions/setup-pixi/action.yml:
- Line 18: The GitHub Action reference for setup-pixi is using a mutable tag, so
update the uses entry in the workflow/action definition to pin
prefix-dev/setup-pixi to its commit SHA instead of v0.9.6. Locate the existing
setup-pixi reference in the action configuration and replace the version tag
with the exact SHA so the workflow remains stable if the tag changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 106ce262-e999-4a25-a43e-fca181c2d57b
📒 Files selected for processing (3)
.github/actions/setup-pixi/action.yml.github/workflows/editor-test.yml.github/workflows/native-test.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/editor-test.yml
- .github/workflows/native-test.yml
Replace matrix strategy with individual named jobs so each cross test only waits for its own build, not all cross builds. Job name: fields preserve the same CI labels as before.
Move the shared build→test pattern into cross-pair.yml and call it 3 times from cross-test.yml. Each pair remains independent with per-config test dependencies, but the logic is defined only once.
- actions/checkout v4 → v7 - actions/cache v4 → v6 - actions/upload-artifact v4 → v7 - actions/download-artifact v4 → v8
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bc781fe6a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Cross builds intentionally disable CLICE_CI_ENVIRONMENT because CI-only toolchain tests detect host toolchains and would produce wrong results on cross-compilation target runners.
16bit-ykiko
changed the title
With build+test merged into one job, the stop-server step must run after all tests, not between build and test. Otherwise sccache may still hold file locks during pixi post-cleanup on Windows.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
.github/workflows/cross-pair.yml (1)
65-73: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRe-add the build-tree
.llvmpayload to the cross artifact.This artifact only includes
bin/andlib/, so the test runner can miss LLVM shared libraries still resolved frombuild/RelWithDebInfo/.llvm. That is the same cross-runner startup failure the old workflow already had to fix. Include.llvm/here and enable hidden-file upload.Suggested change
- name: Upload binaries uses: actions/upload-artifact@v7 with: name: cross-build-${{ inputs.target_triple }} path: | build/RelWithDebInfo/bin/ build/RelWithDebInfo/lib/ + build/RelWithDebInfo/.llvm/ + include-hidden-files: true if-no-files-found: error retention-days: 1🤖 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 @.github/workflows/cross-pair.yml around lines 65 - 73, The Upload binaries step in the cross-pair workflow is missing the build-tree .llvm payload, so the cross artifact can still fail to resolve LLVM shared libraries at runtime. Update the actions/upload-artifact@v7 configuration in the Upload binaries step to include build/RelWithDebInfo/.llvm/ alongside the existing bin/ and lib/ paths, and enable hidden-file upload so the .llvm directory is preserved in the artifact.
🤖 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.
Inline comments:
In @.github/workflows/benchmark.yml:
- Around line 17-18: The Checkout repository step in the benchmark workflow is
leaving persisted Git credentials enabled, which is unnecessary here. Update the
actions/checkout usage to disable credential persistence by setting
persist-credentials to false so later steps cannot reuse the repo token. Use the
existing Checkout repository step in the benchmark job as the location to make
this change.
In @.github/workflows/check-format.yml:
- Around line 10-11: The checkout step in the check-format workflow is
persisting credentials unnecessarily for a job that only runs formatting and git
diff. Update the actions/checkout configuration in the Checkout code step to
disable credential persistence by setting persist-credentials to false, keeping
the change localized to that workflow step.
In @.github/workflows/deploy-docs.yml:
- Line 11: The workflow’s actions/checkout step is leaving credentials available
to later steps, which should be disabled. Update the existing checkout
configuration in the deploy-docs workflow to set persist-credentials to false on
actions/checkout so the token is not retained.
In @.github/workflows/publish-clice.yml:
- Around line 63-64: The release job’s repository checkout is leaving persisted
Git credentials enabled, which exposes an unnecessary token to later build
steps. Update the Checkout repository step that uses actions/checkout@v7 to
disable persisted credentials so subsequent commands like pixi run package and
cmake-build cannot reuse an authenticated git config. Keep the change scoped to
this workflow job and verify no later step relies on git push or fetch access.
---
Duplicate comments:
In @.github/workflows/cross-pair.yml:
- Around line 65-73: The Upload binaries step in the cross-pair workflow is
missing the build-tree .llvm payload, so the cross artifact can still fail to
resolve LLVM shared libraries at runtime. Update the actions/upload-artifact@v7
configuration in the Upload binaries step to include build/RelWithDebInfo/.llvm/
alongside the existing bin/ and lib/ paths, and enable hidden-file upload so the
.llvm directory is preserved in the artifact.
🪄 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
Run ID: bd40a7f3-a583-4520-83f0-10bc1a636376
📒 Files selected for processing (12)
.github/workflows/benchmark.yml.github/workflows/build-llvm.yml.github/workflows/check-format.yml.github/workflows/cross-pair.yml.github/workflows/cross-test.yml.github/workflows/deploy-docs.yml.github/workflows/editor-test.yml.github/workflows/main.yml.github/workflows/native-test.yml.github/workflows/publish-clice.yml.github/workflows/publish-vscode.yml.github/workflows/upload-llvm.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/native-test.yml
- .github/workflows/editor-test.yml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bde07d9cfd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
cross-pair.yml) so each test only waits for its own build.native-test.yml— self-contained matrix, build+test in same jobcross-test.yml→cross-pair.yml— 3 independent build→test pairs, no matrix barriereditor-test.yml— extracted to separate reusable workflowmain.yml— 3 clean workflow calls instead of monolithic build/testactions/checkoutv4 → v7actions/cachev4 → v6actions/upload-artifactv4 → v7actions/download-artifactv4 → v8prefix-dev/setup-pixiv0.9.3 → v0.9.6dorny/paths-filterv3 → v4Swatinem/rust-cachev2 → v2.9.1re-actors/alls-greenrelease/v1 → v1.2.2svenstaro/upload-release-actionv2 → 2.11.5softprops/action-gh-releasev2 → v3huacnlee/autocorrect-actionv2 → v2.5.4Test plan
Summary by CodeRabbit