Coverage ratchet is unclosable when the merge base predates the app's PHPUnit setup (dev→main PRs)

[rubenvdlinde]

Summary

On a pull request whose merge base predates the app's PHPUnit setup, the
coverage ratchet fails with no action available to the author. This is the
unclosable-gate shape. It bites development → main / development → beta
release-promotion PRs, where the merge base can be months old.

Confirmed on zaakafhandelapp PR #294 (development → main), run
31478794540,
job PHPUnit (PHP 8.3, NC stable32) (93739014068).

What the log shows

The tests are fine — Run PHPUnit tests is a green check:

OK (242 tests, 772 assertions)

The failing step is Measure merge-base coverage:

head=4fa6d02… base=b233af99… merge-base=b233af99…
HEAD is now at b233af9 chore(ci): add missing canonical workflows (#244)
No composer.lock file present. Updating dependencies to latest instead.
…
/home/runner/work/_temp/….sh: line 9: ./vendor/bin/phpunit: No such file or directory
##[error]Could not measure coverage at merge base b233af99… — refusing to report a ratchet that did not run.

Verified against the repo rather than inferred:

• git cat-file -e b233af99…:composer.lock → fatal: Not a valid object name
• git show b233af99…:composer.json has no phpunit/phpunit in require-dev

Today's development has both. So at the merge base the test tooling simply did
not exist yet, composer install resolves without phpunit, no base-clover.xml
is produced, and the test -s guard fires.

Why the single-cell shape is a red herring

quality.yml:1829 restricts the step to one matrix cell:

if: ${{ inputs.enable-coverage-guard && github.event_name == 'pull_request'
        && matrix.php-version == inputs.php-version
        && matrix.nextcloud-ref == fromJSON(inputs.nextcloud-test-refs)[0] }}

so 8.3/stable32 is the only cell that runs it. The other three "passes" never
executed the step. This shape is being misread across the fleet as an
environment-dependent test failure (class_exists() autoload) or a wiped
vendor/. Control pair that rules both out: the same commit 8a790664 passed
this exact cell on the push run 31478791392 and failed on the PR run — the only
difference is github.event_name.

The guard itself is correct — please do not soften it

The step's own comment explains why a missing/zero clover must be fatal: a base
measured as 0% sets the floor to zero and passes every possible drop. So all of
these are wrong fixes:

• || true on the clover check, or deleting the test -s guard
• treating an unmeasurable base as 0%
• skipping the ratchet whenever it is inconvenient

Suggested direction

The defect is that an unmeasurable base is reported as a failed base. A
merge base predating the suite has undefined coverage, not zero.

The trigger is cleanly detectable and distinguishable from "tests ran and
produced nothing": ./vendor/bin/phpunit absent after composer install at the
merge base. In that case the defensible fallback is the absolute check this
same workflow already runs on every push —

php scripts/coverage-guard.php coverage/clover.xml

against the committed .coverage-baseline. That is a real check rather than a
skip, so the ratchet is never silently demoted to nothing, and the PR author has
something they can actually satisfy.

Also affected

openconnector is reported to hit the same failure mode. Worth sweeping every repo
that sets enable-coverage-guard and has a long-lived main/beta divergence.

Not fixed in this issue, deliberately

I have authority to change .github, but this is a behaviour change to a workflow
every app inherits, and #347 is currently being held back specifically to avoid
reddening ten repos mid-flight. Filing the diagnosis rather than shipping the
change seemed the right trade; happy to implement it once the fleet is quiescent.

[rubenvdlinde]

Sequencing note from the gate-fixes wave (2026-08-11). Landed this session, smallest blast radius first, each with a fixture observed red-before / green-after: #366 (the gate-16 fixture #364 shipped without), #368 (gate-7 hasPermission/canAccess, #360), #370 (gate-24 both registration APIs, #349), #371 (gate-61 honest empty-scope reason, #347).

This one was deliberately NOT landed in that wave. The per-issue reason is in the next comment / in fleet-board/findings/github-gate-fixes.md; the common one is that every remaining item either changes what a gate counts rather than what it says, or lives in quality.yml where run-helper-suites.sh cannot reach it — and four gate changes in one session is already the limit at which the next fleet measurement stays attributable.

[rubenvdlinde]

Second app, same unclosable shape, different mechanism — openconnector

Confirmed by: openconnector run
31526802260
(Release: merge development into beta), job PHPUnit (PHP 8.3, NC stable32)
93897177676, read from that job's own stdout via
gh api repos/ConductionNL/openconnector/actions/jobs/93897177676/logs (207 643
bytes — an empty log greps exactly like a clean one, so the size is stated).

The tests are fine. The HEAD arm is a clean run:

Tests: 2225, Assertions: 8023, Warnings: 2, Deprecations: 1, PHPUnit Deprecations: 1.

Zero failures, zero errors. The failing step is Measure merge-base coverage,
and the three neighbouring matrix cells (PHP 8.4/stable32, 8.3/stable33,
8.4/stable33) are all green because quality.yml restricts the step to one
cell, exactly as this issue already documents.

What is different here, and why it is worth a second entry: in
zaakafhandelapp the base had no phpunit at all. Here it has phpunit and the
phpunit crashes:

head=8bd40c54… base=01a15d4f… merge-base=01a15d4f…
HEAD is now at 01a15d4f Merge pull request 'openconnector: release development into beta' (#123)
…
  - Upgrading nextcloud/ocp (v32.0.9 => dev-stable29 6e97cdf): Extracting archive
…
PHP Fatal error:  Declaration of OC\Files\Node\LazyFolder::listen($scope, $method, callable $callback)
  must be compatible with OC\Hooks\Emitter::listen(string $scope, string $method, callable $callback): void
  in …/server/lib/private/Files/Node/LazyFolder.php on line 77
…
##[error]Could not measure coverage at merge base 01a15d4f… — refusing to report a ratchet that did not run.

Verified against the repo rather than inferred:

	origin/beta (= the merge base)	origin/development
composer.lock → nextcloud/ocp	dev-stable29	v32.0.9
tests/stubs/OC/Hooks/Emitter.php	present	present

The base checkout's composer install downgrades nextcloud/ocp three
major lines, and stable29's untyped LazyFolder::listen() is then incompatible
with the app's own typed OC\Hooks\Emitter stub. PHPUnit fatals before the
first test, no base-clover.xml is written, and the test -s guard fires.

🔑 So the generalisation is a little wider than "the merge base predates the
app's PHPUnit setup": the merge base only has to be old enough that its
pinned dependency set no longer boots on today's runner. A release-promotion
PR is the one place that is routinely true, and the app author has no move —
the base tree is immutable history, so nothing landed on development can
change what the ratchet measures.

The guard itself is right and should not be relaxed — "refusing to report a
ratchet that did not run" is the honest behaviour, and softening it would turn
this into a gate that prints PASS over an empty measurement, which is the
failure mode half of this programme is currently unpicking. The fix belongs in
what the step does when the base cannot be measured on a release-promotion PR,
not in the assertion.

I also confirmed this mechanism a third time by accident, locally: a vendor/
copied from another worktree carried nextcloud/ocp dev-stable29 while
composer.lock said v32.0.9, and the suite came back 62 errors / 1
failure — the failure being a test that names the cause
(isEnabledForAnyone() is @since 32.0.0 … if it is missing, nextcloud/ocp is pinned below the declared floor again). Syncing vendor to the lock took it to
0 failures. A stale ocp is a very convincing impersonation of an application
regression.