Repository navigation
CFFI dylib release - #4037
CFFI dylib release#4037
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a reusable matrix workflow for building and validating ChangesCFFI release flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant BuildWorkflow
participant Cargo
participant GitHubRelease
participant ManifestGenerator
ReleaseWorkflow->>BuildWorkflow: Invoke matrix CFFI build
BuildWorkflow->>Cargo: Build bridge_cffi for target
Cargo-->>BuildWorkflow: Return cdylib
BuildWorkflow->>ReleaseWorkflow: Upload library and checksum artifact
ReleaseWorkflow->>GitHubRelease: Publish or verify release assets
ReleaseWorkflow->>ManifestGenerator: Generate manifest with CFFI directory
ManifestGenerator-->>ReleaseWorkflow: Return CFFI URLs and SHA256 values
Possibly related PRs
Suggested reviewers: 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 |
⏭️ 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):
|
Binary size checks passed✅ 7 passed
Generated by |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/build2-bridge-cffi.reusable.yaml (1)
212-224: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueAvoid direct
${{ }}step-output interpolation inrun:(zizmor: template-injection).
steps.artifact.outputs.dir/.assetare expanded directly into the shell command. They're currently derived only from the hardcoded matrixtargetliteral, so this isn't exploitable today, but it's the pattern static analysis flags as risky if these outputs ever become less trusted. Pass them throughenv:instead so they're shell-quoted as variables rather than expanded into the script text.🛡️ Proposed fix
- name: Smoke test the built library if: matrix._.smoke working-directory: baml_language + env: + ARTIFACT_DIR: ${{ steps.artifact.outputs.dir }} + ARTIFACT_ASSET: ${{ steps.artifact.outputs.asset }} run: | set -euo pipefail expected="$(jq -r .canonical_version ../release-plan.json)" python3 ../scripts/smoke-bridge-cffi.py \ - "${{ steps.artifact.outputs.dir }}/${{ steps.artifact.outputs.asset }}" \ + "${ARTIFACT_DIR}/${ARTIFACT_ASSET}" \ "$expected"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build2-bridge-cffi.reusable.yaml around lines 212 - 224, Update the “Smoke test the built library” step to pass steps.artifact.outputs.dir and steps.artifact.outputs.asset through the step’s env configuration, then reference those environment variables in the python3 command instead of direct GitHub expression interpolation inside run:. Preserve the existing smoke-test arguments and behavior.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In @.github/workflows/build2-bridge-cffi.reusable.yaml:
- Around line 212-224: Update the “Smoke test the built library” step to pass
steps.artifact.outputs.dir and steps.artifact.outputs.asset through the step’s
env configuration, then reference those environment variables in the python3
command instead of direct GitHub expression interpolation inside run:. Preserve
the existing smoke-test arguments and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 405345e9-28b7-4a67-a3f8-18e01f4748a9
📒 Files selected for processing (6)
.github/workflows/build2-bridge-cffi.reusable.yaml.github/workflows/release-baml-language.ymlbaml_language/Cargo.tomlbaml_language/crates/baml_release/src/manifest.rsscripts/baml-release-manifestsscripts/smoke-bridge-cffi.py
Summary by CodeRabbit
New Features
Bug Fixes
Tests