Repository navigation
Make BAML agent skill installs reproducible - #3753
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds a deterministic agent-skills packager and integrates it into CI releases; refactors the CLI to load skills from multiple sources and normalize legacy layouts; updates tests; and updates the release workflow to build and publish agent-skills artifacts with updated job dependencies. ChangesAgent Skills Packaging and Release
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
⏭️ 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):
|
rossirpaulo
marked this pull request as ready for review
June 12, 2026 08:59
rossirpaulo
enabled auto-merge
June 12, 2026 08:59
|
No description provided. |
![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: 3
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_cli/src/agent_command.rs (1)
410-417:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
SKILL.mdfrontmatter parsing is LF-only.
split_frontmatter()only accepts---\n/\n---\n, so--fromon a Windows checkout or archive with CRLFSKILL.mdfiles fails with “missing opening frontmatter marker”. The new packaging script has the same delimiter assumption, so the local package/install path breaks end-to-end on CRLF inputs.Suggested fix
fn split_frontmatter(content: &str) -> Result<(&str, &str)> { let rest = content - .strip_prefix("---\n") + .strip_prefix("---\r\n") + .or_else(|| content.strip_prefix("---\n")) .ok_or_else(|| anyhow!("SKILL.md is missing opening frontmatter marker"))?; - let Some((frontmatter, body)) = rest.split_once("\n---\n") else { + let Some((frontmatter, body)) = rest + .split_once("\r\n---\r\n") + .or_else(|| rest.split_once("\n---\n")) + else { anyhow::bail!("SKILL.md is missing closing frontmatter marker"); }; Ok((frontmatter, body)) }🤖 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 410 - 417, The split_frontmatter function currently matches only LF delimiters and fails on CRLF files; update split_frontmatter to tolerate CRLF by normalizing input line endings (replace "\r\n" with "\n" at function start) or by checking both "\n" and "\r\n" variants for the opening ("---\n" / "---\r\n") and closing ("\n---\n" / "\r\n---\r\n") markers; ensure you still return the frontmatter and body as &str slices consistently (adjust logic in split_frontmatter to operate on the normalized string or to compute offsets so returned slices refer to the correct underlying data).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-baml-language.yml:
- Around line 288-291: The all-builds job/gate is missing the build-agent-skills
job, which allows publish-pypi, publish-wrapper-release, and publish-nodejs-sdk
(which depend solely on all-builds) to run even if build-agent-skills fails;
update the workflow so the all-builds gate/job includes build-agent-skills (add
build-agent-skills as a dependency/member of all-builds) so that publish-pypi,
publish-wrapper-release, and publish-nodejs-sdk cannot proceed unless
build-agent-skills completes successfully, keeping publish-toolchain-release
(which already depends on build-agent-skills) consistent with the rest of the
pipeline.
In `@scripts/package-agent-skills`:
- Around line 144-149: The tar creation is nondeterministic because gzip embeds
timestamps and tar preserves file metadata; update the archive step that uses
COPYFILE_DISABLE and the tar invocation to produce reproducible output by (1)
exporting GZIP=-n (to strip gzip timestamps), (2) adding deterministic tar flags
like --sort=name --mtime='1970-01-01' --numeric-owner --owner=0 --group=0 (and
--no-acls/--no-xattrs if available), and (3) keeping the same archive name path
used later for sha256 generation so the resulting "$archive" and
"$archive_name.sha256" are stable across builds.
- Around line 16-26: For the option parsing branches handling --source,
--version, and --out-dir (which set source_dir, version, out_dir and then call
shift 2), add a guard to validate that a value exists in "$2" before performing
shift 2; if "$2" is empty or missing, call usage() and exit 2. Locate the blocks
that currently do: "--source) source_dir=\"${2:-}\" shift 2 ;;", "--version)
version=\"${2:-}\" shift 2 ;;", and "--out-dir) out_dir=\"${2:-}\" shift 2 ;;"
and insert a simple check (e.g., test -z "${2:-}") to handle the missing-value
case before assigning and shifting.
---
Outside diff comments:
In `@baml_language/crates/baml_cli/src/agent_command.rs`:
- Around line 410-417: The split_frontmatter function currently matches only LF
delimiters and fails on CRLF files; update split_frontmatter to tolerate CRLF by
normalizing input line endings (replace "\r\n" with "\n" at function start) or
by checking both "\n" and "\r\n" variants for the opening ("---\n" / "---\r\n")
and closing ("\n---\n" / "\r\n---\r\n") markers; ensure you still return the
frontmatter and body as &str slices consistently (adjust logic in
split_frontmatter to operate on the normalized string or to compute offsets so
returned slices refer to the correct underlying data).
🪄 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: 18fc4f46-efe1-468d-a075-60fb8957361e
📒 Files selected for processing (3)
.github/workflows/release-baml-language.ymlbaml_language/crates/baml_cli/src/agent_command.rsscripts/package-agent-skills
rossirpaulo
enabled auto-merge
June 12, 2026 09:12
Contributor
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 (2)
scripts/package-agent-skills (1)
136-141:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDirect-layout skill discovery is depth-mismatched and misses valid
SKILL.mdfiles.At Line 141,
find "$source_dir/skills" -mindepth 2 -maxdepth 2 ... -path '*/baml-*/SKILL.md'cannot matchskills/*/baml-*/SKILL.mdbecause those files are depth 3 underskills. This will skip direct-layout skills and can incorrectly trigger the "no BAML agent skills found" failure.Suggested minimal fix
- done < <(find "$source_dir/skills" -mindepth 2 -maxdepth 2 -type f -path '*/baml-*/SKILL.md' -print0 | sort -z) + done < <(find "$source_dir/skills" -mindepth 3 -maxdepth 3 -type f -path '*/baml-*/SKILL.md' -print0 | sort -z)🤖 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/package-agent-skills` around lines 136 - 141, The find invocation that discovers SKILL.md uses -mindepth 2 -maxdepth 2 and -path '*/baml-*/SKILL.md', which misses direct-layout skills located at skills/*/baml-*/SKILL.md (depth 3). Update the find call used for skill discovery (the one piping into the while loop that reads skill_file and calls stage_skill) to include depth 3 results (e.g., change -mindepth/-maxdepth to 3 or remove the depth limits) or add an alternative -path clause to also match */*/baml-*/SKILL.md so stage_skill will be invoked for those SKILL.md files and count will reflect them..github/workflows/release-baml-language.yml (1)
297-301:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin
BAML_AGENT_SKILLS_REPOSITORYcheckout to an immutable ref.This checkout currently follows the repository default branch, so reruns for the same release version can package different skill contents. That undermines reproducibility for the versioned
baml-agent-skills-*.tar.gzartifact. Please use an immutable tag or commit SHA (ideally emitted by the release plan) viaref:.🔧 Suggested direction
- uses: actions/checkout@v4 with: repository: ${{ env.BAML_AGENT_SKILLS_REPOSITORY }} + ref: ${{ needs.plan.outputs.agent_skills_ref }} # immutable tag or commit SHA path: agent-skills-source persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-baml-language.yml around lines 297 - 301, The checkout step using actions/checkout@v4 for repository ${{ env.BAML_AGENT_SKILLS_REPOSITORY }} is not pinned to an immutable ref, causing non-reproducible artifact contents; update the checkout step (the actions/checkout@v4 invocation that sets path: agent-skills-source) to include a ref: that points to an immutable tag or commit SHA emitted by the release plan (e.g. use a new env like BAML_AGENT_SKILLS_REF or the release SHA) so the workflow always checks out the exact revision used to build baml-agent-skills-*.tar.gz.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/release-baml-language.yml:
- Around line 297-301: The checkout step using actions/checkout@v4 for
repository ${{ env.BAML_AGENT_SKILLS_REPOSITORY }} is not pinned to an immutable
ref, causing non-reproducible artifact contents; update the checkout step (the
actions/checkout@v4 invocation that sets path: agent-skills-source) to include a
ref: that points to an immutable tag or commit SHA emitted by the release plan
(e.g. use a new env like BAML_AGENT_SKILLS_REF or the release SHA) so the
workflow always checks out the exact revision used to build
baml-agent-skills-*.tar.gz.
In `@scripts/package-agent-skills`:
- Around line 136-141: The find invocation that discovers SKILL.md uses
-mindepth 2 -maxdepth 2 and -path '*/baml-*/SKILL.md', which misses
direct-layout skills located at skills/*/baml-*/SKILL.md (depth 3). Update the
find call used for skill discovery (the one piping into the while loop that
reads skill_file and calls stage_skill) to include depth 3 results (e.g., change
-mindepth/-maxdepth to 3 or remove the depth limits) or add an alternative -path
clause to also match */*/baml-*/SKILL.md so stage_skill will be invoked for
those SKILL.md files and count will reflect them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 71f46635-ce1a-4d87-b02f-890b9fda0bac
📒 Files selected for processing (3)
.github/workflows/release-baml-language.ymlbaml_language/crates/baml_cli/src/agent_command.rsscripts/package-agent-skills
This was referenced Jun 17, 2026
This was referenced Jul 10, 2026
feat(toolchain): outdated warnings for toolchain + agent skills, install skills from repo head
#3970
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
Make
baml agent installreproducible by default by installing agent skills from the matching BAML language release asset instead of the mutableBoundaryML/baml-skillmain branch.Changes
--latestand--frominstall escape hatches for mutable upstream, archive URLs, local archives, and local directories.skills/baml-*/SKILL.mdentries from release archives, with legacyplugins/baml/skills/*/SKILL.mdnormalization..sha256sidecar before installing.scripts/package-agent-skillsto build normalized release artifacts and checksums.Validation
cargo test -p baml_cli agent_command -- --nocapturecargo build -p baml_cli --bin baml-clishellcheck scripts/package-agent-skillsactionlint .github/workflows/release-baml-language.ymlBoundaryML/baml-skillmain usingscripts/package-agent-skillsandbaml-cli agent install --fromSummary by CodeRabbit
New Features
Chores
Tests