Skip to content

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

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

ci(quality): enable the Code Quality gates this repo was silently skipping#26
rubenvdlinde merged 1 commit 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 ran, but check:manifest was not in the list ["test:unit", "check:specs", "check:manifest"]
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:

  • #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.
  • #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 FAILSpages[8].type: "roadmap" not in v1.1 enum. It is not part of this repo's check:specs aggregate (json-strict + manifest-v2 + register + registry), which is why it ran nowhere.

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 = 45.56 — this repo's own measured coverage, 272 of 597
    statements, read from clover.xml inside the coverage-report artifact of
    run 30911249478.

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.

…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.
@rubenvdlinde
rubenvdlinde requested a review from Rem-Dam as a code owner August 4, 2026 14:50
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/petstore @ cc831cf

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
test-unit
check-specs
check-manifest
composer ✅ 100/100
npm ✅ 771/771
PHPUnit
Newman
Playwright

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

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

What the run actually produced

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

Jobs: 32 → 33 (skipped: 6 → 4). 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 33 jobs, and no job was cancelled (a cancelled
job is no verdict at all).

Attribution

Caused by this change: Frontend Check (check:manifest) — real Ajv schema violations detailed below. Everything else newly enabled (Coverage Baseline Protection, Hydra Gates) is green, as are Newman and E2E.

Failing jobs

  • Frontend Check (check:manifest)failure (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.

Where check:manifest does fail in CI, the cause is entirely different and
real: the manifest does not conform to nc-vue's app-manifest.schema.json
v1.8.0. Measured causes, from the job logs:

  • nextcloud-app-template/pages/0, /pages/1, /pages/2 must NOT have
    additional properties; /pages/0/config/layout/{0,1,2}/id must be string.
  • petstore/pages/0 additional properties, plus config/widgets/* on
    pages 2, 4 and 6 carrying properties the schema forbids.
  • portaliq/pages/0/config/layout/{0,1,2}/id must be string, plus
    config/widgets/* additional properties on pages 2, 4, 6 and 8.
  • opencatalogi/pages/3 must have required property '_note', and fails
    both the if/then and if/else branches.

That is live drift between three apps' dashboard/widget manifests and the shared
schema, and none of it was visible while the job was not being emitted.

@rubenvdlinde
rubenvdlinde merged commit d8f70b1 into development Aug 4, 2026
34 of 36 checks passed
@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):

18 gates FAIL on the full tree.

  • gate-5 route-auth 🔴 security
  • gate-9 semantic-auth 🔴 security
  • gate-14 route-reachability
  • gate-16 spec-coverage
  • gate-19 e2e-coverage
  • gate-25 contract-coverage
  • gate-26 visual-coverage
  • gate-30 public-monitoring
  • gate-32 semantic-controls
  • gate-38 skip-link
  • gate-40 form-label-association
  • gate-46 spec-anchor-existence
  • gate-49 controller-exception-translation
  • gate-51 schema-property-titles
  • gate-54 relation-dialect
  • gate-58 e2e-networkidle
  • gate-61 listener-work-placement
  • gate-63 (gate-63)

The security ones

Of those, gate-5 route-auth, 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