Repository navigation
release: build nodejs sdk in baml language graph - #3694
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
PR changed again? Review this PR in Change Stack to compare snapshots and stay oriented. 📝 WalkthroughWalkthroughThis PR adds a reusable Node.js SDK native build and npm publishing workflow, integrates them into the main release graph, and updates supporting infrastructure. The new ChangesNode.js SDK Release Build and Publish Integration
Sequence Diagram(s)sequenceDiagram
participant plan as Plan Job
participant build2 as build2-nodejs-sdk<br/>(8-target matrix)
participant artifacts as GitHub Artifacts
plan->>build2: release_plan_json + source_sha
build2->>build2: checkout, stamp plan
build2->>build2: setup tooling (Node/Rust)
build2->>build2: pnpm install, build proto
build2->>build2: pnpm build:napi-release --target
build2->>build2: napi create-npm-dirs + artifacts
build2->>artifacts: upload nodejs-sdk-<target>
Note over build2,artifacts: On x86_64-unknown-linux-gnu:<br/>also upload nodejs-umbrella-dist
sequenceDiagram
participant plan as Plan Job
participant pub2 as publish2-nodejs-sdk<br/>(reusable)
participant npm as npm registry
plan->>pub2: release_plan_json + source_sha + channel
pub2->>pub2: download nodejs-sdk-* artifacts
pub2->>pub2: setup Node.js, validate npm >= 11.5.1
pub2->>pub2: check OIDC availability
pub2->>pub2: write + stamp release-plan.json
pub2->>pub2: napi pre-publish (wire optionalDeps)
pub2->>npm: npm publish with OIDC + skip-if-exists
Note over pub2,npm: For each package:<br/>check npm view <name>@<version><br/>skip if already published
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR introduces substantial new reusable workflows with multi-target cross-compilation matrix logic, npm OIDC/trusted-publishing integration with idempotency guards, and significant release graph restructuring. The changes span 8 files with heterogeneous concerns (CI orchestration, packaging configuration, npm client setup, GitHub Actions workflows, and documentation updates), requiring separate reasoning for each checkpoint and understanding of the release architecture evolution. 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 |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
![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: ac14aa68c3
ℹ️ 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".
| RUSTFLAGS=-C target-feature=-crt-static --cfg tracing_unstable | ||
| EOF | ||
| napi_args: --use-napi-cross |
There was a problem hiding this comment.
In the aarch64-unknown-linux-musl matrix row, this flag makes napi build use @napi-rs/cross-toolchain instead of the musl.cc toolchain downloaded just above. I checked the pinned @napi-rs/[email protected] dependency in baml_language/sdks/nodejs/bridge_nodejs/pnpm-lock.yaml, and its optional toolchain packages only cover GNU triples, not aarch64-unknown-linux-musl; the CLI's download(process.arch, this.target.triple) path therefore throws Unsupported arch ... triple: aarch64-unknown-linux-musl for this release matrix leg, causing the newly added Node.js SDK build gate to fail on every release run.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
TASK/docs/release-maintainer.md (1)
37-37: ⚡ Quick winTighten wording on release-plan transport path.
This sentence currently implies all build jobs download the artifact directly. Reusable SDK jobs actually receive the same frozen plan via workflow inputs, then stamp locally. Clarifying that split will avoid maintainer confusion.
Suggested wording tweak
-Build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json` before compiling. The Python and Node.js SDK native builds are wired in as reusable workflow jobs from `.github/workflows/release-baml-language.yml`, so dry runs exercise both SDK matrices with the same frozen plan and source SHA as the toolchain builds. +Top-level matrix build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json` before compiling. The Python and Node.js SDK native builds run as reusable workflow jobs from `.github/workflows/release-baml-language.yml`, receiving the same frozen plan and source SHA via inputs before stamping/building, so dry runs exercise both SDK matrices consistently with toolchain builds.🤖 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 `@TASK/docs/release-maintainer.md` at line 37, Clarify that not all build jobs download the release-plan artifact: update the sentence referencing release-plan.json and scripts/baml-language-version stamp to state that primary toolchain build jobs download the single release-plan.json artifact and run `scripts/baml-language-version stamp --plan release-plan.json`, while the Python and Node.js SDK native builds (wired in as reusable workflow jobs from `.github/workflows/release-baml-language.yml`) receive the same frozen plan via workflow inputs and perform the stamping locally; keep references to `release-plan.json`, `scripts/baml-language-version stamp`, and `.github/workflows/release-baml-language.yml` so maintainers can locate the related 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 @.github/workflows/build-nodejs-sdk.reusable.yaml:
- Around line 82-84: Add a pinned SHA-256 integrity check for the downloaded
musl toolchain before extraction: after the curl step that fetches
aarch64-linux-musl-cross.tgz compute its SHA-256 (or use a hardcoded expected
SHA256 variable) and compare it to the expected value, failing the job if it
does not match; only run tar -xzf and append to GITHUB_PATH when the checksum
verification passes, and ensure the step is idempotent and exits non-zero on
mismatch to prevent extracting a tampered archive.
---
Nitpick comments:
In `@TASK/docs/release-maintainer.md`:
- Line 37: Clarify that not all build jobs download the release-plan artifact:
update the sentence referencing release-plan.json and
scripts/baml-language-version stamp to state that primary toolchain build jobs
download the single release-plan.json artifact and run
`scripts/baml-language-version stamp --plan release-plan.json`, while the Python
and Node.js SDK native builds (wired in as reusable workflow jobs from
`.github/workflows/release-baml-language.yml`) receive the same frozen plan via
workflow inputs and perform the stamping locally; keep references to
`release-plan.json`, `scripts/baml-language-version stamp`, and
`.github/workflows/release-baml-language.yml` so maintainers can locate the
related logic.
🪄 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: 391fde78-38db-4fa1-90e7-4734041e57fc
📒 Files selected for processing (3)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/release-baml-language.ymlTASK/docs/release-maintainer.md
Binary size checks passed✅ 7 passed
Generated by |
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 4, 2026 19:48
ac14aa6 to
c3b4e5d
Compare
Contributor
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-baml-language.yml:
- Around line 577-590: The current Check existing npm release step only checks
`@boundaryml/baml-core-node` and must be extended to enumerate the full publish
set (the root package.json plus every generated npm/* package) and query npm for
each package's name/version; update the step (the "Check existing npm release"
job logic) to build two lists (published and unpublished) by reading each
package's package.json (e.g., node -p "require('./package.json').name" and
version) and calling npm view for each name@version, then either (A) fail fast
if you detect a mixed state (published and unpublished both non-empty) by
exiting non-zero with a clear message, or (B) record which packages are already
published and emit that list to GITHUB_OUTPUT so the downstream publish loop can
skip those individual packages instead of attempting to republish; implement one
of these two behaviors and expose outputs like published_packages and
unpublished_packages for the publish step to consume.
In `@TASK/docs/release-maintainer.md`:
- Line 37: Update the maintainer notes to accurately reflect the current
workflow: clarify that not all build jobs download and stamp release-plan.json
by noting the build-wrapper job does not perform the download/stamp (refer to
the build-wrapper job name) and that only the jobs which run
scripts/baml-language-version stamp --plan release-plan.json do so; also revise
the npm publishing sentence to remove hypothetical language and state that the
workflow now includes a publish-npm job (refer to publish-npm) so npm publishing
is an active step rather than a possibility.
🪄 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: 281c0833-4762-426c-a53f-fea2b7e594b8
📒 Files selected for processing (4)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/release-baml-language.ymlTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
✅ Files skipped from review due to trivial changes (1)
- baml_language/sdks/nodejs/bridge_nodejs/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build-nodejs-sdk.reusable.yaml
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 4, 2026 19:59
c3b4e5d to
9c43d42
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
TASK/docs/release-maintainer.md (1)
61-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocumentation uses hypothetical language but npm publishing is now added.
Line 61 reads "If npm publishing is added…" but this PR introduces the
publish-npmjob. Update to definitive wording:-If npm publishing is added, keep that publish job top-level in `release-baml-language.yml` for the same trusted-publishing reason, and have it consume the `nodejs-sdk-*` artifacts from the reusable build. +The npm publish job follows the same trusted-publishing constraint and consumes the `nodejs-sdk-*` artifacts from the reusable build.🤖 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 `@TASK/docs/release-maintainer.md` at line 61, Update the documentation sentence that currently reads "If npm publishing is added…" to reflect that npm publishing already exists: state that the `publish-npm` job is now present in `release-baml-language.yml`, keep that job top-level (like `publish-pypi`) because trusted publishing/OIDC is bound to the workflow identity, and note it should consume the `nodejs-sdk-*` artifacts produced by the reusable Node.js native addon build; also remove hypothetical phrasing and leave the PyPI environment guidance unchanged.
🧹 Nitpick comments (1)
.github/workflows/release-baml-language.yml (1)
548-551: ⚡ Quick winAlign
actions/checkout@v6with the rest of the workflow (v4).This workflow uses
actions/checkout@v4in every job except the publish/release job at lines 548-551, which usesactions/checkout@v6. Thev6tag exists (so it’s not an unknown version), but the inconsistency is worth checking—either justify@v6for this job or switch it to@v4like the others.- uses: actions/checkout@v6 with: ref: ${{ needs.plan.outputs.source_sha }} 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-baml-language.yml around lines 548 - 551, The workflow uses actions/checkout@v6 in the publish/release job while other jobs use actions/checkout@v4; update the publish/release job to use the same checkout version as the rest of the workflow (replace the actions/checkout@v6 usage) or add a brief comment justifying why `@v6` is required for that job; locate the checkout step by the string "uses: actions/checkout@v6" and change it to "uses: actions/checkout@v4" (or document the rationale next to that line) so the workflow is consistent.
🤖 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.
Duplicate comments:
In `@TASK/docs/release-maintainer.md`:
- Line 61: Update the documentation sentence that currently reads "If npm
publishing is added…" to reflect that npm publishing already exists: state that
the `publish-npm` job is now present in `release-baml-language.yml`, keep that
job top-level (like `publish-pypi`) because trusted publishing/OIDC is bound to
the workflow identity, and note it should consume the `nodejs-sdk-*` artifacts
produced by the reusable Node.js native addon build; also remove hypothetical
phrasing and leave the PyPI environment guidance unchanged.
---
Nitpick comments:
In @.github/workflows/release-baml-language.yml:
- Around line 548-551: The workflow uses actions/checkout@v6 in the
publish/release job while other jobs use actions/checkout@v4; update the
publish/release job to use the same checkout version as the rest of the workflow
(replace the actions/checkout@v6 usage) or add a brief comment justifying why
`@v6` is required for that job; locate the checkout step by the string "uses:
actions/checkout@v6" and change it to "uses: actions/checkout@v4" (or document
the rationale next to that line) so the workflow is consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5977fab6-56ba-4130-bedd-3f481caf47e8
📒 Files selected for processing (8)
.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/build2-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/release-baml-language.ymlTASK/MERGE-TO-CANARY.mdTASK/TASK.mdTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
✅ Files skipped from review due to trivial changes (2)
- TASK/TASK.md
- TASK/MERGE-TO-CANARY.md
🚧 Files skipped from review as they are similar to previous changes (1)
- baml_language/sdks/nodejs/bridge_nodejs/package.json
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 4, 2026 21:11
9c43d42 to
f8cee00
Compare
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 4, 2026 21:15
f8cee00 to
a0dd207
Compare
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 4, 2026 21:16
a0dd207 to
3e2ca9a
Compare
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 4, 2026 21:19
3e2ca9a to
26cae93
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/release-baml-language.yml (1)
594-607:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the npm publish path idempotent across the full package set.
Line 594 only checks
@boundaryml/baml-core-node, but Lines 641-646 publish every generatednpm/*package before the root package. If a run dies after publishing any platform package and before the root package, the rerun still seesexists=falsehere and then aborts on the already-published child package version, leaving the release in a mixed state and blockingpublish-pkg-boundaryml-com. npm rejects reusing an existing package name/version pair, so this needs to enumerate the full publish set up front and either fail fast on mixed state or skip already-published packages individually. (docs.npmjs.com)Also applies to: 633-646
🤖 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-baml-language.yml around lines 594 - 607, The current check only queries `@boundaryml/baml-core-node` and can leave the release in a mixed state; update the "Check existing npm release" step (id: npm) to enumerate all generated packages (e.g., all folders under npm/* and the root package.json), read each package's name and version, and run npm view for each to build a map of published status (e.g., exists_map JSON output). Then update the publish logic to either (a) fail fast if some but not all packages are already published (mixed state) or (b) skip publishing packages already published and only publish the missing ones—implement this by using the exists_map output from the npm step to conditionally publish each package instead of relying on a single `@boundaryml/baml-core-node` check. Ensure the step emits clear outputs (exists_map or a fail flag) so downstream publish steps (the npm publish loop) can decide per-package skip vs fail.TASK/docs/release-maintainer.md (1)
37-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign these maintainer notes with the workflow that actually ships.
Line 37 still reads as if all build jobs download and stamp
release-plan.json, butbuild-wrapperdoes not and the reusable SDK jobs consume the frozen plan via workflow inputs instead. Line 61 also still describes npm publishing as hypothetical even though this PR addspublish-npm. Tightening both sentences will keep future release changes from following the wrong pattern.📝 Suggested wording
-Build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json` before compiling. The Python and Node.js SDK native builds are wired in as reusable workflow jobs from `.github/workflows/release-baml-language.yml`, so dry runs exercise both SDK matrices with the same frozen plan and source SHA as the toolchain builds. +The toolchain and VSIX build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json` before compiling. The Python and Node.js SDK native builds are wired in as reusable workflow jobs from `.github/workflows/release-baml-language.yml`, so dry runs exercise both SDK matrices with the same frozen plan and source SHA as the rest of the release graph. -The Python wheel build and Node.js native addon build are reusable, but the PyPI upload is a top-level `publish-pypi` job in `.github/workflows/release-baml-language.yml`. Registry publish jobs stay top-level whenever OIDC/trusted publishing is bound to the workflow identity that performs the upload. Before production PyPI publishing, update or validate the PyPI trusted-publisher binding for project `baml-core` so it authorizes `release-baml-language.yml`; leave the PyPI environment blank unless the job declares a matching GitHub Actions `environment`. If npm publishing is added, keep that publish job top-level in `release-baml-language.yml` for the same trusted-publishing reason, and have it consume the `nodejs-sdk-*` artifacts from the reusable build. +The Python wheel build and Node.js native addon build are reusable, but the PyPI upload and npm upload stay as top-level jobs in `.github/workflows/release-baml-language.yml`. Registry publish jobs stay top-level whenever OIDC/trusted publishing is bound to the workflow identity that performs the upload. Before production PyPI publishing, update or validate the PyPI trusted-publisher binding for project `baml-core` so it authorizes `release-baml-language.yml`; leave the PyPI environment blank unless the job declares a matching GitHub Actions `environment`. The npm publish job follows the same trusted-publishing constraint and consumes the `nodejs-sdk-*` artifacts from the reusable build.Also applies to: 61-61
🤖 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 `@TASK/docs/release-maintainer.md` at line 37, Update the maintainer note so it matches the actual workflow: change the sentence that currently states "Build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json`" to clarify that only the toolchain builds perform that download-and-stamp step while `build-wrapper` receives the frozen plan via reusable workflow inputs (from `.github/workflows/release-baml-language.yml`), and also change the line describing npm publishing to reflect that `publish-npm` is now part of the shipping workflow rather than hypothetical; reference `build-wrapper`, `scripts/baml-language-version stamp --plan release-plan.json`, `.github/workflows/release-baml-language.yml`, and `publish-npm` when making these wording updates.
🤖 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.
Duplicate comments:
In @.github/workflows/release-baml-language.yml:
- Around line 594-607: The current check only queries `@boundaryml/baml-core-node`
and can leave the release in a mixed state; update the "Check existing npm
release" step (id: npm) to enumerate all generated packages (e.g., all folders
under npm/* and the root package.json), read each package's name and version,
and run npm view for each to build a map of published status (e.g., exists_map
JSON output). Then update the publish logic to either (a) fail fast if some but
not all packages are already published (mixed state) or (b) skip publishing
packages already published and only publish the missing ones—implement this by
using the exists_map output from the npm step to conditionally publish each
package instead of relying on a single `@boundaryml/baml-core-node` check. Ensure
the step emits clear outputs (exists_map or a fail flag) so downstream publish
steps (the npm publish loop) can decide per-package skip vs fail.
In `@TASK/docs/release-maintainer.md`:
- Line 37: Update the maintainer note so it matches the actual workflow: change
the sentence that currently states "Build jobs download the single
`release-plan.json` artifact and run `scripts/baml-language-version stamp --plan
release-plan.json`" to clarify that only the toolchain builds perform that
download-and-stamp step while `build-wrapper` receives the frozen plan via
reusable workflow inputs (from `.github/workflows/release-baml-language.yml`),
and also change the line describing npm publishing to reflect that `publish-npm`
is now part of the shipping workflow rather than hypothetical; reference
`build-wrapper`, `scripts/baml-language-version stamp --plan release-plan.json`,
`.github/workflows/release-baml-language.yml`, and `publish-npm` when making
these wording updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c5869dd0-2b7d-4ad9-874e-6b98b3131b51
📒 Files selected for processing (8)
.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/build2-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/release-baml-language.ymlTASK/MERGE-TO-CANARY.mdTASK/TASK.mdTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
✅ Files skipped from review due to trivial changes (3)
- baml_language/sdks/nodejs/bridge_nodejs/package.json
- TASK/TASK.md
- TASK/MERGE-TO-CANARY.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci.yaml
- .github/workflows/build2-nodejs-sdk.reusable.yaml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
TASK/docs/release-maintainer.md (1)
61-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate npm publishing wording from hypothetical to current behavior.
This line still says “If npm publishing is added,” but this stack already introduces
publish-npm. Please describe npm publishing as active and top-level (parallel to PyPI) to match the current release graph.🤖 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 `@TASK/docs/release-maintainer.md` at line 61, The paragraph must be updated to reflect that npm publishing is already active and top-level: change the conditional phrasing “If npm publishing is added” to state that `publish-npm` is an existing top-level job in `release-baml-language.yml` (parallel to `publish-pypi`), note it must remain top-level when OIDC/trusted publishing is used, and state that it consumes the `nodejs-sdk-*` artifacts from the reusable node build; keep the guidance about leaving the npm environment blank unless the job declares a matching GitHub Actions `environment` and about validating trusted-publisher bindings for the `baml-core` project.
🤖 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-baml-language.yml:
- Around line 430-454: The publish-npm job currently only checks out the SHA and
downloads nodejs-sdk-* artifacts (steps using actions/checkout and
actions/download-artifact) but never builds/assembles the npm package, stamps
the release version, or runs npm publish; update the publish-npm job (within the
same job block) to: 1) reconstruct/assemble the npm package from the downloaded
artifacts under env BRIDGE_DIR (e.g., a step invoking your package assembly
script or npm pack to create the .tgz), 2) stamp/replace the package version
(use your release version source or the planned SHA/metadata) before packaging
(a step that updates package.json or runs your stamp script), and 3)
authenticate and run npm publish (or npm publish --registry ...) with the
correct npm token/environment and two-factor/publish flags as needed so the
canary run truly publishes the Node.js package. Ensure these steps run after the
Download Node SDK core step and reuse BRIDGE_DIR to locate artifacts.
---
Duplicate comments:
In `@TASK/docs/release-maintainer.md`:
- Line 61: The paragraph must be updated to reflect that npm publishing is
already active and top-level: change the conditional phrasing “If npm publishing
is added” to state that `publish-npm` is an existing top-level job in
`release-baml-language.yml` (parallel to `publish-pypi`), note it must remain
top-level when OIDC/trusted publishing is used, and state that it consumes the
`nodejs-sdk-*` artifacts from the reusable node build; keep the guidance about
leaving the npm environment blank unless the job declares a matching GitHub
Actions `environment` and about validating trusted-publisher bindings for the
`baml-core` project.
🪄 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: 687d4275-8933-4de2-af9a-53b2d00dffa4
📒 Files selected for processing (8)
.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/build2-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/release-baml-language.ymlTASK/MERGE-TO-CANARY.mdTASK/TASK.mdTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
✅ Files skipped from review due to trivial changes (3)
- baml_language/sdks/nodejs/bridge_nodejs/package.json
- TASK/TASK.md
- TASK/MERGE-TO-CANARY.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yaml
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 4, 2026 21:33
26cae93 to
6bbc66a
Compare
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 5, 2026 00:41
c4291ae to
e40690b
Compare
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 5, 2026 00:47
e40690b to
90d2a9e
Compare
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 5, 2026 08:15
90d2a9e to
3a8827b
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
TASK/docs/release-maintainer.md (1)
61-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate maintainer docs to match the now-active npm publish flow.
Line 61 still uses hypothetical wording (“If npm publishing is added...”), but this PR stack describes npm publishing as already wired. Also, Line 37’s “Build jobs ... stamp” phrasing is broad and can mislead if some build-path jobs only consume pre-stamped artifacts. Please tighten both sentences to reflect the current workflow contract.
Also applies to: 37-37
🤖 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 `@TASK/docs/release-maintainer.md` at line 61, Update the paragraph that currently says “If npm publishing is added...” (line referencing the sentence with that phrase) to state that npm publishing is already wired: say the npm publish job is a top-level publish-npm job in release-baml-language.yml that consumes nodejs-sdk-* artifacts from the reusable build and remains top-level due to trusted-publisher/OIDC constraints; also tighten the earlier sentence that says “Build jobs ... stamp” (the phrase on line 37) to clarify that only build jobs which produce artifacts must apply the stamp, whereas build-path jobs that only consume pre-stamped artifacts do not need to stamp them, and ensure both sentences mention the trusted-publishing requirement and the publish job names (publish-pypi and publish-npm) so readers can map to the workflow.
🧹 Nitpick comments (2)
.github/workflows/publish-nodejs-sdk.yaml (1)
10-24: 💤 Low valueWorkflow-level
id-token: writeis redundant.The
publish-npmjob at lines 67-69 already declaresid-token: writeat the job level, which takes precedence. The workflow-level declaration at line 20 is unnecessary and expands the token scope to jobs that don't need it (e.g., theplanjob).♻️ Suggested fix
permissions: contents: read - id-token: write🤖 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/publish-nodejs-sdk.yaml around lines 10 - 24, Remove the workflow-level "id-token: write" permission so only the "publish-npm" job retains id-token: write at its job-level; specifically delete the id-token entry under the top-level permissions block (which currently grants it globally) and ensure the "publish-npm" job still declares id-token: write in its job permissions so other jobs like "plan" do not receive expanded token scope..github/workflows/publish2-nodejs-sdk.yaml (1)
99-120: ⚖️ Poor tradeoffConsider handling partial publish failures gracefully.
If a per-platform package publish fails mid-loop (line 118), subsequent packages won't be published, but some may already have been published. This can leave npm in an inconsistent state where some platform packages exist but not others.
For a canary/nightly release, this may be acceptable since users can retry. However, for critical releases, you may want to either:
- Add
|| trueto continue on individual failures and collect errors- Document this as expected behavior and rely on idempotent re-runs
🤖 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/publish2-nodejs-sdk.yaml around lines 99 - 120, The per-package publish loop (for dir in npm/*/) currently aborts on the first failure, risking partial publishes; modify the loop that runs pnpm publish --access public --no-git-checks --tag "$tag" so each publish is executed but failures are captured (e.g., run the publish in a subshell, record non-zero exit codes or package names into a failures list, and continue to the next package) and after the loop check the failures list and fail the job if any packages failed (also apply the same capture/continue behavior to the final pnpm --filter=`@boundaryml/baml-core-node` publish invocation or clearly document why it should not be continued).
🤖 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.
Duplicate comments:
In `@TASK/docs/release-maintainer.md`:
- Line 61: Update the paragraph that currently says “If npm publishing is
added...” (line referencing the sentence with that phrase) to state that npm
publishing is already wired: say the npm publish job is a top-level publish-npm
job in release-baml-language.yml that consumes nodejs-sdk-* artifacts from the
reusable build and remains top-level due to trusted-publisher/OIDC constraints;
also tighten the earlier sentence that says “Build jobs ... stamp” (the phrase
on line 37) to clarify that only build jobs which produce artifacts must apply
the stamp, whereas build-path jobs that only consume pre-stamped artifacts do
not need to stamp them, and ensure both sentences mention the trusted-publishing
requirement and the publish job names (publish-pypi and publish-npm) so readers
can map to the workflow.
---
Nitpick comments:
In @.github/workflows/publish-nodejs-sdk.yaml:
- Around line 10-24: Remove the workflow-level "id-token: write" permission so
only the "publish-npm" job retains id-token: write at its job-level;
specifically delete the id-token entry under the top-level permissions block
(which currently grants it globally) and ensure the "publish-npm" job still
declares id-token: write in its job permissions so other jobs like "plan" do not
receive expanded token scope.
In @.github/workflows/publish2-nodejs-sdk.yaml:
- Around line 99-120: The per-package publish loop (for dir in npm/*/) currently
aborts on the first failure, risking partial publishes; modify the loop that
runs pnpm publish --access public --no-git-checks --tag "$tag" so each publish
is executed but failures are captured (e.g., run the publish in a subshell,
record non-zero exit codes or package names into a failures list, and continue
to the next package) and after the loop check the failures list and fail the job
if any packages failed (also apply the same capture/continue behavior to the
final pnpm --filter=`@boundaryml/baml-core-node` publish invocation or clearly
document why it should not be continued).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8c4a7b6e-71cc-458a-bfa5-459ecc08ea1d
📒 Files selected for processing (11)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/build2-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish-nodejs-sdk.yaml.github/workflows/publish2-nodejs-sdk.yaml.github/workflows/release-baml-language.ymlTASK/MERGE-TO-CANARY.mdTASK/TASK.mdTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
💤 Files with no reviewable changes (1)
- .github/workflows/build-nodejs-sdk.reusable.yaml
✅ Files skipped from review due to trivial changes (3)
- TASK/TASK.md
- TASK/MERGE-TO-CANARY.md
- baml_language/sdks/nodejs/bridge_nodejs/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci.yaml
- .github/workflows/release-baml-language.yml
sxlijin
force-pushed
the
codex/tie-nodejs-sdk-release
branch
from
June 5, 2026 08:41
3a8827b to
cf190e4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
TASK/docs/release-maintainer.md (2)
37-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify Line 37: SDK reusable builds don’t download the plan artifact.
Line 37 currently reads as universal behavior, but the Python/Node SDK reusable workflows receive
release_plan_jsonas input and stamp from that input rather than downloadingrelease-plan.jsonartifacts. Please scope this sentence to the jobs that actually download artifacts.🤖 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 `@TASK/docs/release-maintainer.md` at line 37, The sentence stating "Build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json`..." should be scoped to only the jobs that actually download artifacts: update the sentence to specify that repository-hosted build jobs (the toolchain builds) download `release-plan.json` and run `scripts/baml-language-version stamp --plan release-plan.json`, and clarify that the Python and Node SDK reusable workflows referenced in `.github/workflows/release-baml-language.yml` instead receive `release_plan_json` as an input and stamp from that input rather than downloading the artifact.
61-61:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate Line 61 to reflect that npm publishing is already implemented.
The sentence still uses hypothetical wording (“If npm publishing is added”), but the Node npm publish flow is already present. Please switch this to present-tense guidance so maintainers don’t treat npm publish as optional future work.
🤖 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 `@TASK/docs/release-maintainer.md` at line 61, Update the sentence that currently reads "If npm publishing is added" to present-tense guidance noting that npm publishing is already implemented; specifically modify the paragraph referencing the top-level `publish-pypi` job in `.github/workflows/release-baml-language.yml` so it states that npm publishing is already present and should remain a top-level job (and consume `nodejs-sdk-*` artifacts) for the same trusted-publishing/OIDC reasons, and keep the instruction about leaving the PyPI environment blank unless the job declares a matching GitHub Actions `environment`.
🤖 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/publish-nodejs-sdk.yaml:
- Around line 18-20: Move the workflow-level OIDC permission so only the publish
job can issue tokens: remove "id-token: write" from the top-level permissions
block (leaving "contents: read"), then add "permissions: id-token: write" (or
include "permissions: { contents: read, id-token: write }" as appropriate) to
the publish job (publish-npm) only; verify other jobs such as "plan" do not
inherit or declare "id-token: write" so token issuance is restricted to the
publish step.
---
Duplicate comments:
In `@TASK/docs/release-maintainer.md`:
- Line 37: The sentence stating "Build jobs download the single
`release-plan.json` artifact and run `scripts/baml-language-version stamp --plan
release-plan.json`..." should be scoped to only the jobs that actually download
artifacts: update the sentence to specify that repository-hosted build jobs (the
toolchain builds) download `release-plan.json` and run
`scripts/baml-language-version stamp --plan release-plan.json`, and clarify that
the Python and Node SDK reusable workflows referenced in
`.github/workflows/release-baml-language.yml` instead receive
`release_plan_json` as an input and stamp from that input rather than
downloading the artifact.
- Line 61: Update the sentence that currently reads "If npm publishing is added"
to present-tense guidance noting that npm publishing is already implemented;
specifically modify the paragraph referencing the top-level `publish-pypi` job
in `.github/workflows/release-baml-language.yml` so it states that npm
publishing is already present and should remain a top-level job (and consume
`nodejs-sdk-*` artifacts) for the same trusted-publishing/OIDC reasons, and keep
the instruction about leaving the PyPI environment blank unless the job declares
a matching GitHub Actions `environment`.
🪄 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: f60e0726-c710-4750-8b0a-c90bf4e690ca
📒 Files selected for processing (11)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/build2-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish-nodejs-sdk.yaml.github/workflows/publish2-nodejs-sdk.yaml.github/workflows/release-baml-language.ymlTASK/MERGE-TO-CANARY.mdTASK/TASK.mdTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
💤 Files with no reviewable changes (1)
- .github/workflows/build-nodejs-sdk.reusable.yaml
✅ Files skipped from review due to trivial changes (2)
- TASK/TASK.md
- TASK/MERGE-TO-CANARY.md
🚧 Files skipped from review as they are similar to previous changes (3)
- baml_language/sdks/nodejs/bridge_nodejs/package.json
- .github/workflows/ci.yaml
- .github/workflows/release-baml-language.yml
Comment on lines
+18
to
+20
| permissions: | ||
| contents: read | ||
| id-token: write |
Contributor
There was a problem hiding this comment.
Scope OIDC permission to the publish job only.
id-token: write at workflow scope grants unnecessary token-issuance capability to non-publish jobs (notably plan). Keep it only on publish-npm.
Suggested change
permissions:
contents: read
- id-token: write🧰 Tools
🪛 zizmor (1.25.2)
[error] 20-20: overly broad permissions (excessive-permissions): id-token: write is overly broad at the workflow level
(excessive-permissions)
🤖 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/publish-nodejs-sdk.yaml around lines 18 - 20, Move the
workflow-level OIDC permission so only the publish job can issue tokens: remove
"id-token: write" from the top-level permissions block (leaving "contents:
read"), then add "permissions: id-token: write" (or include "permissions: {
contents: read, id-token: write }" as appropriate) to the publish job
(publish-npm) only; verify other jobs such as "plan" do not inherit or declare
"id-token: write" so token issuance is restricted to the publish step.
Contributor
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 `@TASK/docs/release-maintainer.md`:
- Line 37: Update the inaccurate wording in the release-maintainer doc: replace
the generic phrase "Build jobs download..." with the precise "Toolchain and VSIX
build jobs download..." to reflect that build-wrapper does not perform the
download/stamp, and change the hypothetical "If npm publishing is added"
phrasing to indicate npm publishing is active now (referencing the
publish-nodejs-sdk workflow and .github/workflows/release-baml-language.yml) so
the text accurately states that the Node.js SDK is published rather than
suggesting it may be added.
🪄 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: 682318e8-e4bc-4b82-a6e7-3e4ca04fa389
📒 Files selected for processing (11)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/build2-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish-nodejs-sdk.yaml.github/workflows/publish2-nodejs-sdk.yaml.github/workflows/release-baml-language.ymlTASK/MERGE-TO-CANARY.mdTASK/TASK.mdTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
💤 Files with no reviewable changes (1)
- .github/workflows/build-nodejs-sdk.reusable.yaml
✅ Files skipped from review due to trivial changes (2)
- TASK/TASK.md
- TASK/MERGE-TO-CANARY.md
🚧 Files skipped from review as they are similar to previous changes (3)
- baml_language/sdks/nodejs/bridge_nodejs/package.json
- .github/workflows/ci.yaml
- .github/workflows/release-baml-language.yml
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
TASK/docs/release-maintainer.md (1)
37-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPast review concerns remain unaddressed.
The past review comment correctly identified two documentation accuracy issues that are still present:
Line 37: "Build jobs download..." should be "Toolchain and VSIX build jobs download..." because
build-wrapperdoesn't perform the download/stamp operation. The context snippets confirm that SDK build jobs do perform stamping, but the generic phrasing conflates distinct job categories.Line 61: "If npm publishing is added" uses hypothetical language despite this PR adding the
publish-nodejs-sdkworkflow. Context snippet 3 shows thatpublish-nodejs-sdkis defined in.github/workflows/release-baml-language.ymlwith the expected OIDC/trusted-publishing pattern.The suggested diffs in the past review accurately resolve both issues.
Also applies to: 61-61
🤖 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 `@TASK/docs/release-maintainer.md` at line 37, Update the inaccurate generic phrasing on line 37 to explicitly state that "Toolchain and VSIX build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json`" (call out that `build-wrapper` does not perform the download/stamp), and replace the hypothetical wording on line 61 ("If npm publishing is added") with a factual reference to the existing `publish-nodejs-sdk` workflow defined in `.github/workflows/release-baml-language.yml` (mention `publish-nodejs-sdk` and the OIDC/trusted-publishing pattern) so the doc matches the actual workflow names and behavior.
🤖 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.
Duplicate comments:
In `@TASK/docs/release-maintainer.md`:
- Line 37: Update the inaccurate generic phrasing on line 37 to explicitly state
that "Toolchain and VSIX build jobs download the single `release-plan.json`
artifact and run `scripts/baml-language-version stamp --plan release-plan.json`"
(call out that `build-wrapper` does not perform the download/stamp), and replace
the hypothetical wording on line 61 ("If npm publishing is added") with a
factual reference to the existing `publish-nodejs-sdk` workflow defined in
`.github/workflows/release-baml-language.yml` (mention `publish-nodejs-sdk` and
the OIDC/trusted-publishing pattern) so the doc matches the actual workflow
names and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1b745a4c-f9dd-466c-8083-639609164d75
📒 Files selected for processing (11)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/build2-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish-nodejs-sdk.yaml.github/workflows/publish2-nodejs-sdk.yaml.github/workflows/release-baml-language.ymlTASK/MERGE-TO-CANARY.mdTASK/TASK.mdTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
💤 Files with no reviewable changes (1)
- .github/workflows/build-nodejs-sdk.reusable.yaml
✅ Files skipped from review due to trivial changes (2)
- TASK/MERGE-TO-CANARY.md
- TASK/TASK.md
🚧 Files skipped from review as they are similar to previous changes (3)
- baml_language/sdks/nodejs/bridge_nodejs/package.json
- .github/workflows/ci.yaml
- .github/workflows/release-baml-language.yml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
TASK/docs/release-maintainer.md (1)
37-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate release-maintainer wording to match the current release graph behavior.
At Line 37, “Build jobs download…” is still too broad; this should explicitly call out the toolchain/VSIX path.
At Line 61, “If npm publishing is added” is outdated now that Node.js publish wiring exists in this stack.Suggested doc patch
-Build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json` before compiling. The Python and Node.js SDK native builds are wired in as reusable workflow jobs from `.github/workflows/release-baml-language.yml`, so dry runs exercise both SDK matrices with the same frozen plan and source SHA as the toolchain builds. +Toolchain and VSIX build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json` before compiling. The Python and Node.js SDK native builds are wired in as reusable workflow jobs from `.github/workflows/release-baml-language.yml`, so dry runs exercise both SDK matrices with the same frozen plan and source SHA as the toolchain builds. @@ -The Python wheel build and Node.js native addon build are reusable, but the PyPI upload is a top-level `publish-pypi` job in `.github/workflows/release-baml-language.yml`. Registry publish jobs stay top-level whenever OIDC/trusted publishing is bound to the workflow identity that performs the upload. Before production PyPI publishing, update or validate the PyPI trusted-publisher binding for project `baml-core` so it authorizes `release-baml-language.yml`; leave the PyPI environment blank unless the job declares a matching GitHub Actions `environment`. If npm publishing is added, keep that publish job top-level in `release-baml-language.yml` for the same trusted-publishing reason, and have it consume the `nodejs-sdk-*` artifacts from the reusable build. +The Python wheel build and Node.js native addon build are reusable, but the PyPI upload and npm upload stay as top-level jobs in `.github/workflows/release-baml-language.yml`. Registry publish jobs stay top-level whenever OIDC/trusted publishing is bound to the workflow identity that performs the upload. Before production PyPI publishing, update or validate the PyPI trusted-publisher binding for project `baml-core` so it authorizes `release-baml-language.yml`; leave the PyPI environment blank unless the job declares a matching GitHub Actions `environment`. The npm publish job follows the same trusted-publishing constraint and consumes the `nodejs-sdk-*` artifacts from the reusable build.Also applies to: 61-61
🤖 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 `@TASK/docs/release-maintainer.md` at line 37, Update the wording in release-maintainer.md to reflect the current release graph: replace the broad sentence that begins with “Build jobs download the single `release-plan.json` artifact and run `scripts/baml-language-version stamp --plan release-plan.json` …” with a sentence that explicitly states toolchain and VSIX build jobs download the release-plan.json artifact and run that stamp command (call out the toolchain/VSIX path), and change the phrasing at the sentence referencing “If npm publishing is added” to acknowledge that Node.js publishing is already wired (remove the conditional and state that Node.js publish wiring exists in the reusable workflow jobs).
🤖 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/build2-nodejs-sdk.reusable.yaml:
- Around line 166-176: The workflow step invoking "pnpm exec napi build ... --js
native.js --dts native.d.ts" is hardcoding root entry filenames that mismatch
the package's "build:napi-release" output ("./dist/native.js" and
"./dist/native.d.ts") and the repo's only existing declaration
"typescript_src/native.d.ts"; update the workflow (the pnpm exec napi build
invocation) to reference the actual output paths (e.g., --js ./dist/native.js
--dts ./dist/native.d.ts) or alternatively change the package.json
"build:napi-release" script to emit root-level native.js/native.d.ts, and ensure
"build:proto" actually generates the expected files (or add an explicit step to
copy/emit types to the expected locations) so the napi build uses the correct
entrypoints.
In @.github/workflows/publish-nodejs-sdk.yaml:
- Around line 10-16: The workflow defined in publish-nodejs-sdk.yaml triggers
real nightly publishes on pushes to branch codex/tie-nodejs-sdk-release; update
the workflow to prevent real publishes on push by gating the publisher job(s)
(the reusable publisher invoked by publish2-nodejs-sdk.yaml and the publish-npm
step) so they only run on manual dispatch or when a non-publishable tag/version
is present: either restrict the top-level on.push branch rule (remove or change
codex/tie-nodejs-sdk-release), or add an explicit job-level condition (if:
github.event_name == 'workflow_dispatch') on the publish-npm/publisher
invocation, or alter generated versions to include a rehearsal marker so
publish2-nodejs-sdk.yaml does not perform a live dist-tag publish. Ensure you
change the workflow that calls publish2-nodejs-sdk.yaml and the publish-npm job
conditions accordingly.
In @.github/workflows/publish2-nodejs-sdk.yaml:
- Around line 40-45: The Publish to npm step runs npm publish in a loop and can
fail on reruns due to partially published packages; update that step to make it
restart-safe by, for each package unpacked by the "Download per-platform npm
packages" step, reading the package name and version from package.json and
running npm view <package>@<version> version (or equivalent registry query)
before calling npm publish, skipping publish when the version already exists;
ensure this per-package idempotency guard is applied inside the loop that
currently invokes npm publish so reruns continue past already-published
packages.
- Around line 137-141: The publish step must be made restart-safe: before
running the existing npm publish command inside the for loop (the one under "for
dir in npm/*/; do" and the final umbrella "npm publish --access public --tag
\"$tag\" --loglevel verbose"), query the registry for the package@version (read
package.json in "$dir" to get name/version) and skip publishing if that exact
version already exists; alternately wrap the npm publish invocation to detect
the "already published" failure and treat it as non-fatal so the loop continues
(ensure this check/guard is applied both inside the loop and for the final
umbrella publish).
---
Duplicate comments:
In `@TASK/docs/release-maintainer.md`:
- Line 37: Update the wording in release-maintainer.md to reflect the current
release graph: replace the broad sentence that begins with “Build jobs download
the single `release-plan.json` artifact and run `scripts/baml-language-version
stamp --plan release-plan.json` …” with a sentence that explicitly states
toolchain and VSIX build jobs download the release-plan.json artifact and run
that stamp command (call out the toolchain/VSIX path), and change the phrasing
at the sentence referencing “If npm publishing is added” to acknowledge that
Node.js publishing is already wired (remove the conditional and state that
Node.js publish wiring exists in the reusable workflow jobs).
🪄 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: 62121015-25db-453a-bca4-d694267515ac
📒 Files selected for processing (11)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/build2-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish-nodejs-sdk.yaml.github/workflows/publish2-nodejs-sdk.yaml.github/workflows/release-baml-language.ymlTASK/MERGE-TO-CANARY.mdTASK/TASK.mdTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
💤 Files with no reviewable changes (1)
- .github/workflows/build-nodejs-sdk.reusable.yaml
✅ Files skipped from review due to trivial changes (2)
- TASK/TASK.md
- TASK/MERGE-TO-CANARY.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/ci.yaml
- baml_language/sdks/nodejs/bridge_nodejs/package.json
- .github/workflows/release-baml-language.yml
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
.github/workflows/build2-nodejs-sdk.reusable.yaml (2)
81-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify the musl cross-toolchain tarball before extracting it.
This step untars a third-party archive straight from the network and prepends its binaries to
PATHwithout any integrity check. If that archive is tampered with, the release job can silently ship compromised binaries. Please pin a SHA-256 and fail before extraction on mismatch.Suggested hardening
- curl -LO https://musl.cc.timfish.dev/aarch64-linux-musl-cross.tgz - tar -xzf aarch64-linux-musl-cross.tgz + curl -fL --retry 3 --retry-delay 2 -o aarch64-linux-musl-cross.tgz https://musl.cc.timfish.dev/aarch64-linux-musl-cross.tgz + echo "<PINNED_SHA256> aarch64-linux-musl-cross.tgz" | sha256sum -c - + tar -xzf aarch64-linux-musl-cross.tgz echo "$PWD/aarch64-linux-musl-cross/bin" >> "$GITHUB_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 @.github/workflows/build2-nodejs-sdk.reusable.yaml around lines 81 - 84, The workflow currently downloads and extracts the musl cross-toolchain with curl -LO and tar -xzf and then modifies PATH (echo "$PWD/aarch64-linux-musl-cross/bin" >> "$GITHUB_PATH") without verifying integrity; add a pinned SHA-256 check: fetch the expected digest (hardcode the pinned SHA-256), compute the downloaded file's SHA-256 (e.g., via sha256sum or shasum -a 256), compare it to the pinned value, and if it doesn't match, fail the job before running tar; only proceed to tar -xzf and updating GITHUB_PATH when the checksum verification succeeds.
41-42:⚠️ Potential issue | 🟠 MajorScope
MACOSX_DEPLOYMENT_TARGETper macOS target + verify musl cross-toolchain integrity
.github/workflows/build2-nodejs-sdk.reusable.yamlsetsMACOSX_DEPLOYMENT_TARGET: "10.13"globally, but the matrix builds bothaarch64-apple-darwinandx86_64-apple-darwin; scope the deployment target per matrix entry (arm64 min macOS is >= 11) instead of exporting one value for both.- The
aarch64-unknown-linux-musljob downloadshttps://musl.cc.timfish.dev/aarch64-linux-musl-cross.tgzand runstar -xzf ...without any checksum/signature verification; add integrity checking before extracting.🤖 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/build2-nodejs-sdk.reusable.yaml around lines 41 - 42, Scope MACOSX_DEPLOYMENT_TARGET per macOS matrix entry instead of a global env: set MACOSX_DEPLOYMENT_TARGET="11.0" (or >=11) for the aarch64-apple-darwin matrix entry and keep "10.13" for x86_64-apple-darwin so arm64 builds target a supported minimum macOS; update the matrix or job env usage rather than a single global env. For the aarch64-unknown-linux-musl job that downloads https://musl.cc.timfish.dev/aarch64-linux-musl-cross.tgz, add integrity verification before extracting: download a checksum or signature alongside the tarball, verify the checksum/signature (e.g., sha256 and shasum -c or gpg --verify) and fail the job if verification fails, and only then run tar -xzf to extract.
🤖 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.
Duplicate comments:
In @.github/workflows/build2-nodejs-sdk.reusable.yaml:
- Around line 81-84: The workflow currently downloads and extracts the musl
cross-toolchain with curl -LO and tar -xzf and then modifies PATH (echo
"$PWD/aarch64-linux-musl-cross/bin" >> "$GITHUB_PATH") without verifying
integrity; add a pinned SHA-256 check: fetch the expected digest (hardcode the
pinned SHA-256), compute the downloaded file's SHA-256 (e.g., via sha256sum or
shasum -a 256), compare it to the pinned value, and if it doesn't match, fail
the job before running tar; only proceed to tar -xzf and updating GITHUB_PATH
when the checksum verification succeeds.
- Around line 41-42: Scope MACOSX_DEPLOYMENT_TARGET per macOS matrix entry
instead of a global env: set MACOSX_DEPLOYMENT_TARGET="11.0" (or >=11) for the
aarch64-apple-darwin matrix entry and keep "10.13" for x86_64-apple-darwin so
arm64 builds target a supported minimum macOS; update the matrix or job env
usage rather than a single global env. For the aarch64-unknown-linux-musl job
that downloads https://musl.cc.timfish.dev/aarch64-linux-musl-cross.tgz, add
integrity verification before extracting: download a checksum or signature
alongside the tarball, verify the checksum/signature (e.g., sha256 and shasum -c
or gpg --verify) and fail the job if verification fails, and only then run tar
-xzf to extract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e3126394-86c3-4f06-8d29-c29871d3bb2c
📒 Files selected for processing (10)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/build2-nodejs-sdk.reusable.yaml.github/workflows/build2-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish2-nodejs-sdk.yaml.github/workflows/release-baml-language.ymlTASK/MERGE-TO-CANARY.mdTASK/TASK.mdTASK/docs/release-maintainer.mdbaml_language/sdks/nodejs/bridge_nodejs/package.json
💤 Files with no reviewable changes (1)
- .github/workflows/build-nodejs-sdk.reusable.yaml
✅ Files skipped from review due to trivial changes (2)
- TASK/TASK.md
- TASK/MERGE-TO-CANARY.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci.yaml
- .github/workflows/release-baml-language.yml
This was referenced Jun 12, 2026
This was referenced Jul 15, 2026
This was referenced Jul 22, 2026
This branch had an error being deployed
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
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
Release Notes
New Features
@boundaryml/baml-core-node) now published to npm via secure OIDC trusted publishingChores