Fix release.yml failures on dev branch - #457
Conversation
docs(core): clarify IMG2NUM_BUILD_C is required for WASM builds
|
Warning Review limit reached
More reviews will be available in 59 minutes and 2 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe release workflow now runs for pull requests to ChangesRelease workflow and build settings
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (7 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
ad185bf to
4bdc41e
Compare
4bdc41e to
2c59904
Compare
Ryan-Millard
changed the title
release.yml failures omn dev branchrelease.yml failures on dev branch
5d78c7d to
2286680
Compare
trigger workflow fjkhd
2286680 to
65106d9
Compare
coderabbitai
Bot
added
the
github_actions
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 @.github/workflows/release.yml:
- Around line 6-7: Add an event guard in the release workflow so PR-triggered
runs on dev cannot reach mutating release actions. Update the build-js job or,
at minimum, the release draft and npm publish steps using github.event_name !=
'pull_request' so the softprops/action-gh-release and Publish to npm operations
only run on push-based release events. Use the existing build-js job and the
release/publish steps as the anchor points when applying the condition.
- Around line 166-169: Disable the implicit caching behavior in the release
workflow’s actions/setup-node step by explicitly turning off cache for the node
setup used before publishing. Update the setup-node configuration in the release
job so it does not auto-enable package caching from the packageManager field,
keeping the publish path free of cache-poisoning risk. Focus on the
actions/setup-node step in the release workflow and add the explicit no-cache
setting there.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4ce3fb2d-9fdd-4101-baa9-58556c16020e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (4)
.github/workflows/release.ymlCMakeLists.txtpackage.jsonpnpm-workspace.yaml
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: build-c-cpp-native (ubuntu-latest, c)
- GitHub Check: build-c-cpp-native (macos-latest, cpp)
- GitHub Check: build-py (macos-latest)
- GitHub Check: build-c-cpp-native (macos-latest, c)
- GitHub Check: build-c-cpp-native (ubuntu-latest, cpp)
- GitHub Check: build-c-cpp-native (windows-latest, cpp)
- GitHub Check: build-c-cpp-native (windows-latest, c)
- GitHub Check: build-py (windows-latest)
- GitHub Check: build-py (ubuntu-latest)
- GitHub Check: build-js
- GitHub Check: Build C/C++ / Build Python
- GitHub Check: Build C/C++ / Build C & C++
- GitHub Check: Build Documentation Site / Build Docusaurus Site
- GitHub Check: Build C/C++ / Build WASM (bindings/js)
- GitHub Check: Lint & Validate Code
🧰 Additional context used
📓 Path-based instructions (5)
{CMakeLists.txt,**/*.cmake}
📄 CodeRabbit inference engine (.editorconfig)
{CMakeLists.txt,**/*.cmake}: Use 2-space indentation for CMake configuration files
Maintain 120 character maximum line length for CMake files
Files:
CMakeLists.txt
**/*.txt
📄 CodeRabbit inference engine (.editorconfig)
**/*.txt: Do not enforce maximum line length for text files
Use 2-space indentation for text files
Files:
CMakeLists.txt
**
⚙️ CodeRabbit configuration file
**: # Contributing to Img2NumWant to contribute to Img2Num? There are a few things you need to know.
We wrote a contribution guide to help you get started.
A few important points:
- Add tests with your PR — new features and bug fixes must include tests where appropriate. PRs without tests are unlikely to be approved.
- Follow the repository's coding style rules.
- Use the issue and PR templates when filing issues or submitting code. Your PR will be rejected if you don't.
If you're unsure what to change, open a discussion and someone will assist you.
Questions?
If you have questions or need help:
- Open a discussion
- Create an issue
- Check existing PRs for ideas
Thank you for improving Img2Num! 🎨🚀
**: BasedOnStyle: LLVM
Standard: c++20--- Basic formatting ---
IndentWidth: 4
ColumnLimit: 100
TabWidth: 4--- Braces ---
Cpp11BracedListStyle: true
SpaceBeforeCpp11BracedList: true
BreakBeforeBraces: Attach--- Braced initializers ---
Cpp11BracedListStyle: true
--- Constructor initializer lists ---
PackConstructorInitializers: Never
BreakConstructorInitializers: BeforeComma
ConstructorInitializerIndentWidth: 4--- Alignment ---
AlignAfterOpenBracket: BlockIndent
--- Pointers ---
PointerAlignment: Left
--- Includes ---
IncludeBlocks: Regroup
SortIncludes: CaseInsensitive--- Extern "C" cleanliness ---
IndentExternBlock: NoIndent
--- Lambdas ---
AllowShortLambdasOnASingleLine: Inline
--- Functions ---
AllowShortFunctionsOnASingleLine: None
**: root = true-------------------------
Global defaults
-------------------------
[*]
charset = utf-8
end_of_line = lf
indent_style = space
indent_size = 2
trim_trailing_whitespace = true
insert_final_newline = true
max_line_lengt...
Files:
CMakeLists.txtpackage.jsonpnpm-workspace.yaml
**/*.json
📄 CodeRabbit inference engine (.editorconfig)
Do not trim trailing whitespace in JSON files
Files:
package.json
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: GitHub Actions workflows. Review for:
- SHA-pinned action versions for third-party actions (security best practice).
- Secrets accessed only via ${{ secrets.* }} — never hardcoded.
- Least-privilege permissions on each job/workflow.
- Correct job dependency ordering (needs:) and if/condition logic.
Files:
.github/workflows/release.yml
🧠 Learnings (2)
📚 Learning: 2026-05-01T22:50:11.527Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 339
File: release-please-config.json:18-47
Timestamp: 2026-05-01T22:50:11.527Z
Learning: In this repo, release-please-action v4 preserves '/' verbatim in slash-containing path-based package keys when emitting GitHub Actions output names (e.g., `bindings/c--release_created`). When referencing these step outputs in `job.outputs` (and other expressions), use bracket notation with the exact output name: `${{ steps.release.outputs['bindings/c--release_created'] }}` rather than dot notation. If needed, map the complex step output to a clean job-level output alias so downstream jobs can use dot notation via that alias.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-05-19T17:30:09.565Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 375
File: .github/workflows/cmake-build.yml:86-88
Timestamp: 2026-05-19T17:30:09.565Z
Learning: In Ryan-Millard/Img2Num CI/workflow YAMLs, any `uv sync` command used for the Python package build must include `--no-build-isolation` (do not remove it). If you need deterministic dependency installs for CI, you may add `--frozen` alongside it (e.g., `uv sync --frozen --no-build-isolation`), and it should not conflict with the repo’s build setup.
Applied to files:
.github/workflows/release.yml
🪛 zizmor (1.26.1)
.github/workflows/release.yml
[info] 195-195: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 216-216: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[error] 166-166: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
[info] 200-200: action functionality is already included by the runner (superfluous-actions): use gh release in a script step
(superfluous-actions)
77d3923 to
5ec53b9
Compare
5ec53b9 to
8e8dff5
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.github/workflows/release.yml (4)
179-198: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winWrite the WASM build to the path you package.
cmake -B build-wasmcreates the build directory at repo root, but the tar step packagespackages/js/build-wasm, so the wasm-only tarball will fail or be empty.Proposed fix
- emcmake cmake -B build-wasm \ + emcmake cmake -B packages/js/build-wasm \ @@ - cmake --build build-wasm + cmake --build packages/js/build-wasm🤖 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/release.yml around lines 179 - 198, The WASM build output path in the workflow does not match the path later packaged for the JS release artifact. Update the Build WASM step in the release workflow so the cmake build directory is created under the same packages/js/build-wasm location that the Package step tars up, using the existing Build WASM and Package steps as the reference points. Keep the JS build unchanged, but ensure the wasm-only tarball contains the actual build output by aligning the build directory and packaging path.
159-163: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable persisted checkout credentials before running package scripts.
build-jshascontents: write, then runs PR-controlledpnpm install/pnpm build; withoutpersist-credentials: false, those scripts can access the checkout token from git config. As per path instructions, workflow jobs should use least-privilege permissions.Proposed fix
- uses: actions/checkout@900f2210b1d28bbbd0bd22d17926b9e224e8f231 with: ref: ${{ needs.release-please.outputs.is_dry_run == 'true' && github.sha || needs.release-please.outputs.js_tag }} submodules: true + persist-credentials: false🤖 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/release.yml around lines 159 - 163, The checkout step in the release workflow is leaving persisted Git credentials available to later package scripts, which is too permissive for the build-js job; update the actions/checkout usage in this workflow to disable credential persistence so PR-controlled pnpm install and pnpm build cannot read the checkout token from git config. Make the change on the checkout step used by build-js, using the actions/checkout configuration to set persisted credentials off while keeping the existing ref and submodules behavior intact.Source: Path instructions
285-308: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLet the Python publish job run on dry-run releases.
Line 308 now has a dry-run tag, but the job-level
ifonly allowspy_created == 'true', so the dry-run/TestPyPI path is skipped when onlyis_dry_runis true.Proposed fix
- if: ${{ needs.release-please.outputs.py_created == 'true' }} + if: ${{ needs.release-please.outputs.py_created == 'true' || needs.release-please.outputs.is_dry_run == 'true' }}🤖 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/release.yml around lines 285 - 308, The publish-py job is still gated by release-please py_created, so the dry-run/TestPyPI path never runs even though softprops/action-gh-release already handles the dry-run tag. Update the job-level if condition on publish-py to also allow release-please dry runs, using the existing needs.release-please.outputs.is_dry_run signal alongside py_created. Keep the change localized to the publish-py job so the wheelhouse download and gh-release steps run for both real Python releases and dry-run releases.
125-145: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPackage native dry-run artifacts before uploading the draft release.
On dry-run runs, the release step executes, but both package steps only run when
created == 'true', leavingenv.ARCHIVEunset and the draft release without the built artifact.Proposed fix
- if: ${{ steps.component.outputs.created == 'true' && runner.os != 'Windows' }} + if: ${{ (steps.component.outputs.created == 'true' || needs.release-please.outputs.is_dry_run == 'true') && runner.os != 'Windows' }} @@ - if: ${{ steps.component.outputs.created == 'true' && runner.os == 'Windows' }} + if: ${{ (steps.component.outputs.created == 'true' || needs.release-please.outputs.is_dry_run == 'true') && runner.os == 'Windows' }}🤖 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/release.yml around lines 125 - 145, The draft release step in the release workflow can run during dry-run even when the packaging steps are skipped, leaving ARCHIVE unset. Update the packaging conditions in the “Package (Linux/macOS)” and “Package (Windows)” steps so they also run for dry-run executions, and keep using the ARCHIVE env value consumed by softprops/action-gh-release. Use the existing steps.component.outputs.created and needs.release-please.outputs.is_dry_run checks to ensure the native artifact is built before uploading the draft release.
🤖 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.
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 179-198: The WASM build output path in the workflow does not match
the path later packaged for the JS release artifact. Update the Build WASM step
in the release workflow so the cmake build directory is created under the same
packages/js/build-wasm location that the Package step tars up, using the
existing Build WASM and Package steps as the reference points. Keep the JS build
unchanged, but ensure the wasm-only tarball contains the actual build output by
aligning the build directory and packaging path.
- Around line 159-163: The checkout step in the release workflow is leaving
persisted Git credentials available to later package scripts, which is too
permissive for the build-js job; update the actions/checkout usage in this
workflow to disable credential persistence so PR-controlled pnpm install and
pnpm build cannot read the checkout token from git config. Make the change on
the checkout step used by build-js, using the actions/checkout configuration to
set persisted credentials off while keeping the existing ref and submodules
behavior intact.
- Around line 285-308: The publish-py job is still gated by release-please
py_created, so the dry-run/TestPyPI path never runs even though
softprops/action-gh-release already handles the dry-run tag. Update the
job-level if condition on publish-py to also allow release-please dry runs,
using the existing needs.release-please.outputs.is_dry_run signal alongside
py_created. Keep the change localized to the publish-py job so the wheelhouse
download and gh-release steps run for both real Python releases and dry-run
releases.
- Around line 125-145: The draft release step in the release workflow can run
during dry-run even when the packaging steps are skipped, leaving ARCHIVE unset.
Update the packaging conditions in the “Package (Linux/macOS)” and “Package
(Windows)” steps so they also run for dry-run executions, and keep using the
ARCHIVE env value consumed by softprops/action-gh-release. Use the existing
steps.component.outputs.created and needs.release-please.outputs.is_dry_run
checks to ensure the native artifact is built before uploading the draft
release.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a853e032-a874-4d52-a8f7-592db8fbb65e
📒 Files selected for processing (1)
.github/workflows/release.yml
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: build-py (ubuntu-latest)
- GitHub Check: build-c-cpp-native (macos-latest, c)
- GitHub Check: build-js
- GitHub Check: build-c-cpp-native (windows-latest, c)
- GitHub Check: build-c-cpp-native (macos-latest, cpp)
- GitHub Check: build-c-cpp-native (ubuntu-latest, cpp)
- GitHub Check: build-c-cpp-native (ubuntu-latest, c)
- GitHub Check: build-py (macos-latest)
- GitHub Check: build-c-cpp-native (windows-latest, cpp)
- GitHub Check: build-py (windows-latest)
- GitHub Check: build-py-sdist
- GitHub Check: Build C/C++ / Build C & C++
- GitHub Check: Build C/C++ / Build Python
- GitHub Check: Build C/C++ / Build WASM (bindings/js)
- GitHub Check: Build Documentation Site / Build Docusaurus Site
- GitHub Check: Lint & Validate Code
- GitHub Check: Analyze (c-cpp)
🧰 Additional context used
📓 Path-based instructions (1)
.github/workflows/**
⚙️ CodeRabbit configuration file
.github/workflows/**: GitHub Actions workflows. Review for:
- SHA-pinned action versions for third-party actions (security best practice).
- Secrets accessed only via ${{ secrets.* }} — never hardcoded.
- Least-privilege permissions on each job/workflow.
- Correct job dependency ordering (needs:) and if/condition logic.
Files:
.github/workflows/release.yml
🧠 Learnings (2)
📚 Learning: 2026-05-01T22:50:11.527Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 339
File: release-please-config.json:18-47
Timestamp: 2026-05-01T22:50:11.527Z
Learning: In this repo, release-please-action v4 preserves '/' verbatim in slash-containing path-based package keys when emitting GitHub Actions output names (e.g., `bindings/c--release_created`). When referencing these step outputs in `job.outputs` (and other expressions), use bracket notation with the exact output name: `${{ steps.release.outputs['bindings/c--release_created'] }}` rather than dot notation. If needed, map the complex step output to a clean job-level output alias so downstream jobs can use dot notation via that alias.
Applied to files:
.github/workflows/release.yml
📚 Learning: 2026-05-19T17:30:09.565Z
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 375
File: .github/workflows/cmake-build.yml:86-88
Timestamp: 2026-05-19T17:30:09.565Z
Learning: In Ryan-Millard/Img2Num CI/workflow YAMLs, any `uv sync` command used for the Python package build must include `--no-build-isolation` (do not remove it). If you need deterministic dependency installs for CI, you may add `--frozen` alongside it (e.g., `uv sync --frozen --no-build-isolation`), and it should not conflict with the repo’s build setup.
Applied to files:
.github/workflows/release.yml
🪛 zizmor (1.26.1)
.github/workflows/release.yml
[warning] 213-213: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🔇 Additional comments (1)
.github/workflows/release.yml (1)
6-7: LGTM!Also applies to: 170-170, 203-203, 212-218
833f599 to
a1c0e3a
Compare
a1c0e3a to
65756b6
Compare
|
@coderabbitai make a commit message and description for this PR. |
|
Tip For best results, initiate chat on the files or code changes.
Commit message PR description ## Summary
This PR fixes `release.yml` failures on the `dev` branch introduced by the repository restructuring in `#449`.
It updates the release workflow to match the current project layout, adjusts the JS/WASM pipeline, and makes dry-run draft releases safer by using unique tag names. It also includes a small wording improvement in `CMakeLists.txt` to make the WASM/C bindings requirement clearer.
## Changes
### Release workflow
- Run the release workflow on `pull_request` events targeting `dev`
- Use dry-run-specific `tag_name` values for draft releases in:
- native artifacts
- JS/WASM
- Python
- Avoid collisions with normal release tags during validation/testing
### JS/WASM pipeline
- Set up Node.js 24
- Use `pnpm@11.4.0`
- Install dependencies with `pnpm install`
- Build WASM with explicit `IMG2NUM_*` CMake flags
- Build WASM in a dedicated `build-wasm` directory
- Build the JS package from `packages/js`
- Package JS distribution files and WASM artifacts into separate tarballs
- Publish with `pnpm publish --provenance` while preserving dry-run behavior
### Workspace/package manager updates
- Update the root `packageManager` field to `pnpm@11.4.0`
- Add `allowBuilds` entries in `pnpm-workspace.yaml`
### CMake wording
- Clarify that C bindings are required for WASM builds in the `IMG2NUM_BUILD_C` option description
## Why
After the restructuring in `#449`, the release workflow no longer aligned with the current layout/build flow on `dev`. These updates bring the workflow back in sync and make it safer to validate release behavior on the branch without conflicting with real release tags.
## Testing
- Validate the updated workflow against `dev` by temporarily changing the target branch as needed
- Confirm JS/WASM packaging and draft release asset generation succeed
- Revert temporary branch-target changes after verificationIf you want, I can also turn this into:
|
2 participants
What was changed & why
A moment ago, the
release.ymlworkflow failed on thedevbranch (see here) due to the restructuring done in #449. This update was necessary to remedy that.Changes
release.ymlCMakeLists.txtstring to be more explanatory for WebAssembly usersTesting & Verification
I'll test it on this branch by changing the target, then reverting.
Additional Resources