Repository navigation
Guard release tail from expected nightly skips - #4218
Conversation
|
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):
|
📝 WalkthroughWalkthroughThe release workflow now requires successful prerequisite jobs and non-cancellation status for downstream canary release jobs. Contract tests verify these conditions across publishing, smoke-test, channel-publishing, and nightly-dispatch stages. ChangesRelease workflow gating
Estimated code review effort: 2 (Simple) | ~10 minutes 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/tests/test_release_pipeline_contract.py (1)
913-973: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert guards in each job’s actual
if:expression.These checks search the entire job block, so they can pass if a guard is moved into a comment or unrelated step. They also will not catch a future direct
needsdependency added without a matching success check. Extract theif:expression and validate it against the declaredneedslist.🤖 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` around lines 913 - 973, Update test_expected_nightly_skips_do_not_poison_release_tail to extract each job’s actual if expression and declared needs list, then assert every declared dependency has a matching success result guard alongside !cancelled(). Remove broad whole-block substring checks so guards in comments or unrelated steps cannot satisfy the test.
🤖 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 `@scripts/tests/test_release_pipeline_contract.py`:
- Around line 913-973: Update
test_expected_nightly_skips_do_not_poison_release_tail to extract each job’s
actual if expression and declared needs list, then assert every declared
dependency has a matching success result guard alongside !cancelled(). Remove
broad whole-block substring checks so guards in comments or unrelated steps
cannot satisfy the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 795ec231-7f67-4e1e-ba22-49b4e788886e
📒 Files selected for processing (2)
.github/workflows/release-baml-language.ymlscripts/tests/test_release_pipeline_contract.py
Binary size checks passed✅ 7 passed
Generated by |
…ses (BoundaryML#5515) Nightly `0.23.1-nightly.20261103.a` can't pass `Pack BAML wrapper candidates`. BoundaryML#5422 made that job and `Wrapper tests` gate `all-builds`. - [Run 41798478057](https://github.com/BoundaryML/baml/actions/runs/41798478057): the BCS project and environment variables were unset. They are now set as GitHub environment variables; B-2011 tracks moving them to secrets. - [Run 41823460220](https://github.com/BoundaryML/baml/actions/runs/41823460220), on canary after BoundaryML#5491: `Cannot embed telemetry: API-key verification failed … requires the 200 JSON introspection response`. Production BCS still answers `POST /v1/auth/verify {"fullResponse":true}` with `204`. `bcs_api/README.md` notes that server support has to be deployed before embedding can work. The candidates aren't shipped yet, so this PR stops nightlies from requiring them: - `build2-baml-wrapper.reusable.yaml` and `wrapper-tests.reusable.yaml` take a new `enabled` input, which defaults to `true`, so `ci.yaml` is unchanged. `release-baml-language.yml` passes `enabled: channel != 'nightly'`. - The jobs still run on nightly, as no-ops; they are not skipped. A skipped ancestor of `all-builds` would also skip every publisher that uses the implicit `success()` (see BoundaryML#4218). The pack job's steps are all guarded, and in wrapper tests only the `tests` matrix is skipped, while `platforms` still runs. - The release fingerprint gate requires every fingerprinted artifact family, so `tools/release-fingerprints` `check` takes a new `allow_missing` family name. `all-builds` passes `--allow_missing baml-wrapper-candidates` on nightly. A candidates artifact that is present is still checked. - Canary releases still require all of it. Revert once production BCS supports key introspection and nightly packing passes. Validation: - `actionlint` passes on the three workflows. - `baml-cli test --from tools/release-fingerprints` passes 2/2, using this branch's toolchain. - Ran `check` against a local artifacts directory: without the flag it reports the missing candidates; with the flag that problem goes away; a present but empty candidates artifact still fails. - Nightly dry run [41810686735](https://github.com/BoundaryML/baml/actions/runs/41810686735), on the first commit: both wrapper jobs succeeded as no-ops and `All builds gate` ran instead of being skipped. It then failed only on the missing candidates artifact, which the second commit fixes. - Nightly dry run [41834585621](https://github.com/BoundaryML/baml/actions/runs/41834585621), rebased on BoundaryML#5491: pending. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Chores** * Nightly release workflows now skip BAML wrapper candidate builds and wrapper tests. These steps remain enabled for other release channels. * Reusable build and test workflows now support an optional enable/disable setting, enabled by default, so their corresponding steps can be skipped while the workflow remains runnable. * Nightly release checks allow the BAML wrapper candidate artifact to be absent. Other release channels still require it, and commit verification remains in place. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5.5 <[email protected]>
Summary
success()propagationRoot cause
Nightly releases intentionally skip the Gradle Plugin Portal publisher.
All pre-manifest publishers completecorrectly accepts that state and succeeds, but GitHub still propagates the skipped transitive ancestor through the implicitsuccess()added to downstream job conditions. This skippedPublish pkg.boundaryml.com manifests, which skipped Go publishing and made the final fan-in fail withGO: skipped.The guarded conditions use
!cancelled()to suppress the implicit status check while remaining fail-closed through explicit direct dependency-result checks.Impact
Nightly and canary releases can proceed from the policy-aware publisher fan-in through exact-version manifests, Go publish/smoke, and mutable channel promotion without an expected skipped publisher poisoning the tail of the graph. Failed or cancelled direct prerequisites still block publication.
Validation
mise exec -- actionlint .github/workflows/release-baml-language.ymlpython3 -m unittest scripts.tests.test_release_pipeline_contractyaml.safe_loadgit diff --checkSummary by CodeRabbit
Bug Fixes
Tests