feat(toolchain): select a local baml-cli by path - #4264
Conversation
Running a locally built baml-cli meant either repointing BAML_HOME and hand-seeding a toolchain directory or bypassing the wrapper entirely with BAML_CLI_ALLOW_DIRECT. Neither survives a project that pins [toolchain]. A selector containing a path separator now names a baml-cli binary the wrapper does not manage, accepted anywhere a selector is: BAML_VERSION, [toolchain] path in baml.toml, and baml toolchain use. Channels and versions never contain a separator, so no prefix or flag is needed to tell them apart. Paths in baml.toml resolve relative to that file, so a relative path is committable. Resolved paths are absolutized once and canonicalized when the target exists, keeping ../ out of --version, status, and the config file; a path that does not exist yet keeps its joined form so the error names what was asked for. Path toolchains skip the VERSION check, the channel-outdated warning, and the background manifest refresh, so they always take the exec fast path. install and uninstall reject them, update no-ops, and status and list report them without touching the network. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9aTrkwDQgGr9HSvdEcq61
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ 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):
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe wrapper now supports local path selectors alongside managed channels and versions. It normalizes and verifies local ChangesPath toolchain support
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant ToolchainCLI
participant SelectorResolver
participant LocalBamlCLI
participant IDECommand
ToolchainCLI->>SelectorResolver: resolve path selector
SelectorResolver->>LocalBamlCLI: verify local binary
LocalBamlCLI-->>ToolchainCLI: return usability result
ToolchainCLI->>LocalBamlCLI: execute with local-toolchain marker
IDECommand->>LocalBamlCLI: check VSIX asset
LocalBamlCLI-->>IDECommand: report local binary context
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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 `@baml_language/crates/baml/src/main.rs`:
- Around line 207-220: Update the path-selector branch in print_version so
verify_path_toolchain receives path_selector_origin(&selector.source) instead of
an empty origin. Preserve the existing success annotation and failure output
while ensuring verification errors identify whether the selected path came from
the environment, project configuration, or default configuration.
🪄 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 Plus
Run ID: f439737d-cf5f-4deb-afb7-9df80ba48a0c
📒 Files selected for processing (1)
baml_language/crates/baml/src/main.rs
Binary size checks passed✅ 7 passed
Generated by |
Follow-up to the path-selector change, from an adversarial review. Drop [toolchain] path from baml.toml. A checked-out repository could otherwise choose which binary runs: a committed path plus a mode-755 file (git preserves the exec bit) executed on any baml invocation, with no prompt and deliberately nothing on stderr. rustup avoids this by requiring an absolute path, which would make the field useless to a teammate anyway, so the field is refused outright and the refusal explains where to set a local toolchain instead. $BAML_VERSION and baml toolchain use are unaffected; both are typed by the developer. Refuse to exec the wrapper itself. baml and baml-cli build into the same directory, so pointing at the wrapper is a one-character slip, and it re-resolved the same selector and exec'd itself forever: silent and unkillable-looking on unix, an unbounded process tree elsewhere. Tidy paths lexically instead of canonicalizing. Canonicalizing resolved symlinks away, so a current -> release-N indirection was frozen to the target it had the day it was set and stopped following repoints. It also produced \\?\C:\... verbatim paths on Windows, which would have been written into config.toml and printed by --version and status. Also: fall back to .exe on Windows when the path has no extension; refuse a relative default.selector in the global config, which would otherwise resolve against cwd and run a different binary per directory; surface resolution errors from baml toolchain list instead of swallowing them and reporting a selector that cannot run; attribute exec failures that verify_path_toolchain cannot catch, such as a wrong architecture or a noexec mount; and make baml ide install say that a local toolchain has no bundled extension rather than reporting a missing asset file. The wrapper now passes BAML_WRAPPER_LOCAL_TOOLCHAIN for a path toolchain, leaving BAML_WRAPPER_RESOLVED_TOOLCHAIN version-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9aTrkwDQgGr9HSvdEcq61
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/crates/baml/src/main.rs (1)
494-532: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
~-prefixed default selectors can silently regain cwd-dependence whenHOMEis unset.The relative-path guard exempts any selector starting with
~on the assumption tilde always resolves to an absoluteHOMEpath. But pertilde_without_home_is_left_relative(line ~1729), whenHOMEis unset,resolve_selector_path_with_homejoins the literal~/...string ontobase(cwd here) instead of erroring — so a manually-edited global config withdefault.selector = "~/builds/baml-cli"and noHOMEset would silently produce a different binary per directory, exactly the failure mode this check exists to prevent.🐛 Proposed fix
if is_path_selector(selector) - && !selector.starts_with('~') + && (!selector.starts_with('~') || env::var_os("HOME").is_none()) && !Path::new(selector).is_absolute() {🤖 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/src/main.rs` around lines 494 - 532, Update the default-selector validation in active_selector so a ~-prefixed path is rejected when HOME is unset and would remain relative to the current directory. Preserve the existing acceptance of ~ paths when they resolve independently of cwd, while retaining the relative-path error for other non-absolute selectors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@baml_language/crates/baml/src/main.rs`:
- Around line 494-532: Update the default-selector validation in active_selector
so a ~-prefixed path is rejected when HOME is unset and would remain relative to
the current directory. Preserve the existing acceptance of ~ paths when they
resolve independently of cwd, while retaining the relative-path error for other
non-absolute selectors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 47b83e34-0fe2-48e9-b186-65114166af81
📒 Files selected for processing (2)
baml_language/crates/baml/src/main.rsbaml_language/crates/baml_cli/src/ide_command.rs
baml --version and baml toolchain status now ask a local toolchain what version it is, instead of printing only a path and leaving anything that reads them with no version at all. The call is capped at three seconds and the child is killed on timeout, so a binary that will not answer degrades to "version unknown" rather than hanging the command. Both now always name the setting that selected the binary, including the global config, which selector_origin stays quiet about and which is the one most likely to have been forgotten. Verification failures in --version carry that attribution too, so a broken local toolchain can be traced to whatever chose it. baml toolchain uninstall on a path now says how to get back to a managed toolchain rather than only refusing. Guard the global-config check on whether a selector can actually stand on its own instead of on a bare `~` prefix. With no home directory `~/x` falls back to being joined onto the current directory, and `~user` is not a form we expand, so both slipped through a prefix test and reintroduced the per-directory behaviour the check exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y9aTrkwDQgGr9HSvdEcq61
## Summary - bump the BAML wrapper from 0.2.3 to 0.2.4 - publish the local baml-cli path selection support merged in BoundaryML#4264 ## Root cause The wrapper 0.2.3 release predates BoundaryML#4264. Because that PR changed wrapper behavior without advancing the independently versioned wrapper package, users on the latest wrapper still interpreted an absolute baml-cli path as a version and attempted to fetch a manifest for it. ## Impact The next wrapper release will support local toolchain selection with commands such as: baml toolchain use ./target/release/baml-cli ## Validation - scripts/baml-wrapper-version check - python3 -m unittest scripts.tests.test_release_pipeline_contract - cargo check --manifest-path baml_language/Cargo.toml -p baml --release - git diff --check - pre-commit hooks <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Version updated to 0.2.4 <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Lets you point the
bamlwrapper at a locally builtbaml-cli.Why
Today the only ways to run a local build are repointing
BAML_HOMEand hand-seedingtoolchains/<v>/bin/baml-cli+VERSION(whatfreshness_e2e.rsdoes), or bypassing the wrapper entirely withBAML_CLI_ALLOW_DIRECT. Neither survives a project that pins[toolchain].Design
A path is recognized by shape, not by a prefix. A selector containing a path separator (or starting with
~or.) is a binary path. Channels and versions never contain one, so nopath:prefix, subcommand, or flag is needed to disambiguate.The one input this does not catch is a bare filename with no separator:
baml toolchain use baml-clireads as a version../baml-cliis the answer, same convention as running any local executable.Two places to set it, both typed by the developer, with precedence unchanged (
BAML_VERSION>baml.toml>~/.baml/config.toml>canary):Deliberately not settable from
baml.toml. A committedpathplus a mode-755 file (git preserves the exec bit) would execute on anybamlinvocation in that tree, with no prompt and nothing on stderr. rustup avoids this by requiringpathto be an absolute path, which also makes the field useless to a teammate, so there is little left to want. The field is refused with a message pointing at the two supported ways:Refuses to exec the wrapper itself.
bamlandbaml-clibuild into the same directory, so pointing atbamlis a one-character slip. Without a guard it re-resolves the same selector and execs itself forever: silent on unix, an unbounded process tree on Windows where the non-unix branch spawns a child per iteration.Paths are tidied lexically, not canonicalized. Canonicalizing resolves symlinks, which would freeze a
current -> release-Nindirection to whatever it pointed at the day it was set. It also yields\\?\C:\...verbatim paths on Windows, which would land inconfig.tomland in--versionoutput. Lexical..removal keeps the symlink and reads the same on every platform. If lexical tidying would produce a path that does not exist while the untidied one does (a symlinked directory followed by..), the untidied form is kept, so tidying can never break a working path.Path toolchains skip the version bookkeeping: no
VERSIONcheck, no channel-outdated warning, no background manifest refresh. That keeps theexec()fast path unconditional and keeps stderr clean for parsed output.Subcommands:
installanduninstallreject a path,updateno-ops,statusandlistreport the path and its origin without touching the network.listsurfaces resolution errors rather than swallowing them, since it is what you run to find out why nothing works.baml ide installneeds a managed toolchain and now says so. A local build has noassets/beside it, so it previously reported a missing file the developer never expected to exist.baml playgroundandbaml packalso degrade under a local toolchain: both look for siblings of the executable, which only exist in the installed layout.Errors name the source, so a forgotten override is traceable:
Failures that cannot be caught up front (wrong architecture, a
noexecmount, a missing interpreter) carry the same attribution.Windows
The separator rule is split into
is_path_selector_on(selector, windows)so the Windows behavior is exercised by tests on every platform rather than only compiling undercfg(windows). A path with no extension falls back to.exe, sobaml toolchain use .\target\debug\baml-cliworks. A local Windows compile-check was not possible here (the target needs mingw for a TLS dependency's build script); thecargo test (windows)CI job covers it, since it runsbamlas part of--workspace.Testing
34 unit tests and the 2 existing e2e tests pass;
cargo fmtandcargo clippyare clean.Manually exercised end to end against both a stub and the real
baml-cli: env var with relative and~paths,toolchain use <path>followed by a plain invocation, a symlinked path surviving a repoint, the self-exec guard (direct and through a symlink), thebaml.tomlrefusal, a relativedefault.selectorbeing refused,listsurfacing that error,baml ide installrefusing a local toolchain, and the directory / missing / non-executable errors. Channels and pinned versions still resolve exactly as before.Review note
The security, self-exec, symlink, Windows
.exe,list, and global-config issues above were found by an adversarial review pass over the first commit and fixed in the second.🤖 Generated with Claude Code
https://claude.ai/code/session_01Y9aTrkwDQgGr9HSvdEcq61
Summary by CodeRabbit
baml-clibinary as an active toolchain.baml toolchain use <path>and$BAML_VERSION, including local version detection.--versionnow identify active local binaries and usability.