Repository navigation
Fix release pipeline contract and integrate Swift fan-in - #4158
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 Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis change adds contract-driven C# SDK packaging and verification, registry-specific release metadata, package digest recording, expanded release workflow orchestration, stricter Swift checks, reproducible native build flags, and CI coverage for release-plan and NuGet invariants. ChangesRelease contracts and version identities
Workflow execution and publishing
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ReleasePlan
participant PrepareCSharp
participant VerifyCSharp
participant NuGet
ReleasePlan->>PrepareCSharp: provide frozen plan and source SHA
PrepareCSharp->>VerifyCSharp: upload neutral preparation artifact
VerifyCSharp->>NuGet: stage and verify native package
NuGet-->>VerifyCSharp: return existing-package comparison
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/sdks/csharp/bridge_csharp/tools/Baml.NuGetNormalizer/Program.cs (1)
602-633: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse length-aware reads to avoid a false "not equal" on partial reads.
Stream.Readmay return fewer bytes than requested, so two byte-identical (equal-length) files can produce differingleftRead/rightReadat different chunk boundaries and returnfalse. On the compare path that surfaces as a spurious "different product-owned content" failure, blocking an otherwise-valid republish. PreferReadExactly/ReadAtLeast(available on net10.0) or fully fill each buffer before comparing.🤖 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/sdks/csharp/bridge_csharp/tools/Baml.NuGetNormalizer/Program.cs` around lines 602 - 633, Update FilesEqual to use length-aware reads, such as ReadExactly or ReadAtLeast, so each comparison processes complete chunks until the remaining file length is exhausted. Preserve the existing length check, byte-by-byte comparison, and equal-file result while preventing differing partial-read boundaries from causing false mismatches.
🤖 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
`@baml_language/sdks/csharp/bridge_csharp/tools/Baml.NuGetNormalizer/Program.cs`:
- Around line 602-633: Update FilesEqual to use length-aware reads, such as
ReadExactly or ReadAtLeast, so each comparison processes complete chunks until
the remaining file length is exhausted. Preserve the existing length check,
byte-by-byte comparison, and equal-file result while preventing differing
partial-read boundaries from causing false mismatches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 617330db-4127-4964-b43d-040d33c1a5f9
📒 Files selected for processing (27)
.github/workflows/build2-bridge-cffi.reusable.yaml.github/workflows/build2-java-sdk.reusable.yaml.github/workflows/cargo-tests.reusable.yaml.github/workflows/ci.yaml.github/workflows/prepare-csharp-sdk.reusable.yaml.github/workflows/publish2-csharp-sdk.yaml.github/workflows/release-baml-language.yml.github/workflows/verify-csharp-product-slice.reusable.yamlbaml_language/Cross.tomlbaml_language/crates/baml_release/src/manifest.rsbaml_language/crates/baml_release/src/platforms.rsbaml_language/crates/baml_version/src/lib.rsbaml_language/sdks/csharp/DEVELOPMENT.mdbaml_language/sdks/csharp/bridge_csharp/src/Baml.Bridge.csprojbaml_language/sdks/csharp/bridge_csharp/tools/Baml.NuGetNormalizer/Program.csbaml_language/sdks/csharp/bridge_csharp/tools/Baml.NuGetNormalizer/README.mdbaml_language/sdks/csharp/bridge_csharp/tools/pack-product.shrelease/csharp-package-size-policy.jsonrelease/platforms.jsonscripts/assemble-swift-sdk-mirrorscripts/baml-csharp-release-contractscripts/baml-language-versionscripts/baml-release-manifestsscripts/tests/test_baml_language_version.pyscripts/tests/test_baml_release_manifests.pyscripts/tests/test_release_pipeline_contract.pyscripts/tests/test_swift_release_contract.py
rossirpaulo
force-pushed
the
paulo/release-pipeline-fix
branch
from
July 23, 2026 21:52
5f36b27 to
3892d56
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/release-baml-language.yml (2)
1597-1650: 🔒 Security & Privacy | 🟠 MajorBind release-plan values to env vars before interpolating into the manifest-generation script.
Static analysis (zizmor) flags Lines 1633, 1634, 1636, 1647, 1648, 1649 as template-injection risk:
${{ needs.plan.outputs.* }}expressions are expanded directly into therun:shell body instead of being passed through anenv:block first, unlike the pattern used elsewhere in this same file (e.g.EXPECTED_VERSION: ${{ needs.plan.outputs.version }}inverify-swift-sdk). Direct expansion means any value containing shell metacharacters gets spliced verbatim into the script before bash ever runs, which is the classic GitHub Actions script-injection vector — even though these particular values currently originate from the internally computed release plan, this pattern degrades the workflow's injection-safety guarantees. Same issue recurs indry-run-artifacts(Lines 1976-2018). See consolidated comment for both sites.🤖 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 1597 - 1650, Update the release-manifest generation step to pass every needs.plan.outputs value used by the shell script—crates_io_version, nuget_version, swiftpm_version, channel, version, released_at, pypi_version, and wrapper_version—through the step’s env block, then reference those environment variables inside the run script instead of directly interpolating expressions. Apply the same env-binding pattern to the dry-run-artifacts manifest-generation step.
1939-2018: 🔒 Security & Privacy | 🟠 MajorSame template-injection pattern as the real manifest job.
Lines 1980, 2001, 2002, 2004, 2015 mirror the direct
${{ needs.plan.outputs.* }}expansion flagged inpublish-pkg-boundaryml-com(Lines 1597-1650). See consolidated comment.🤖 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 1939 - 2018, Update the dry-run-artifacts job’s “Generate dry-run files” step to avoid direct `${{ needs.plan.outputs.* }}` interpolation in the shell script, matching the safe handling used by publish-pkg-boundaryml-com. Pass the referenced plan outputs through environment variables or another non-template-injection mechanism, then use those values for the arguments to scripts/baml-release-manifests, including the version, channel, release timestamp, package versions, and wrapper-change flag.
🧹 Nitpick comments (3)
scripts/tests/test_release_pipeline_contract.py (1)
134-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
typing.Callableinstead of thecallablebuiltin in the annotation.
callableis a builtin function, not a type; type checkers reject it in an annotation. It's harmless at runtime here (local annotation isn't evaluated), so this is purely for type-check cleanliness.♻️ Proposed change
- cases: list[tuple[str, callable]] = [ + cases: list[tuple[str, Callable[[dict], object]]] = [Add the import near the top:
+from typing import Callable🤖 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/tests/test_release_pipeline_contract.py` at line 134, Update the type annotation for cases to use typing.Callable rather than the callable builtin, and add the corresponding Callable import at the top of the module..github/workflows/release-baml-language.yml (2)
563-606: 📐 Maintainability & Code Quality | 🔵 TrivialDuplicated Swift native-version verification logic.
This
main.swift+ build/codesign/run block is byte-for-byte duplicated insmoke-swift-release(Lines 1759-1773). Both must independently stay correct for the release-version guarantee to hold; consider extracting the verifier program and shell steps into a shared script/composite action consumed by both jobs. See consolidated comment.🤖 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 563 - 606, Extract the duplicated Swift native-version verifier and its build, codesign, and execution steps into one shared script or composite action, then invoke it from both this release verification block and the smoke-swift-release job. Preserve each job’s existing inputs, including EXPECTED_VERSION/BAML_EXPECTED_VERSION, and keep both jobs independently validating the planned version.
1731-1774: 📐 Maintainability & Code Quality | 🔵 TrivialSame native-version verification duplicated from
verify-swift-sdk.Env wiring at Lines 1742-1743 is correct, but the
main.swifttemplate and build/run steps at Lines 1759-1773 duplicateverify-swift-sdk(Lines 563-606). See consolidated comment.🤖 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 1731 - 1774, Remove the duplicated native-version verification and build/run logic from smoke-swift-release, reusing the existing verify-swift-sdk mechanism instead. Update the smoke job’s dependencies and invocation so it consumes the published package while preserving the planned-versus-native version check currently implemented by verify-swift-sdk.
🤖 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/build2-bridge-cffi.reusable.yaml:
- Around line 175-211: Update the RUSTFLAGS setup in the workflow to preserve
the wasm32 target configuration that is clobbered by the exported flags. Add the
target-scoped --cfg getrandom_backend="wasm_js" replacement when TARGET is
wasm32-unknown-unknown, alongside the existing Windows and musl-specific
handling, without changing other target behavior.
---
Outside diff comments:
In @.github/workflows/release-baml-language.yml:
- Around line 1597-1650: Update the release-manifest generation step to pass
every needs.plan.outputs value used by the shell script—crates_io_version,
nuget_version, swiftpm_version, channel, version, released_at, pypi_version, and
wrapper_version—through the step’s env block, then reference those environment
variables inside the run script instead of directly interpolating expressions.
Apply the same env-binding pattern to the dry-run-artifacts manifest-generation
step.
- Around line 1939-2018: Update the dry-run-artifacts job’s “Generate dry-run
files” step to avoid direct `${{ needs.plan.outputs.* }}` interpolation in the
shell script, matching the safe handling used by publish-pkg-boundaryml-com.
Pass the referenced plan outputs through environment variables or another
non-template-injection mechanism, then use those values for the arguments to
scripts/baml-release-manifests, including the version, channel, release
timestamp, package versions, and wrapper-change flag.
---
Nitpick comments:
In @.github/workflows/release-baml-language.yml:
- Around line 563-606: Extract the duplicated Swift native-version verifier and
its build, codesign, and execution steps into one shared script or composite
action, then invoke it from both this release verification block and the
smoke-swift-release job. Preserve each job’s existing inputs, including
EXPECTED_VERSION/BAML_EXPECTED_VERSION, and keep both jobs independently
validating the planned version.
- Around line 1731-1774: Remove the duplicated native-version verification and
build/run logic from smoke-swift-release, reusing the existing verify-swift-sdk
mechanism instead. Update the smoke job’s dependencies and invocation so it
consumes the published package while preserving the planned-versus-native
version check currently implemented by verify-swift-sdk.
In `@scripts/tests/test_release_pipeline_contract.py`:
- Line 134: Update the type annotation for cases to use typing.Callable rather
than the callable builtin, and add the corresponding Callable import at the top
of the module.
🪄 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: d9b0302c-1049-4dde-b726-410177c70298
📒 Files selected for processing (27)
.github/workflows/build2-bridge-cffi.reusable.yaml.github/workflows/build2-java-sdk.reusable.yaml.github/workflows/cargo-tests.reusable.yaml.github/workflows/ci.yaml.github/workflows/prepare-csharp-sdk.reusable.yaml.github/workflows/publish2-csharp-sdk.yaml.github/workflows/release-baml-language.yml.github/workflows/verify-csharp-product-slice.reusable.yamlbaml_language/Cross.tomlbaml_language/crates/baml_release/src/manifest.rsbaml_language/crates/baml_release/src/platforms.rsbaml_language/crates/baml_version/src/lib.rsbaml_language/sdks/csharp/DEVELOPMENT.mdbaml_language/sdks/csharp/bridge_csharp/src/Baml.Bridge.csprojbaml_language/sdks/csharp/bridge_csharp/tools/Baml.NuGetNormalizer/Program.csbaml_language/sdks/csharp/bridge_csharp/tools/Baml.NuGetNormalizer/README.mdbaml_language/sdks/csharp/bridge_csharp/tools/pack-product.shrelease/csharp-package-size-policy.jsonrelease/platforms.jsonscripts/assemble-swift-sdk-mirrorscripts/baml-csharp-release-contractscripts/baml-language-versionscripts/baml-release-manifestsscripts/tests/test_baml_language_version.pyscripts/tests/test_baml_release_manifests.pyscripts/tests/test_release_pipeline_contract.pyscripts/tests/test_swift_release_contract.py
🚧 Files skipped from review as they are similar to previous changes (17)
- baml_language/sdks/csharp/bridge_csharp/tools/Baml.NuGetNormalizer/README.md
- release/csharp-package-size-policy.json
- .github/workflows/build2-java-sdk.reusable.yaml
- baml_language/sdks/csharp/bridge_csharp/src/Baml.Bridge.csproj
- .github/workflows/publish2-csharp-sdk.yaml
- .github/workflows/ci.yaml
- scripts/baml-release-manifests
- scripts/assemble-swift-sdk-mirror
- .github/workflows/cargo-tests.reusable.yaml
- .github/workflows/prepare-csharp-sdk.reusable.yaml
- scripts/baml-csharp-release-contract
- baml_language/crates/baml_release/src/platforms.rs
- release/platforms.json
- .github/workflows/verify-csharp-product-slice.reusable.yaml
- baml_language/sdks/csharp/bridge_csharp/tools/Baml.NuGetNormalizer/Program.cs
- baml_language/sdks/csharp/bridge_csharp/tools/pack-product.sh
- scripts/baml-language-version
This comment has been minimized.
This comment has been minimized.
meefs
pushed a commit
to meefs/baml
that referenced
this pull request
Jul 24, 2026
…aryML#4169) ## Summary - replace the stale 11-file C# generator assertion with a preparation-time SHA-256 manifest that binds the complete generated source tree through verification and publication - retain essential generated entry-point checks and exact copy verification in the clean package consumer - attach the reusable NuGet publisher to the existing `boundary-tools-prod` environment so Infisical-hydrated `NUGET_USER` is resolved where `NuGet/login` runs - align the documented trusted-publishing identity with `publish2-csharp-sdk.yaml` and remove the invalid caller-to-reusable-workflow secret forwarding ## Why The release integration itself was green after BoundaryML#4158, but the production release failed during C# preparation because Dhilan's Swift/release work also expanded the generated C# stdlib surface from 11 to 35 legitimate `.g.cs` files. The hard-coded filename snapshot was not a sound product invariant. This keeps Dhilan's generator output and changes the release contract to pin what preparation actually produced, including paths and hashes, so later additions, removals, or content changes still fail closed. The NuGet trusted-publisher policy is configured for the reusable workflow and existing `boundary-tools-prod` environment. GitHub environment secrets are resolved in the called job; they are not passed from the caller. ## Validation - regenerated `sdk_tests/crates/csharp/primitive_slice` with the current CLI - wrote and verified a manifest for all 35 generated `.g.cs` files - `python3 -m unittest scripts.tests.test_release_pipeline_contract` (9 tests) - Python compile checks - workflow YAML parse checks - `bash -n` on the package consumer - `git diff --check` - repository pre-commit hooks, including YAML validation ## Safety No compiler/runtime semantics, stdlib implementation, platform support, or shared CFFI artifact format is changed. This PR does not trigger a production release. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Added SHA-256–based integrity validation for generated C# client sources via a generated `generated-sources.sha256` manifest. * Workflow verification now produces and checks the manifest for the generated sources directory, including carrying it into product outputs. * NuGet publishing workflow updates to rely on the reusable workflow’s own trusted-publishing secret configuration. * **Bug Fixes** * Replaced brittle hardcoded `*.g.cs` filename list comparisons with manifest-driven validation (missing/unexpected/modified detection). * **Tests** * Expanded contract and pipeline tests to cover manifest “growth,” exact pinned content, missing/extra inputs, and invalid manifest data. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This was referenced Jul 24, 2026
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
BamlRuntime.nativeVersion()so the native binary cannot be dead-stripped, and make the mirror assembler refuse every pre-existing output path instead of treating a forgeableSOURCE_SHAmarker as recursive-delete authoritySwift review
Dhilan's merged PR #4149 chose the right overall direction: one XCFramework build, verification before publication, an immutable GitHub release asset, and a passive
baml-swiftmirror consumed from a clean project. I kept that architecture.It did not yet fully adhere to the shared release plan: the Swift publisher was outside the final publisher fan-in, Swift was absent from the frozen registry plan and manifests, nightly dispatch did not wait for the Swift smoke, and both Swift checks only imported/printed without calling native code. Its normal CI passed, but the release-dispatch job was skipped, so those edges were not exercised end to end. This PR closes those correctness gaps. Explicit Swift package-size history and broader generated/simulator coverage should remain measured follow-ups rather than an invented policy in this change.
Validation
baml_releaseRust tests passactionlintand YAML parsingcargo-hawk, which was skipped after its environment failed becausegofmtis unavailable; cargo fmt, clippy, and YAML hooks pass independentlyRelease readiness
No production release was triggered. NuGet package ownership/trusted-publisher configuration still needs an administrative confirmation before declaring the production publisher ready.
Summary by CodeRabbit
released_at, plus verified package digests in generated manifests.