Skip to content

ci(quality): enable the Code Quality gates this repo was silently skipping - #728

Merged
rubenvdlinde merged 2 commits into
developmentfrom
chore/enable-skipped-quality-gates
Aug 4, 2026
Merged

ci(quality): enable the Code Quality gates this repo was silently skipping#728
rubenvdlinde merged 2 commits into
developmentfrom
chore/enable-skipped-quality-gates

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What

Enables the Code Quality gates this repo was silently skipping.

gate was now
Frontend Check skipped (frontend-checks: []) ["check:manifest", "check:vue3-compile", "test:l10n"]
Hydra Gates skipped (enable-hydra-gates unset) on, pinned v1.0.1

Why

A skipped job and a passing job are indistinguishable in the Quality Report.
Every gate listed above reported skipped in this repository's runs, which reads
as "fine". This turns them on.

Two prerequisites landed on ConductionNL/.github@main first and are what make
this viable:

  • Implement: Procest App Scaffold #149 — hydra-gates gate-7 (no-admin-idor) now follows delegation, and
    gates 6/7 no longer pass on an empty scope. Before that, gate-7 flagged
    correctly-guarded methods whose guard is reached through a helper, which is
    why 19 of 20 repos kept the whole tier switched off.
  • Implement: Procest Case Management #150 — an empty frontend-checks list deleted the Frontend Check job
    from the run rather than skipping it, because inputs.frontend-checks != '[]'
    was a literal string comparison.

Not enabled, on purpose

  • Journeydoc Capture — deliberately left off everywhere.
  • enable-axe — it produces the report hydra-gates gate-33 consumes, but a
    vanilla Nextcloud 34 with no app installed already returns three
    serious/critical violations from core's own UI. Turning it on in the same
    change as the gates would confuse "this app has an accessibility defect" with
    "Nextcloud core does". Separate change.

On red

Some legs below were measured failing before this PR was opened, and are
enabled anyway. The defects are pre-existing; the only thing that changed is
that CI can now see them. Per the brief, a gate is not switched back off because
it failed on arrival — the failure is the result.

Measured before flipping, not after

Every leg below was run against this branch's tree before it was enabled:

  • test:l10n PASSES — 2102 keys used, 3100 in en.json, and en.json/nl.json key sets match.
  • check:manifest FAILS on three counts — pages[4] and pages[5] (type=custom requires component field) and pages[52].type: "roadmap" not in v1.2 enum.
  • check:vue3-compile not measurable locally — it needs @vue/compiler-sfc, which only exists after the leg's own npm ci. CI is the first place it gets a real verdict; that is stated rather than guessed.

Left off, with reasons

  • Integration Tests (Newman) — left OFF. I initially enabled it, then reverted: the existing enable-newman: false records that the ZGW compliance collections fail at 95%+ because the ZGW API implementation is still in progress. Four collections under data/ would meet that same known cause. It goes back on in the commit that gets the core CRUD assertions passing.
  • Coverage ratchet — already enabled here (one of only two repos in the fleet that had .coverage-baseline committed); unchanged at 29.6.
  • test / test:unit — already run by the shared Frontend Tests (unit) job.

…pping

A skipped job and a passing job are indistinguishable in the Quality
Report. Every gate turned on here reported 'skipped' in every run.

Each newly-enabled leg was measured against this tree BEFORE being
enabled; the results are in the PR description. Legs that were measured
failing are enabled anyway - the defects are pre-existing, and the only
thing that changed is that CI can now see them.

Journeydoc Capture and enable-axe are deliberately NOT enabled.
…pt-outs

A second key in the same with: block is accepted last-one-wins by every
YAML parser involved, so the file reads as configured while the value is
decided somewhere else in it.

Where enable-newman: false already carried a specific, still-live reason
(OR React/Async await + SQLite REGEXP; ZGW API incomplete; collections
hard-coding non-CI URLs and unseeded registers), it is left off and the
reason is quoted rather than overridden. Where it was a bare line with no
comment, the stale line is removed instead of shadowed.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/procest @ c8671ff

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue3-compile
test-l10n
composer ✅ 100/100
npm ✅ 550/550
PHPUnit
Newman ⏭️
Playwright

Quality workflow — 2026-08-04 15:30 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

What the run actually produced

Run 30921048391 — the measurement that matters, since it is the one that decides the verdict.

Jobs: 29 → 31 (skipped: 7 → 5). Counted from the jobs API, not from the
Quality Report — a malformed caller produces a startup_failure whose
jobs.total_count is 0 and which emits no contexts at all, and that reads exactly
like success. This run emitted 31 jobs, and no job was cancelled (a cancelled
job is no verdict at all).

Attribution

Caused by this change: Hydra Gates (gate-5 route-auth, 4 routed methods — detailed below). Not caused by this change: E2E Tests (Playwright), which was already failing on the previous PR run (30910281884) and on a development push (30903829414), and was cancelled on the one before that. This PR does not touch Playwright config or inputs. All three newly-enabled Frontend Check legs are green — including check:vue3-compile, which could not be measured locally.

Failing jobs

  • E2E Tests (Playwright)failure (23m)
  • Hydra Gatesfailure (0m)
  • Quality Reportfailure (0m)

Correction to the pre-measurement in the description

The description says check:manifest fails on pages[N].type: "roadmap" not in v1.x enum, and reports that as a fleet-wide defect. That measurement was
wrong, and wrong in an instructive way.

I ran the script locally with no node_modules present. Without Ajv installed,
tests/validate-manifest.js silently falls back to a hand-rolled structural
lint
whose local allowedTypes set does not contain roadmap. In CI the leg
runs npm ci first, so the script resolves the real schema —
node_modules/@conduction/nextcloud-vue/src/schemas/app-manifest.schema.json,
v1.8.0 — and runs actual Ajv validation, which accepts roadmap.

So I measured the fallback path, not the path CI takes, and drew a fleet-wide
conclusion from it. The gate itself is what corrected me: check:manifest is
green in CI in decidesk, doriath, nldesign, openbuild, shillinq, procest,
pipelinq, softwarecatalog, launchpad and zaakafhandelapp — every repo where I
had predicted the roadmap failure.

Hydra Gates: the one failing gate is the same one in all three repos

gate-5 route-auth — controller methods registered in appinfo/routes.php with
no auth attribute (#[PublicPage] / #[NoAdminRequired] / #[NoCSRFRequired] /
#[AuthorizedAdminSetting]). Nextcloud's middleware rejects such a request
before the controller runs, so these endpoints are unreachable at runtime
while the code reads as if they work.

  • docudesk: 3 routed methods
  • procest: 4 routed methods
  • scholiq: 4 routed methods

Every other gate passed. The run prints COVERAGE: 58 of 61 declared gates reported a result with GATES THAT DID NOT RUN: 4 24 33 — read that line, not
the banner: gate-33 is the axe gate, which has no report to consume because
enable-axe is deliberately off.

@rubenvdlinde
rubenvdlinde merged commit 19324df into development Aug 4, 2026
32 of 35 checks passed
@rubenvdlinde
rubenvdlinde deleted the chore/enable-skipped-quality-gates branch August 4, 2026 15:42
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Note on the Hydra Gates red in this PR — and on its 57 greens

The gate-5 route-auth failure I reported is real and correctly attributed:
gate-5 reads appinfo/routes.php wholesale rather than diff-scoped, which is
exactly why it caught something when the other gates did not.

The other 57 gates in that same run are a different matter. They are
diff-scoped, and the job log shows what they scoped to:

[hydra-gates] Scope: diff vs origin/development — 3 changed file(s)

Those three files were mine — the workflow caller, .coverage-baseline and
scripts/coverage-guard.php. No application code was in scope. So
"1 gate failed, 57 passed" should be read as "1 gate that scans the whole tree
failed; 57 gates that scan the diff had nothing to look at".

Re-running the package against the full tree in comparable repos turns up between
6 and 23 failing gates each, in all 15 re-scanned so far. Please do not read the
57 greens here as a clean bill of health.

Also worth reading in that log, because the package prints it and it qualifies
the same greens further:

[hydra-gates] WAIVERS: N file(s) carry an '@spec exclude <reason>' tag.
[hydra-gates] WAIVERS: N file(s) carry an '@e2e exclude <reason>' tag.
[hydra-gates] a green above is partly a statement that someone justified them,
[hydra-gates] not that the underlying coverage exists.

docudesk carries 130 @e2e exclude waivers; hermiq carries 44 @spec exclude. Gates 16/19/26 honour them, so their greens are partly a record of
justified exclusions rather than of coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant