chore: use sccache for cargo tests#3612
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 GitHub Actions rust-cache with sccache (optional R2/S3 backend), adds sccache exports to Changessccache with R2 backend caching
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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: d8e9e359dd
ℹ️ 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".
| echo "AWS_ACCESS_KEY_ID=${BAML_SCCACHE_R2_ACCESS_KEY_ID}" | ||
| echo "AWS_SECRET_ACCESS_KEY=${BAML_SCCACHE_R2_SECRET_ACCESS_KEY}" |
There was a problem hiding this comment.
In ci.yaml, pull_request runs invoke this reusable workflow with secrets: inherit before running PR-controlled cargo build/test code. Writing AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY to $GITHUB_ENV makes those long-lived R2 credentials available to every later cargo build script and test in the job, so a same-repo PR can exfiltrate them or write arbitrary cache entries. Gate the remote cache setup to trusted refs/events, or avoid placing the secrets in the job environment used by cargo code.
Useful? React with 👍 / 👎.
| # Shared sccache configuration. Keep credential values out of the repo: | ||
| # put BAML_SCCACHE_R2_ACCESS_KEY_ID and BAML_SCCACHE_R2_SECRET_ACCESS_KEY | ||
| # in $HOME/.envrc.baml for local shells, or in GitHub Actions secrets for CI. | ||
| export RUSTC_WRAPPER="${RUSTC_WRAPPER:-baml-sccache}" |
There was a problem hiding this comment.
When direnv is loaded on a machine that has not separately installed sccache (it is not provisioned by the repo's mise.toml or flake.nix), this default makes every local cargo invocation execute tools/baml-sccache; that wrapper ends with exec sccache, so cargo fails before invoking rustc instead of building normally. Only set RUSTC_WRAPPER when sccache exists, provision it in the dev environment, or leave it opt-in for local shells.
Useful? React with 👍 / 👎.
![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
🤖 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 @.envrc:
- Around line 12-18: The .envrc currently exports SCCACHE_* R2/S3 vars
unconditionally and tools/baml-sccache only sets
AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY when BAML_SCCACHE_R2_* are present, so
local builds can try to use remote storage without credentials; update
tools/baml-sccache to detect the absence of BAML_SCCACHE_R2_* (or missing AWS
creds) and either set SCCACHE_S3_NO_CREDENTIALS=true or unset/override
SCCACHE_BUCKET, SCCACHE_ENDPOINT and SCCACHE_S3_KEY_PREFIX before exec sccache
"$@", and also ensure AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY are unset when
switching to no-creds mode; reference the BAML_SCCACHE_R2_* vars, the
SCCACHE_S3_NO_CREDENTIALS env var, and the script tools/baml-sccache when making
the change.
In @.github/actions/setup-sccache/action.yml:
- Around line 42-50: The step is leaking R2 credentials by appending
AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY to $GITHUB_ENV; remove the two lines
that echo AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY so only SCCACHE_BUCKET,
SCCACHE_REGION, SCCACHE_ENDPOINT and SCCACHE_S3_KEY_PREFIX are written to
GITHUB_ENV. Locate the conditional that checks BAML_SCCACHE_R2_ACCESS_KEY_ID and
BAML_SCCACHE_R2_SECRET_ACCESS_KEY and delete the two echo lines for
AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY (keep the other echo lines intact);
if needed, ensure the sccache daemon is started with those credentials in a
restricted environment rather than exporting them to GITHUB_ENV.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 927ba0ea-2f9c-4fea-8250-7ff6df010eb9
📒 Files selected for processing (3)
.envrc.github/actions/setup-sccache/action.yml.github/workflows/cargo-tests.reusable.yaml
| if [[ -n "${BAML_SCCACHE_R2_ACCESS_KEY_ID}" && -n "${BAML_SCCACHE_R2_SECRET_ACCESS_KEY}" ]]; then | ||
| { | ||
| echo "SCCACHE_BUCKET=baml-build1" | ||
| echo "SCCACHE_REGION=auto" | ||
| echo "SCCACHE_ENDPOINT=https://321ca319116f1e5eefa9135d9d019a5a.r2.cloudflarestorage.com" | ||
| echo "SCCACHE_S3_KEY_PREFIX=${SCCACHE_S3_KEY_PREFIX}" | ||
| echo "AWS_ACCESS_KEY_ID=${BAML_SCCACHE_R2_ACCESS_KEY_ID}" | ||
| echo "AWS_SECRET_ACCESS_KEY=${BAML_SCCACHE_R2_SECRET_ACCESS_KEY}" | ||
| } >> "$GITHUB_ENV" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does sccache read AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY at server start, so the client process running cargo does not need them when an authenticated server is already running?
💡 Result:
Yes, sccache reads environment variables like AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY at server start [1]. Because sccache operates on a client-server architecture, the server process maintains the cache connection state [2][3]. When you run a cargo build, the client (your cargo process) sends the command to the already-running server daemon [2][1]. Since the server is the component responsible for performing the S3 operations and maintaining the authentication state, it must have access to the credentials when it starts [1]. Consequently, once the server is successfully running with the necessary credentials, the client process itself does not need these environment variables to be set in its own environment [2][3]. If you restart the server, however, it will need to be able to resolve the credentials again (e.g., from the environment, shared config files, or IAM instance metadata) to initialize the S3 connection [4][5]. Note that environment variables are only read at the time the server process is launched; they are not dynamically re-read from the client's environment during subsequent requests [1].
Citations:
- 1: https://docs.rs/crate/sccache/0.2.6
- 2: https://docs.rs/crate/sccache/0.2.5/source/README.md
- 3: https://github.com/wasmerio/sccache
- 4: https://crates.io/crates/sccache/0.3.3
- 5: https://github.com/mozilla/sccache/blob/main/docs/S3.md
Stop exporting R2 AWS_* credentials to GITHUB_ENV
sccache reads AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY from its environment when the server daemon starts; once that daemon is running, later cargo invocations are just clients and don’t need AWS_* set themselves. Appending the R2 keys to $GITHUB_ENV unnecessarily exposes them to all subsequent steps (including PR-controlled build/test), creating a credential leak and cache-poisoning risk. Remove the two echo AWS_* lines.
🔒️ Keep R2 keys out of the shared job environment
{
echo "SCCACHE_BUCKET=baml-build1"
echo "SCCACHE_REGION=auto"
echo "SCCACHE_ENDPOINT=https://321ca319116f1e5eefa9135d9d019a5a.r2.cloudflarestorage.com"
echo "SCCACHE_S3_KEY_PREFIX=${SCCACHE_S3_KEY_PREFIX}"
- echo "AWS_ACCESS_KEY_ID=${BAML_SCCACHE_R2_ACCESS_KEY_ID}"
- echo "AWS_SECRET_ACCESS_KEY=${BAML_SCCACHE_R2_SECRET_ACCESS_KEY}"
} >> "$GITHUB_ENV"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [[ -n "${BAML_SCCACHE_R2_ACCESS_KEY_ID}" && -n "${BAML_SCCACHE_R2_SECRET_ACCESS_KEY}" ]]; then | |
| { | |
| echo "SCCACHE_BUCKET=baml-build1" | |
| echo "SCCACHE_REGION=auto" | |
| echo "SCCACHE_ENDPOINT=https://321ca319116f1e5eefa9135d9d019a5a.r2.cloudflarestorage.com" | |
| echo "SCCACHE_S3_KEY_PREFIX=${SCCACHE_S3_KEY_PREFIX}" | |
| echo "AWS_ACCESS_KEY_ID=${BAML_SCCACHE_R2_ACCESS_KEY_ID}" | |
| echo "AWS_SECRET_ACCESS_KEY=${BAML_SCCACHE_R2_SECRET_ACCESS_KEY}" | |
| } >> "$GITHUB_ENV" | |
| if [[ -n "${BAML_SCCACHE_R2_ACCESS_KEY_ID}" && -n "${BAML_SCCACHE_R2_SECRET_ACCESS_KEY}" ]]; then | |
| { | |
| echo "SCCACHE_BUCKET=baml-build1" | |
| echo "SCCACHE_REGION=auto" | |
| echo "SCCACHE_ENDPOINT=https://321ca319116f1e5eefa9135d9d019a5a.r2.cloudflarestorage.com" | |
| echo "SCCACHE_S3_KEY_PREFIX=${SCCACHE_S3_KEY_PREFIX}" | |
| } >> "$GITHUB_ENV" |
🤖 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-sccache/action.yml around lines 42 - 50, The step is
leaking R2 credentials by appending AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY
to $GITHUB_ENV; remove the two lines that echo AWS_ACCESS_KEY_ID and
AWS_SECRET_ACCESS_KEY so only SCCACHE_BUCKET, SCCACHE_REGION, SCCACHE_ENDPOINT
and SCCACHE_S3_KEY_PREFIX are written to GITHUB_ENV. Locate the conditional that
checks BAML_SCCACHE_R2_ACCESS_KEY_ID and BAML_SCCACHE_R2_SECRET_ACCESS_KEY and
delete the two echo lines for AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY (keep
the other echo lines intact); if needed, ensure the sccache daemon is started
with those credentials in a restricted environment rather than exporting them to
GITHUB_ENV.
Binary size checks passed✅ 7 passed
Generated by |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/cargo-tests.reusable.yaml (1)
58-88: 🏗️ Heavy liftConsider extracting sccache setup to a composite action.
This ~25-line shell script is duplicated verbatim across 8 jobs (linux, macos, windows, sdk-tests, wasm, msrv, cargo-doc, snapshot-tests). Any bug fix or enhancement would require updating all 8 locations.
Consider extracting this into a local composite action (e.g.,
.github/actions/setup-sccache) that encapsulates the direnv loading, credential handling, server startup, and GITHUB_ENV persistence. The action could accept the two R2 secrets as inputs.🤖 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/cargo-tests.reusable.yaml around lines 58 - 88, The sccache setup block (step named "Load .envrc") is duplicated across multiple jobs; extract it into a reusable local composite action (e.g., .github/actions/setup-sccache) that accepts inputs for BAML_SCCACHE_R2_ACCESS_KEY_ID and BAML_SCCACHE_R2_SECRET_ACCESS_KEY and encapsulates the direnv loading, export/eval of RUSTC_WRAPPER/SCCACHE_ERROR_LOG, the AWS credential conditional, sccache --stop-server / --start-server, and the loop that writes variables to GITHUB_ENV; replace the inline script with a uses: ./.github/actions/setup-sccache step that maps the two secrets to inputs and preserves the same exported variables (SCCACHE_*, AWS_ACCESS_KEY_ID/SECRET, RUSTC_WRAPPER, and SCCACHE_ERROR_LOG) so behavior with variables like SCCACHE_BUCKET, SCCACHE_BASEDIRS and RUNNER_TEMP remains identical.
🤖 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 @.envrc:
- Around line 27-29: The PATH_add call is brittle because it assumes "brew
--prefix llvm" succeeds; guard it by first checking that Homebrew reports llvm
is installed (or that brew --prefix llvm returns a valid directory) before
calling PATH_add. Update the .envrc snippet around the PATH_add invocation so
that after confirming command -v brew and CI checks, you verify llvm exists
(e.g., via brew ls/ls --versions or by capturing brew --prefix llvm and testing
the resulting directory) and only then call PATH_add with the llvm bin path;
reference the PATH_add invocation and the brew --prefix llvm call to locate the
change.
---
Nitpick comments:
In @.github/workflows/cargo-tests.reusable.yaml:
- Around line 58-88: The sccache setup block (step named "Load .envrc") is
duplicated across multiple jobs; extract it into a reusable local composite
action (e.g., .github/actions/setup-sccache) that accepts inputs for
BAML_SCCACHE_R2_ACCESS_KEY_ID and BAML_SCCACHE_R2_SECRET_ACCESS_KEY and
encapsulates the direnv loading, export/eval of RUSTC_WRAPPER/SCCACHE_ERROR_LOG,
the AWS credential conditional, sccache --stop-server / --start-server, and the
loop that writes variables to GITHUB_ENV; replace the inline script with a uses:
./.github/actions/setup-sccache step that maps the two secrets to inputs and
preserves the same exported variables (SCCACHE_*, AWS_ACCESS_KEY_ID/SECRET,
RUSTC_WRAPPER, and SCCACHE_ERROR_LOG) so behavior with variables like
SCCACHE_BUCKET, SCCACHE_BASEDIRS and RUNNER_TEMP remains identical.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 287d85e2-2952-4fb3-9b40-44380bfbfc97
📒 Files selected for processing (3)
.envrc.github/workflows/cargo-tests.reusable.yamlmise.toml
✅ Files skipped from review due to trivial changes (1)
- mise.toml
| if [ "${CI:-}" != "true" ] && command -v brew >/dev/null 2>&1; then | ||
| PATH_add "$(brew --prefix llvm)/bin" | ||
| fi |
There was a problem hiding this comment.
Guard brew --prefix llvm before PATH_add to avoid brittle direnv loads.
If Homebrew exists but llvm isn’t installed, brew --prefix llvm can fail and make local shell init noisy/fragile. Add a lightweight existence check first.
Proposed patch
- if [ "${CI:-}" != "true" ] && command -v brew >/dev/null 2>&1; then
- PATH_add "$(brew --prefix llvm)/bin"
+ if [ "${CI:-}" != "true" ] && command -v brew >/dev/null 2>&1; then
+ if brew --prefix llvm >/dev/null 2>&1; then
+ PATH_add "$(brew --prefix llvm)/bin"
+ fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ "${CI:-}" != "true" ] && command -v brew >/dev/null 2>&1; then | |
| PATH_add "$(brew --prefix llvm)/bin" | |
| fi | |
| if [ "${CI:-}" != "true" ] && command -v brew >/dev/null 2>&1; then | |
| if brew --prefix llvm >/dev/null 2>&1; then | |
| PATH_add "$(brew --prefix llvm)/bin" | |
| fi | |
| fi |
🤖 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 @.envrc around lines 27 - 29, The PATH_add call is brittle because it assumes
"brew --prefix llvm" succeeds; guard it by first checking that Homebrew reports
llvm is installed (or that brew --prefix llvm returns a valid directory) before
calling PATH_add. Update the .envrc snippet around the PATH_add invocation so
that after confirming command -v brew and CI checks, you verify llvm exists
(e.g., via brew ls/ls --versions or by capturing brew --prefix llvm and testing
the resulting directory) and only then call PATH_add with the llvm bin path;
reference the PATH_add invocation and the brew --prefix llvm call to locate the
change.
sxlijin
force-pushed
the
sam/sccache-ci
branch
2 times, most recently
from
739ea16 to
207b915
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/cargo-tests.reusable.yaml (1)
70-100: ⚖️ Poor tradeoffConsider extracting the repeated "Load .envrc" block into a composite action.
This ~30-line script is duplicated verbatim across 8 jobs (cargo-test-linux, cargo-test-macos, cargo-test-windows, sdk-tests, cargo-test-wasm, cargo-build-msrv, cargo-doc, snapshot-tests). Any future bug fix or enhancement would require updating all 8 copies, risking drift.
A composite action at
.github/actions/load-sccache-env/action.ymlcould encapsulate this logic and accept the R2 secrets as inputs, reducing maintenance burden and ensuring consistency.🤖 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/cargo-tests.reusable.yaml around lines 70 - 100, Extract the repeated "Load .envrc" step into a reusable composite action (e.g., .github/actions/load-sccache-env/action.yml) that accepts inputs for BAML_SCCACHE_R2_ACCESS_KEY_ID and BAML_SCCACHE_R2_SECRET_ACCESS_KEY and exposes outputs or writes to GITHUB_ENV; move the logic that sets RUSTC_WRAPPER, SCCACHE_ERROR_LOG (using RUNNER_TEMP), conditionally exports/unsets AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY and other SCCACHE_* vars, runs sccache --stop-server && sccache --start-server, and appends the selected env vars to GITHUB_ENV into the composite action; then replace each "Load .envrc" job step with a single uses: ./.github/actions/load-sccache-env and pass the two secrets via with:, ensuring the composite action mirrors behavior for the loop that writes variables into GITHUB_ENV and preserves SCCACHE_ERROR_LOG and RUNNER_TEMP usage.
🤖 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.
Nitpick comments:
In @.github/workflows/cargo-tests.reusable.yaml:
- Around line 70-100: Extract the repeated "Load .envrc" step into a reusable
composite action (e.g., .github/actions/load-sccache-env/action.yml) that
accepts inputs for BAML_SCCACHE_R2_ACCESS_KEY_ID and
BAML_SCCACHE_R2_SECRET_ACCESS_KEY and exposes outputs or writes to GITHUB_ENV;
move the logic that sets RUSTC_WRAPPER, SCCACHE_ERROR_LOG (using RUNNER_TEMP),
conditionally exports/unsets AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY and other
SCCACHE_* vars, runs sccache --stop-server && sccache --start-server, and
appends the selected env vars to GITHUB_ENV into the composite action; then
replace each "Load .envrc" job step with a single uses:
./.github/actions/load-sccache-env and pass the two secrets via with:, ensuring
the composite action mirrors behavior for the loop that writes variables into
GITHUB_ENV and preserves SCCACHE_ERROR_LOG and RUNNER_TEMP usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f0f62ab2-1ccf-4790-9489-ba75b5351f7a
📒 Files selected for processing (3)
.envrc.github/workflows/cargo-tests.reusable.yamlmise.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- mise.toml
- .envrc
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/cargo-tests.reusable.yaml (1)
70-100: ⚖️ Poor tradeoffConsider extracting the "Load .envrc" block into a reusable composite action.
The ~30-line shell script for loading
.envrc, mapping R2 secrets, restarting sccache, and persisting env vars is duplicated verbatim across 8 jobs. Extracting it into a dedicated composite action (e.g.,.github/actions/setup-sccache) would reduce maintenance burden and risk of drift when updating the logic.This is a good-to-have improvement that can be deferred if the explicit inline approach is preferred for CI debuggability.
Also applies to: 169-199, 261-291, 378-408, 477-507, 577-607, 657-687, 737-767
🤖 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/cargo-tests.reusable.yaml around lines 70 - 100, The "Load .envrc" job block is duplicated across many jobs; extract that ~30-line shell snippet into a reusable composite GitHub Action (e.g., .github/actions/setup-sccache) that exposes inputs for the secret names and any toggles and encapsulates the steps: run direnv allow/eval, export RUSTC_WRAPPER and SCCACHE_ERROR_LOG, conditionally map BAML_SCCACHE_R2_ACCESS_KEY_ID/BAML_SCCACHE_R2_SECRET_ACCESS_KEY to AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY, restart sccache, and persist the environment variables (the same list used in the for loop) to GITHUB_ENV; then replace each inline "Load .envrc" step with a uses: call to the new action passing the required secrets/inputs..github/actions/setup-mise/action.yml (1)
52-60: 💤 Low valueRemove unused
install_argsfrom mise-action invocation.Since
install: false(line 55), theinstall_argsparameter on line 60 is not used byjdx/mise-action. The actual install is now delegated to the explicitmise installsteps below. This parameter can be removed to avoid confusion.🧹 Proposed cleanup
- name: Install mise uses: jdx/[email protected] with: install: false # Our own prefix (not the upstream default), shared across every # baml_language workflow. Bump the trailing number to invalidate # every consumer's cache at once. cache_key_prefix: mise-baml1 - install_args: ${{ inputs.install_args }}🤖 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-mise/action.yml around lines 52 - 60, Remove the unused install_args input from the jdx/mise-action invocation: since the action is called with install: false (in the step using jdx/[email protected]) the install_args parameter is ignored and only causes confusion, so delete the install_args: ${{ inputs.install_args }} line from that action step and keep the explicit subsequent mise install steps as the source of truth.
🤖 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.
Nitpick comments:
In @.github/actions/setup-mise/action.yml:
- Around line 52-60: Remove the unused install_args input from the
jdx/mise-action invocation: since the action is called with install: false (in
the step using jdx/[email protected]) the install_args parameter is ignored and
only causes confusion, so delete the install_args: ${{ inputs.install_args }}
line from that action step and keep the explicit subsequent mise install steps
as the source of truth.
In @.github/workflows/cargo-tests.reusable.yaml:
- Around line 70-100: The "Load .envrc" job block is duplicated across many
jobs; extract that ~30-line shell snippet into a reusable composite GitHub
Action (e.g., .github/actions/setup-sccache) that exposes inputs for the secret
names and any toggles and encapsulates the steps: run direnv allow/eval, export
RUSTC_WRAPPER and SCCACHE_ERROR_LOG, conditionally map
BAML_SCCACHE_R2_ACCESS_KEY_ID/BAML_SCCACHE_R2_SECRET_ACCESS_KEY to
AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY, restart sccache, and persist the
environment variables (the same list used in the for loop) to GITHUB_ENV; then
replace each inline "Load .envrc" step with a uses: call to the new action
passing the required secrets/inputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c63447b5-ad36-4dbc-b1e6-53bf4b2791f5
📒 Files selected for processing (4)
.envrc.github/actions/setup-mise/action.yml.github/workflows/cargo-tests.reusable.yamlmise.toml
✅ Files skipped from review due to trivial changes (1)
- mise.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- .envrc
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/cargo-tests.reusable.yaml (2)
65-71: ⚖️ Poor tradeoffStatic analysis flags unpinned action reference.
Swatinem/rust-cache@v2uses a version tag rather than a commit hash. For enhanced supply-chain security, consider pinning to a specific commit hash. This applies to all 8 usages in this file.Example pinning
-- uses: Swatinem/rust-cache@v2 +# Pinned from v2.x.x (update hash when upgrading) +- uses: Swatinem/rust-cache@<commit-sha>🤖 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/cargo-tests.reusable.yaml around lines 65 - 71, The workflow uses the unpinned action reference Swatinem/rust-cache@v2 which should be replaced with a specific commit SHA for supply-chain security; locate every usage of the identifier Swatinem/rust-cache@v2 in the file (all 8 occurrences) and update each to Swatinem/rust-cache@<commit-sha> using the exact git commit hash for the version you vetted, ensuring the same pinned SHA is applied consistently and updating any accompanying comments or save-if/shared-key parameters unchanged.
73-103: ⚖️ Poor tradeoffConsider extracting the
.envrcloading logic to a composite action.This ~30-line bash script is duplicated verbatim across 8 jobs. Extracting it to a composite action (e.g.,
.github/actions/load-envrc-sccache/action.yml) would centralize maintenance and reduce the risk of drift when updates are needed.🤖 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/cargo-tests.reusable.yaml around lines 73 - 103, Extract the repeated "Load .envrc" bash block into a reusable composite action and replace each duplicated job step with a single uses: call; specifically, move the logic that handles direnv allow/eval, exports RUSTC_WRAPPER and SCCACHE_ERROR_LOG, conditionally maps BAML_SCCACHE_R2_ACCESS_KEY_ID/BAML_SCCACHE_R2_SECRET_ACCESS_KEY to AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY (or unsets SCCACHE_*/AWS_* and emits the notice), restarts sccache (sccache --stop-server || true; sccache --start-server) and the for-loop that writes envs to GITHUB_ENV into the composite action (e.g., accept inputs for the two secrets and RUNNER_TEMP, and expose any needed outputs or simply set environment in the caller); then update every job that currently runs the "Load .envrc" step to call this composite action to centralize maintenance and avoid drift.
🤖 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/cargo-tests.reusable.yaml:
- Around line 99-103: The env propagation loop in
.github/workflows/cargo-tests.reusable.yaml omits SCCACHE_SERVER_UDS so later
steps don't receive the UDS path; update the list of variable names iterated
(the for ... in ...) to include SCCACHE_SERVER_UDS so the existing check if [[
-n "${!name+x}" ]] and the printf "%s=%s\n" "$name" "${!name}" >> "$GITHUB_ENV"
will persist SCCACHE_SERVER_UDS into GITHUB_ENV and ensure clients/servers use
the same socket endpoint.
---
Nitpick comments:
In @.github/workflows/cargo-tests.reusable.yaml:
- Around line 65-71: The workflow uses the unpinned action reference
Swatinem/rust-cache@v2 which should be replaced with a specific commit SHA for
supply-chain security; locate every usage of the identifier
Swatinem/rust-cache@v2 in the file (all 8 occurrences) and update each to
Swatinem/rust-cache@<commit-sha> using the exact git commit hash for the version
you vetted, ensuring the same pinned SHA is applied consistently and updating
any accompanying comments or save-if/shared-key parameters unchanged.
- Around line 73-103: Extract the repeated "Load .envrc" bash block into a
reusable composite action and replace each duplicated job step with a single
uses: call; specifically, move the logic that handles direnv allow/eval, exports
RUSTC_WRAPPER and SCCACHE_ERROR_LOG, conditionally maps
BAML_SCCACHE_R2_ACCESS_KEY_ID/BAML_SCCACHE_R2_SECRET_ACCESS_KEY to
AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY (or unsets SCCACHE_*/AWS_* and emits the
notice), restarts sccache (sccache --stop-server || true; sccache
--start-server) and the for-loop that writes envs to GITHUB_ENV into the
composite action (e.g., accept inputs for the two secrets and RUNNER_TEMP, and
expose any needed outputs or simply set environment in the caller); then update
every job that currently runs the "Load .envrc" step to call this composite
action to centralize maintenance and avoid drift.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a878ae66-02ca-4fdf-aa7e-b251269a4d10
📒 Files selected for processing (4)
.envrc.github/actions/setup-mise/action.yml.github/workflows/cargo-tests.reusable.yamlmise.toml
✅ Files skipped from review due to trivial changes (1)
- mise.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/actions/setup-mise/action.yml
- .envrc
Merging this PR will not alter performance
Comparing 
Footnotes
|
Route RUSTC_WRAPPER through a baml-sccache wrapper (tools/baml-sccache on POSIX, the native tools_sccache crate on Windows) backed by R2, configured entirely from .envrc so CI matches local shells. mise installs sccache + direnv; each cargo job loads .envrc via `direnv export gha`. Swatinem/rust-cache still caches the cargo registry/git download state, with cache-targets: false so sccache owns target/ and the two caches don't compete. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
1 participant
Use sccache (R2-backed) for Rust compilation artifacts in the cargo CI jobs, configured entirely from
.envrcso CI matches local shells.tools_sccachecrate /tools/baml-sccachewrapper: aRUSTC_WRAPPERthat mapsBAML_SCCACHE_R2_*→AWS_*and execs sccache (native crate on Windows, shell script on POSIX)..envrcviadirenv export gha(the single source of truth for the sccache/R2 config).cache-targets: falseso sccache ownstarget/and the two caches don't compete. Fork PRs without R2 secrets fall back to the runner-local cache.Follow-up #3624 replaces Swatinem for the download caches with a granular, Cargo.lock-driven R2 action (
cache-cargo-home); this PR is the sccache base it stacks on.🤖 Generated with Claude Code