Merged
Conversation
📝 WalkthroughWalkthroughReplaces a legacy multi-job daybook-compose CI workflow with a consolidated "build artifacts" workflow; introduces profile-aware debug/release handling across Gradle and build scripts; exposes a reusable desktop runtime library path in Nix shells; updates build scripts and test wasm paths; and bumps a pre-commit hook rev. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant GH as "GitHub Actions"
participant Meta as "resolve-release-meta"
participant Job as "Platform Job\n(linux/android/windows/macos)"
participant Builder as "Toolchains\n(Gradle/Rust/Protoc/others)"
participant Store as "Artifact Store / Upload"
participant Release as "GitHub Release"
Dev->>GH: push / PR / workflow_dispatch
GH->>Meta: compute release_tag/kind, cargo_version, matrix outputs
Meta-->>GH: set job matrix & outputs
GH->>Job: spawn platform-specific job
Job->>Builder: setup toolchain & deps
Builder->>Builder: build & package artifacts (profile-aware)
Builder->>Store: upload artifacts (non-release)
Job->>GH: emit artifact paths to GITHUB_OUTPUT
GH->>Release: if release_kind != none -> attach artifacts & publish
Release-->>GH: release published
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 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/builds.yml:
- Around line 36-47: The env expressions (DAYBOOK_RELEASE_KIND,
DAYBOOK_IS_RELEASE_BUILD, DAYBOOK_COMPOSE_PROFILE, DAYBOOK_RUST_PROFILE_DIR,
DAYBOOK_ANDROID_GRADLE_TASK, DAYBOOK_WINDOWS_GRADLE_TASK,
DAYBOOK_MACOS_GRADLE_TASK) currently gate ref-based release detection behind
"github.event_name == 'push'", causing workflow_dispatch with build_profile:
auto to always pick debug; update each expression to first honor inputs when
event is workflow_dispatch (inputs.build_profile == 'release'/'debug') and
otherwise resolve release vs debug by checking the ref (startsWith(github.ref,
'refs/tags/v') || github.ref == 'refs/heads/main') without requiring event_name
== 'push' so manual runs with refs can select release automatically.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/build-daybook-compose.yml.github/workflows/builds.yml.pre-commit-config.yamlflake.nixsrc/daybook_compose/composeApp/build.gradle.ktsx/build-a-dayb.tsx/build-appimage-dayb.ts
💤 Files with no reviewable changes (1)
- .github/workflows/build-daybook-compose.yml
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/builds.yml (1)
66-96:⚠️ Potential issue | 🟠 Major
build_profile: autoignores ref and defaults to release on PR/manual runs
compose_profilestarts asreleaseand only changes whenbuild_profileis explicitly set, soautoyields release even for PRs and non‑main branches. This contradicts the input description and can unexpectedly run release builds. Consider deriving anis_ref_releaseflag from the ref and defaultingcompose_profileto debug for auto.🛠️ Suggested fix
event_name='${{ github.event_name }}' ref='${{ github.ref }}' manual_build_profile='${{ inputs.build_profile || 'auto' }}' + is_ref_release=0 + if [[ "${ref}" == refs/tags/v* || "${ref}" == "refs/heads/main" ]]; then + is_ref_release=1 + fi + is_push_tag_release=0 is_push_main=0 if [[ "${event_name}" == "push" && "${ref}" == refs/tags/v* ]]; then is_push_tag_release=1 fi if [[ "${event_name}" == "push" && "${ref}" == "refs/heads/main" ]]; then is_push_main=1 fi @@ - compose_profile="release" + compose_profile="debug" if [[ "${manual_build_profile}" == "release" || "${manual_build_profile}" == "debug" ]]; then compose_profile="${manual_build_profile}" - elif [[ "${is_release_build}" == "1" ]]; then + elif [[ "${is_ref_release}" == "1" ]]; then compose_profile="release" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/builds.yml around lines 66 - 96, The compose_profile logic currently defaults to "release" and ignores manual_build_profile='auto'; modify it to derive an is_ref_release flag from ref (reuse/refactor the existing checks for is_push_tag_release and is_push_main or create is_ref_release) and when manual_build_profile == "auto" set compose_profile="debug" unless is_ref_release or is_release_build is true, otherwise set compose_profile="release"; keep explicit manual settings for "release"/"debug" to override this behavior and update uses of compose_profile, is_push_tag_release, is_push_main, is_release_build and manual_build_profile accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/builds.yml:
- Around line 66-96: The compose_profile logic currently defaults to "release"
and ignores manual_build_profile='auto'; modify it to derive an is_ref_release
flag from ref (reuse/refactor the existing checks for is_push_tag_release and
is_push_main or create is_ref_release) and when manual_build_profile == "auto"
set compose_profile="debug" unless is_ref_release or is_release_build is true,
otherwise set compose_profile="release"; keep explicit manual settings for
"release"/"debug" to override this behavior and update uses of compose_profile,
is_push_tag_release, is_push_main, is_release_build and manual_build_profile
accordingly.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
src/wflow/test/recover_from_log.rs (2)
50-50: Same wasm_path hard-coding concern as in src/wflow/test/fails_until_told.rs Line 12.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wflow/test/recover_from_log.rs` at line 50, The test hard-codes the WASM path in the wasm_path field ("../../target/wasm32-wasip2/debug/test_wflows.wasm"), which is brittle; change the test to compute the path at runtime instead of hard-coding it — e.g., derive the path from environment/Cargo variables (use CARGO_MANIFEST_DIR or CARGO_TARGET_DIR and the active profile) or a shared test helper so both recover_from_log.rs (wasm_path) and the other test fails_until_told.rs reuse the same resolution logic; update the wasm_path assignment to call that helper or construct the path from env vars so tests work across environments and CI.
25-25: Same wasm_path hard-coding concern as in src/wflow/test/fails_until_told.rs Line 12.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wflow/test/recover_from_log.rs` at line 25, The wasm_path field is hard-coded; change its construction to derive the wasm file location dynamically (e.g., use env!("CARGO_MANIFEST_DIR") or a WASM_PATH env var) instead of the literal "../../target/wasm32-wasip2/debug/test_wflows.wasm". Update the wasm_path assignment in src/wflow/test/recover_from_log.rs (the wasm_path field) to join CARGO_MANIFEST_DIR with "target/wasm32-wasip2/debug/test_wflows.wasm" or read from std::env::var("WASM_PATH") with a sensible fallback, and apply the same change to the duplicate occurrence in src/wflow/test/fails_until_told.rs.src/wflow/test/fails_once.rs (2)
66-66: Same wasm_path hard-coding concern as in src/wflow/test/fails_until_told.rs Line 12.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wflow/test/fails_once.rs` at line 66, The test currently hard-codes the wasm path string ("../../target/wasm32-wasip2/debug/test_wflows.wasm"); change it to compute the path dynamically instead of embedding a literal: replace the literal used for wasm_path with code that derives the path from build/runtime environment (e.g., use CARGO_MANIFEST_DIR and target triple/target dir or read CARGO_TARGET_DIR from std::env) so the same approach used in src/wflow/test/fails_until_told.rs is applied here; update the wasm_path variable in fails_once.rs accordingly (identify and modify the wasm_path binding/variable where the literal appears).
14-14: Same wasm_path hard-coding concern as in src/wflow/test/fails_until_told.rs Line 12.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wflow/test/fails_once.rs` at line 14, The test currently hard-codes the wasm path string ("../../target/wasm32-wasip2/debug/test_wflows.wasm"); replace that literal with the shared helper used by the other test (the wasm_path helper used in the fails_until_told test) or a single reusable function (e.g., wasm_path("test_wflows.wasm")) so both fails_once and fails_until_told use the same platform-aware resolution; update the code in the fails_once test to call that helper instead of the hard-coded path.src/wflow/test/fails_until_told.rs (1)
83-83: Same wasm_path hard-coding concern as Line 12.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/wflow/test/fails_until_told.rs` at line 83, The hard-coded wasm_path string should be replaced with a deterministic, environment-aware construction to avoid brittle paths; update the wasm_path initialization (the wasm_path field in the test setup in fails_until_told.rs) to build the path dynamically (for example using env!("CARGO_MANIFEST_DIR") or std::env::var("CARGO_MANIFEST_DIR") combined with Path/PathBuf join operations, or using CARGO_TARGET_DIR if available) so it points to target/wasm32-wasip2/debug/test_wflows.wasm relative to the repo root instead of the literal "../../target/wasm32-wasip2/debug/test_wflows.wasm".
🤖 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/builds.yml:
- Around line 111-123: The code sets compose_profile="release" unconditionally
at the top, causing build_profile=auto to default to release for PRs; change the
logic so compose_profile is initialized to "debug" (or empty) and then set to
manual_build_profile if it's "release" or "debug", else set to "release" only
when is_release_build=="1", otherwise leave it "debug" — update the block using
the existing symbols compose_profile, manual_build_profile, and is_release_build
so auto yields release on main/v* and debug for PRs/non-main branches.
In `@src/daybook_core/build.rs`:
- Line 12: The build script currently uses CARGO_TARGET_DIR as-is which can be a
relative path and later is resolved relative to the build script cwd, causing
mismatches; update the code that computes target_dir and wflows_target_dir so
that if the retrieved CARGO_TARGET_DIR is not absolute you join/resolve it
against the workspace root (or canonicalize it) before using it—check the places
where target_dir and wflows_target_dir are set (and the other two occurrences
noted around the same logic) and replace the plain join with a normalized
absolute path by using Path::is_absolute and, when false,
workspace_root.join(&target_dir) (or .canonicalize()) so subsequent path
resolution is consistent.
In `@src/wflow/test/fails_until_told.rs`:
- Line 12: The test currently hardcodes wasm_path as
"../../target/wasm32-wasip2/debug/test_wflows.wasm", which breaks when
CARGO_TARGET_DIR is set; change the test to compute the path at runtime by
reading CARGO_TARGET_DIR (std::env::var("CARGO_TARGET_DIR").unwrap_or_else(|_|
"target".into())), then build the path with the target triple and profile
segments (e.g. join target_dir, "wasm32-wasip2", "debug", "test_wflows.wasm")
into a PathBuf and assign that to wasm_path; apply the same runtime resolution
pattern to the other tests that set wasm_path (recover_from_log.rs,
fails_until_told.rs, fails_once.rs, cancel_job.rs) so artifact lookup honors
CARGO_TARGET_DIR.
---
Duplicate comments:
In `@src/wflow/test/fails_once.rs`:
- Line 66: The test currently hard-codes the wasm path string
("../../target/wasm32-wasip2/debug/test_wflows.wasm"); change it to compute the
path dynamically instead of embedding a literal: replace the literal used for
wasm_path with code that derives the path from build/runtime environment (e.g.,
use CARGO_MANIFEST_DIR and target triple/target dir or read CARGO_TARGET_DIR
from std::env) so the same approach used in src/wflow/test/fails_until_told.rs
is applied here; update the wasm_path variable in fails_once.rs accordingly
(identify and modify the wasm_path binding/variable where the literal appears).
- Line 14: The test currently hard-codes the wasm path string
("../../target/wasm32-wasip2/debug/test_wflows.wasm"); replace that literal with
the shared helper used by the other test (the wasm_path helper used in the
fails_until_told test) or a single reusable function (e.g.,
wasm_path("test_wflows.wasm")) so both fails_once and fails_until_told use the
same platform-aware resolution; update the code in the fails_once test to call
that helper instead of the hard-coded path.
In `@src/wflow/test/fails_until_told.rs`:
- Line 83: The hard-coded wasm_path string should be replaced with a
deterministic, environment-aware construction to avoid brittle paths; update the
wasm_path initialization (the wasm_path field in the test setup in
fails_until_told.rs) to build the path dynamically (for example using
env!("CARGO_MANIFEST_DIR") or std::env::var("CARGO_MANIFEST_DIR") combined with
Path/PathBuf join operations, or using CARGO_TARGET_DIR if available) so it
points to target/wasm32-wasip2/debug/test_wflows.wasm relative to the repo root
instead of the literal "../../target/wasm32-wasip2/debug/test_wflows.wasm".
In `@src/wflow/test/recover_from_log.rs`:
- Line 50: The test hard-codes the WASM path in the wasm_path field
("../../target/wasm32-wasip2/debug/test_wflows.wasm"), which is brittle; change
the test to compute the path at runtime instead of hard-coding it — e.g., derive
the path from environment/Cargo variables (use CARGO_MANIFEST_DIR or
CARGO_TARGET_DIR and the active profile) or a shared test helper so both
recover_from_log.rs (wasm_path) and the other test fails_until_told.rs reuse the
same resolution logic; update the wasm_path assignment to call that helper or
construct the path from env vars so tests work across environments and CI.
- Line 25: The wasm_path field is hard-coded; change its construction to derive
the wasm file location dynamically (e.g., use env!("CARGO_MANIFEST_DIR") or a
WASM_PATH env var) instead of the literal
"../../target/wasm32-wasip2/debug/test_wflows.wasm". Update the wasm_path
assignment in src/wflow/test/recover_from_log.rs (the wasm_path field) to join
CARGO_MANIFEST_DIR with "target/wasm32-wasip2/debug/test_wflows.wasm" or read
from std::env::var("WASM_PATH") with a sensible fallback, and apply the same
change to the duplicate occurrence in src/wflow/test/fails_until_told.rs.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/DEVDOC/todo.mdis excluded by!docs/DEVDOC/**and included by**/*
📒 Files selected for processing (7)
.github/workflows/builds.yml.github/workflows/checks.ymlsrc/daybook_core/build.rssrc/wflow/test/cancel_job.rssrc/wflow/test/fails_once.rssrc/wflow/test/fails_until_told.rssrc/wflow/test/recover_from_log.rs
✅ Files skipped from review due to trivial changes (1)
- src/wflow/test/cancel_job.rs
| } else { | ||
| cwd.join("../../target/").canonicalize().unwrap() | ||
| }; | ||
| let wflows_target_dir = target_dir.join("wasm"); |
There was a problem hiding this comment.
Normalize relative CARGO_TARGET_DIR before re-rooting the build.
If CARGO_TARGET_DIR is relative (e.g., target), you now pass it to Cargo while running in the workspace root, but later resolve paths relative to the build script’s cwd. That mismatch can make the script look in the wrong directory and fail to find the wasm.
✅ Proposed fix (normalize to workspace root)
let cwd = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR")?);
+let workspace_root = cwd.join("../../").canonicalize().unwrap();
let out_dir = PathBuf::from(std::env::var("OUT_DIR")?);
let profile = std::env::var("PROFILE")?;
let target_dir = if let Ok(dir) = std::env::var("CARGO_TARGET_DIR") {
- PathBuf::from(dir)
+ let p = PathBuf::from(dir);
+ if p.is_absolute() { p } else { workspace_root.join(p) }
} else {
- cwd.join("../../target/").canonicalize().unwrap()
+ workspace_root.join("target")
};
let wflows_target_dir = target_dir.join("wasm");
...
build_wflows
@@
- .current_dir(cwd.join("../../"))
+ .current_dir(&workspace_root)
.env("CARGO_TARGET_DIR", &wflows_target_dir);Also applies to: 60-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/daybook_core/build.rs` at line 12, The build script currently uses
CARGO_TARGET_DIR as-is which can be a relative path and later is resolved
relative to the build script cwd, causing mismatches; update the code that
computes target_dir and wflows_target_dir so that if the retrieved
CARGO_TARGET_DIR is not absolute you join/resolve it against the workspace root
(or canonicalize it) before using it—check the places where target_dir and
wflows_target_dir are set (and the other two occurrences noted around the same
logic) and replace the plain join with a normalized absolute path by using
Path::is_absolute and, when false, workspace_root.join(&target_dir) (or
.canonicalize()) so subsequent path resolution is consistent.
| let test_cx = WflowTestContext::builder() | ||
| .initial_workloads(vec![InitialWorkload { | ||
| wasm_path: "../../target/wasm32-wasip2/release/test_wflows.wasm".into(), | ||
| wasm_path: "../../target/wasm32-wasip2/debug/test_wflows.wasm".into(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for test_wflows.wasm references and target directory patterns
rg -n "test_wflows\.wasm" --type rsRepository: dman-os/townframe
Length of output: 87
🏁 Script executed:
# Check for CARGO_TARGET_DIR, build directory patterns, and path resolution
rg -n "CARGO_TARGET_DIR|wasm32-wasip2|target/wasm" --type rsRepository: dman-os/townframe
Length of output: 87
🏁 Script executed:
# Look at the specific file to understand context
cat -n src/wflow/test/fails_until_told.rsRepository: dman-os/townframe
Length of output: 4691
🏁 Script executed:
# Check for any build.rs or build configuration
fd -e toml | head -20 | xargs grep -l "test_wflows\|wasm32-wasip2" 2>/dev/nullRepository: dman-os/townframe
Length of output: 195
🏁 Script executed:
# Search for any helper functions or macros that might resolve paths
rg -n "wasm_path|artifact|target_dir" --type rs -iRepository: dman-os/townframe
Length of output: 87
🏁 Script executed:
# Find all references to test_wflows
rg -n "test_wflows"Repository: dman-os/townframe
Length of output: 1202
🏁 Script executed:
# Search for CARGO_TARGET_DIR usage
rg -n "CARGO_TARGET_DIR"Repository: dman-os/townframe
Length of output: 354
🏁 Script executed:
# Look for any build.rs files
find . -name "build.rs" | head -20Repository: dman-os/townframe
Length of output: 122
🏁 Script executed:
# Check other test files to see if they have similar path patterns
find . -path "*/test/*.rs" -type f | head -20Repository: dman-os/townframe
Length of output: 232
🏁 Script executed:
# Look at Cargo.toml to understand project structure
head -50 Cargo.tomlRepository: dman-os/townframe
Length of output: 1118
🏁 Script executed:
# Check the test_wflows crate configuration
cat -n src/test_wflows/Cargo.tomlRepository: dman-os/townframe
Length of output: 616
🏁 Script executed:
# Look for how test_wflows.wasm is built - check if there's a build process
rg -n "wasm32-wasip2|test_wflows\.wasm" src/test_wflows/ -A 3 -B 3Repository: dman-os/townframe
Length of output: 43
🏁 Script executed:
# Check the daybook_core build.rs that uses CARGO_TARGET_DIR to understand how it's used
cat -n src/daybook_core/build.rsRepository: dman-os/townframe
Length of output: 7256
🏁 Script executed:
# Check if there's any pattern in the codebase that handles wasm paths dynamically
rg -n "wasm_path|env::var.*target" -A 2 -B 2Repository: dman-os/townframe
Length of output: 13746
Use environment-aware path resolution for test_wflows.wasm to support CARGO_TARGET_DIR overrides.
The hard-coded relative path ../../target/wasm32-wasip2/debug/test_wflows.wasm will break if CARGO_TARGET_DIR is set. The codebase already uses CARGO_TARGET_DIR in build scripts (e.g., src/daybook_core/build.rs, src/wash_plugin_pglite/build.rs), and when set, artifact locations shift outside the default target/ directory. Consider deriving the wasm path at runtime using CARGO_TARGET_DIR or providing a centralized helper function. This affects all test files using this pattern: recover_from_log.rs, fails_until_told.rs, fails_once.rs, and cancel_job.rs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wflow/test/fails_until_told.rs` at line 12, The test currently hardcodes
wasm_path as "../../target/wasm32-wasip2/debug/test_wflows.wasm", which breaks
when CARGO_TARGET_DIR is set; change the test to compute the path at runtime by
reading CARGO_TARGET_DIR (std::env::var("CARGO_TARGET_DIR").unwrap_or_else(|_|
"target".into())), then build the path with the target triple and profile
segments (e.g. join target_dir, "wasm32-wasip2", "debug", "test_wflows.wasm")
into a PathBuf and assign that to wasm_path; apply the same runtime resolution
pattern to the other tests that set wasm_path (recover_from_log.rs,
fails_until_told.rs, fails_once.rs, cancel_job.rs) so artifact lookup honors
CARGO_TARGET_DIR.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/builds.yml (1)
86-116:⚠️ Potential issue | 🟠 Major
autoprofile ignores ref on workflow_dispatch.
compose_profileonly flips to release whenis_release_buildis1, but that flag is set only for push events. So manual runs on main/tags withbuild_profile=autostill build debug, contradicting the input description. Consider basing the auto profile on the ref independently of push while keeping release uploads gated to push.🛠️ Suggested adjustment
- is_push_tag_release=0 - is_push_main=0 - if [[ "${event_name}" == "push" && "${ref}" == refs/tags/v* ]]; then - is_push_tag_release=1 - fi - if [[ "${event_name}" == "push" && "${ref}" == "refs/heads/main" ]]; then - is_push_main=1 - fi + is_ref_tag_release=0 + is_ref_main=0 + if [[ "${ref}" == refs/tags/v* ]]; then + is_ref_tag_release=1 + fi + if [[ "${ref}" == "refs/heads/main" ]]; then + is_ref_main=1 + fi + is_push_tag_release=0 + is_push_main=0 + if [[ "${event_name}" == "push" && "${is_ref_tag_release}" == "1" ]]; then + is_push_tag_release=1 + fi + if [[ "${event_name}" == "push" && "${is_ref_main}" == "1" ]]; then + is_push_main=1 + fi ... - elif [[ "${is_release_build}" == "1" ]]; then + elif [[ "${is_ref_tag_release}" == "1" || "${is_ref_main}" == "1" ]]; then compose_profile="release" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/builds.yml around lines 86 - 116, The auto profile handling ignores ref for workflow_dispatch; change the compose_profile logic so when manual_build_profile is "auto" it inspects ref (the existing ref variable) regardless of event_name to choose "release" for refs/tags/v* or "release" for refs/heads/main (or "prerelease" if you prefer), while keeping is_release_build and release_kind calculations gated to push events; update the block that sets compose_profile (using manual_build_profile, ref, is_release_build and compose_profile) so ref pattern checks run even when event_name != "push" but do not alter is_release_build or release_kind which remain determined only for push.
🧹 Nitpick comments (1)
.github/workflows/builds.yml (1)
33-40: Scopecontents: writeto release jobs only.
contents: writeis granted for all events, including PR checks. Consider setting workflow-level permissions to read and elevating to write only on jobs that call the release upload action.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/builds.yml around lines 33 - 40, The workflow currently sets permissions: contents: write at workflow-level which grants write to PR checks; change the top-level permissions to contents: read and then add permissions: contents: write only to the specific job that performs release uploads (the job that calls the release upload action), so update the global permissions block to "contents: read" and add a job-level permissions override in the release/upload job to "contents: write"; keep the env block unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/builds.yml:
- Around line 86-116: The auto profile handling ignores ref for
workflow_dispatch; change the compose_profile logic so when manual_build_profile
is "auto" it inspects ref (the existing ref variable) regardless of event_name
to choose "release" for refs/tags/v* or "release" for refs/heads/main (or
"prerelease" if you prefer), while keeping is_release_build and release_kind
calculations gated to push events; update the block that sets compose_profile
(using manual_build_profile, ref, is_release_build and compose_profile) so ref
pattern checks run even when event_name != "push" but do not alter
is_release_build or release_kind which remain determined only for push.
---
Nitpick comments:
In @.github/workflows/builds.yml:
- Around line 33-40: The workflow currently sets permissions: contents: write at
workflow-level which grants write to PR checks; change the top-level permissions
to contents: read and then add permissions: contents: write only to the specific
job that performs release uploads (the job that calls the release upload
action), so update the global permissions block to "contents: read" and add a
job-level permissions override in the release/upload job to "contents: write";
keep the env block unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores
Tests