Repository navigation
Fix Windows bridge export containment - #4176
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe release pipeline now maintains a canonical CFFI export allowlist, validates Windows DLL exports against it, disables AWS-LC jitter entropy exports, uses the allowlist during packaging, and tests these release-contract requirements. ChangesCFFI export hygiene
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant WindowsBuild
participant Dumpbin
participant Allowlist
participant SmokeTest
WindowsBuild->>Dumpbin: Enumerate bridge_cffi.dll exports
Dumpbin->>Allowlist: Compare sorted export names
Allowlist-->>WindowsBuild: Pass or report differences
WindowsBuild->>SmokeTest: Run ABI smoke test
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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):
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 381-398: Update the export comparison in the PowerShell block
using $actualExports, $expectedExports, and $exportDifference so both
Sort-Object calls and Compare-Object use case-sensitive comparison. Preserve the
existing export extraction, deduplication, reporting, and failure behavior.
🪄 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: 9c78a8d6-2429-4a24-93f9-cd1c67d776ad
📒 Files selected for processing (4)
.github/workflows/build2-bridge-cffi.reusable.yamlbaml_language/sdks/csharp/bridge_csharp/tools/pack-product.shrelease/bridge-cffi-public-exports.txtscripts/tests/test_release_pipeline_contract.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/tests/test_release_pipeline_contract.py (1)
640-656: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBind hygiene assertions to the actual Windows smoke step.
The global
assertIn/indexchecks can be satisfied by comments or unrelated workflow steps, allowing a future regression in the real export-validation step to pass contract tests. Scope these checks to the named Windows smoke step and assert the exact opt-out write, export comparison, and ordering there.🤖 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 640 - 656, Update the contract assertions in the test covering the Windows smoke step so export-validation checks are extracted from and bound to the named step’s script body, not the entire builder text. Within that scoped body, assert the exact opt-out write, dumpbin/export-list handling, Compare-Object validation, and ordering before artifact upload; keep the public export-file content checks unchanged.
🤖 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 640-656: Update the contract assertions in the test covering the
Windows smoke step so export-validation checks are extracted from and bound to
the named step’s script body, not the entire builder text. Within that scoped
body, assert the exact opt-out write, dumpbin/export-list handling,
Compare-Object validation, and ordering before artifact upload; keep the public
export-file content checks unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 624275ff-3345-4f8d-bdcb-c4409b0dbe94
📒 Files selected for processing (2)
.github/workflows/build2-bridge-cffi.reusable.yamlscripts/tests/test_release_pipeline_contract.py
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/build2-bridge-cffi.reusable.yaml
Binary size checks passed✅ 7 passed
Generated by |
What changed
AWS_LC_SYS_NO_JITTER_ENTROPY=1controlbridge_cffi.dllexport table in the producer workflow before artifact uploadrelease/so the producer and C# package verifier share one ABI contractRoot cause
The bundled jitter-entropy library declares nine internal entry points with
__declspec(dllexport)under MSVC. Because AWS-LC is linked statically intobridge_cffi.dll, those linker directives widened the DLL's public ABI. Path hygiene was already fixed; the nightly release then correctly exposed this latent export violation during C# product assembly.Impact
Windows retains AWS-LC as its crypto backend and continues using Windows OS entropy. Only the optional CPU-jitter source is excluded. Any future dependency export leakage now fails in the real Windows producer matrix before upload rather than during the production release.
Validation
python3 -m unittest scripts.tests.test_release_pipeline_contractmise exec actionlint -- actionlint .github/workflows/build2-bridge-cffi.reusable.yamlshellcheck baml_language/sdks/csharp/bridge_csharp/tools/pack-product.shbash -n baml_language/sdks/csharp/bridge_csharp/tools/pack-product.shscripts/baml-language-version checkThe decisive runtime validation is the PR's real x64 and ARM64 Windows release build plus final-DLL export inspection.
Summary by CodeRabbit
New Features
register_unhandled_spawn_error_callbackandshutdown_runtime.Bug Fixes
Tests