Use flake for PR build workflows#533
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR migrates CI workflows to Nix-driven dev shells, expands flake.nix to provide multi-environment devShells/checks/apps, adds a shared Bash CI helper library, implements platform-specific CI entrypoint scripts (android/desktop/ios/frontend/rust), and standardizes ONNX runtime provisioning and deterministic artifact hashing. ChangesNix-Based CI Infrastructure and Script Modernization
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying maple with
|
| Latest commit: |
a002114
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3f87441d.maple-ca8.pages.dev |
| Branch Preview URL: | https://codex-reproducible-flake-bui.maple-ca8.pages.dev |
f4bca39 to
343abb1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@scripts/ci/ios-pr.sh`:
- Around line 43-47: The script copies iOS files into a temp dir without
checking for their existence which can fail before the trap is installed; set up
ios_project_state_dir and install the trap (trap restore_ios_build_state EXIT)
before attempting any copies, and guard each cp of
"${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" and
"${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" with existence checks
(e.g. [ -f ... ] or similar) so the script continues gracefully when those files
are absent, optionally emitting a warning when a file is missing.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 13b9d476-beee-4574-a178-6e394a447bc3
📒 Files selected for processing (15)
.github/workflows/android-pr-build.yml.github/workflows/desktop-pr-build.yml.github/workflows/frontend-tests.yml.github/workflows/mobile-pr-build.yml.github/workflows/rust-tests.ymlflake.nixfrontend/src-tauri/scripts/onnxruntime-pins.shfrontend/src-tauri/scripts/provide-linux-onnxruntime.shfrontend/src-tauri/scripts/setup-ios-cargo-config.shscripts/ci/_common.shscripts/ci/android-pr.shscripts/ci/desktop-pr.shscripts/ci/frontend.shscripts/ci/ios-pr.shscripts/ci/rust.sh
| cd "${TAURI_DIR}" | ||
| ios_project_state_dir="$(mktemp -d)" | ||
| cp "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" "${ios_project_state_dir}/Info.plist" | ||
| cp "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" "${ios_project_state_dir}/maple_iOS.entitlements" | ||
| trap restore_ios_build_state EXIT |
There was a problem hiding this comment.
State preservation may fail if iOS project files don't exist.
Lines 45-46 copy files without checking if they exist first. On a fresh checkout where bun tauri ios init hasn't been run yet, these files may not exist and the script will fail before the trap is set.
Consider adding existence checks or letting these operations fail gracefully:
Proposed fix
cd "${TAURI_DIR}"
ios_project_state_dir="$(mktemp -d)"
-cp "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" "${ios_project_state_dir}/Info.plist"
-cp "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" "${ios_project_state_dir}/maple_iOS.entitlements"
+if [ -f "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" ]; then
+ cp "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" "${ios_project_state_dir}/Info.plist"
+fi
+if [ -f "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" ]; then
+ cp "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" "${ios_project_state_dir}/maple_iOS.entitlements"
+fi
trap restore_ios_build_state EXIT📝 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.
| cd "${TAURI_DIR}" | |
| ios_project_state_dir="$(mktemp -d)" | |
| cp "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" "${ios_project_state_dir}/Info.plist" | |
| cp "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" "${ios_project_state_dir}/maple_iOS.entitlements" | |
| trap restore_ios_build_state EXIT | |
| cd "${TAURI_DIR}" | |
| ios_project_state_dir="$(mktemp -d)" | |
| if [ -f "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" ]; then | |
| cp "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" "${ios_project_state_dir}/Info.plist" | |
| fi | |
| if [ -f "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" ]; then | |
| cp "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" "${ios_project_state_dir}/maple_iOS.entitlements" | |
| fi | |
| trap restore_ios_build_state EXIT |
🤖 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 `@scripts/ci/ios-pr.sh` around lines 43 - 47, The script copies iOS files into
a temp dir without checking for their existence which can fail before the trap
is installed; set up ios_project_state_dir and install the trap (trap
restore_ios_build_state EXIT) before attempting any copies, and guard each cp of
"${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" and
"${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" with existence checks
(e.g. [ -f ... ] or similar) so the script continues gracefully when those files
are absent, optionally emitting a warning when a file is missing.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/ci/ios-pr.sh (1)
44-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInstall the EXIT trap before copying state files, and guard missing files.
Line 45 and Line 46 can fail before
trap restore_ios_build_state EXITis active, which skips cleanup/setup recovery on missing generated iOS files.Suggested patch
cd "${TAURI_DIR}" ios_project_state_dir="$(mktemp -d)" -cp "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" "${ios_project_state_dir}/Info.plist" -cp "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" "${ios_project_state_dir}/maple_iOS.entitlements" trap restore_ios_build_state EXIT +if [ -f "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" ]; then + cp "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" "${ios_project_state_dir}/Info.plist" +fi +if [ -f "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" ]; then + cp "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" "${ios_project_state_dir}/maple_iOS.entitlements" +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 `@scripts/ci/ios-pr.sh` around lines 44 - 47, Install the EXIT trap before performing any file operations so restore_ios_build_state will run on early failures; move the trap restore_ios_build_state EXIT line above the cp commands that copy into ios_project_state_dir and add existence checks for "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" and "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" (or bail with an error) to guard against missing generated iOS files before copying into ios_project_state_dir.
🤖 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 `@scripts/ci/android-pr.sh`:
- Around line 43-44: Add a guard after computing toolchain_prebuilt to fail fast
when no prebuilt directory was found: check if the variable toolchain_prebuilt
is empty or unset, and if so print a clear error to stderr (e.g., printf 'Error:
no prebuilt toolchain found in %s\n' "$NDK_HOME/toolchains/llvm/prebuilt" >&2)
and exit with a non‑zero status; otherwise continue to export PATH as before.
Ensure you reference the existing toolchain_prebuilt variable in the check so
the invalid PATH is never exported.
---
Duplicate comments:
In `@scripts/ci/ios-pr.sh`:
- Around line 44-47: Install the EXIT trap before performing any file operations
so restore_ios_build_state will run on early failures; move the trap
restore_ios_build_state EXIT line above the cp commands that copy into
ios_project_state_dir and add existence checks for
"${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" and
"${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" (or bail with an
error) to guard against missing generated iOS files before copying into
ios_project_state_dir.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04a4cefe-9ce3-4ba8-87fc-49dfcfbc911e
📒 Files selected for processing (15)
.github/workflows/android-pr-build.yml.github/workflows/desktop-pr-build.yml.github/workflows/frontend-tests.yml.github/workflows/mobile-pr-build.yml.github/workflows/rust-tests.ymlflake.nixfrontend/src-tauri/scripts/onnxruntime-pins.shfrontend/src-tauri/scripts/provide-linux-onnxruntime.shfrontend/src-tauri/scripts/setup-ios-cargo-config.shscripts/ci/_common.shscripts/ci/android-pr.shscripts/ci/desktop-pr.shscripts/ci/frontend.shscripts/ci/ios-pr.shscripts/ci/rust.sh
| toolchain_prebuilt="$(find "${NDK_HOME}/toolchains/llvm/prebuilt" -mindepth 1 -maxdepth 1 -type d | sort | head -n 1)" | ||
| export PATH="${toolchain_prebuilt}/bin:${PATH}" |
There was a problem hiding this comment.
Add an explicit guard for empty toolchain_prebuilt.
If find returns no child prebuilt directory, Line 44 exports an invalid toolchain path and failures happen later with poor diagnostics. Fail fast here.
Suggested patch
-toolchain_prebuilt="$(find "${NDK_HOME}/toolchains/llvm/prebuilt" -mindepth 1 -maxdepth 1 -type d | sort | head -n 1)"
+toolchain_prebuilt="$(find "${NDK_HOME}/toolchains/llvm/prebuilt" -mindepth 1 -maxdepth 1 -type d | sort | head -n 1 || true)"
+if [ -z "${toolchain_prebuilt}" ] || [ ! -d "${toolchain_prebuilt}/bin" ]; then
+ echo "Could not resolve an NDK LLVM prebuilt toolchain under ${NDK_HOME}/toolchains/llvm/prebuilt" >&2
+ exit 1
+fi
export PATH="${toolchain_prebuilt}/bin:${PATH}"📝 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.
| toolchain_prebuilt="$(find "${NDK_HOME}/toolchains/llvm/prebuilt" -mindepth 1 -maxdepth 1 -type d | sort | head -n 1)" | |
| export PATH="${toolchain_prebuilt}/bin:${PATH}" | |
| toolchain_prebuilt="$(find "${NDK_HOME}/toolchains/llvm/prebuilt" -mindepth 1 -maxdepth 1 -type d | sort | head -n 1 || true)" | |
| if [ -z "${toolchain_prebuilt}" ] || [ ! -d "${toolchain_prebuilt}/bin" ]; then | |
| echo "Could not resolve an NDK LLVM prebuilt toolchain under ${NDK_HOME}/toolchains/llvm/prebuilt" >&2 | |
| exit 1 | |
| fi | |
| export PATH="${toolchain_prebuilt}/bin:${PATH}" |
🤖 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 `@scripts/ci/android-pr.sh` around lines 43 - 44, Add a guard after computing
toolchain_prebuilt to fail fast when no prebuilt directory was found: check if
the variable toolchain_prebuilt is empty or unset, and if so print a clear error
to stderr (e.g., printf 'Error: no prebuilt toolchain found in %s\n'
"$NDK_HOME/toolchains/llvm/prebuilt" >&2) and exit with a non‑zero status;
otherwise continue to export PATH as before. Ensure you reference the existing
toolchain_prebuilt variable in the check so the invalid PATH is never exported.
343abb1 to
0439a4a
Compare
1491d0a to
d94bc93
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/ci/ios-pr.sh (1)
45-48:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInstall the restore trap before snapshot copies and guard missing files.
The snapshot
cpcommands can fail before theEXITtrap is registered. That makes this entrypoint brittle on fresh or partially initialized iOS project state.Proposed fix
cd "${TAURI_DIR}" ios_project_state_dir="$(mktemp -d)" -cp "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" "${ios_project_state_dir}/Info.plist" -cp "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" "${ios_project_state_dir}/maple_iOS.entitlements" trap restore_ios_build_state EXIT +if [ -f "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" ]; then + cp "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" "${ios_project_state_dir}/Info.plist" +fi +if [ -f "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" ]; then + cp "${TAURI_DIR}/gen/apple/maple_iOS/maple_iOS.entitlements" "${ios_project_state_dir}/maple_iOS.entitlements" +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 `@scripts/ci/ios-pr.sh` around lines 45 - 48, The EXIT trap is registered too late (after the cp calls) and the cp commands assume files exist; move the trap call (restore_ios_build_state) to immediately after ios_project_state_dir is created so cleanup is guaranteed, and guard each copy of "${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" and ".../maple_iOS.entitlements" by testing file existence (e.g., [ -f ... ]) before running cp, logging or skipping if missing to avoid hard failures; reference ios_project_state_dir, restore_ios_build_state, and the two cp lines to locate the changes.
🧹 Nitpick comments (2)
flake.nix (1)
126-139: 💤 Low valuePotential argument parsing issue in
target addwhen using--toolchainflag.When
--toolchainor-tis encountered,shift 2is used, but if the toolchain value is missing (e.g.,--toolchainis the last argument), this will cause an error or skip the next positional argument incorrectly.🛡️ Safer argument parsing
--toolchain | -t) - shift 2 + shift + [ "$#" -gt 0 ] && shift ;;🤖 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 `@flake.nix` around lines 126 - 139, The argument parsing loop that handles the --toolchain/-t case (inside the while ... case block that appends to requested_targets) currently does an unconditional shift 2 which misbehaves if --toolchain is the last argument; update the --toolchain|-t branch to validate that a following value exists before shifting twice (e.g., check that "$#" -gt 1), and if missing emit an error or handle the missing toolchain value safely; ensure the rest of the case branches and the requested_targets array behavior remain unchanged..github/workflows/desktop-pr-build.yml (1)
18-35: 💤 Low valueXcode selection logic is duplicated across workflows.
The same Xcode 26.5 selection logic appears in both
desktop-pr-build.ymlandmobile-pr-build.yml. Consider extracting this into a reusable composite action or a shared script to reduce duplication and ensure 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/desktop-pr-build.yml around lines 18 - 35, Extract the duplicated "Select Xcode 26.5" step logic into a single reusable unit and update both workflows to call it: create a composite GitHub Action (e.g., .github/actions/select-xcode-26-5 with an action.yml) or a shared script (e.g., scripts/select_xcode_26_5.sh) that contains the for-loop/xcode-select/xcodebuild logic currently under the step named "Select Xcode 26.5", then replace the inline step in desktop-pr-build.yml and mobile-pr-build.yml with a uses: reference to the new action or a run: invoking the shared script so both workflows reuse the exact same selection logic.
🤖 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 `@scripts/ci/rust.sh`:
- Line 10: The CI Rust test invocation runs cargo test --all-targets without
enforcing the lockfile, allowing Cargo to update Cargo.lock during CI; update
the test command in scripts/ci/rust.sh to run cargo test --all-targets --locked
(i.e., add the --locked flag) so dependency resolution uses the checked-in
Cargo.lock and fails the job if the lockfile would change.
---
Duplicate comments:
In `@scripts/ci/ios-pr.sh`:
- Around line 45-48: The EXIT trap is registered too late (after the cp calls)
and the cp commands assume files exist; move the trap call
(restore_ios_build_state) to immediately after ios_project_state_dir is created
so cleanup is guaranteed, and guard each copy of
"${TAURI_DIR}/gen/apple/maple_iOS/Info.plist" and ".../maple_iOS.entitlements"
by testing file existence (e.g., [ -f ... ]) before running cp, logging or
skipping if missing to avoid hard failures; reference ios_project_state_dir,
restore_ios_build_state, and the two cp lines to locate the changes.
---
Nitpick comments:
In @.github/workflows/desktop-pr-build.yml:
- Around line 18-35: Extract the duplicated "Select Xcode 26.5" step logic into
a single reusable unit and update both workflows to call it: create a composite
GitHub Action (e.g., .github/actions/select-xcode-26-5 with an action.yml) or a
shared script (e.g., scripts/select_xcode_26_5.sh) that contains the
for-loop/xcode-select/xcodebuild logic currently under the step named "Select
Xcode 26.5", then replace the inline step in desktop-pr-build.yml and
mobile-pr-build.yml with a uses: reference to the new action or a run: invoking
the shared script so both workflows reuse the exact same selection logic.
In `@flake.nix`:
- Around line 126-139: The argument parsing loop that handles the --toolchain/-t
case (inside the while ... case block that appends to requested_targets)
currently does an unconditional shift 2 which misbehaves if --toolchain is the
last argument; update the --toolchain|-t branch to validate that a following
value exists before shifting twice (e.g., check that "$#" -gt 1), and if missing
emit an error or handle the missing toolchain value safely; ensure the rest of
the case branches and the requested_targets array behavior remain unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37977db8-aee3-40bf-b0c3-861923cf4fac
📒 Files selected for processing (17)
.github/workflows/android-pr-build.yml.github/workflows/desktop-pr-build.yml.github/workflows/frontend-tests.yml.github/workflows/mobile-pr-build.yml.github/workflows/rust-tests.ymlflake.nixfrontend/src-tauri/scripts/build-ios-onnxruntime-all.shfrontend/src-tauri/scripts/onnxruntime-pins.shfrontend/src-tauri/scripts/provide-linux-onnxruntime.shfrontend/src-tauri/scripts/setup-ios-cargo-config.shscripts/ci/_common.shscripts/ci/android-pr.shscripts/ci/desktop-pr.shscripts/ci/frontend.shscripts/ci/ios-onnxruntime.shscripts/ci/ios-pr.shscripts/ci/rust.sh
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/rust-tests.yml
| prepare_linux_onnxruntime | ||
|
|
||
| cd "${TAURI_DIR}" | ||
| cargo test --all-targets |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Cargo test invocations under CI scripts:"
rg -n --type=sh '\bcargo\s+test\b' scripts/ci
echo
echo "Cargo.lock files present in repository:"
fd -HI '^Cargo.lock$'Repository: OpenSecretCloud/Maple
Length of output: 273
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "---- scripts/ci/rust.sh (with line numbers) ----"
nl -ba scripts/ci/rust.sh | sed -n '1,120p'
echo
echo "---- Other 'cargo test' invocations under scripts/ci ----"
rg -n --type=sh '\bcargo\s+test\b' scripts/ci
echo
echo "---- Cargo.lock locations ----"
fd -HI '^Cargo.lock$'Repository: OpenSecretCloud/Maple
Length of output: 175
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "---- scripts/ci/rust.sh (cat -n) ----"
cat -n scripts/ci/rust.sh | sed -n '1,120p'
echo
echo "---- Other 'cargo test' invocations under scripts/ci ----"
rg -n --type=sh '\bcargo\s+test\b' scripts/ci
echo
echo "---- Cargo.lock locations ----"
fd -HI '^Cargo.lock$'Repository: OpenSecretCloud/Maple
Length of output: 584
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "---- Find TAURI_DIR definition ----"
rg -n --hidden '^\s*(export\s+)?TAURI_DIR=' scripts/ci -S || true
rg -n --hidden 'TAURI_DIR' scripts/ci/_common.sh scripts/ci -S || true
echo
echo "---- Show scripts/ci/_common.sh (first 200 lines) ----"
sed -n '1,200p' scripts/ci/_common.shRepository: OpenSecretCloud/Maple
Length of output: 10108
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "---- Search for --locked usage in scripts/ci ----"
rg -n --hidden '\bcargo\s+test\b.*--locked\b|--locked\b' scripts/ci || true
echo
echo "---- Inspect .cargo config generation/usage in scripts/ci/_common.sh ----"
sed -n '380,520p' scripts/ci/_common.sh
echo
echo "---- Show actual frontend/src-tauri/.cargo/config.toml if present ----"
if [ -f "frontend/src-tauri/.cargo/config.toml" ]; then
sed -n '1,200p' frontend/src-tauri/.cargo/config.toml
else
echo "frontend/src-tauri/.cargo/config.toml not found"
fiRepository: OpenSecretCloud/Maple
Length of output: 2386
Enforce lockfile usage in CI Rust tests
scripts/ci/rust.sh runs cargo test --all-targets from frontend/src-tauri (which contains Cargo.lock), but without --locked Cargo can update the lockfile during CI. Add --locked to keep dependency resolution reproducible.
Proposed fix
-cargo test --all-targets
+cargo test --all-targets --locked📝 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.
| cargo test --all-targets | |
| cargo test --all-targets --locked |
🤖 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 `@scripts/ci/rust.sh` at line 10, The CI Rust test invocation runs cargo test
--all-targets without enforcing the lockfile, allowing Cargo to update
Cargo.lock during CI; update the test command in scripts/ci/rust.sh to run cargo
test --all-targets --locked (i.e., add the --locked flag) so dependency
resolution uses the checked-in Cargo.lock and fails the job if the lockfile
would change.
180ebcb to
03cf21f
Compare
fd25dde to
d31f784
Compare
| exit 1 | ||
| fi | ||
|
|
||
| configure_reproducible_build_metadata |
There was a problem hiding this comment.
🔴 pub_date in latest.json hardcoded to 1980-01-01 due to reproducible build epoch
The source_date_rfc3339() function at scripts/ci/_common.sh:251 formats SOURCE_DATE_EPOCH into an RFC 3339 date. configure_reproducible_build_metadata() at scripts/ci/_common.sh:124-126 defaults SOURCE_DATE_EPOCH to 315532800 (1980-01-01T00:00:00Z) when unset. The update-latest-json job in release.yml:419-422 does not set SOURCE_DATE_EPOCH externally, so latest-json.sh:52 calls source_date_rfc3339() which produces 1980-01-01T00:00:00Z. Every release's latest.json auto-updater manifest will report pub_date: "1980-01-01T00:00:00Z" instead of the actual release date. The old workflow used date -u +"%Y-%m-%dT%H:%M:%SZ" to get the real current time.
Was this helpful? React with 👍 or 👎 to provide feedback.
4f1d8c5 to
12f5d8b
Compare
| export CARGO_TARGET_AARCH64_APPLE_DARWIN_RUSTFLAGS="${CARGO_TARGET_AARCH64_APPLE_DARWIN_RUSTFLAGS:+${CARGO_TARGET_AARCH64_APPLE_DARWIN_RUSTFLAGS} }${macos_link_flags}" | ||
| export CARGO_TARGET_X86_64_APPLE_DARWIN_RUSTFLAGS="${CARGO_TARGET_X86_64_APPLE_DARWIN_RUSTFLAGS:+${CARGO_TARGET_X86_64_APPLE_DARWIN_RUSTFLAGS} }${macos_link_flags}" |
There was a problem hiding this comment.
🟡 CARGO_TARGET_*_APPLE_DARWIN_RUSTFLAGS overrides RUSTFLAGS, losing --remap-path-prefix reproducibility flags on macOS
In use_xcode_toolchain() (line 354-355), CARGO_TARGET_AARCH64_APPLE_DARWIN_RUSTFLAGS and CARGO_TARGET_X86_64_APPLE_DARWIN_RUSTFLAGS are set with only the -C link-arg=-isysroot linker flags. Per Cargo's documented flag resolution, per-target rustflags (target.<triple>.rustflags) take precedence over global build.rustflags/RUSTFLAGS. Since configure_reproducible_rust_paths() (scripts/ci/_common.sh:189-198) sets --remap-path-prefix flags only in RUSTFLAGS (via append_rustflag_once), these remap flags are effectively ignored for macOS darwin targets when Cargo resolves flags. This means host-specific absolute paths (repo root, cargo home, rustup home) remain embedded in the compiled macOS binaries, undermining the cross-machine reproducibility that the rest of the infrastructure carefully establishes. The same-machine unsigned-vs-signed comparison still passes because both builds use the same environment, so the bug is silent in CI.
Was this helpful? React with 👍 or 👎 to provide feedback.
a9bfd51 to
8919240
Compare
| EOF | ||
| nix develop .#ci -c ./scripts/ci/attestation-manifest.sh \ | ||
| latest-json-artifacts.sha256 \ | ||
| frontend/src-tauri/target/reproducibility/latest-json-final.sha256 \ |
There was a problem hiding this comment.
🟡 Duplicate file passed to attestation-manifest.sh for latest-json step
The same file frontend/src-tauri/target/reproducibility/latest-json-final.sha256 is passed as both the second argument (final manifest, whose contents are copied to the output) and the third argument (proof file, whose own hash is appended to the output). This causes the attestation manifest to contain the entries from the final manifest plus a redundant self-referential hash entry. All other invocations of attestation-manifest.sh in the PR use distinct glob patterns for the proof files (e.g., android-release-*.sha256), suggesting this is a copy-paste error where the third argument should either be omitted or use a different pattern.
Was this helpful? React with 👍 or 👎 to provide feedback.
b6fec23 to
a1c3260
Compare
| } | ||
|
|
||
| source_date_rfc3339() { | ||
| date -u -d "@${SOURCE_DATE_EPOCH:?SOURCE_DATE_EPOCH is required}" +"%Y-%m-%dT%H:%M:%SZ" |
There was a problem hiding this comment.
🟡 source_date_rfc3339() uses GNU-only date -d flag, fails on macOS
source_date_rfc3339() at scripts/ci/_common.sh:251 uses date -u -d "@${SOURCE_DATE_EPOCH}" which is GNU coreutils syntax. On macOS (BSD date), -d means DST adjustment, not date-string parsing. The macOS equivalent is date -u -r "${SOURCE_DATE_EPOCH}". While this function is currently only called from latest-json.sh which runs on Linux in CI (release.yml runs update-latest-json on ubuntu-latest), it is defined in _common.sh and the script is exposed as a flake app (ci-latest-json in flake.nix:473) on all platforms including macOS, making local development/testing broken on macOS.
Was this helpful? React with 👍 or 👎 to provide feedback.
f0173d9 to
5837978
Compare
5837978 to
a002114
Compare
Summary
Local verification
Android is flake-evaluated and guarded locally; the full Android PR build requires x86_64-linux because Google Linux NDK tools in this SDK package are x86_64-only.
Summary by CodeRabbit
Chores
Bug Fixes / Improvements