fix(gate-28): license-triangle reported PASS on repos with no lib/, having opened zero files - #172
Merged
Merged
Conversation
…aving opened zero files
`_pass 28` was called unconditionally after a comparison that only ran inside
`if [ -n "${_composer_lic}" ] && [ -d lib ]`. Every repo without a lib/
directory therefore printed
[gate-28] license-triangle: PASS
having inspected nothing, and the coverage accounting counted it as a gate that
reported a result. A PASS and a no-op were byte-identical in the output — the
falsely-GREEN shape the coverage block exists to make impossible.
This is not hypothetical. Four Python ExApp sidecars in the fleet — valtimo,
openklant, opentalk, openzaak — have no lib/ at all (their application code is
a single ex_app/lib/main.py and the only PHP is phpcs-custom-sniffs/). All four
have been reporting gate-28 PASS on a --full scan of their entire tree.
The gate now counts the files it actually compared and routes on that:
- FAIL unchanged: any file whose @license is outside composer.json's set
- PASS only when >= 1 file was really compared
- NOT APPLICABLE (na) no lib/ — there is nothing to compare and there
cannot be; the gate becomes applicable when PHP app
code lands under lib/
- SKIPPED (structural) lib/ exists but composer.json declares no license,
or no in-scope lib/**/*.php carried an @license tag
Measured on three controls, not asserted:
A valtimo (no lib/) was PASS -> NOT APPLICABLE
COVERAGE 29 of 63 -> 28 of 63 reported
(30 not applicable; 28 of 33 applicable ran)
B openbuild (lib/, agreeing) PASS -> PASS (unchanged)
C fixture (lib/, drifting) FAIL 1 -> FAIL 1 (identical old
and new — the failure path is untouched)
Control C was run against BOTH the old and the new script on the same fixture
to show the change removes a false green without weakening the real assertion.
Also corrects the header comment, which oversold the gate's reach: it describes
"the three locations a Conduction app declares its license" including
appinfo/info.xml, but the code never reads info.xml. It compares two locations.
The comment now says so, so nothing here can be read as a verdict on
<licence>. (The ADR-014 claim that info.xml must say `agpl` is separately
contradicted by the whole fleet and by the appstore's own info.xsd, which
enumerates EUPL-1.2 — that is an ADR decision and is deliberately not resolved
in a gate.)
This was referenced Aug 5, 2026
The first cut of this change classified every "lib/ exists but no composer license" case as `structural`, which counts against coverage. That is right for a composer.json that exists and omits `license` — a real, fixable gap — and wrong for a repo that has no composer.json at all. With no composer.json there is no second declaration to compare against and no change inside that repo can create one; the gate simply has no subject matter. Caught by the package's own test 4c-i, which builds a fixture with lib/ and no composer.json and asserts `--require-full-coverage` exits 0 on it. It got 98. The invariant was correct and the classification was not. hydra-gates entry-point tests: 30 passed, 1 failed -> 31 passed, 0 failed Four controls, old script vs new, on purpose-built fixtures: no lib/, composer.json present OLD: PASS NEW: NOT APPLICABLE lib/, no composer.json OLD: PASS NEW: NOT APPLICABLE lib/, licences agree OLD: PASS NEW: PASS lib/, one file drifting OLD: FAIL 1 NEW: FAIL 1 The two PASS -> NOT APPLICABLE rows are the false greens being removed. The bottom two rows are the controls that show nothing was weakened: the real verdicts are byte-identical between the old and new script.
rubenvdlinde
pushed a commit
that referenced
this pull request
Aug 5, 2026
Three reconciliations, each caught by an existing assertion rather than by reading the diff: * gate-28 — #172 gave this gate a four-way skip taxonomy (`na` for no lib/, `na` for no composer.json, `structural` for a composer.json without a `license`, `structural` for nothing carrying a tag) and made PASS conditional on having actually compared a file. Moving the read into a helper collapsed all of that into one `na`, undoing it. The taxonomy is restored verbatim; the helper now returns the compared-file count on stderr so `_lt_checked` still gates the PASS. The one state that short-circuits it is a MISSING HELPER — which #172's chain would have called `structural`, i.e. a claim about the REPOSITORY, when the repository is fine and the gate is broken. Those must not wear the same words. * gates 40 and 46 — an empty in-scope file set is ordinary ADR-020 diff scoping, not "not applicable". tests/test-hydra-gates-bin.sh asserts that no src-guarded gate reports NOT APPLICABLE while src/ exists, and gate-40 was tripping it. Answering differently from every sibling gate drifts the applicability table away from the guards it mirrors — the one way that table could hide a live gate. * quality.yml — #173 added a preflight for the FLAG direction of the floating-caller / pinned-callee desync (does the pinned runner understand the coverage flag?). This branch adds one for the PATH direction (does the pinned package contain the files the workflow executes by name?). Both are kept: they are different halves of the same interface, and #168 broke the half #173 does not cover. And one contract change, made explicit rather than incidental: test-hydra-gates-bin.sh's "empty diff exits 0" fixture set BASE_SHA to HEAD, so it was really asserting "scoping a commit against itself exits 0" — the shillinq shape. Those are different facts and only one is legitimate. The original assertion now runs against a real base-behind-HEAD empty diff and still expects 0; a new assertion covers base == HEAD, expects 99, and checks that NO gate printed PASS. Verified on the merged tree: hydra-gates entry-point tests: 36 passed, 0 failed (was 31 passed, 3 failed) helper suites: 23 passed, 0 failed gate-46 fleet: 1,995 -> 918 gate-40 fleet: 1,211 -> 517 (unchanged by the merge)
This was referenced Aug 5, 2026
Merged
Merged
rubenvdlinde
added a commit
that referenced
this pull request
Aug 6, 2026
…-020 working (#182) gate-28 turned RED on every PR in the fleet whose diff does not touch lib/**/*.php, which is most of them. It reported the DIFF's shape as the REPOSITORY's defect. `_lt_files` empty had two causes, and the final `else` stated them as one: a) the repo has lib/**/*.php but this diff touches none of them — ADR-020 diff-scoping working, the same state gate-4 reports NOT APPLICABLE for the same diff; b) lib/ exists but holds no tracked .php at all. Neither is (c) "files WERE in scope and none carried a declaration", the genuine structural gap that branch describes. All three printed (c), and for (a) that message is FALSE: with zero files in scope, "0 in-scope file carried a declaration" is true only vacuously. Not cosmetic. hydra-gates-require-full-coverage defaults to TRUE (#164) and a structural gap fails the run with exit 98, so this was a fleet-wide falsely-RED gate — the mirror of the falsely-GREEN shape #172 built this taxonomy to kill. It surfaced the moment a repo unpinned to main, because gate-28 does not exist at v1.3.0. Measured on nldesign against the real runner, three arms, before and after: diff = .github/workflows/code-quality.yml before SKIPPED(structural) -> exit 98 after NOT APPLICABLE diff = 3 files under lib/ before PASS after PASS diff = one lib/*.php declaring AGPL while composer.json declares EUPL-1.2 after FAIL — 1 file(s) The third arm is the one that matters: turning a red into a not-applicable is exactly how a gate gets muted, so the mismatch it exists to catch was injected and confirmed still caught. The (c) message now also states the in-scope count, so a reader can tell it apart from the case it used to be conflated with. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
rubenvdlinde
added a commit
that referenced
this pull request
Aug 6, 2026
…g one state too far (#180) * fix(gate-28): an EMPTY DIFF SCOPE is not a structural gap — #172 swung one state too far #172 was right that gate-28 must not report PASS having opened zero files. But there are TWO ways to read zero files and it gave them the same word: (a) lib/**/*.php files ARE in this diff and none carries a licence tag — a genuine gap in the repository, correctly `structural`; (b) no lib/**/*.php file is in this diff AT ALL — diff-scoping working exactly as ADR-020 designed it, and no statement about the repo. (b) is the ordinary case. `_in_scope` filters to the PR's diff, so every workflow-only and frontend-only PR reads zero lib PHP files. Calling that `structural` counts it against coverage, and with hydra-gates-require-full-coverage on by default the run fails with exit 98 for a licence problem the repository does not have. MEASURED on the fleet's own unpinning PRs, each a single-file workflow diff: hrmq#74 CLEAN baseline -> RED on nothing but gate-28 app-versions#129 gate 28 its only gate that did not run opencatalogi#813 " nextcloud-app-template#132 " launchpad#60 " hrmq's 168 lib PHP files all carry their tags. The gate had simply not been handed any of them to read, and then reported that as the repository's fault. This is the "a gate that is legitimately not applicable must NOT fail the run" rule broken — the same shape as #173, one gate lower down. It surfaced now because the fleet just unpinned (#177): at v1.3.0 gate-28 reported PASS on an empty read, so the false GREEN hid what is now a false RED. Neither was a verdict. The `structural` branch is preserved for case (a) and now names the count of files that WERE in scope, so the two states no longer read alike. TESTS — scripts/lib/test_gate_license_triangle_scope.sh, four control pairs, discovered automatically by tests/run-helper-suites.sh: wf-only diff, tagged repo -> NOT APPLICABLE (b, the false red) untagged PHP in the diff -> structural (a, must stay red) tagged PHP in the diff -> PASS (the gate compared something) no lib/ at all -> NOT APPLICABLE (#172's original case) Verified in BOTH directions rather than assumed: against the fixed runner 5/5 green; against the runner at 954a78a the first two FAIL and the last three still pass, so the suite discriminates and the fix is targeted rather than a mute for #172. The did-not-run assertion took two attempts to measure anything — v1 matched only a summary line this suite never triggers, v2 matched a roster the runner also prints for NOT-APPLICABLE gates. Both are recorded in the file, because a dead assertion inside a suite meant to tell two states apart is the exact defect the suite exists to catch. * fix(test): explicit if/else in _expect — `A && _ok || _bad` can count one assertion twice ShellCheck SC2015 on the new suite, and it is not a style note here. In `grep -q ... && _ok "$d" || _bad "$d"`, a non-zero return from _ok makes _bad run as well, so the SAME assertion is counted once as a pass and once as a failure. In a harness whose entire job is to count assertions correctly, that is instrument corruption — the class this suite exists to catch, one level up. Also adds a `*)` arm: an unknown expectation string now fails loudly instead of matching no case and silently asserting nothing. Re-verified in both directions after the change: fixed runner 5 passed / 0 failed, runner at 954a78a 3 passed / 2 failed, entry-point invariants 39/0. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
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
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.
The defect
_pass 28was called unconditionally, after a comparison that only runs inside:So a repo with no
lib/printedhaving opened zero files — and the coverage accounting counted it as a gate that reported a result. A PASS and a no-op were byte-identical in the output. That is the falsely-GREEN shape the coverage block exists to make impossible, appearing inside the thing meant to detect it.
Not hypothetical. Four Python ExApp sidecars in the fleet —
valtimo,openklant,opentalk,openzaak— have nolib/at all (their application is a singleex_app/lib/main.py; the only PHP isphpcs-custom-sniffs/). All four report gate-28 PASS on a--fullscan of their entire tree.The fix
Count the files actually compared, and route on that. The
na/structuralvocabulary already exists — this gate just wasn't using it.@license∉ composer.json's setFAILFAIL(untouched)PASSPASS(untouched)lib/at allPASSNOT APPLICABLElib/exists, composer.json declares nolicensePASSSKIPPED (structural)lib/exists, 0 in-scope file carried an@licensetagPASSSKIPPED (structural)Three controls — measured, not asserted
A — no
lib/(valtimo,--full):B —
lib/present, licences agree (openbuild,--full):PASS→PASS,COVERAGE: 61 of 63. Unchanged.C —
lib/present, one file drifting (fixture:composer.jsonEUPL-1.2, one file@license AGPL-3.0-or-later):Control C was run against both the old and the new script on the same fixture, deliberately: it shows this removes a false green without weakening the real assertion. No waiver, no threshold change, no
continue-on-error.Also: the header comment oversold the gate
It described "the three locations a Conduction app declares its license", naming
appinfo/info.xml— but the code never readsinfo.xml. It compares two locations:composer.json .licenseand per-file@licensetags underlib/. The comment now says so, so nothing here can be read as a verdict on<licence>.🔴 Related, and deliberately NOT resolved here
That same comment cites ADR-014, which says
appinfo/info.xmlMUST use<licence>agpl</licence>because "Nextcloud app store does not recognise EUPL".That rationale is factually false. The live appstore schema — https://apps.nextcloud.com/schema/apps/info.xsd — enumerates
EUPL-1.2as an allowed<licence>value. (It does not enumerateeupl.)And the fleet already contradicts the ADR:
openregister,opencatalogi,openconnector,docudesk,nldesign,softwarecatalog,larpingapp,pipelinq,procest,openbuild,decidesk,portaliq— 12 of 12 checked declareEUPL-1.2.Amending an ADR that six repos mirror and this gate cites is a decision, not a cleanup. Written up with the evidence in ConductionNL/openbuild#135; no ADR is touched by this PR.