Skip to content

chore: ci improvements#1469

Merged
ctrlc03 merged 5 commits intomainfrom
chore/ci-improvements
Mar 25, 2026
Merged

chore: ci improvements#1469
ctrlc03 merged 5 commits intomainfrom
chore/ci-improvements

Conversation

@ctrlc03
Copy link
Collaborator

@ctrlc03 ctrlc03 commented Mar 23, 2026

Summary by CodeRabbit

  • Chores
    • Optimized continuous integration workflow configuration to improve testing efficiency and reliability through refined matrix strategies and conditional dependency caching.

@ctrlc03 ctrlc03 requested a review from cedoor March 23, 2026 09:36
@vercel
Copy link

vercel bot commented Mar 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
crisp Ready Ready Preview, Comment Mar 25, 2026 2:18pm
enclave-docs Ready Ready Preview, Comment Mar 25, 2026 2:18pm

Request Review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 23, 2026

Warning

Rate limit exceeded

@cedoor has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 38 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4ad4de63-ee46-45a9-9b0d-5aa7b363122f

📥 Commits

Reviewing files that changed from the base of the PR and between 014f684 and 6665c67.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml
📝 Walkthrough

Walkthrough

Changes to GitHub Actions CI workflow restructure the crisp_unit job from fixed sequential steps into a matrix-driven architecture with conditional tool setup, modify the detect_changes output condition for docker_ciphernode, update ciphernode_integration_test strategy behavior, and normalize YAML formatting.

Changes

Cohort / File(s) Summary
CI Workflow Matrix Restructuring
..github/workflows/ci.ymlcrisp_unit job
Reworked from four fixed test steps into a matrix-driven job using test-suite includes (test:circuits, test:sdk, test:contracts, cargo test) with dynamic command execution; added conditional caching and tool installation (Rust/Foundry/solc for cargo test, pnpm/Node for non-cargo test, contract compilation for test:contracts).
CI Workflow Logic Updates
..github/workflows/ci.ymldetect_changes, ciphernode_integration_test, build_enclave_cli jobs
Updated docker_ciphernode output condition by restructuring CONTRACTS as separate gating factor; changed ciphernode_integration_test strategy.fail-fast to true and removed commented TODO; normalized runs-on: whitespace in build_enclave_cli.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • PR #1113: Reworks crisp_unit job in the same workflow into matrix-driven structure with adjusted command execution and artifact consumption from a new build step.
  • PR #980: Modifies crisp_unit job with matrix additions and related step restructuring in .github/workflows/ci.yml.
  • PR #1428: Makes overlapping changes to the same workflow file affecting crisp_unit and build_enclave_cli job blocks.

Suggested reviewers

  • 0xjei

Poem

🐰 The CI pipeline hops anew,
With matrices that test both old and new,
Conditional tools spring into place,
Caching speeds the testing race,
Workflows refined with care and grace!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'chore: ci improvements' is vague and generic, using non-descriptive terms that don't convey specific information about the actual changes made to the CI workflow. Consider a more specific title that highlights the main change, such as 'chore: refactor ci matrix strategy and detect_changes logic' or 'chore: optimize ci workflow with matrix-driven testing'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/ci-improvements

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.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/ci.yml:
- Line 111: The docker_ciphernode change detector removed $CONTRACTS and must
include it so contract-only PRs trigger build_ciphernode_image; update the echo
line that sets docker_ciphernode (the any call for variables like $FORCE $RUST
$DOCKER $CI) to also pass $CONTRACTS (i.e., call any $FORCE $RUST $DOCKER $CI
$CONTRACTS) so the workflow rebuilds the Docker image when contract artifacts
change.
- Around line 585-588: The matrix entry "cargo test" is expanded as "pnpm cargo
test" and fails because package.json has no "cargo" script; update the
strategy.matrix to either (A) remove the plain "cargo test" string and run Rust
tests directly via an include entry like: add strategy.matrix.include with an
item that sets test-suite: "cargo test" and a separate command field "cargo
test" (and working-directory if needed), or (B) change the list to only npm
script names and add a new job/step that invokes "cargo test" directly (e.g.,
run: cargo test) so the Rust tests execute with the correct runner instead of
being prefixed by pnpm.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 30c0b32a-dde6-4004-ad60-448e1d6e4137

📥 Commits

Reviewing files that changed from the base of the PR and between 12e2752 and def6975.

📒 Files selected for processing (2)
  • .github/workflows/ci.yml
  • tests/integration/persist.sh

ctrlc03 and others added 2 commits March 25, 2026 14:48
- Include CONTRACTS in docker_ciphernode change detection so contract-only PRs rebuild the image
- Use matrix include with explicit commands so cargo test runs without pnpm cargo test

Made-with: Cursor
cedoor added 2 commits March 25, 2026 15:03
- Rust cache, toolchain, Foundry, solc: only for cargo test
- Nargo: only for test:circuits
- pnpm, Node, pnpm install: only for non-Rust suites
- Enclave contract compilation: only for test:contracts
- SDK artifact download: only for test:sdk

Made-with: Cursor
Copy link
Member

@0xjei 0xjei left a comment

Choose a reason for hiding this comment

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

utACK

@ctrlc03 ctrlc03 merged commit 849b65a into main Mar 25, 2026
31 checks passed
@ctrlc03 ctrlc03 deleted the chore/ci-improvements branch March 25, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants