Repository navigation
Overhaul BAML language release pipeline - #3639
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds canonical version stamping, a baml_release manifest/fetcher library, a new baml wrapper CLI with toolchain lifecycle and self-update, IDE/VSIX protocol and per-project clients, channel-aware release workflows and reusable build stamping, installers/package artifacts, TASK docs, and test/infra updates. ChangesWrapper and Release Pipeline Overhaul
Sequence Diagram (release flow) sequenceDiagram
participant Planner as scripts/baml-language-version
participant ReleaseWF as release-baml-language.yml
participant BuildSDK as build-*-sdk.reusable
participant PublishPyPI as publish-python-pypi.yml
participant Host as pkg.boundaryml.com
ReleaseWF->>Planner: plan / compute / stamp
ReleaseWF->>BuildSDK: pass release_plan_json + source_sha
BuildSDK->>BuildSDK: stamp release-plan.json
ReleaseWF->>PublishPyPI: pre-check & publish wheels
ReleaseWF->>Host: upload manifests & install scripts
Estimated code review effort 🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Binary size checks passed✅ 7 passed
Generated by |
![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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
typescript2/app-vscode-ext/src/extension.ts (2)
194-226:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
knownProjectsis global but now mutated per-client, breaking the multi-project model.With per-project clients, every client overwrites the single global
knownProjectsonbaml/listProjects(Line 223), and any client'sStoppedtransition clears it entirely (Line 204). With two BAML projects open, the status-bar tooltip will reflect only the last client to fire a notification, and stopping one project's server wipes the project list for the still-running ones.Consider keying project lists per client (e.g.
Map<projectRoot, string[]>) and aggregating for the tooltip, and onStoppedonly removing that client's entry.🤖 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 `@typescript2/app-vscode-ext/src/extension.ts` around lines 194 - 226, The global knownProjects array is being overwritten per-client and cleared on any client's Stopped state; change knownProjects to a Map keyed by the client/project root (e.g. Map<string, string[]>) so each client stores its own project list, modify the 'baml/listProjects' handler to set knownProjects.set(clientIdOrProjectRoot, params.projects ?? []), change the onDidChangeState Stopped branch to remove only that client's entry (knownProjects.delete(clientIdOrProjectRoot)) instead of clearing everything, and update refreshTooltip() to aggregate across all Map values when rendering the status-bar tooltip; use the client instance or its projectRoot as the unique key to locate per-client entries.
312-317:⚠️ Potential issue | 🟠 MajorNormalize
projectPathfrombaml/listProjectsbeforeclients.get()inbaml.openPlayground.
ListProjectssends project identifiers fromprojects.keys(); those roots are computed byget_baml_project_root(), which returns thebaml_src/directory itself when the project path’s filename isbaml_src. The extension’sclientsmap is keyed byfindBamlProjectRoot(), which only resolves to the nearest directory containingbaml.toml(or workspace root). So clicking a tooltip entry whereprojectPathis abaml_src/...path won’t match any key and incorrectly shows “BAML Language Server is not running.” Updatebaml.openPlayground(orfindBamlProjectRoot) to resolve/normalize server-provided project paths to the same key format used inclients.🤖 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 `@typescript2/app-vscode-ext/src/extension.ts` around lines 312 - 317, The command handler registered as 'baml.openPlayground' is using the raw server-provided projectPath to look up clients in the clients Map, but server paths from baml/listProjects may point at baml_src/ and therefore won't match keys created by findBamlProjectRoot(); fix by normalizing the incoming projectPath with the same resolution logic used for client keys (call findBamlProjectRoot(projectPath) or otherwise strip/resolve baml_src to the project root) before doing clients.get(projectPath), so the lookup and the activeClient() fallback use the same canonical project root; update the handler around clients.get(...) (or update findBamlProjectRoot to accept server paths) to perform this normalization.
🧹 Nitpick comments (7)
baml_language/crates/baml_cli/src/ide_command.rs (1)
26-32: ⚡ Quick winEnforce
--cursor/--codemutual exclusivity at clap parse time
IdeInstallArgs::resolve_editorcurrently does a runtime check (if self.cursor && self.code { ... }) and returns ananyhow!error after dispatch. Declaring the conflict in the clap derive surfaces the error during argument parsing with the standard usage message.♻️ Example using
conflicts_with#[derive(Args, Clone, Debug)] pub(crate) struct IdeInstallArgs { - #[arg(long)] + #[arg(long, conflicts_with = "code")] pub cursor: bool, #[arg(long)] pub code: bool, }Then remove the
self.cursor && self.codeguard fromresolve_editor.🤖 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 `@baml_language/crates/baml_cli/src/ide_command.rs` around lines 26 - 32, Add a clap-level conflict so the parser rejects using both flags: annotate the IdeInstallArgs fields to declare mutual exclusivity (use the clap attribute like conflicts_with on the cursor and/or code fields) and then remove the runtime check in IdeInstallArgs::resolve_editor that returns an anyhow error for self.cursor && self.code; keep resolve_editor focused on resolving the Editor and rely on clap to enforce the conflict at parse time.baml_language/crates/bex_project/src/bex_lsp/protocol.rs (1)
1-10: 💤 Low valueProtocol constants/capabilities are duplicated across Rust and TypeScript.
These values mirror
compat.ts(BAML_LSP_PROTOCOL_*) and the capability array inextension.ts(Line 172). They must be bumped in lockstep; a mismatch silently breaks compatibility negotiation. Consider a comment cross-referencing the TS counterpart, or a generated/shared source if this grows.🤖 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 `@baml_language/crates/bex_project/src/bex_lsp/protocol.rs` around lines 1 - 10, The protocol constants (BAML_LSP_PROTOCOL_VERSION, BAML_PLAYGROUND_PROTOCOL_VERSION, MIN_SUPPORTED_VSCODE_LSP_PROTOCOL, MIN_SUPPORTED_PLAYGROUND_PROTOCOL) and the CAPABILITIES array are duplicated in TypeScript (compat.ts and extension.ts) and must be kept in sync; update this Rust file by adding a clear comment above these declarations pointing to the exact TypeScript counterparts (compat.ts for BAML_* values and extension.ts for the capabilities list) and include a short TODO/instruction that any change here must be mirrored in those TS files (or consider centralizing generation later) so future maintainers will update both places atomically.baml_language/crates/bex_project/src/bex_lsp/multi_project/request.rs (1)
80-89: ⚡ Quick winPin the initialize metadata with a unit test.
This
experimental.bamlpayload plusserver_info.versionis now an IDE compatibility contract. A small unit test aroundserver_capabilities()/on_request_initialize()would catch accidental key-name or version-source drift before it breaks the extension or playground wiring. As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible".Also applies to: 148-152
🤖 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 `@baml_language/crates/bex_project/src/bex_lsp/multi_project/request.rs` around lines 80 - 89, Add a unit test that pins the initialize metadata by asserting the exact experimental.baml JSON payload and the server version returned by server_capabilities() / on_request_initialize(); locate the experimental payload construction in the multi-project initialize logic (the block that sets experimental: Some(serde_json::json!({...}))) and the functions server_capabilities() and on_request_initialize() and create a test that calls those functions (or constructs the same payload path) and compares the resulting JSON object and server_info.version against a hard-coded expected value to detect accidental key/name or version-source drift; ensure the test lives alongside the module as a standard Rust #[cfg(test)] unit test and fails on any change to keys, values, or version constants..github/workflows/release-baml-language.yml (1)
66-69: ⚡ Quick winConsider adding
persist-credentials: falseto checkout steps.Multiple checkout steps in this workflow do not set
persist-credentials: false. While the workflow does upload artifacts that could potentially include the.gitdirectory, the default behavior ofactions/checkout(persist-credentials: true) leaves the GitHub token in the git config, which could be extracted from artifacts or subsequent steps.For defense-in-depth, consider adding
persist-credentials: falseto all checkout actions unless git operations (push/commit) are explicitly needed in that job. The jobs that do need credentials (publish-homebrew, publish-aur) appear to use dedicated secrets rather than the default token.Also applies to: 160-160, 205-205, 255-255, 364-364, 467-467, 502-502, 552-552
🤖 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 66 - 69, Update every actions/checkout@v4 step to include persist-credentials: false unless that job intentionally needs GitHub token; specifically locate the checkout steps (the uses: actions/checkout@v4 occurrences around the shown diff and also at the other occurrences noted) and add the persist-credentials: false key alongside ref and fetch-depth so the checkout does not leave the default token in the checked-out repo; leave checkout entries in jobs that explicitly require pushing/committing unchanged if they truly need credentials..github/workflows/publish-python-pypi.yml (1)
33-36: 💤 Low valueOptional: Workflow-level
id-token: writemay be redundant.The
publish-to-pypijob (lines 57-59) already declares job-levelid-token: writepermissions. The workflow-level declaration at line 35 may be unnecessary and contributes to the static analysis warning about overly broad permissions.Consider removing the workflow-level
id-token: writeand relying on the job-level permissions for clarity and least-privilege.🤖 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-python-pypi.yml around lines 33 - 36, The workflow-level permissions block currently sets "id-token: write" broadly; remove the workflow-level "id-token: write" entry and keep the existing job-level "id-token: write" declaration in the publish-to-pypi job (referencing the publish-to-pypi job name) so permissions follow least-privilege and eliminate the redundant workflow-level id-token setting.scripts/baml-wrapper-version (1)
49-57: 💤 Low valueConsider validating that the version field belongs to the
[package]section.The regex
r'^version = "([^"]+)"$'withMULTILINEwill match any line with that pattern, not necessarily the[package]section's version. If the Cargo.toml has other version fields (e.g., in dependencies), this could match the wrong line.Given that
check()validates absence ofversion.workspace, the risk is reduced but not eliminated. Consider either:
- Parsing the TOML properly (via
tomllibwhich is already available in Python 3.11+), or- Using a more specific regex that ensures the match is within the
[package]section.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/baml-wrapper-version` around lines 49 - 57, The read_version() function currently uses a multiline regex that can match any "version = ..." line; update it to ensure the version is read from the [package] table (instead of arbitrary sections). Replace the regex approach in read_version() to parse WRAPPER_CARGO_TOML using tomllib (available in Python 3.11+), extract data["package"]["version"], then call Version.parse(...) and fail(...) on missing keys or parse errors; alternatively, if you prefer a regex solution keep WRAPPER_CARGO_TOML.read_text() but scope the regex to the [package] section before matching the version line to avoid picking up dependency/version fields.scripts/baml-language-version (1)
346-353: ⚡ Quick winFragile insertion marker for
__version__in PY_INIT.The logic inserts
__version__after"atexit.register(flush_events)\n"if the version line is missing. If that marker line changes format or is removed, the insertion will silently fail (no error, version line not added).Consider making the fallback more robust by appending to the end of the file or using a more stable insertion point, and adding validation that the insertion succeeded.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/baml-language-version` around lines 346 - 353, The current insertion for __version__ in PY_INIT relies on the fragile marker "atexit.register(flush_events)\n" and may silently fail if that line changes; update the fallback to append version_line to the end of PY_INIT (or insert at another stable anchor) instead of using text.replace with the marker, then validate the result by checking that '__version__ = "' appears in the final text and raise/exit with a clear error if it does not; reference PY_INIT, version_line, marker, and the atexit.register(flush_events) anchor when making the change.
🤖 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 `@baml_language/crates/baml_release/src/lib.rs`:
- Around line 152-162: In install_to_toolchain_root, avoid downloading the
archive when the version is already installed by moving the VERSION file
idempotency check to before the call to fetch_archive(); keep the existing
fs::create_dir_all and InstallLock::acquire(root.join(format!(".{}.lock",
self.spec.version))) calls, compute install_dir = root.join(&self.spec.version)
and read install_dir.join("VERSION") to compare with self.spec.version, and
return Ok(install_dir) if it matches; only call self.fetch_archive() (and
subsequent install steps) when the VERSION check indicates the toolchain is not
already installed.
- Around line 380-405: The retry loop can panic because a 3xx response leaves
last_error None; update the Ok(response) branch that checks
response.status().is_redirection() (and the surrounding retry loop that sets
last_error and ultimately returns Err(FetchError::Network { url, source:
last_error.expect(...) })) to handle 3xx explicitly: either immediately return a
concrete FetchError for redirects (e.g., construct and return a FetchError
describing the 3xx status using response.status() and url) or ensure last_error
is set to a valid reqwest::Error before continuing (for example by converting
the 3xx response into an Err and storing that in last_error) so that the final
Err(FetchError::Network { ... source: last_error.expect(...) }) cannot panic;
touch the branches around response.status().is_redirection(),
response.error_for_status(), last_error, and the final Err(FetchError::Network)
return.
In `@baml_language/crates/baml/src/main.rs`:
- Around line 291-303: The code currently swallows
toml::from_str::<ProjectConfig>(&text) parse errors when reading a candidate
baml.toml (variables: candidate, ProjectConfig, toolchain), causing silent
fallback; change the logic to surface parse failures instead of ignoring
them—match on toml::from_str and on Err return or propagate a descriptive error
(including candidate path and the toml error) so the caller sees a malformed
local baml.toml rather than falling back to parent/default toolchain selection.
- Around line 97-99: The current global check using args.iter().any(|arg| arg ==
"--version" || arg == "-V") in main.rs accidentally intercepts flags meant for
subcommands; restrict the wrapper-owned version handling to the top-level
invocation only by checking the first user argument (e.g. use args.get(1) or
args.iter().skip(1).next()) and compare that single argument to "--version" or
"-V" (or ensure no subcommand is present) before printing and returning from
main; update the version-check code that references args so it only triggers for
that top-level position (preserving downstream pass-through to baml-cli).
- Around line 617-624: self_update currently writes to current.tmp and calls
fs::rename(tmp, current) which fails on Windows when the running EXE is locked;
on Windows, change the flow so after writing tmp you do not rename but instead
spawn a short-lived updater process (via std::process::Command) that takes
arguments like --replace <tmp> <current>, then exit the running process;
implement a new branch in main (or a new function handler) that recognizes the
--replace args, waits a moment for the parent to exit, atomically moves/renames
tmp to current (retrying on sharing errors) and then optionally relaunches the
replaced executable—use the existing symbols tmp, current, self_update, and
fs::rename to locate where to add the platform-gated spawn and the
replacement-handler logic.
- Around line 433-440: The blocking fetch uses reqwest::blocking::get(&url) with
no timeout causing possible indefinite hangs; replace it by constructing a
reqwest::blocking::Client via
reqwest::blocking::Client::builder().timeout(std::time::Duration::from_secs(N)).build()?
(choose a sensible N, e.g. 10–30s), then call client.get(&url).send() and
proceed with .error_for_status()?.text()?; update both occurrences (the block
that sets fetched_remote = true and the other fetch at the second occurrence) to
use this client (preferably create the client once and reuse) so requests have
an explicit timeout. Ensure you import std::time::Duration and handle the
Client::build() error via existing ? handling.
In `@scripts/baml-package-manager-artifacts`:
- Around line 88-109: The aur_srcinfo function hardcodes pkgdesc causing
.SRCINFO to diverge from PKGBUILD; change aur_srcinfo to accept a pkgdesc
parameter (remove the unused pkgbuild parameter) and use that value instead of
the hardcoded "BAML wrapper - toolchain manager and launcher", then update the
call sites that generate baml and baml-bin to pass the exact pkgdesc strings
from their PKGBUILD files ("...launcher (prebuilt binary)" and "...launcher
(built from source)"). Ensure the returned string uses the provided pkgdesc and
keep the rest of the fields unchanged.
In `@scripts/baml-release-manifests`:
- Line 78: The line using vsix = next(args.vsix_dir.glob("*.vsix")) can raise
StopIteration with no message; change this to explicitly handle the empty-case
(e.g., use next(args.vsix_dir.glob("*.vsix"), None) or
list(args.vsix_dir.glob("*.vsix")) and check length) and when no .vsix is found
raise a clear, user-facing error (consistent with other missing-artifact
handling) referencing args.vsix_dir and the vsix variable so the log/exit
message states that no .vsix files were found in the provided directory.
In `@scripts/install.sh`:
- Around line 45-46: The tmp directory creation is insecure: replace the
predictable tmp="${TMPDIR:-/tmp}/baml-install.$$" and mkdir -p "$tmp" usage with
a secure temporary directory created via mktemp -d; update any code that
references the tmp variable (tmp) to use the mktemp-produced path and ensure you
handle mktemp failure (check for non-empty/exit) and clean up the directory on
exit so tmp is exclusively owned and unpredictable.
- Around line 78-91: The check using grep -qx 'bin/baml' against the entries
file fails for tar members prefixed with ./; update the installer to accept both
forms by normalizing or adjusting the pattern: either strip leading ./ from the
variable that stores entries (entries="$tmp/entries.txt") after creating it, or
change the existence test to use a regex accepting (^|\\./)bin/baml$ (the
current grep call grep -qx 'bin/baml' "$entries" and the tar listing step using
tar -tzf "$archive" are the relevant symbols to modify). Ensure the same
normalization or pattern is applied to any other safety checks that inspect
$entries so both 'bin/baml' and './bin/baml' are accepted.
In `@TASK/PRIOR-ART.md`:
- Line 233: The word "foresseable" in the document should be corrected to
"foreseeable"; locate the occurrence of "foresseable" in TASK/PRIOR-ART.md (the
sentence "This baml wrapper is too old to read manifest schema v2." context) and
replace that misspelling with "foreseeable" to fix the typo.
In `@TASK/USER-PATHS.md`:
- Line 233: Replace the misspelled word "foresseable" with the correct spelling
"foreseeable" in the sentence that currently reads 'All other foresseable SDKs
don't have any issues with this system.'; search for the exact token
"foresseable" in the document and update it to "foreseeable" so the sentence
reads correctly.
In `@typescript2/README.md`:
- Around line 82-86: The README step 4 currently documents running "pnpm
--filter app-vscode-ext build" but the package.json script
"vscode:package:build" actually runs "pnpm --filter baml-language build"; update
the README to match the real command (replace "app-vscode-ext" with
"baml-language") or, if the intended turbo target is different, change the
"vscode:package:build" script to use "app-vscode-ext" so the docs and
package.json are consistent; reference the README step and the
"vscode:package:build" script and ensure the documented command matches the
turbo target that emits the extension host bundle.
---
Outside diff comments:
In `@typescript2/app-vscode-ext/src/extension.ts`:
- Around line 194-226: The global knownProjects array is being overwritten
per-client and cleared on any client's Stopped state; change knownProjects to a
Map keyed by the client/project root (e.g. Map<string, string[]>) so each client
stores its own project list, modify the 'baml/listProjects' handler to set
knownProjects.set(clientIdOrProjectRoot, params.projects ?? []), change the
onDidChangeState Stopped branch to remove only that client's entry
(knownProjects.delete(clientIdOrProjectRoot)) instead of clearing everything,
and update refreshTooltip() to aggregate across all Map values when rendering
the status-bar tooltip; use the client instance or its projectRoot as the unique
key to locate per-client entries.
- Around line 312-317: The command handler registered as 'baml.openPlayground'
is using the raw server-provided projectPath to look up clients in the clients
Map, but server paths from baml/listProjects may point at baml_src/ and
therefore won't match keys created by findBamlProjectRoot(); fix by normalizing
the incoming projectPath with the same resolution logic used for client keys
(call findBamlProjectRoot(projectPath) or otherwise strip/resolve baml_src to
the project root) before doing clients.get(projectPath), so the lookup and the
activeClient() fallback use the same canonical project root; update the handler
around clients.get(...) (or update findBamlProjectRoot to accept server paths)
to perform this normalization.
---
Nitpick comments:
In @.github/workflows/publish-python-pypi.yml:
- Around line 33-36: The workflow-level permissions block currently sets
"id-token: write" broadly; remove the workflow-level "id-token: write" entry and
keep the existing job-level "id-token: write" declaration in the publish-to-pypi
job (referencing the publish-to-pypi job name) so permissions follow
least-privilege and eliminate the redundant workflow-level id-token setting.
In @.github/workflows/release-baml-language.yml:
- Around line 66-69: Update every actions/checkout@v4 step to include
persist-credentials: false unless that job intentionally needs GitHub token;
specifically locate the checkout steps (the uses: actions/checkout@v4
occurrences around the shown diff and also at the other occurrences noted) and
add the persist-credentials: false key alongside ref and fetch-depth so the
checkout does not leave the default token in the checked-out repo; leave
checkout entries in jobs that explicitly require pushing/committing unchanged if
they truly need credentials.
In `@baml_language/crates/baml_cli/src/ide_command.rs`:
- Around line 26-32: Add a clap-level conflict so the parser rejects using both
flags: annotate the IdeInstallArgs fields to declare mutual exclusivity (use the
clap attribute like conflicts_with on the cursor and/or code fields) and then
remove the runtime check in IdeInstallArgs::resolve_editor that returns an
anyhow error for self.cursor && self.code; keep resolve_editor focused on
resolving the Editor and rely on clap to enforce the conflict at parse time.
In `@baml_language/crates/bex_project/src/bex_lsp/multi_project/request.rs`:
- Around line 80-89: Add a unit test that pins the initialize metadata by
asserting the exact experimental.baml JSON payload and the server version
returned by server_capabilities() / on_request_initialize(); locate the
experimental payload construction in the multi-project initialize logic (the
block that sets experimental: Some(serde_json::json!({...}))) and the functions
server_capabilities() and on_request_initialize() and create a test that calls
those functions (or constructs the same payload path) and compares the resulting
JSON object and server_info.version against a hard-coded expected value to
detect accidental key/name or version-source drift; ensure the test lives
alongside the module as a standard Rust #[cfg(test)] unit test and fails on any
change to keys, values, or version constants.
In `@baml_language/crates/bex_project/src/bex_lsp/protocol.rs`:
- Around line 1-10: The protocol constants (BAML_LSP_PROTOCOL_VERSION,
BAML_PLAYGROUND_PROTOCOL_VERSION, MIN_SUPPORTED_VSCODE_LSP_PROTOCOL,
MIN_SUPPORTED_PLAYGROUND_PROTOCOL) and the CAPABILITIES array are duplicated in
TypeScript (compat.ts and extension.ts) and must be kept in sync; update this
Rust file by adding a clear comment above these declarations pointing to the
exact TypeScript counterparts (compat.ts for BAML_* values and extension.ts for
the capabilities list) and include a short TODO/instruction that any change here
must be mirrored in those TS files (or consider centralizing generation later)
so future maintainers will update both places atomically.
In `@scripts/baml-language-version`:
- Around line 346-353: The current insertion for __version__ in PY_INIT relies
on the fragile marker "atexit.register(flush_events)\n" and may silently fail if
that line changes; update the fallback to append version_line to the end of
PY_INIT (or insert at another stable anchor) instead of using text.replace with
the marker, then validate the result by checking that '__version__ = "' appears
in the final text and raise/exit with a clear error if it does not; reference
PY_INIT, version_line, marker, and the atexit.register(flush_events) anchor when
making the change.
In `@scripts/baml-wrapper-version`:
- Around line 49-57: The read_version() function currently uses a multiline
regex that can match any "version = ..." line; update it to ensure the version
is read from the [package] table (instead of arbitrary sections). Replace the
regex approach in read_version() to parse WRAPPER_CARGO_TOML using tomllib
(available in Python 3.11+), extract data["package"]["version"], then call
Version.parse(...) and fail(...) on missing keys or parse errors; alternatively,
if you prefer a regex solution keep WRAPPER_CARGO_TOML.read_text() but scope the
regex to the [package] section before matching the version line to avoid picking
up dependency/version fields.
🪄 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: 1fa8baed-9841-415a-b6dd-290bfc93770b
⛔ Files ignored due to path filters (2)
baml_language/Cargo.lockis excluded by!**/*.lockbaml_language/sdks/python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
.github/workflows/build-python-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish-python-pypi.yml.github/workflows/release-baml-language-alpha.yml.github/workflows/release-baml-language.yml.github/workflows/release-cli.yaml.github/workflows/release-pkg-boundaryml-com.ymlTASK/AMENDMENTS.mdTASK/COMMANDS.mdTASK/FAILURE-MODES.mdTASK/OPEN-QUESTIONS.mdTASK/PRIOR-ART.mdTASK/TASK.mdTASK/USER-PATHS.mdTASK/docs/install.mdTASK/docs/release-maintainer.mdTASK/docs/toolchain-system.mdbaml_language/Cargo.tomlbaml_language/crates/baml/Cargo.tomlbaml_language/crates/baml/src/main.rsbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_cli/src/commands.rsbaml_language/crates/baml_cli/src/ide_command.rsbaml_language/crates/baml_cli/src/lib.rsbaml_language/crates/baml_cli/src/main.rsbaml_language/crates/baml_cli/src/pack_command.rsbaml_language/crates/baml_lsp_server/Cargo.tomlbaml_language/crates/baml_lsp_server/src/lib.rsbaml_language/crates/baml_lsp_server/src/playground_server.rsbaml_language/crates/baml_lsp_server/src/playground_ws.rsbaml_language/crates/baml_release/Cargo.tomlbaml_language/crates/baml_release/src/lib.rsbaml_language/crates/baml_release/src/manifest.rsbaml_language/crates/baml_version/Cargo.tomlbaml_language/crates/baml_version/src/lib.rsbaml_language/crates/bex_project/Cargo.tomlbaml_language/crates/bex_project/src/bex_lsp/mod.rsbaml_language/crates/bex_project/src/bex_lsp/multi_project/request.rsbaml_language/crates/bex_project/src/bex_lsp/protocol.rsbaml_language/crates/bridge_cffi/Cargo.tomlbaml_language/crates/bridge_cffi/src/ffi/runtime.rsbaml_language/crates/bridge_wasm/Cargo.tomlbaml_language/crates/bridge_wasm/src/lib.rsbaml_language/release.tomlbaml_language/sdks/nodejs/bridge_nodejs/Cargo.tomlbaml_language/sdks/nodejs/bridge_nodejs/package.jsonbaml_language/sdks/nodejs/bridge_nodejs/src/lib.rsbaml_language/sdks/python/pyproject.tomlbaml_language/sdks/python/rust/bridge_python/Cargo.tomlbaml_language/sdks/python/rust/bridge_python/src/lib.rsbaml_language/sdks/python/src/baml_core/__init__.pybaml_language/stow.tomlpackaging/aur/baml-bin/PKGBUILDpackaging/aur/baml/PKGBUILDscripts/baml-language-versionscripts/baml-package-manager-artifactsscripts/baml-release-manifestsscripts/baml-wrapper-versionscripts/install.ps1scripts/install.shtools/pkg_boundaryml_com/data/index.htmltools/pkg_boundaryml_com/lib/pkg-boundaryml-com-stack.tstypescript2/README.mdtypescript2/app-vscode-ext/package.jsontypescript2/app-vscode-ext/src/compat.tstypescript2/app-vscode-ext/src/extension.tstypescript2/package.jsontypescript2/pkg-playground/src/ports/WebSocketRuntimePort.tstypescript2/pkg-playground/src/protocol.ts
💤 Files with no reviewable changes (3)
- .github/workflows/release-pkg-boundaryml-com.yml
- .github/workflows/release-cli.yaml
- .github/workflows/release-baml-language-alpha.yml
rossirpaulo
marked this pull request as ready for review
June 3, 2026 05:14
…-release-pipeline # Conflicts: # .github/workflows/release-baml-language-alpha.yml # baml_language/sdks/nodejs/bridge_nodejs/package.json
⏭️ 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):
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
TASK/MERGE-TO-CANARY.md (1)
7-14: ⚡ Quick winMove this merge playbook out of the long-lived repo before merging.
The branch SHAs, divergence counts, and one-off conflict decisions here become stale immediately once
paulo/baml-language-release-pipelinemoves or the merge lands. Keeping this in-tree will leave misleading operator guidance next to the real release docs.Also applies to: 15-21, 411-420
🤖 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/MERGE-TO-CANARY.md` around lines 7 - 14, The MERGE-TO-CANARY playbook currently embeds ephemeral branch SHAs and divergence info for paulo/baml-language-release-pipeline and origin/canary; remove those stale, in-repo details before merging by extracting the full playbook content to an external, versioned runbook/wiki (or private ops storage) and replace the in-repo MERGE-TO-CANARY playbook with a short pointer and a note that SHA/divergence lines are ephemeral; update the same change in the other occurrences referenced (the blocks around the earlier 15-21 and 411-420 sections) so no hard-coded SHAs (e.g., merge base 1742580..., branch head 12463db..., origin/canary 9cff3559...) remain in-tree..github/workflows/build-nodejs-sdk.reusable.yaml (1)
20-24: ⚡ Quick winEnsure the push rehearsal exercises the stamped release path.
sam/publish-nodejsruns fromon: push, and for push-triggered workflow runs theinputscontext is not populated (even if the workflow also hasworkflow_callinputs). Since the “Stamp release plan” step is guarded byif: ${{ inputs.release_plan_json != '' }}, it will be skipped on that rehearsal branch, so the stamped release path isn’t exercised. Add a push-compatible way to provide a fixturerelease_plan_json(or use aworkflow_dispatchrehearsal path) so the stamp step 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/build-nodejs-sdk.reusable.yaml around lines 20 - 24, The "Stamp release plan" step is skipped on push because inputs.release_plan_json is empty for push-triggered runs; to fix, add a push-compatible fixture and broaden the guard: create a small preparatory step (run-on-push) that sets an env/output like RELEASE_PLAN_JSON with a fixture JSON when github.event_name == 'push', then change the "Stamp release plan" condition from if: ${{ inputs.release_plan_json != '' }} to check either source (e.g. if: ${{ inputs.release_plan_json != '' || env.RELEASE_PLAN_JSON != '' }} or use steps.prepare.outputs.release_plan_json), and update the step to read the release plan from inputs.release_plan_json || env.RELEASE_PLAN_JSON (or the prepare step output) so the stamped release path runs during the sam/publish-nodejs push rehearsal.
🤖 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 45-50: The comment claiming “Target glibc 2.17” for the aarch64
target in build-nodejs-sdk.reusable.yaml is incorrect because the container used
(ghcr.io/rust-cross/manylinux_2_28-cross:aarch64) targets glibc >= 2.28; either
replace that container with a manylinux_2_17-compatible cross image or sysroot
for target aarch64-unknown-linux-gnu, or update the inline comment and any
release/docs to state the true minimum glibc (2.28) for the aarch64 build;
locate the aarch64-unknown-linux-gnu block and change the container string or
the comment to reflect the chosen, consistent glibc floor.
- Around line 106-110: Update the local composite action's input schema so
actionlint can parse the node-action-version input: open the action definition
that declares the input named node-action-version (in the action.yml for the
setup-node composite action) and remove or replace unsupported keys like type:
choice and the options array with an actionlint-compatible declaration (e.g.,
use a simple string type or omit type/options entirely and provide a
description/default), ensuring the input name node-action-version remains and
the setup usage (uses: ./.github/actions/setup-node with: node-action-version:
v6) still works.
---
Nitpick comments:
In @.github/workflows/build-nodejs-sdk.reusable.yaml:
- Around line 20-24: The "Stamp release plan" step is skipped on push because
inputs.release_plan_json is empty for push-triggered runs; to fix, add a
push-compatible fixture and broaden the guard: create a small preparatory step
(run-on-push) that sets an env/output like RELEASE_PLAN_JSON with a fixture JSON
when github.event_name == 'push', then change the "Stamp release plan" condition
from if: ${{ inputs.release_plan_json != '' }} to check either source (e.g. if:
${{ inputs.release_plan_json != '' || env.RELEASE_PLAN_JSON != '' }} or use
steps.prepare.outputs.release_plan_json), and update the step to read the
release plan from inputs.release_plan_json || env.RELEASE_PLAN_JSON (or the
prepare step output) so the stamped release path runs during the
sam/publish-nodejs push rehearsal.
In `@TASK/MERGE-TO-CANARY.md`:
- Around line 7-14: The MERGE-TO-CANARY playbook currently embeds ephemeral
branch SHAs and divergence info for paulo/baml-language-release-pipeline and
origin/canary; remove those stale, in-repo details before merging by extracting
the full playbook content to an external, versioned runbook/wiki (or private ops
storage) and replace the in-repo MERGE-TO-CANARY playbook with a short pointer
and a note that SHA/divergence lines are ephemeral; update the same change in
the other occurrences referenced (the blocks around the earlier 15-21 and
411-420 sections) so no hard-coded SHAs (e.g., merge base 1742580..., branch
head 12463db..., origin/canary 9cff3559...) remain in-tree.
🪄 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: d4af2bbe-adc6-4a3c-af76-bd1836c371a0
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish-python-pypi.ymlTASK/MERGE-TO-CANARY.mdbaml_language/Cargo.tomlbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/compound_ops.bamlbaml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/test_complex_models.pybaml_language/sdk_tests/crates/typescript_node/type_shapes/customizable/roundtrip_complex_models.test.ts
💤 Files with no reviewable changes (2)
- baml_language/sdk_tests/crates/typescript_node/type_shapes/customizable/roundtrip_complex_models.test.ts
- baml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/test_complex_models.py
✅ Files skipped from review due to trivial changes (1)
- baml_language/crates/baml_cli/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- baml_language/Cargo.toml
- .github/workflows/publish-python-pypi.yml
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🧹 Nitpick comments (2)
TASK/MERGE-TO-CANARY.md (1)
7-14: ⚡ Quick winMove this merge playbook out of the long-lived repo before merging.
The branch SHAs, divergence counts, and one-off conflict decisions here become stale immediately once
paulo/baml-language-release-pipelinemoves or the merge lands. Keeping this in-tree will leave misleading operator guidance next to the real release docs.Also applies to: 15-21, 411-420
🤖 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/MERGE-TO-CANARY.md` around lines 7 - 14, The MERGE-TO-CANARY playbook currently embeds ephemeral branch SHAs and divergence info for paulo/baml-language-release-pipeline and origin/canary; remove those stale, in-repo details before merging by extracting the full playbook content to an external, versioned runbook/wiki (or private ops storage) and replace the in-repo MERGE-TO-CANARY playbook with a short pointer and a note that SHA/divergence lines are ephemeral; update the same change in the other occurrences referenced (the blocks around the earlier 15-21 and 411-420 sections) so no hard-coded SHAs (e.g., merge base 1742580..., branch head 12463db..., origin/canary 9cff3559...) remain in-tree..github/workflows/build-nodejs-sdk.reusable.yaml (1)
20-24: ⚡ Quick winEnsure the push rehearsal exercises the stamped release path.
sam/publish-nodejsruns fromon: push, and for push-triggered workflow runs theinputscontext is not populated (even if the workflow also hasworkflow_callinputs). Since the “Stamp release plan” step is guarded byif: ${{ inputs.release_plan_json != '' }}, it will be skipped on that rehearsal branch, so the stamped release path isn’t exercised. Add a push-compatible way to provide a fixturerelease_plan_json(or use aworkflow_dispatchrehearsal path) so the stamp step 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/build-nodejs-sdk.reusable.yaml around lines 20 - 24, The "Stamp release plan" step is skipped on push because inputs.release_plan_json is empty for push-triggered runs; to fix, add a push-compatible fixture and broaden the guard: create a small preparatory step (run-on-push) that sets an env/output like RELEASE_PLAN_JSON with a fixture JSON when github.event_name == 'push', then change the "Stamp release plan" condition from if: ${{ inputs.release_plan_json != '' }} to check either source (e.g. if: ${{ inputs.release_plan_json != '' || env.RELEASE_PLAN_JSON != '' }} or use steps.prepare.outputs.release_plan_json), and update the step to read the release plan from inputs.release_plan_json || env.RELEASE_PLAN_JSON (or the prepare step output) so the stamped release path runs during the sam/publish-nodejs push rehearsal.
🤖 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 45-50: The comment claiming “Target glibc 2.17” for the aarch64
target in build-nodejs-sdk.reusable.yaml is incorrect because the container used
(ghcr.io/rust-cross/manylinux_2_28-cross:aarch64) targets glibc >= 2.28; either
replace that container with a manylinux_2_17-compatible cross image or sysroot
for target aarch64-unknown-linux-gnu, or update the inline comment and any
release/docs to state the true minimum glibc (2.28) for the aarch64 build;
locate the aarch64-unknown-linux-gnu block and change the container string or
the comment to reflect the chosen, consistent glibc floor.
- Around line 106-110: Update the local composite action's input schema so
actionlint can parse the node-action-version input: open the action definition
that declares the input named node-action-version (in the action.yml for the
setup-node composite action) and remove or replace unsupported keys like type:
choice and the options array with an actionlint-compatible declaration (e.g.,
use a simple string type or omit type/options entirely and provide a
description/default), ensuring the input name node-action-version remains and
the setup usage (uses: ./.github/actions/setup-node with: node-action-version:
v6) still works.
---
Nitpick comments:
In @.github/workflows/build-nodejs-sdk.reusable.yaml:
- Around line 20-24: The "Stamp release plan" step is skipped on push because
inputs.release_plan_json is empty for push-triggered runs; to fix, add a
push-compatible fixture and broaden the guard: create a small preparatory step
(run-on-push) that sets an env/output like RELEASE_PLAN_JSON with a fixture JSON
when github.event_name == 'push', then change the "Stamp release plan" condition
from if: ${{ inputs.release_plan_json != '' }} to check either source (e.g. if:
${{ inputs.release_plan_json != '' || env.RELEASE_PLAN_JSON != '' }} or use
steps.prepare.outputs.release_plan_json), and update the step to read the
release plan from inputs.release_plan_json || env.RELEASE_PLAN_JSON (or the
prepare step output) so the stamped release path runs during the
sam/publish-nodejs push rehearsal.
In `@TASK/MERGE-TO-CANARY.md`:
- Around line 7-14: The MERGE-TO-CANARY playbook currently embeds ephemeral
branch SHAs and divergence info for paulo/baml-language-release-pipeline and
origin/canary; remove those stale, in-repo details before merging by extracting
the full playbook content to an external, versioned runbook/wiki (or private ops
storage) and replace the in-repo MERGE-TO-CANARY playbook with a short pointer
and a note that SHA/divergence lines are ephemeral; update the same change in
the other occurrences referenced (the blocks around the earlier 15-21 and
411-420 sections) so no hard-coded SHAs (e.g., merge base 1742580..., branch
head 12463db..., origin/canary 9cff3559...) remain in-tree.
🪄 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: d4af2bbe-adc6-4a3c-af76-bd1836c371a0
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/ci.yaml.github/workflows/publish-python-pypi.ymlTASK/MERGE-TO-CANARY.mdbaml_language/Cargo.tomlbaml_language/crates/baml_cli/Cargo.tomlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/invalid_assignment/compound_ops.bamlbaml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/test_complex_models.pybaml_language/sdk_tests/crates/typescript_node/type_shapes/customizable/roundtrip_complex_models.test.ts
💤 Files with no reviewable changes (2)
- baml_language/sdk_tests/crates/typescript_node/type_shapes/customizable/roundtrip_complex_models.test.ts
- baml_language/sdk_tests/crates/python_pydantic2/type_shapes/customizable/test_complex_models.py
✅ Files skipped from review due to trivial changes (1)
- baml_language/crates/baml_cli/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- baml_language/Cargo.toml
- .github/workflows/publish-python-pypi.yml
🛑 Comments failed to post (2)
.github/workflows/build-nodejs-sdk.reusable.yaml (2)
45-50:
⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift🧩 Analysis chain
🌐 Web query:
What minimum glibc version does the manylinux_2_28 policy target for Linux aarch64 wheels/containers?💡 Result:
The manylinux_2_28 policy targets a minimum glibc version of 2.28 for all supported architectures, including aarch64 [1][2][3]. This policy is based on the glibc version provided by the underlying distribution used for the manylinux_2_28 images (AlmaLinux 8) [1][4][5]. Wheels built under this policy are expected to be compatible with any Linux distribution that uses glibc 2.28 or later [1][5].
Citations:
- 1: https://github.com/pypa/manylinux
- 2: pypa/manylinux#1585
- 3: pypa/manylinux#1282
- 4: https://github.com/pypa/manylinux?tab=readme-ov-file
- 5: https://github.com/pypa/manylinux/blob/main/README.rst
🏁 Script executed:
#!/bin/bash set -euo pipefail file=".github/workflows/build-nodejs-sdk.reusable.yaml" # Print the relevant section around the snippet sed -n '35,70p' "$file" | cat -n # Search for all mentions of glibc floor / manylinux policy / 2.17 / 2.28 rg -n "glibc|manylinux|2\.17|2\.28" "$file" || true # Show any release metadata stamping / inputs around those lines if present rg -n "release|Stamp|release_plan_json|abi|target" "$file" || trueRepository: BoundaryML/baml
Length of output: 3741
🏁 Script executed:
#!/bin/bash set -euo pipefail rg -n "glibc\s*2\.17|2\.17-compatible|manylinux_2_28|glibc\s*2\.28|2\.28-compatible" .github/workflows/build-nodejs-sdk.reusable.yaml # If you want broader confirmation of release metadata claims, also scan the repo for 2.17 mentions (cap output kept small). rg -n "glibc\s*2\.17|2\.17-compatible" . | head -n 50Repository: BoundaryML/baml
Length of output: 640
Fix glibc compatibility claim for
aarch64-unknown-linux-gnuinbuild-nodejs-sdk.reusable.yaml
build-nodejs-sdk.reusable.yamlstates “Target glibc 2.17” foraarch64-unknown-linux-gnu, but the container used isghcr.io/rust-cross/manylinux_2_28-cross:aarch64(manylinux_2_28 targets a minimum glibc of 2.28). This makes the artifact misrepresented as 2.17-compatible on distros with glibc 2.17–2.27. Either switch to a true manylinux/2.17-cross (or equivalent 2.17 sysroot) for that target, or update the documented support floor/release notes accordingly.🤖 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/build-nodejs-sdk.reusable.yaml around lines 45 - 50, The comment claiming “Target glibc 2.17” for the aarch64 target in build-nodejs-sdk.reusable.yaml is incorrect because the container used (ghcr.io/rust-cross/manylinux_2_28-cross:aarch64) targets glibc >= 2.28; either replace that container with a manylinux_2_17-compatible cross image or sysroot for target aarch64-unknown-linux-gnu, or update the inline comment and any release/docs to state the true minimum glibc (2.28) for the aarch64 build; locate the aarch64-unknown-linux-gnu block and change the container string or the comment to reflect the chosen, consistent glibc floor.
106-110:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Inspect the local action definition that actionlint could not parse. fd -p 'action.y*ml' .github/actions/setup-node -x sed -n '1,160p' {} rg -n 'node-action-version|options:' .github/actions/setup-nodeRepository: BoundaryML/baml
Length of output: 2695
🏁 Script executed:
#!/bin/bash set -euo pipefail command -v actionlint >/dev/null 2>&1 && echo "actionlint: $(actionlint -version || true)" || echo "actionlint: not installed" # Lint the specific workflow that calls the local action. if command -v actionlint >/dev/null 2>&1; then actionlint .github/workflows/build-nodejs-sdk.reusable.yaml || true fi # If the action.yml exists, attempt to lint it as well (best-effort; actionlint behavior may vary). if command -v actionlint >/dev/null 2>&1; then actionlint .github/actions/setup-node/action.yml || true fiRepository: BoundaryML/baml
Length of output: 1538
Fix
.github/actions/setup-node/action.ymlso actionlint can parse thenode-action-versioninput.
actionlintfails parsing the local action used by.github/workflows/build-nodejs-sdk.reusable.yaml(line 107), specifically rejecting theoptionskey for thenode-action-versioninput—so static validation for thatwith:contract is lost at the call site. Adjust thenode-action-versioninput schema to whatactionlintaccepts (e.g., change/removetype: choice+options) so linting remains safe.🧰 Tools
🪛 actionlint (1.7.12)
[error] 107-107: could not parse action metadata in "/home/jailuser/git/.github/actions/setup-node": line 5: unexpected key "options" for definition of input "node-action-version"
(action)
🤖 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/build-nodejs-sdk.reusable.yaml around lines 106 - 110, Update the local composite action's input schema so actionlint can parse the node-action-version input: open the action definition that declares the input named node-action-version (in the action.yml for the setup-node composite action) and remove or replace unsupported keys like type: choice and the options array with an actionlint-compatible declaration (e.g., use a simple string type or omit type/options entirely and provide a description/default), ensuring the input name node-action-version remains and the setup usage (uses: ./.github/actions/setup-node with: node-action-version: v6) still works.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-python-sdk.reusable.yaml (1)
85-93:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDirect rehearsal paths skip stamping entirely.
When
release_plan_jsonis empty, this step is skipped. That happens on this workflow'spushtrigger and on manualpublish-python-pypi.ymlruns, so those builds read whatever version is already in checkout instead of the canary-stamped surfaces the release graph will publish. The Node reusable workflow in this PR already handles the empty-input case correctly; this one should generate a canary plan and stamp unconditionally too.Suggested change
- name: Stamp release plan - if: ${{ inputs.release_plan_json != '' }} env: RELEASE_PLAN_JSON: ${{ inputs.release_plan_json }} run: | set -euo pipefail - printf '%s\n' "$RELEASE_PLAN_JSON" > release-plan.json + if [[ -n "${RELEASE_PLAN_JSON:-}" ]]; then + printf '%s\n' "$RELEASE_PLAN_JSON" > release-plan.json + else + scripts/baml-language-version plan --channel canary --out release-plan.json + fi scripts/baml-language-version stamp --plan release-plan.json🤖 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/build-python-sdk.reusable.yaml around lines 85 - 93, The "Stamp release plan" step currently skips when inputs.release_plan_json is empty, causing builds to use unchecked versions; update this step so it always produces and stamps a release plan: if RELEASE_PLAN_JSON is empty, have the step generate a canary release-plan.json (e.g., create a minimal canary plan JSON) before calling scripts/baml-language-version stamp --plan release-plan.json, and ensure the environment variable RELEASE_PLAN_JSON is used when present; keep the step name "Stamp release plan" and the stamp command intact so scripts/baml-language-version runs unconditionally with either the provided plan or the generated canary plan.
🤖 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-python-sdk.reusable.yaml:
- Around line 81-84: The checkout step using actions/checkout@v6 with ref: ${{
inputs.source_sha || github.sha }} is leaving persistable credentials; add
persist-credentials: false under the same with block to prevent checkout
credentials from being available to later steps. Update the step that references
actions/checkout@v6 and the with: ref entry to include persist-credentials:
false so the runner does not retain repository credentials.
In @.github/workflows/publish-python-pypi.yml:
- Around line 96-135: When RELEASE_PLAN_JSON is present (plan_json) and you set
version = json.loads(plan_json)["pypi_version"], validate that the wheels in
dist/ actually match that version before querying PyPI: after computing
local_files (or wheels) check that every local wheel filename contains the exact
version string (e.g. using a regex similar to r"^baml_core-(.+?)-" or by
asserting version in path.name); if any local wheel does not match, print an
error (to stderr) and exit non‑zero to fail fast rather than publishing
mismatched artifacts; otherwise continue with the existing PyPI URL/query logic.
---
Outside diff comments:
In @.github/workflows/build-python-sdk.reusable.yaml:
- Around line 85-93: The "Stamp release plan" step currently skips when
inputs.release_plan_json is empty, causing builds to use unchecked versions;
update this step so it always produces and stamps a release plan: if
RELEASE_PLAN_JSON is empty, have the step generate a canary release-plan.json
(e.g., create a minimal canary plan JSON) before calling
scripts/baml-language-version stamp --plan release-plan.json, and ensure the
environment variable RELEASE_PLAN_JSON is used when present; keep the step name
"Stamp release plan" and the stamp command intact so
scripts/baml-language-version runs unconditionally with either the provided plan
or the generated canary plan.
🪄 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: bc353522-68dd-41e0-848e-3912bc715f33
⛔ Files ignored due to path filters (1)
baml_language/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.github/workflows/build-nodejs-sdk.reusable.yaml.github/workflows/build-python-sdk.reusable.yaml.github/workflows/publish-python-pypi.yml.github/workflows/release-baml-language.ymlTASK/OPEN-QUESTIONS.mdTASK/docs/toolchain-system.mdbaml_language/Cargo.tomlbaml_language/crates/baml/Cargo.tomlbaml_language/crates/baml/src/main.rsbaml_language/crates/baml_cli/Cargo.tomlbaml_language/stow.tomltools/pkg_boundaryml_com/lib/pkg-boundaryml-com-stack.ts
✅ Files skipped from review due to trivial changes (2)
- baml_language/crates/baml_cli/Cargo.toml
- TASK/docs/toolchain-system.md
🚧 Files skipped from review as they are similar to previous changes (7)
- baml_language/Cargo.toml
- tools/pkg_boundaryml_com/lib/pkg-boundaryml-com-stack.ts
- baml_language/crates/baml/Cargo.toml
- baml_language/stow.toml
- .github/workflows/release-baml-language.yml
- baml_language/crates/baml/src/main.rs
- TASK/OPEN-QUESTIONS.md
This was referenced Jul 10, 2026
feat(toolchain): outdated warnings for toolchain + agent skills, install skills from repo head
#3970
Merged
Merged
This branch was previously 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
Overhauls the BAML language release path around the new wrapper/toolchain split.
bamlwrapper crate plus sharedbaml_releaseand stampedbaml_versioncratesbaml_language/release.tomlandscripts/baml-language-versionbaml lspthrough the wrapper, and adds LSP/playground compatibility metadataValidation
cargo fmtcargo stowcargo clippy --workspace --all-targets --all-features -- -D warningsactionlint .github/workflows/release-baml-language.yml .github/workflows/publish-python-pypi.yml .github/workflows/build-python-sdk.reusable.yamlcargo check --manifest-path baml_language/Cargo.toml -p baml -p baml_release -p baml_clicargo test --manifest-path baml_language/Cargo.toml -p baml_releasescripts/baml-language-version checkscripts/baml-wrapper-version checktypescript2/app-vscode-ext:./node_modules/.bin/tsc --noEmitRelease Follow-ups
Before enabling production publishes:
.github/workflows/publish-python-pypi.ymlscripts/install.ps1on Windows/PowerShellTASK/*planning files should remain in the branchSummary by CodeRabbit
New Features
Installers & Packaging
Documentation
CI / Releases