The binary-compatibility gate can now block a merge by rbuergi · Pull Request #2407 · Systemorph/MeshWeaver

rbuergi · 2026-08-26T17:16:21Z

It could not block anything

record-signatures — the job carrying both binary-compatibility checks (public record primary constructors, and public types moving assembly without a TypeForwardedTo) — was absent from collect-results' needs. And Consolidate test results is the repo's only required status check.

required check:  Consolidate test results        (= the collect-results job)
its needs:       [precheck, preflight, build, test, plugin-gate,
                  landed-modules-gate, doc-gate, licence-gate, clients-gate]
the gates live in:  record-signatures            (dotnet-test.yml:212)

So the gate ran, failed, and nothing read the verdict.

Why this is worse than the trapdoor already documented

AGENTS.md guards five times over against a gate that renders a passing tick without running. This one genuinely runs and genuinely fails — and from the merge button's point of view those are indistinguishable. A red that stops nothing is decorative.

The cost is already paid: #2283 moved four public types out of MeshWeaver.AI and shipped a TypeLoadException to production on every MCP tool call (#2370). A module's IL binds a NAME scoped to an AssemblyRef, so no source build in either repo can see it — and the verification that was run before merging (building the plugin's source against the branch) answers source compatibility, which is a different question. The gate for exactly that class now exists and is proven live in CI. As wired, it would have gone red on #2283 and #2283 would have merged regardless.

The change

Two edits, in the shape this file already prescribes for every other gate:

record-signatures added to collect-results' needs.
An explicit fail step — because needs: alone is not enough under if: always(): a skipped dependency does not fail an always() job, which would re-open the trapdoor one level up. That lesson is already written beside plugin-gate, doc-gate, licence-gate and clients-gate; this makes the fifth consistent with them.

The failure message names the remedy and its limits, because the wrong fix here is tempting:

leave [assembly: TypeForwardedTo] in the original assembly — one type identity; a shim mints a second and reintroduces the is/as trap-door;
a forwarder cannot rename, so a moved type keeps its original name;
do not satisfy it by rebuilding the consumer — the point is that modules already shipped bind the old name;
where a forwarder is impossible because it would need a reference cycle, the move must be reverted or the set republished atomically (7 public types that left GitSync/Hosting for MeshWeaver.AI CANNOT be forwarded — move back, or republish atomically #2398 tracks two such groups).

What this will cost

Honestly: any PR that legitimately breaks a public surface now needs the forwarder in the same change. That is the intent, and it is friction. The alternative is the status quo, in which we find out from production.

Verification

dotnet-test.yml parses; WhatsNewEntryIntegrityTest + DocumentationLinkIntegrityTest 2/2, built first (the docs ship as an <EmbeddedResource>, so --no-build would have tested stale text).

🚨 This PR cannot fully demonstrate itself: its own run exercises the wiring, not a real binary break. The gate's ability to detect one is already proven — replaying #2283 names exactly the four types, with 20 self-tests running before the gate so a broken matcher fails loudly.

Refs #2398.

🤖 Generated with Claude Code

It could not. `record-signatures` — which carries BOTH binary-compat checks (public record primary constructors, and public types moving assembly without a TypeForwardedTo) — was absent from `collect-results`' needs, and "Consolidate test results" is the repo's ONLY required status check. So the gate ran, failed, and nothing read its verdict. That is a variant of the skip-trapdoor this file already guards against five times over, with a twist: the documented case is a gate that renders a passing tick WITHOUT running. This one genuinely runs and genuinely fails — and from the merge button's point of view those are the same thing. A red that stops nothing is decorative. The cost is already paid. #2283 moved four public types out of MeshWeaver.AI and shipped a TypeLoadException to production on every MCP tool call (#2370): a module's IL binds a NAME scoped to an AssemblyRef, so no source build in either repo can see the break — and the verification that WAS run (building the plugin's source against the branch) answers source compatibility, not this. The gate for exactly that class now exists, is proven live in CI, and as wired would have gone red on #2283 while #2283 merged anyway. Two changes, matching the shape this file already prescribes: * record-signatures added to `needs`; * an explicit fail step, because `needs:` alone is NOT enough under `if: always()` — a skipped dependency does not fail an always() job, which would re-open the trapdoor one level up. The failure message names the remedy and its limits: a forwarder left in the ORIGINAL assembly (one type identity — a shim would mint a second and reintroduce the is/as trap-door), that a forwarder cannot rename, and that where a forwarder is impossible because it would need a reference cycle the move must be reverted or the set republished atomically (#2398). Verified: dotnet-test.yml parses; WhatsNewEntryIntegrityTest and DocumentationLinkIntegrityTest 2/2 (built first — docs are an embedded resource). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR makes the “Public surface (binary compatibility)” CI gate able to actually block merges by wiring the record-signatures job into the required Consolidate test results job and failing that required job when the gate fails.

Changes:

Add record-signatures to collect-results.needs so the binary-compatibility gate is part of the required merge check.
Add an explicit “fail” step in collect-results to turn a binary-compatibility gate failure into a failing required status check.
Add a “What’s New” entry documenting that module-breaking binary changes now block merges.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-26-a-binary-break-can-no-longer-merge.md`	Adds a changelog entry explaining the new merge-blocking behavior for binary-break detection.
`.github/workflows/dotnet-test.yml`	Connects `record-signatures` into the required `collect-results` job and adds an explicit failure step for the gate.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    - name: Fail if the binary-compatibility gate failed
+      if: needs.record-signatures.result == 'failure'
+      run: |


github-actions · 2026-08-26T17:27:25Z