fix(quality): a failed sibling-app install must fail the step, not pass [DRAFT — do not merge]

[rubenvdlinde]

🚧 DRAFT — DO NOT MERGE

.github@main is consumed live by every repo, so this lands fleet-wide the instant it merges. The flip is Ruben's call.

Unlike the companion phpcs PR (#483), this one has no debt precondition — it does not turn any existing green cell red; it only stops a failed step from reporting success. It is deliberately kept separate from #483 so it can be sequenced independently rather than waiting on the 707-error phpcs cleanup.

---

The defect

.github/workflows/quality.yml, "Checkout additional apps" (Nextcloud test-matrix job):

git clone --depth 1 --branch "$ref" "https://github.com/$repo.git" "server/apps/$name"
if [ -f "server/apps/$name/composer.json" ]; then
  cd "server/apps/$name"
  composer install --no-progress --prefer-dist --optimize-autoloader --no-dev 2>/dev/null || true
  cd -
fi

2>/dev/null hides the error and || true discards the exit code. A failed dependency install is therefore invisible, and the suite runs against a half-installed sibling app.

This is the only composer install of the 20+ in this workflow with either its stderr discarded or its status thrown away — and the git clone on the line directly above it has always failed loudly. The asymmetry was not a deliberate policy.

Worse: the damage surfaces as the consuming repo's fault, because the missing classes belong to the sibling.

---

Measured evidence — decidesk#517, 2026-08-17

A PR whose diff is exactly one .ts file and zero PHP.

Every one of its six PHPUnit cells hit a transient HTTP 504 while installing openregister's dependencies. Verified per job that the 504 falls inside this step — between the Checking out ConductionNL/openregister line and the next step group — not somewhere else in the log.

cell	504 inside the step	package the 504 removed	outcome
1	yes	Masterminds/html5-php	red, 87 errors
2	yes	symfony/deprecation-contracts	red
3	yes	symfony/deprecation-contracts	red
4	yes	php-http/discovery	green
5	yes	Masterminds/html5-php	green
6	yes	php-http/discovery	green

6 of 6 installs failed. 6 of 6 steps reported success.

The first error in the red cells:

Error: Class "Twig\Extension\AbstractExtension" not found
  in .../server/apps/openregister/lib/Twig/MappingExtension.php:41

cascading into ~86 further UnknownTypeException: Class or interface "OCA\OpenRegister\Service\FileService" does not exist — openregister's autoloader was broken, so all of its classes became unmockable.

development and the sibling PR #516 ran green in the same window, which is what rules out the PR's own diff.

The part that makes this worth fixing rather than retrying

The three green cells were exactly as half-installed as the three red ones. The only difference was which package the network happened to drop — html5-php and deprecation-contracts broke the autoload chain, php-http/discovery did not.

So this step is producing half-installed sibling apps routinely, and whether that becomes a red cell is a lottery. A retry would have "fixed" #517 and taught us nothing.

---

The fix

Keep stderr. Fail the step, naming the app and its ref. Retrying becomes an operator's decision taken knowingly rather than a silent default. Also drops the cd/cd - dance for a subshell.

Verified with a shell control that failure propagates correctly out of the ... | while read pipeline subshell under bash -e (how Actions runs run: blocks) — a check that can say both yes and no:

scenario	old step exit	new step exit
sibling install succeeds	0	0
sibling install fails	0 ← the hole	1 ← fixed

The other three "Checkout additional apps" blocks in this file were checked and are already loud; they are deliberately left untouched. Measure widely, change narrowly.

---

Relationship to #483

These two defects are the same shape from opposite directions, and they are the reason both were found tonight:

• phpcs measures correctly, then discards the verdict.
• this step discards the measurement, then reports success.

Both produce a green cell over a real failure.

---

⚠️ This is one of THREE, and the second half of an existing PR

Line ownership — read this before reviewing

#355 (draft, opened 2026-08-11, by @rubenvdlinde) already fixes the sibling line to this one, four lines away in the same step:

php occ app:enable "$name" || echo "::warning::Failed to enable $name, continuing..."

PR	owns	status
#355	php occ app:enable … || echo "::warning::…"	draft, CONFLICTING, untouched since 2026-08-11
#484 (this)	composer install … 2>/dev/null || true	draft
#483	the phpcs case discarding its own verdict	draft

#355 and this PR are two halves of one hole — an app that will not enable, and an app whose dependencies never installed. #355's own body already names the second as a cause of the first: "…or its composer install did not complete." They should land together or not at all.

#355's measurement, independently re-verified tonight

Before writing this PR I re-ran #355's evidence rather than restating it. hermiq run 31490144919, six PHPUnit cells:

cells	app:enable	result
3	failed	Tests: 1500, Assertions: 4647, Errors: 12
3	succeeded	Tests: 1500, Assertions: 4657, Errors: 5, Failures: 1

Perfect 6/6, same commit, same application code, environment the only variable. It reproduces exactly.

#355 is closer to landing than its metadata suggests

It is 98 commits behind main, over which quality.yml grew +944/−39. But a local dry-run merge produces exactly one conflict region — everything else auto-merges. It needs a refresh, not a rewrite. (I did not touch the branch: no rebase, no force-push.)

---

The three defects are one story

#	where	what it does
#483	phpcs case	measures correctly, then discards the verdict (exit 1 → success)
#484	composer install for siblings	discards the measurement, reports success
#355	app:enable for siblings	discards the measurement, reports success

Three places where the shared workflow reports success over a failure it had already observed. In each case the information existed and was thrown away — #483 threw away a verdict it had computed; #484 and #355 threw away the status of a step that had already failed.

The finding that outlives all three fixes

The app:enable defect was found, measured AND fixed on 2026-08-11. The fix never landed. In the five days since, the fleet has spent real effort misattributing environment failures to application debt — including tonight, on decidesk#517, where 87 errors were read as decidesk's.

An unmerged fix is indistinguishable from an undiscovered bug — except that the second time, you also pay for the rediscovery. Whatever caused #355 to stall is now itself a finding, and it is the most portable thing in this whole batch.