Repository navigation
fix(cli): make baml agent install work without the GitHub API and install into the right directory - #3984
Merged
Conversation
…helpers New baml_release::skills module shared by the wrapper and baml-cli: skill repo URL builders (env-overridable for mirrors/tests), [skills] provenance state in state.toml written via read-modify-write so it never clobbers the wrapper-owned [channels.*] sections, a latest-commit freshness cache, and a GitHub API fetch for the skill repo head commit. Co-authored-by: Cursor <[email protected]>
…rovenance baml agent install now defaults to the current head of BoundaryML/baml-skill: it resolves the main commit SHA first, downloads the tarball pinned to that exact commit, and records [skills] provenance in state.toml (plus the freshness cache) after a successful install. The --latest flag is removed since it is the standard behavior; --from remains for custom sources and the toolchain-release snapshot survives only behind BAML_AGENT_SKILLS_RELEASE_VERSION. Includes an e2e test that runs the real binary against a stubbed GitHub server and asserts the recorded provenance. Co-authored-by: Cursor <[email protected]>
…ated The wrapper now surfaces staleness on normal commands: before exec'ing baml-cli it compares the recorded channel version against the cached channel manifest and the [skills] provenance against the cached latest skill commit, printing a styled bold-yellow warning: line on stderr when either is behind. Caches auto-refresh over the network at most once per 24h window (5s timeout, silent on failure, throttled by a .last-check marker so offline machines do not retry every command), activating the previously dead [update] auto_check config flag with default-on/opt-out semantics. toolchain status gains a skill section and toolchain use/update do a best-effort skill check afterward. The wrapper's State struct now round-trips foreign state.toml sections so it cannot drop the CLI-written [skills] entry. Covered by unit tests plus an e2e suite that runs the real wrapper binary against a temp BAML_HOME. Co-authored-by: Cursor <[email protected]>
Projects without any baml-* agent skills now get a warning on every command (No baml skill is installed, set it up with baml agent install), instead of staying silent. Projects with skills keep the upgrade warning when provenance is missing or behind the cached latest commit; the project-skills check is now the primary gate, so global provenance no longer triggers upgrade warnings in skill-less projects. Decision logic extracted into a pure skill_warning_message helper with unit tests, plus updated e2e coverage. Co-authored-by: Cursor <[email protected]>
CI runs clippy with -D warnings, which rejected the numeric casts in the skills date helper (cast_possible_wrap/truncation/sign_loss), a nonminimal_bool in file_older_than, and format_push_string in the e2e test. Replace the casts with try_from with in-range fallbacks and simplify the boolean with is_none_or. Co-authored-by: Cursor <[email protected]>
…se pipeline
agent install now pulls the skill repo head directly from GitHub, so the
versioned baml-agent-skills-{version}.tar.gz release asset has no
consumers. Remove the build-agent-skills job, the packaging script, and
the BAML_AGENT_SKILLS_RELEASE_VERSION escape hatch that pointed at it.
Co-authored-by: Cursor <[email protected]>
- agent install --from now clears recorded provenance instead of leaving a stale commit that misreports custom content as current - write_skills_state treats only a missing state.toml as empty; corrupt or unreadable files error instead of being silently replaced - passive cache refreshes share one 5s wall-clock budget instead of up to 5s each - skill detection requires a baml-* directory containing SKILL.md, so leftover empty directories don't suppress the install prompt Co-authored-by: Cursor <[email protected]>
…uto-check The passive path inlined its own fetch-then-persist sequence; route it through the same helper the explicit toolchain commands use so there is exactly one fetch-and-cache path and one compare-and-warn path, with trigger policy (TTL-gated vs forced) as the only difference. Co-authored-by: Cursor <[email protected]>
…ad of before it Warnings from existing caches still print immediately (no network). When a refresh is due (at most once per TTL window), it now runs on a background thread while baml-cli executes; the wrapper spawns and waits instead of exec'ing on that rare path, joins the refresh with whatever remains of the 5s budget after the command exits, and prints only warnings the fresh caches newly justify (never duplicating one already shown). The common no-refresh case keeps the zero-overhead exec path. Co-authored-by: Cursor <[email protected]>
…Hub API baml agent install resolved the skill repo head via an unauthenticated GET to api.github.com, whose 60-requests/hour per-IP rate limit made installs hard-fail with HTTP 403 on shared-IP environments (CI, agent sandboxes) — with no fallback and no offline path. The REST call only existed to learn the head SHA before downloading the tarball from codeload.github.com, which is not API-rate-limited and needs no credentials. Drop it: download the main-branch tarball directly, and recover the exact commit for provenance from the pax global header (comment=<sha>) that git archive embeds in every GitHub tarball. Archives without the header (custom mirrors) install fine and just skip provenance, matching --from semantics. The download error now points at the --from escape hatch. The wrapper's passive freshness check still uses the REST API, but it is throttled to once per day and silent on failure, so a rate limit there can no longer block anything. Co-Authored-By: Claude Fable 5 <[email protected]>
… dir Without --dir, baml agent install picked its root via an unbounded ancestor walk that accepted any directory owning a bare baml_src/. A stray ~/baml_src made installs run from ~/some/project land the skills in $HOME instead of the project. Detection is now scoped to where a project can plausibly be: inside a git repo the walk never leaves the repo (nearest baml.toml or baml_src owner, else the repo root, where agent harnesses discover .claude/skills); outside one it stops before the home directory and falls back to the current directory. The decision lives in a pure detect_install_root_in so the boundaries are unit-testable without env or cwd mutation. Co-Authored-By: Claude Fable 5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
⏭️ 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
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe agent install flow derives default-install commit provenance from tarball pax headers, constrains automatic install-root detection, and expands unit and end-to-end coverage for archive metadata, routing, missing provenance, and installation failures. ChangesAgent install behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant codeload_archive
participant skills_from_archive
participant state_toml
CLI->>codeload_archive: request main tarball
codeload_archive->>skills_from_archive: return archive with optional pax comment
skills_from_archive->>CLI: return skills and optional commit
CLI->>state_toml: save installation provenance
Possibly related PRs
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 |
Co-Authored-By: Claude Fable 5 <[email protected]>
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: 2
🧹 Nitpick comments (1)
baml_language/crates/baml_cli/src/agent_command.rs (1)
154-158: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant field-by-field reconstruction.
loadedcan just be returned directly.♻️ Simplify
- let loaded = skills_from_archive(&archive)?; - Ok(LoadedSkills { - skills: loaded.skills, - commit: loaded.commit, - }) + skills_from_archive(&archive)🤖 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/agent_command.rs` around lines 154 - 158, Update the return logic after skills_from_archive to return loaded directly, removing the redundant LoadedSkills field-by-field reconstruction while preserving the existing error propagation.
🤖 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_cli/src/agent_command.rs`:
- Around line 269-274: Update the ancestor collection in the no-git-repository
branch so it stops at the current directory when home is None, while retaining
the existing home-boundary behavior when home is available. Ensure the fallback
cannot walk ancestors up to the filesystem root and preserves the documented
install-root scope.
- Around line 259-268: Update the git_toplevel filtering before the ancestors
computation to reject a toplevel equal to the user’s home directory, while
retaining the existing cwd containment check. Ensure a dotfiles worktree rooted
at $HOME cannot contribute ancestor candidates, but preserve normal project
toplevel handling.
---
Nitpick comments:
In `@baml_language/crates/baml_cli/src/agent_command.rs`:
- Around line 154-158: Update the return logic after skills_from_archive to
return loaded directly, removing the redundant LoadedSkills field-by-field
reconstruction while preserving the existing error propagation.
🪄 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: 2a81def1-ecba-4438-9283-74cf05bba445
📒 Files selected for processing (2)
baml_language/crates/baml_cli/src/agent_command.rsbaml_language/crates/baml_cli/tests/agent_install_e2e.rs
Two review findings on the bounded walk: a git toplevel rooted at (or containing) the home directory — dotfiles worktrees, home-as-git-repo — slipped past the ancestor filter and reopened the stray-~/baml_src hole, and with no determinable home directory the non-git walk was unbounded to the filesystem root. Reject home-containing toplevels, and without a home boundary don't walk above cwd at all: installing at cwd is always safe. Also satisfies clippy's unnecessary_to_owned in the test archive helper. Co-Authored-By: Claude Fable 5 <[email protected]>
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.
Problem
Two bugs in
baml agent install, both hit in the wild:Installs hard-fail on GitHub API rate limits. The default path resolved the
skill repo head via an unauthenticated
GET api.github.com/.../commits/main,which is capped at 60 requests/hour per IP. On shared-IP environments (CI, agent
sandboxes) this 403s and the install dies — no fallback, no offline path. One
agent run lost access to the official skills entirely after 6 failed attempts.
Skills install into the wrong directory. Root detection walked all
ancestors and accepted any directory owning a bare
baml_src/. A stray~/baml_srcmade an install run from~/nomad(no git, no baml.toml) silentlydrop the skills into
$HOMEinstead of the project.Fix
Drop the GitHub API from the install path entirely. The REST call only existed
to learn the head SHA before downloading the tarball from
codeload.github.com—which is not API-rate-limited and needs no credentials. We now download the
main-branch tarball directly and recover the exact commit for provenance from the
pax global header (
comment=<sha>) thatgit archiveembeds in every GitHubtarball. One unauthenticated request, no token ever required. Archives without the
header (custom mirrors) install fine and just skip provenance, matching
--fromsemantics. Download failures now point at the
--fromescape hatch. The wrapper'spassive freshness check still uses the REST API, but it's once-per-day and silent
on failure, so it can't block anything.
Bound root detection to where a project can plausibly be. Inside a git repo the
ancestor walk never leaves the repo (nearest
baml.toml/baml_srcowner, else therepo root, where agent harnesses discover
.claude/skills/). Outside a git repo itstops before
$HOMEand falls back to cwd. A git toplevel that isn't an ancestorof cwd (dotfiles
GIT_DIR/GIT_WORK_TREEpattern) is ignored. The decision livesin a pure
detect_install_root_in(cwd, git_toplevel, home)so every boundary isunit-tested without env/cwd mutation.
Testing
root-detection boundaries (git-scoped, home-bounded,
~/baml_srcregression,foreign-worktree regression, cwd == home).
any future reintroduction of a GitHub API call fails CI.
$HOME, no.claude, no git, no baml project, no tokens, emptyPATH(no git binary),HOME/BAML_HOMEunset — all succeed, skills land in cwd/repo root, recordedprovenance matches the repo head.
overflow-sized records): no panics or hangs; malformed input degrades to
"no provenance recorded", never a failed install.
Summary by CodeRabbit
--fromin error output).