fix: remove ci artifacts deep clean#681
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughIntroduces PR-triggered dry-run releases and event-gated publish steps in the releases workflow. Converts many crates to workspace-managed metadata (version/edition/license), bumps the workspace version to 0.0.1-test, aligns internal dependency versions, marks e3-evm as non-publishable, and sets Cargo resolver to "3" in templates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant GH as GitHub Actions (releases.yml)
participant RP as release-plz
participant NPM as NPM Publish
participant BR as Binary Release
Dev->>GH: Open PR to main
GH->>GH: Setup Node 20 / pnpm, install deps
GH->>RP: release-pr --dry-run
RP-->>GH: Dry-run result (no publishing)
Dev->>GH: Push to main
GH->>GH: Setup Node 20 / pnpm, install deps
GH->>RP: release-pr
RP-->>GH: releases_created = true/false
alt releases_created == true
GH->>NPM: Release NPM Packages
NPM-->>GH: Publish result
GH->>BR: Trigger Binary Release
BR-->>GH: Release result
else releases_created == false
GH-->>Dev: Skip publish steps
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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 |
0xjei
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
crates/evm/build.rs (1)
12-12: Track more inputs that affect artifact freshness.Add package manager inputs so contract changes via dependency bumps or script edits retrigger the build script.
println!("cargo:rerun-if-changed=../../packages/enclave-contracts/contracts"); +println!("cargo:rerun-if-changed=../../packages/enclave-contracts/package.json"); +println!("cargo:rerun-if-changed=../../pnpm-lock.yaml");.github/workflows/releases.yml (1)
46-48: Consider cleaning ignored files as well.If you still hit “uncommitted changes” due to ignored outputs, use
git clean -fdX(ignored) or-ffdx(ignored + untracked). Keep as-is if current setup already passes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/releases.yml(1 hunks)crates/evm/build.rs(1 hunks)examples/CRISP/.gitignore(0 hunks)packages/enclave-contracts/.gitignore(0 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json(0 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json(0 hunks)packages/enclave-contracts/artifacts/contracts/registry/NaiveRegistryFilter.sol/NaiveRegistryFilter.json(0 hunks)
💤 Files with no reviewable changes (5)
- packages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.json
- packages/enclave-contracts/artifacts/contracts/registry/NaiveRegistryFilter.sol/NaiveRegistryFilter.json
- packages/enclave-contracts/.gitignore
- packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json
- examples/CRISP/.gitignore
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/binary-releases.yml (2)
3-20: Builds for workflow_dispatch/repository_dispatch currently use repo HEAD, not the requested tag.Artifacts may not match the release tag when manually dispatched or repository-dispatched. Checkout doesn’t pin to the tag (or version) inputs, so you build whatever HEAD is. Fix by checking out the intended ref before building.
Apply:
@@ - - name: Checkout - uses: actions/checkout@v4 + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: Checkout requested ref (manual/dispatch) + if: ${{ github.event_name == 'workflow_dispatch' || github.event_name == 'repository_dispatch' }} + run: | + set -euo pipefail + git fetch --tags --force + if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then + REF="${{ github.event.inputs.tag }}" + else + REF="v${{ github.event.client_payload.version }}" + fi + if [ -z "$REF" ]; then + echo "Ref not provided" >&2 + exit 1 + fi + if git rev-parse --verify --quiet "$REF" >/dev/null; then + git checkout -f "$REF" + else + echo "Ref $REF not found" >&2 + exit 1 + fiAlso applies to: 41-46, 118-137
164-167: Release notes claim macOS x86_64 support, but matrix builds only aarch64.Either add a macOS x86_64 matrix entry or fix the text. As-is, users will look for a missing asset.
Apply:
- - macOS (x86_64, aarch64 Apple Silicon) + - macOS (aarch64 Apple Silicon)Alternatively, add:
@@ - os: macos-13 target: x86_64-apple-darwin os_name: macos arch: x86_64
🧹 Nitpick comments (2)
.github/workflows/binary-releases.yml (2)
47-61: Speed up builds with caching (pnpm + Rust).Add pnpm cache to setup-node and Rust build cache to cut release times.
Apply:
- name: Setup Node.js uses: actions/setup-node@v4 with: - node-version: "20" + node-version: "20" + cache: "pnpm" @@ - name: Install Rust uses: dtolnay/rust-toolchain@stable + + - name: Cache Rust build + uses: Swatinem/rust-cache@v2 + with: + save-if: ${{ github.ref_type == 'tag' || github.event_name != 'push' }}
140-147: Consider upgrading and pinning actions for supply-chain hygiene.softprops/action-gh-release has v2; also consider pinning actions to commit SHAs.
Example:
- - name: Create Release - uses: softprops/action-gh-release@v1 + - name: Create Release + uses: softprops/action-gh-release@v2And pin other actions (checkout, setup-node, upload/download-artifact) to SHAs if your policy prefers that.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/binary-releases.yml(4 hunks)crates/evm/build.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/evm/build.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test_contracts
- GitHub Check: integration_prebuild
- GitHub Check: test_net
- GitHub Check: build_sdk
- GitHub Check: rust_unit
- GitHub Check: Build & Push Image
🔇 Additional comments (1)
.github/workflows/binary-releases.yml (1)
118-137: Validate repository_dispatch payload to avoid empty/invalid tags.If client_payload.version is missing, you’ll create a “v” tag/release or fail later. Add a guard.
Apply:
- name: Get tag name and prerelease status id: get_tag run: | if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then echo "tag=${{ github.event.inputs.tag }}" >> $GITHUB_OUTPUT echo "prerelease=${{ github.event.inputs.prerelease }}" >> $GITHUB_OUTPUT elif [ "${{ github.event_name }}" = "repository_dispatch" ]; then VERSION="${{ github.event.client_payload.version }}" + if [ -z "$VERSION" ]; then + echo "client_payload.version is required for repository_dispatch" >&2 + exit 1 + fi + if ! [[ "$VERSION" =~ ^[0-9]+(\.[0-9]+){2}(-[A-Za-z0-9.\-]+)?$ ]]; then + echo "Invalid semantic version: $VERSION" >&2 + exit 1 + fi echo "tag=v${VERSION}" >> $GITHUB_OUTPUT echo "prerelease=false" >> $GITHUB_OUTPUT else TAG="${GITHUB_REF#refs/tags/}" echo "tag=${TAG}" >> $GITHUB_OUTPUT
0xjei
force-pushed
the
fix/release-rs
branch
from
95c5a91 to
13d21c3
Compare
0xjei
force-pushed
the
fix/release-rs
branch
from
bf82473 to
2e1fb25
Compare
0xjei
changed the title
0xjei
force-pushed
the
fix/release-rs
branch
from
2bd529e to
81cb62b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/crypto/Cargo.toml (1)
7-7: Repository URL likely incorrect ("cryptography" vs crate path).The crate lives under crates/crypto; link may be broken on crates.io.
-repository = "https://github.com/gnosisguild/enclave/crates/cryptography" +repository = "https://github.com/gnosisguild/enclave"crates/bfv-helpers/Cargo.toml (1)
19-19: Cargo error:gitandversioncannot be combined.
grecodeclares bothgitandversion; Cargo rejects this. Usetag(orrev) withgit, or dropversion.Apply one of:
-greco = { package = "e3-greco-generator", git = "https://github.com/gnosisguild/greco", version = "0.1.0"} +greco = { package = "e3-greco-generator", git = "https://github.com/gnosisguild/greco", tag = "v0.1.0" }-greco = { package = "e3-greco-generator", git = "https://github.com/gnosisguild/greco", version = "0.1.0"} +greco = { package = "e3-greco-generator", git = "https://github.com/gnosisguild/greco" }
♻️ Duplicate comments (1)
.github/workflows/releases.yml (1)
38-47: Node + pnpm setup addressed.This resolves the build.rs dependency on pnpm noted earlier.
🧹 Nitpick comments (5)
crates/init/Cargo.toml (1)
13-13: Trim trailing whitespace.Minor cleanup.
-async-recursion.workspace = true +async-recursion.workspace = truecrates/bfv-helpers/Cargo.toml (1)
18-18: Pin git dependency for reproducibility.Consider pinning
fhe-utilto a tag or commit to avoid drifting builds.Example:
-fhe-util = { git = "https://github.com/gnosisguild/fhe.rs" } +fhe-util = { git = "https://github.com/gnosisguild/fhe.rs", tag = "vX.Y.Z" } +# or +# fhe-util = { git = "https://github.com/gnosisguild/fhe.rs", rev = "<commit-sha>" }.github/workflows/releases.yml (2)
48-52: Add a warn-only cleanliness check to replace the removed deep clean.Matches the PR goal: detect but don’t mutate the working tree.
Insert after this step:
- name: Install dependencies run: | cd packages/enclave-contracts pnpm install --frozen-lockfile + + - name: Verify working tree cleanliness (warn only) + run: | + if [[ -n "$(git status --porcelain)" ]]; then + echo "::warning:: Working tree is dirty after setup. Modified files:" + git status --porcelain + fi
84-85: Gate NPM release on Rust releases being created.Prevents running NPM release when Rust produced no releases.
- if: github.event_name == 'push' + if: github.event_name == 'push' && needs.release-rust.outputs.releases_created == 'true'Cargo.toml (1)
62-86: Optional: ensure member crates consume these viaworkspace = true.To avoid drift, confirm each crate depends on these as
{ workspace = true }. I can script a quick audit if helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
Cargo.lockis excluded by!**/*.lockexamples/CRISP/Cargo.lockis excluded by!**/*.locktemplates/default/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
.github/workflows/releases.yml(5 hunks)Cargo.toml(1 hunks)crates/aggregator/Cargo.toml(1 hunks)crates/bfv-helpers/Cargo.toml(1 hunks)crates/cli/Cargo.toml(1 hunks)crates/compute-provider/Cargo.toml(1 hunks)crates/config/Cargo.toml(1 hunks)crates/crypto/Cargo.toml(1 hunks)crates/data/Cargo.toml(1 hunks)crates/enclaveup/Cargo.toml(1 hunks)crates/entrypoint/Cargo.toml(1 hunks)crates/events/Cargo.toml(1 hunks)crates/evm-helpers/Cargo.toml(1 hunks)crates/evm/Cargo.toml(1 hunks)crates/fhe/Cargo.toml(1 hunks)crates/indexer/Cargo.toml(1 hunks)crates/init/Cargo.toml(1 hunks)crates/keyshare/Cargo.toml(1 hunks)crates/logger/Cargo.toml(1 hunks)crates/net/Cargo.toml(1 hunks)crates/program-server/Cargo.toml(1 hunks)crates/request/Cargo.toml(1 hunks)crates/sdk/Cargo.toml(1 hunks)crates/sortition/Cargo.toml(1 hunks)crates/test-helpers/Cargo.toml(1 hunks)crates/tests/Cargo.toml(1 hunks)templates/default/Cargo.toml(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- crates/fhe/Cargo.toml
- crates/logger/Cargo.toml
- crates/events/Cargo.toml
- crates/test-helpers/Cargo.toml
- crates/data/Cargo.toml
- crates/sdk/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/evm/Cargo.toml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-25T10:28:56.174Z
Learnt from: ctrlc03
PR: gnosisguild/enclave#657
File: Cargo.toml:32-34
Timestamp: 2025-08-25T10:28:56.174Z
Learning: The examples/CRISP directory has its own Cargo.toml workspace configuration with members like "server", "wasm-crypto", "program/core", "program/client", etc. The root workspace intentionally excludes "examples/CRISP/server", "examples/CRISP/program", and "examples/CRISP/wasm-crypto" to prevent double workspace membership, which is the correct approach for self-contained example workspaces.
Applied to files:
crates/config/Cargo.tomlcrates/enclaveup/Cargo.tomlcrates/tests/Cargo.tomlcrates/sortition/Cargo.tomlCargo.tomlcrates/request/Cargo.tomlcrates/program-server/Cargo.tomlcrates/cli/Cargo.tomlcrates/net/Cargo.tomltemplates/default/Cargo.tomlcrates/aggregator/Cargo.tomlcrates/crypto/Cargo.tomlcrates/indexer/Cargo.tomlcrates/init/Cargo.tomlcrates/bfv-helpers/Cargo.tomlcrates/entrypoint/Cargo.tomlcrates/evm-helpers/Cargo.toml
📚 Learning: 2024-10-22T02:36:01.448Z
Learnt from: ryardley
PR: gnosisguild/enclave#145
File: packages/ciphernode/data/Cargo.toml:0-0
Timestamp: 2024-10-22T02:36:01.448Z
Learning: In `packages/ciphernode/data/Cargo.toml`, `actix-rt` is only used in tests and should remain in `[dev-dependencies]`.
Applied to files:
Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: build_sdk
- GitHub Check: test_net
- GitHub Check: build_enclave_cli
- GitHub Check: build_e3_support_dev
- GitHub Check: integration_prebuild
- GitHub Check: Build & Push Image
- GitHub Check: test_contracts
- GitHub Check: rust_unit
🔇 Additional comments (23)
crates/config/Cargo.toml (1)
3-3: LGTM: adopt workspace-managed versionSwitching to
version.workspace = truealigns with the workspace metadata strategy and avoids per-crate bumps. No functional changes.templates/default/Cargo.toml (1)
2-2: Resolver 3: ensure toolchain compatibility in CI
Cargo resolver v3 shipped in Rust/Cargo 1.84.0; ensure your CI and dev toolchains use Rust 1.84.0 or newer.crates/sortition/Cargo.toml (1)
3-3: LGTM: workspace-managed versionConsistent with the workspace metadata approach; no behavior change.
crates/entrypoint/Cargo.toml (1)
3-3: LGTM: workspace-managed versionKeeps versions centralized; build script unchanged.
crates/aggregator/Cargo.toml (1)
3-3: LGTM: workspace-managed versionMatches the rest of the workspace; no runtime impact.
crates/program-server/Cargo.toml (1)
3-3: LGTM: workspace-managed versionConsistent metadata inheritance; dependencies already workspace-scoped.
crates/cli/Cargo.toml (1)
3-3: LGTM: workspace-managed versionFits the centralized versioning; binaries unaffected.
crates/indexer/Cargo.toml (1)
3-3: LGTM: workspace-managed versionAligns with the repo-wide change; no API/public changes.
crates/net/Cargo.toml (2)
3-5: Workspace-managed metadata: looks good.Adopts workspace version/edition/license correctly and keeps dependencies on workspace.
3-5: Confirm root[workspace.package]and consistent inheritance
Ensure the root Cargo.toml defines a[workspace.package]section withversion,edition, andlicense, and that every crate usingversion.workspace = truealso setsedition.workspace = trueandlicense.workspace = true.crates/init/Cargo.toml (1)
3-5: Workspace inheritance applied correctly.crates/enclaveup/Cargo.toml (1)
3-5: Workspace-managed version/edition/license: LGTM.crates/evm-helpers/Cargo.toml (1)
3-5: Consistent switch to workspace-managed metadata.crates/tests/Cargo.toml (1)
3-5: Adopts workspace-managed metadata cleanly.crates/compute-provider/Cargo.toml (1)
3-5: Workspace-managed metadata: OK.crates/request/Cargo.toml (1)
3-5: Workspace-managed metadata: looks good.crates/crypto/Cargo.toml (1)
3-5: Workspace-managed metadata adoption is correct.crates/bfv-helpers/Cargo.toml (1)
3-3: Good move to workspace-managed versioning.Keeps crate in lockstep with the workspace release.
crates/keyshare/Cargo.toml (1)
3-5: LGTM: inherit version/edition/license from workspace.Consistent with the workspace-wide change.
.github/workflows/releases.yml (3)
16-17: PR-triggered dry-run is a solid addition.Avoids accidental publishes while still validating the flow.
61-62: Double-check release-plz input supports the boolean expression.The
commandinput using&& ... || ...should resolve to a string; verify the action supports this pattern, or split into two steps withif:guards.Option B (if needed):
- - name: Run release-plz - id: release-plz - uses: MarcoIeni/release-plz-action@v0.5.64 - ... - with: - version: "0.3.83" - command: ${{ github.event_name == 'pull_request' && 'release-pr --dry-run' || 'release-pr' }} + - name: Run release-plz (dry-run) + if: github.event_name == 'pull_request' + id: release-plz + uses: MarcoIeni/release-plz-action@v0.5.64 + with: + version: "0.3.83" + command: release-pr --dry-run + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }} + + - name: Run release-plz + if: github.event_name == 'push' + id: release-plz + uses: MarcoIeni/release-plz-action@v0.5.64 + with: + version: "0.3.83" + command: release-pr + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + CARGO_REGISTRY_TOKEN: ${{ secrets.CARGO_REGISTRY_TOKEN }}
119-119: Nice: binary release is properly gated on push + releases_created.Prevents noisy dispatches on PRs or dry-runs.
Cargo.toml (1)
55-55: Workspace version bump acknowledged.Aligns with crates using
version.workspace = true.
2 participants
#680 didn't worked out so I'm trying another approach. The release workflow was failing because contract artifacts were tracked in git but became uncommitted during CI runs, causing release-plz to error with "uncommitted changes".
I have removed the aggressive
git cleanand replaced with a verification step that warns about the uncommitted changes to avoid removing the committed artifacts. that's itSummary by CodeRabbit