Skip to content

feat(gates): gate-83 contract-surface-shift — declaring a magic method breaks every consumer - #472

Merged
rubenvdlinde merged 1 commit into
mainfrom
feat/gate-83-contract-surface-shift-v2
Aug 16, 2026
Merged

feat(gates): gate-83 contract-surface-shift — declaring a magic method breaks every consumer#472
rubenvdlinde merged 1 commit into
mainfrom
feat/gate-83-contract-surface-shift-v2

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What

Adds gate-83 contract-surface-shift: a diff-scoped gate that fails when a method on a published contract moves between the MAGIC surface (an @method docblock tag routed through __call()) and the DECLARED surface, unless the change is annotated.

Why

PHPUnit picks its mock builder on exactly that distinction:

addMethods()   throws CannotUseAddMethodsException  if the method EXISTS
onlyMethods()  throws CannotUseOnlyMethodsException if it does NOT

So every consumer that doubles the class has hard-coded an assumption about which surface each method sits on. Moving one breaks them with no commit in their repositories.

Measured, not hypothetical. On 2026-08-15 openregister#2498 published the ObjectService/ObjectEntity interfaces, which forced getUuid(), getRegister() and getSchema() to be declared:

repo fallout
opencatalogi 42 errors, all 6 PHPUnit matrix cells red
decidesk 1 failure

Both development branches went red on commits that had passed hours earlier, and the failures first surfaced on unrelated dependency PRs, where they read as those PRs' fault. Finding that out cost a full baseline-comparison pass across 19 repositories.

Why it lives on the producer side

It cannot be written in the consuming repo. The hydra-gates job checks out exactly two things — the app and the gates package — so a consumer-side checker has no access to the real class and would resolve every external double as "unknown". It would report ZERO findings for the one change that matters, which reads as a pass. The authority for what is declared lives with the class, so the check lives there too.

The escape hatch

@contract-shift <category> — <reason>

Categories are closed: announced, internal-only, new-contract. Same shape as gate-16's @spec exclude and gate-61's @listener-placement. A bare tag or an unknown category fails.

Verification

In a real runner run, against the commit that caused the outage:

[gate-83] contract-surface-shift: FAIL — 5 undeclared shift(s) ...
[hydra-gates] COVERAGE: 64 of 69 declared gates reported a result

It names all five shifted methods — the three that broke consumers plus getOwner/getOrganisation, which nobody happened to have doubled.

Negative control, a diff touching no contract file:

[gate-83] contract-surface-shift: NOT APPLICABLE — the diff against 'HEAD~1'
touched no published-contract file, so no surface shift could occur.

Named, not silent, and still counted in COVERAGE.

Suite (scripts/lib/test_check_contract_surface_shift.sh, auto-discovered by run-helper-suites.sh) — 9 assertions, all paired positive/negative: both shift directions, the annotation grammar, empty scope, not-applicable, unresolvable-base-fails-closed, and the terminal summary the runner greps for.

One of those assertions earned its place immediately

The bare-annotation case caught a live bug in the first draft. The separator was \s*, which matches newlines — so on a reason-less @contract-shift announced the match ran past the line end and captured the docblock's closing */ as the "reason". Non-empty, so the no-reason branch never fired and the escape hatch was silently open to any bare tag. Nothing else would have found it.

…d breaks every consumer

A method on a published contract is served either DECLARED or MAGIC (an
`@method` tag routed through __call()). PHPUnit picks its mock builder on
exactly that distinction — addMethods() refuses a method that exists,
onlyMethods() refuses one that does not — so moving a method between the two
surfaces breaks every consumer that doubles it, with no commit in their repos.

Measured, not hypothetical: openregister#2498 published the
ObjectService/ObjectEntity interfaces, forcing getUuid()/getRegister()/
getSchema() to be declared. opencatalogi went red on 42 errors across all 6
PHPUnit cells, decidesk on 1. Both development branches broke on commits that
had passed hours earlier, and it surfaced on unrelated dependency PRs where it
read as their fault.

The gate is on the PRODUCER side because it cannot work anywhere else: the
gates job checks out only the app and the gates package, so a consumer-side
checker cannot see the real class and would resolve every external double as
unknown — reporting zero for the one change that matters, which reads as a
pass.

Diff-scoped; fails unless the shift carries a reason-bearing
`@contract-shift <announced|internal-only|new-contract> — <reason>`. Declared
not-applicable when lib/Contract/ is absent, so its silence in the 18 consuming
repos is stated by name rather than being a coverage gap.

Recorded in UNCOVERED.md under needs-diff: the subject is a transition, so a
planted/clean directory pair cannot express it. Covered instead by a suite that
builds real git histories (9 assertions), and replayed against the commit that
caused the outage, where it names all five shifted methods.

Verified: acceptance matrix rc 0 (64 of 70 fixtured), package invariants rc 0,
errexit discipline rc 0, helper suite 9 of 9.
@rubenvdlinde
rubenvdlinde merged commit f6e56a3 into main Aug 16, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant