feat(gates): gate-83 contract-surface-shift — declaring a magic method breaks every consumer - #467
Closed
rubenvdlinde wants to merge 4 commits into
Closed
feat(gates): gate-83 contract-surface-shift — declaring a magic method breaks every consumer#467rubenvdlinde wants to merge 4 commits into
rubenvdlinde wants to merge 4 commits into
Conversation
…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. The authority lives with the class, so the check does too. Diff-scoped; fails unless the shift carries a reason-bearing `@contract-shift <announced|internal-only|new-contract> — <reason>`. Verified in a real runner run against the commit that caused the outage: FAIL naming all five shifted methods, counted in COVERAGE (69 declared). Negative control on a diff touching no contract file: NOT APPLICABLE, named rather than silent. The 9-assertion suite is auto-discovered by run-helper-suites.sh; its bare-annotation case caught a live bug in the first draft, where a \s* separator crossed the newline and captured the docblock's closing */ as the reason, leaving the escape hatch open to any reason-less tag.
…fix SC2015 Same two defects the gate-84 branch hit, and both are real here too. Gate-83's `if [ -d lib/Contract ]` guard made it emit NOTHING in every repo that consumes a contract rather than publishing one — 18 of the 19 — and a silence is indistinguishable from a pass. That is the exact failure --require-full-coverage exists to catch, and the package's own all-not-applicable fixture catches it. ShellCheck SC2015: `A && ok || no` is not if-then-else; the failure branch also runs when the check passes and the reporter fails, so one assertion could print both PASS and FAIL. Verified: tests/test-hydra-gates-bin.sh rc 0, and the gate-83 suite is still 9 of 9.
…rrexit The acceptance ratchet refuses a gate that is declared by the runner and covered by neither a fixture bundle nor a reasoned row. gate-83 cannot have a bundle: its subject is a TRANSITION between the magic and declared surfaces, so it is meaningless without two commits, and a planted/clean pair is two directory states with no history between them. Both arms would report EMPTY SCOPE and the planted assertion would grade a scope fault as a gate defect. Records the reason rather than weakening the ratchet, and names what actually covers the gate: scripts/lib/test_check_contract_surface_shift.sh builds real git fixtures with two commits (9 assertions, both shift directions, the annotation grammar, unresolvable-base-fails-closed), and the gate was replayed against openregister@83e8798a4 — the commit that caused the outage it exists for — where it FAILs and names all five shifted methods. Also restores with 'set +e' rather than 'set -e': errexit off is the state this script runs in, and re-arming it would abort the run on the first non-zero anything downstream. Verified: acceptance matrix rc 0, errexit discipline rc 0, package invariants rc 0.
ShellCheck SC2015 on eight assertion sites, not the one I fixed first. Every `[ rc = n ] && ok ... || no ...` reads like if-then-else and is not: the failure branch also runs when the check passes and the reporter fails, so a single assertion could print both PASS and FAIL — a suite that lies in the direction of looking fine. Replaces them with an `expect_rc` helper that branches properly, which also makes each assertion one line instead of three. Still 9 of 9.
Contributor
Author
|
Superseded by the rebased branch. #469 landed first and both branches added a gate block to the same region of |
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
1 participant
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.
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@methoddocblock tag routed through__call()) and the DECLARED surface, unless the change is annotated.Why
PHPUnit picks its mock builder on exactly that distinction:
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#2498published theObjectService/ObjectEntityinterfaces, which forcedgetUuid(),getRegister()andgetSchema()to be declared:Both
developmentbranches 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
Categories are closed:
announced,internal-only,new-contract. Same shape as gate-16's@spec excludeand 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:
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:
Named, not silent, and still counted in COVERAGE.
Suite (
scripts/lib/test_check_contract_surface_shift.sh, auto-discovered byrun-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 announcedthe 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.