From 7c1eef6a980e516f5315a98b1ebb231a50421a66 Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Mon, 17 Aug 2026 04:59:08 +0200 Subject: [PATCH] fix(quality): a failed sibling-app install must fail the step, not pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Checkout additional apps" step in the Nextcloud test-matrix job ended its sibling `composer install` with `2>/dev/null || true` — stderr discarded AND the exit status thrown away. It was the only `composer install` of the 20+ in this workflow to do either, and the `git clone` on the line directly above it has always failed loudly, so the asymmetry was not deliberate. The consequence is the programme's core failure mode in CI plumbing: a step that did not succeed is indistinguishable from one that did. Worse, the damage surfaces as the CONSUMING repo's fault, because the missing classes belong to the sibling. Measured on decidesk#517 (2026-08-17) — a diff of exactly one `.ts` file and zero PHP: * ALL 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 openregister" line and the next step group. * All six reported the step as SUCCESSFUL. * Three cells then went red with 87 errors, the first being `Class "Twig\Extension\AbstractExtension" not found` in server/apps/openregister/lib/Twig/MappingExtension.php. * Three went green. The only difference was WHICH package the 504 removed — html5-php and deprecation-contracts broke the autoload chain, php-http/discovery did not. The green cells were exactly as half-installed as the red ones. That last point is the reason this is worth fixing rather than retrying: the step is producing half-installed sibling apps routinely, and whether that becomes a red cell is a lottery over which package the network drops. The fix keeps stderr and fails 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 the new form propagates correctly out of the `... | while read` pipeline subshell under `bash -e`: install succeeds -> step exit 0 install fails -> step exit 1 (old form: step exit 0) The other three "Checkout additional apps" blocks in this file were checked and are already loud; they are deliberately left untouched. DRAFT — do not merge. `.github@main` is consumed live by every repo. --- .github/workflows/quality.yml | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index f2bf1785..2598d86d 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -2345,9 +2345,36 @@ jobs: echo "Checking out $repo ($name) at $ref..." 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 - + echo "Installing composer dependencies for $name..." + # ⚠️ This install used to end in `2>/dev/null || true` — stderr + # discarded AND the exit status thrown away. It was the ONLY + # `composer install` of the 20+ in this workflow to do either, + # and the `git clone` immediately above it has always failed + # loudly, so the asymmetry was not a deliberate policy. + # + # A sibling app whose dependencies did not install is not a + # neutral condition. The suite then runs against a HALF-INSTALLED + # app, and its missing classes surface as failures of the + # CONSUMING repository — a red cell pointing at the wrong repo. + # + # Measured on decidesk#517 (2026-08-17), a diff of exactly one + # `.ts` file and zero PHP: ALL SIX PHPUnit cells hit a transient + # HTTP 504 while installing openregister's dependencies, and all + # six reported this step as SUCCESSFUL. Three then went red with + # 87 errors — the first being + # Class "Twig\Extension\AbstractExtension" not found + # in server/apps/openregister/lib/Twig/MappingExtension.php + # — while three went green. The only difference was WHICH package + # the 504 happened to remove; the green cells were exactly as + # half-installed as the red ones. + # + # So: keep stderr, and fail the step naming the app and its ref. + # Retrying is an operator's decision to take knowingly, not a + # silent default. + if ! ( cd "server/apps/$name" && composer install --no-progress --prefer-dist --optimize-autoloader --no-dev ); then + echo "::error::Dependency install FAILED for sibling app '$name' ($repo@$ref). Refusing to continue: the suite would otherwise run against a half-installed app and report its missing classes as this repository's failures." + exit 1 + fi fi done