Skip to content

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

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#430
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:vue-demi", "test:l10n"]
Coverage Baseline Protection / Check skipped (enable-coverage-guard unset) on, inputs added here
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:

  • Added standaardVeries array #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.
  • fixed eslint #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:

  • check:manifest PASSES — 30 pages, structural lint 0 issues.
  • check:vue-demi PASSES.
  • test:l10n PASSES — 531 keys used, 690 in en.json.

Coverage baseline inputs

enable-coverage-guard needs scripts/coverage-guard.php and
.coverage-baseline, neither of which existed here.

  • scripts/coverage-guard.phpbyte-identical to the copies in
    openregister and procest, the only two repos in the fleet that already had
    it.
  • .coverage-baseline = 13.01 — this repo's own measured coverage, 4077 of 31330
    statements, read from clover.xml inside the coverage-report artifact of
    run 30911570224.

A note on where that number came from: the job's own error message points you at
a coverage-baseline artifact, and the same log says No files were found with the provided paththat artifact does not exist, because it is only
uploaded on drift. The value was therefore taken from the clover file
coverage-guard.php itself parses, which is the same source the job would use.

Coverage Baseline Protection (PR side) only rejects a lowered baseline and
explicitly accepts a newly-introduced one, so it should pass here.
Coverage Baseline Check (push side) runs only after merge.

⚠️ The baseline here is 13.01% — 4077 of 31330 statements, the lowest in
the fleet by a wide margin. It is recorded as measured rather than rounded up:
the ratchet's job is to stop it falling further, and a baseline set above the
truth fails on the first honest run.

Left off, with reasons

  • Integration Tests (Newman) — left OFF. I initially enabled it, then reverted: the existing enable-newman: false records that the collection hard-codes /var/www/html/custom_apps/openregister/... paths that do not exist on a CI runner and assumes a seeded voorzieningen register / module schema. It goes back on in the commit that rewrites the collection and adds the seed step.

    Worth recording from checking this path: the validator counts collections recursively (find) while the run step globs them flat (for collection in *.postman_collection.json after cd). A newman-collection-path that only satisfies the validator would pass validation and then run nothing.

  • test:l10n:parity — measured short 404+ translations across the required locales. Translation backlog, not a CI defect.

  • 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/softwarecatalog @ cfea6b5

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-vue-demi
test-l10n
composer ✅ 121/121
npm ✅ 849/849
PHPUnit
Newman ⏭️
Playwright

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

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

What the run actually produced

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

Jobs: 27 → 29 (skipped: 8 → 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 29 jobs, and no job was cancelled (a cancelled
job is no verdict at all).

Attribution

Caused by this change: nothing. Every newly-enabled gate — check:manifest, check:vue-demi, test:l10n, Coverage Baseline Protection, Hydra Gates — is green, and the whole run is green. Newman remains deliberately off — see the description.

No failing jobs — the whole run is green with the new gates on.

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.

@rubenvdlinde
rubenvdlinde merged commit 568bbec into development Aug 4, 2026
30 checks passed
@rubenvdlinde
rubenvdlinde deleted the chore/enable-skipped-quality-gates branch August 4, 2026 15:42
rubenvdlinde added a commit that referenced this pull request Aug 4, 2026
This workflow had only a pull_request trigger - the only caller in the
fleet without a push one. SBOM, Features Extract and Coverage Baseline
Check are push-only jobs, so none of them could ever fire here. The last
push run on development is from 2026-03-19.

Nothing showed this: on a pull_request those three jobs are correctly
skipped, so every PR's Quality Report looked complete. The only way to see
it is to ask when a push run last happened.

enable-coverage-guard was switched on in #430; without this its push-side
half would have been dead on arrival.
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

⚠️ Correction: the Hydra Gates green in this PR measured almost nothing

I reported Hydra Gates as PASS here and treated that as a meaningful result.
It was not, and this needs saying plainly because it was the headline claim.

The gates are diff-scoped. The job log says exactly what it looked at:

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

Three files — and they were mine: .github/workflows/code-quality.yml,
.coverage-baseline, scripts/coverage-guard.php. Not one line of lib/,
src/ or appinfo/routes.php was in scope.
58 gates "passed" in about 1.5
seconds because there was nothing for them to read.

A diff-scoped gate's green means "nothing in scope changed", not "the tree is
clean"
. I read it as the second.

What the gates actually say about this repo

Re-run locally against the full tree (--base <empty-tree>, with ajv
installed into the gate package exactly as the CI step does, so gates 22 and 53
run for real rather than refusing):

22 gates FAIL on the full tree.

  • gate-9 semantic-auth 🔴 security
  • gate-12 nc-input-labels
  • gate-16 spec-coverage
  • gate-19 e2e-coverage
  • gate-25 contract-coverage
  • gate-26 visual-coverage
  • gate-32 semantic-controls
  • gate-38 skip-link
  • gate-39 button-name
  • gate-40 form-label-association
  • gate-43 table-headers
  • gate-45 prefers-reduced-motion
  • gate-46 spec-anchor-existence
  • gate-49 controller-exception-translation
  • gate-50 security-config-fail-mode
  • gate-51 schema-property-titles
  • gate-54 relation-dialect
  • gate-55 register-handler-resolution
  • gate-57 orphaned-write-capability
  • gate-58 e2e-networkidle
  • gate-61 listener-work-placement
  • gate-63 (gate-63)

The security ones

Of those, gate-9 semantic-auth are the ADR-005 authorisation family — the tier .github#149 was fixed to make trustworthy. They report PASS in CI here purely because no controller file was in the diff.

None of this is caused by this PR — every one of these findings pre-dates it, and
most pre-date it by a long way. The PR is still the right change: it is what put
the gate in the run at all, and the gate will do real work on the next PR that
touches application code. But the green it produced is not evidence of
anything
, and I presented it as though it were.

What this means for the fleet number

I reported "Hydra Gates: 19 of 22 repos green". Measured properly, 13 of 13
repos I have re-scanned so far carry between 6 and 23 failing gates on their full
tree
— 227 failing gate-checks in total, every one of them behind a green tick.
The three repos that did go red in CI (docudesk, procest, scholiq, all on
gate-5 route-auth) are simply the ones whose failing gate happens not to be
diff-scoped.

The rule worth keeping

When you enable a diff-scoped gate, its first green tells you nothing. Run it
against the base tree directly before reporting a verdict — otherwise you will
both overstate the result and, on the next real PR, mistake long-standing
violations for ones that PR introduced.

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