Skip to content

ci: add the fleet-standard Code Quality pipeline (hrmq had none) - #65

Merged
rubenvdlinde merged 4 commits into
developmentfrom
ci/add-code-quality-workflow
Aug 4, 2026
Merged

ci: add the fleet-standard Code Quality pipeline (hrmq had none)#65
rubenvdlinde merged 4 commits into
developmentfrom
ci/add-code-quality-workflow

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

What those existing runs actually were

hrmq had no .github/workflows/ directory at allgh api .../contents/.github/workflows returns 404, and the Actions workflows API lists exactly two entries, both dynamic:

path name
dynamic/github-code-scanning/codeql CodeQL
dynamic/dependabot/dependabot-updates Dependabot Updates

The runs named "Push on development" / "PR #64" / "Scheduled" are CodeQL default setup (event: dynamic), which names its runs after the trigger rather than the workflow. They look like an app pipeline in the run list and are not one. No PHPCS, PHPMD, Psalm, PHPStan, PHPUnit, ESLint, Stylelint, license, SBOM or E2E check has ever run against this repo, on any commit.

The caller

Modelled on nextcloud-app-template, cross-checked against larpingapp, with inputs tuned to what hrmq actually ships.

Enabled — and why each is satisfiable

input evidence in the repo
enable-psalm psalm.xml + psalm-baseline.xml, vimeo/psalm in require-dev
enable-phpstan phpstan.neon + phpstan-baseline.neon + phpstan-bootstrap.php
enable-phpmd / enable-phpcs (defaults) phpmd.xml + phpmd.baseline.xml; phpcs.xml
enable-phpmetrics phpmetrics/phpmetrics in require-dev
enable-phpunit phpunit.xml + tests/bootstrap.php; tests/Unit/ mirrors lib/ package-by-package
enable-frontend / enable-eslint / enable-stylelint eslint.config.js, stylelint.config.js, lint + stylelint npm scripts
frontend-checks: ["check:manifest","check:widget-keys"] two self-contained node validators that already existed with no runner
enable-playwright playwright.config.ts + 2 specs under tests/e2e/spec-coverage/
enable-license-check, enable-sbom, enable-features-extract (defaults) composer.json + package.json; cyclonedx/cyclonedx-php-composer already in require-dev

nextcloud-test-refs: stable31..33info.xml allows 28–34, but the floor that actually binds is nextcloud/ocp: ^31.0.

additional-apps: openregister@mainsrc/manifest.json declares "dependencies": ["openregister"], and nc-vue's CnAppRoot honours that at runtime: without it, the "Required apps are missing" gate renders instead of the router outlet and every shell assertion fails with "element(s) not found" while the app behaves exactly as designed. hrmq is also a thin client (Employee / EmploymentContract / PayrollRun / Payslip / WageTaxFiling are all OR objects), so core-journeys.spec.ts has nothing to seed against either. Pinned to main rather than development only because this is hrmq's first CI run and the foundation app should not be a moving confound while the baseline is read.

playwright-test-path left at its default tests/e2e. There is no config at that path, so the shared workflow falls back to the root playwright.config.ts — which is the intended one (testDir: './tests/e2e', one chromium project). The "Validate Playwright tests exist" step's find is recursive, so both specs under spec-coverage/ are counted. Setting the input without shipping a config there would change nothing except mislead the reader.

Deliberately NOT enabled

  • enable-newman — no tests/integration/, no Postman collection anywhere. The job's own validation step hard-fails on a missing collection directory: a guaranteed red job with zero assertions.
  • enable-hydra-gates — off on purpose, not by omission. Only 1 of 17 fleet repos sets it, and gate-7 (no-admin-idor) currently false-positives on ordinary responder delegation; that is being fixed separately in fix(hydra-gates): gate-7 follows delegation; gates 6/7 stop passing on an empty scope .github#149. Turning it on here would make hrmq's first-ever pipeline red for a reason that is not hrmq's.
  • enable-axe — only consumed when hydra-gates is on (gate-33 reads tests/axe/report.json), so it would produce an artifact nothing reads. Also measured red against vanilla NC34 before any app is installed.
  • enable-playwright-coverage — needs a defensible threshold; there is no measured V8 baseline for hrmq yet.
  • enable-coverage-guard — the ratchet compares against a committed .coverage-baseline, which this repo does not have.
  • enable-journeydoc-capture — per ADR-030 belongs in its own scheduled docs-capture.yml, not on every push.

Two repo fixes this pipeline required

tests/e2e/base-url.ts — GitHub Actions exemption from FORBIDDEN_HOSTS.
The file already anticipated that the shared workflow exports the instance as BASE_URL and accepted that name — but the value it exports is literally http://localhost:8080, which the guard immediately below it rejects. Enabling Playwright without this fix would have thrown from resolveBaseURL() at config-load time, before a single test ran, with an error blaming a shared dev container that does not exist on a runner. The guard's real question is "is this host disposable"; on a GitHub runner :8080 is the run's own php -S, created and destroyed with the job. GITHUB_ACTIONS is used rather than the far broader CI, which a developer may well export in a shell that can reach the real :8080.

docs/features.json — generated, not disabled.
enable-features-extract defaults to true and the file did not exist, so that gate would have been stale on arrival. Generated from the 55 specs under openspec/specs/ with the shared scripts/extract-features.py (52 features). hrmq can satisfy this gate — it had simply never been asked to.

Pre-flight measured locally

Against a clean npm ci at this commit, before enabling anything:

  • check:manifest PASS (113 pages, schema 2.22.0)
  • check:widget-keys PASS (all 11 keys resolve, incl. 2 via a real probe build)
  • lint PASS · stylelint PASS
  • extract-features.py --check PASS

⚠️ check:widget-keys first appeared to FAIL when run from a stale local checkout — a probe-build crash inside @nextcloud/webpack-vue-config. That was an artifact of the stale tree, not the code; it passes cleanly at this commit. Worth recording because it would have been an easy reason to wrongly drop the job.

Note on counting

This branch name does not match the push: trigger patterns (main, development, feature/**, bugfix/**, hotfix/**), so the PR run is the single measurement — no duplicate. Expect ~25 jobs; a startup_failure from a YAML error would report jobs.total_count = 0 and emit no contexts, which reads exactly like success in a rollup, so the job count is checked directly.

hrmq had NO `.github/workflows/` directory at all. The runs visible in its
Actions tab — "Push on development", "PR #64", "Scheduled" — are GitHub's
DYNAMIC workflows, which have no file in the repo:

  dynamic/github-code-scanning/codeql
  dynamic/dependabot/dependabot-updates

CodeQL default setup names its runs after the trigger, which is why they read
like an app pipeline. They are not one. Until this commit, no PHPCS, PHPMD,
Psalm, PHPStan, PHPUnit, ESLint, Stylelint, license, SBOM or E2E check had
ever run against this repo, on any commit.

Three files:

1. `.github/workflows/code-quality.yml` — a caller for the shared
   `ConductionNL/.github/.github/workflows/quality.yml@main`, modelled on
   nextcloud-app-template with inputs tuned to what hrmq actually ships.
   Every enabled tool has its config committed at the repo root already
   (phpcs.xml, phpmd.xml, psalm.xml, phpstan.neon + all three baselines,
   phpunit.xml, eslint.config.js, stylelint.config.js); nothing is switched
   on speculatively. Newman, hydra-gates, axe, playwright-coverage,
   coverage-guard and journeydoc-capture are OFF, each with its reason in a
   comment at the call site.

2. `tests/e2e/base-url.ts` — exempt GitHub Actions runners from
   FORBIDDEN_HOSTS. The file already anticipated that the shared workflow
   exports the instance as `BASE_URL` and accepted that name, but the VALUE
   it exports is literally `http://localhost:8080`, which the guard below it
   then rejects. Enabling Playwright without this fix would have thrown from
   `resolveBaseURL()` at config-load time — before a single test ran, with an
   error blaming a shared dev container that does not exist on a runner. The
   guard's real question is "is this host disposable", and on a GitHub runner
   :8080 is the run's own `php -S`, created and destroyed with the job.
   `GITHUB_ACTIONS` is used rather than the much broader `CI`, which a
   developer may export in a shell that CAN reach the real :8080.

3. `docs/features.json` — generated from the 55 specs under `openspec/specs/`
   via the shared `scripts/extract-features.py` (52 features). The
   `enable-features-extract` input defaults to TRUE, and the file did not
   exist, so the gate would have been stale on arrival. Generated rather than
   disabled: hrmq can satisfy this gate, it had simply never been asked to.

Verified locally against a clean `npm ci` at this commit before enabling
anything: check:manifest PASS (113 pages, schema 2.22.0), check:widget-keys
PASS (all 11 keys resolve), lint PASS, stylelint PASS,
extract-features --check PASS.
hrmq's first-ever CI run failed all six PHPUnit legs on exactly one test out
of 981 (4004 assertions), with:

  appinfo/info.xml failed to parse via simplexml_load_file() - the same call
  Nextcloud's app:enable uses, so this makes the app uninstallable.
  Failed to load external entity because the resolver function returned null

Nine seconds EARLIER, in the same job, `occ app:enable hrmq` printed
"hrmq 0.2.0 enabled". The test asserted the app was uninstallable immediately
after the same job installed it, which is the tell: the failure was about the
call, not about the file.

Two facts make it deterministic, and neither is visible on a dev machine:

1. `OC\App\InfoParser::parse()` (lib/private/App/InfoParser.php) does
   `simplexml_load_string(file_get_contents($file))`. It has never used
   `simplexml_load_file()`, so the test's stated premise — "the same call
   Nextcloud's app:enable uses" — was simply wrong about which call that is.

2. `tests/bootstrap.php` requires the server's `lib/base.php` whenever a full
   checkout is present. Under the shared quality workflow the app is mounted
   at `server/apps/hrmq`, so it always is; base.php then hardens libxml with
   `libxml_set_external_entity_loader(static fn () => null)`. Under that
   loader `simplexml_load_file()` resolves even the PRIMARY document through
   the resolver and returns false for a perfectly valid file.
   `simplexml_load_string()` takes the bytes directly and is unaffected.

So the test passed for everyone locally (no server checkout → no hardening)
and could only ever fail once CI existed — reporting the precise opposite of
what the run had already demonstrated.

Fixed by mirroring InfoParser exactly. Reproduced and verified under the same
null entity loader:

  simplexml_load_file()   => FALSE  "Failed to load external entity"
  simplexml_load_string() => OK     (id=hrmq)

Positive control, because a passing assertion is only evidence about the
assertion until it has been shown it can still fail — under the SAME hardened
loader, the fixed parse still rejects the exact defect the docblock was
written for (a literal `--` inside an XML comment):

  real info.xml             => OK    (test passes)
  malformed (-- in comment) => FALSE (test fails: "Double hyphen within comment")

No test was disabled and no CI input was weakened to get this green.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Quality Report — ConductionNL/hrmq @ 1c76b70

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-widget-keys
composer ✅ 99/99
npm ✅ 813/813
PHPUnit
Newman ⏭️
Playwright

Quality workflow — 2026-08-04 11:18 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

First run measured — run 30903169627

32 jobs emitted. Counted from actions/runs/.../jobs, not inferred from the rollup — a startup_failure would have reported jobs.total_count = 0 and emitted no contexts, which reads exactly like success. It did not; this is a real run.

22 success · 8 failure · 2 skipped (plus 6 further intentional skips inside that count).

result jobs
✅ success (22) PHP Quality ×6 (lint, phpcs, phpmd, psalm, phpstan, phpmetrics) · Vue Quality ×2 (eslint, stylelint) · Frontend Build · Frontend Tests (unit) · Frontend Check (check:manifest) · Frontend Check (check:widget-keys) · Security ×2 · License ×2 · Features Check
❌ failure (8) PHPUnit ×6 (8.3/8.4 × stable31/32/33) · E2E Tests (Playwright) · Quality Report (rollup of the above)
⏭️ skipped Integration Tests (Newman), Journeydoc Capture, Coverage Baseline Protection, Coverage Baseline Check, Hydra Gates — all deliberately off. Features Extract + SBOM are push-only and correctly skipped on a PR.

Every PHP and frontend static-analysis gate passed on hrmq's first exposure to CI. That is a genuinely good result for a repo that has never had any.


Failure 1 — PHPUnit ×6: a FALSE RED, now fixed (b0f894d)

981 tests, 4004 assertions, exactly 1 failure, identical on all six legs:

1) OCA\Hrmq\Tests\Unit\AppInfoTest::testInfoXmlIsWellFormedXml
appinfo/info.xml failed to parse via simplexml_load_file() - the same call
Nextcloud's app:enable uses, so this makes the app uninstallable.
Failed to load external entity because the resolver function returned null (line 0)

Nine seconds earlier, in the same job, occ app:enable hrmq printed hrmq 0.2.0 enabled. The test asserted the app was uninstallable immediately after the same job installed it. That contradiction is the tell — the failure was about the call, not the file.

Two facts, neither visible on a dev machine:

  1. OC\App\InfoParser::parse() uses simplexml_load_string(file_get_contents($file)). It has never used simplexml_load_file() — so the test's stated premise ("the same call Nextcloud's app:enable uses") was factually wrong about which call that is.
  2. tests/bootstrap.php requires the server's lib/base.php whenever a full checkout is present — always true under this workflow, where the app is mounted at server/apps/hrmq. base.php then calls libxml_set_external_entity_loader(static fn () => null). Under that loader simplexml_load_file() resolves even the primary document through the resolver and returns false for a perfectly valid file.

So the test passed for everyone locally (no server checkout → no hardening) and could only ever fail once CI existed. Reproduced under the same null loader:

simplexml_load_file()   => FALSE  "Failed to load external entity"
simplexml_load_string() => OK     (id=hrmq)

Fixed by mirroring InfoParser exactly. Positive control — under the same hardened loader the fixed parse still rejects the exact defect the docblock was written for:

real info.xml             => OK    (passes)
malformed (-- in comment) => FALSE (fails: "Double hyphen within comment")

Not a dead gate. No test disabled, no CI input weakened.

Fleet check: searched the org three independent ways (gh search code for simplexml_load_file / testInfoXmlIsWellFormedXml, enumeration of tests/Unit/ across all 75 PHP repos, and per-repo search across the 20 NC-app repos), with the known hrmq hit as a positive control first. hrmq is the only repo with this test. 17 other repos load base.php and run PHPUnit against a server checkout, so they are latentnextcloud-app-template most of all, since it is the fleet's copy source.


Failure 2 — E2E (Playwright): RED, and legitimately so

68 failed, 2 passed, 10.1 min (cap is 45, so this is a real verdict, not a timeout, and not a cancellation).

Deliberately left enabled and left red. The job is not misconfigured and the suite is not broken — the CI instance is simply missing hrmq's data.

The suite diagnosed itself:

Could not resolve the hrmq Employee schema on http://localhost:8080 — none of
employee, Employee, hrmq_employee answered 200 on
.../apps/openregister/api/objects/hrmq/<schema>.
Is the hrmq register installed in OpenRegister?

It is not. occ app:enable hrmq exits 0, but the register import runs from lib/Repair/InitializeRegister.php — an IRepairStep, which has no user session, so OpenRegister's RBAC denies it and the step swallows the exception as a warning. Enable succeeds; nothing is imported.

The failure shape confirms it exactly. Every failing page redirects to the same place:

Expected substring: "/mijn/doelen"
Received string:    "/index.php/apps/hrmq/timesheets"

/timesheets is the default route. With no register, every schema-driven manifest page falls back to it. And the only browser test that passed is [index] Timesheets mounts at /timesheets — the one whose expected URL happens to be the fallback destination. (The other pass is manifest sanity, a pure in-process assertion with no browser.) So the two greens are not evidence the app works; they are what "everything falls back to the default" looks like from the outside.

Required follow-up, not a config tweak: hrmq needs a ci-seed.sh that performs the register import over the admin HTTP API and then verifies the register + schema slugs, wired in as playwright-seed-command — exactly the route larpingapp took for the identical IRepairStep/RBAC problem. That is real work and belongs in its own PR; tuning an input cannot fix it.

Two things I did not do, on purpose: I did not set enable-playwright: false, and I did not weaken the specs. A skipped E2E job reports skipped, which reads exactly like a job with nothing to complain about — strictly worse than a red one that names its cause.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Correction to the tally above

I wrote "22 success · 8 failure · 2 skipped" in the previous comment. That was wrong — I tallied by hand instead of counting. The actual figures, from actions/runs/30903169627/jobs:

32 jobs — 17 success · 8 failure · 7 skipped.

The failure and skip lists in the table above were correct; the success count was not (17, not 22). The 7 skips are: Newman, Journeydoc Capture, Coverage Baseline Protection, Coverage Baseline Check, Hydra Gates (all deliberately off) plus Features Extract and SBOM (push-only, correctly skipped on a PR). Nothing else changes — the analysis of both failures stands.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Quality Report — ConductionNL/hrmq @ a69253e

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-widget-keys
composer ✅ 99/99
npm ✅ 813/813
PHPUnit
Newman ⏭️
Playwright

Quality workflow — 2026-08-04 11:34 UTC

Download the full PDF report from the workflow artifacts.

…stall

hrmq's first CI E2E run was 68 failed / 2 passed, and both "passes" were the
default route (/timesheets) — the failure mode and the success signal were the
same page. The suite named the cause itself: "Is the hrmq register installed in
OpenRegister?" It was not, for two independent reasons, both fixed here.

1. appinfo/info.xml declared the register-import repair step under
   <post-migration> ONLY. Nextcloud's Installer::installAppLastSteps() guards
   both the pre- and post-migration blocks with `if ($previousVersion !== '')`,
   so on a fresh install neither runs — only repair-steps/install is
   unconditional. `occ app:enable hrmq` printed "hrmq 0.2.0 enabled" and not one
   line of InitializeRegister's own output. Adds an <install> block.

2. lib/Settings/hrmq_register.json declared no components.registers section at
   all. OpenRegister's ImportHandler creates registers from that key and nowhere
   else, so even a repair step that DID run would have created 54 schemas and
   zero registers — and then skipped all 118 seed objects, whose @self.register
   is resolved through the map that section populates. Declares the register
   with all 54 fragment schema slugs, and bumps info.version 0.13.0 -> 0.14.0.

tests/Unit/Settings/RegisterDeclarationTest.php pins both: the register exists
with the slug the manifest's 176 page configs use, its version tracks
info.version (OpenRegister skips a register import whose version is not newer,
so a frozen version would never pick up later schemas), and its schema list is
EXACTLY the union of the register.d fragments — a register listing only some of
them is the same silent partial outage, one schema at a time. Verified as a
positive control: 3/3 fail against the pre-change file, 3/3 pass after.

tests/e2e/ci-seed.sh + playwright-seed-command make it a GATE rather than a
hope. A repair step cannot gate: InitializeRegister catches \Throwable and
app:enable exits 0 either way, and it runs with no user session while
OpenRegister main (which this workflow pins) calls importFromApp() without the
SystemOperationContext wrapper. The script imports over the admin HTTP API,
reproducing SettingsService::deepMergeConfig() so the register.d fragments are
merged the same way, then verifies the register, all 54 schema slugs and the
object collections the specs read. It logs whether the register was already
present BEFORE it acted — the only evidence that fixes 1 and 2 stand on their
own — and refuses to conclude anything from a non-200 status or a page returned
exactly at the limit, because a truncated list and a missing slug look identical.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Quality Report — ConductionNL/hrmq @ 6d30da0

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-widget-keys
composer ✅ 99/99
npm ✅ 813/813
PHPUnit
Newman ⏭️
Playwright

Quality workflow — 2026-08-04 14:48 UTC

Download the full PDF report from the workflow artifacts.

…e shell

With the register provisioned, run 30919961510 (E2E job 92028085860) was still
67 failed / 3 passed — and every failure read the same:

    Expected substring: "/employees"
    Received string:    "/index.php/apps/hrmq/timesheets"

Cause: both specs addressed the wrong base.

`src/main.js` builds the router with `createWebHistory(generateUrl('/apps/hrmq'))`.
`generateUrl` returns `/apps/hrmq` with the front controller inactive and
`/index.php/apps/hrmq` with it active — so the base is not a constant.

  * core-journeys.spec.ts hardcoded `/apps/hrmq`.
  * manifest-pages.spec.ts probed the two candidates and took the first that
    SERVED THE SHELL — a different question. Under `php -S` BOTH forms reach
    index.php (nextcloud.log: `"url":"/apps/hrmq/dsr-requests","scriptName":"/index.php"`),
    so the probe always matched `/apps/hrmq` while `generateUrl` returned the
    `/index.php` form.

Either way every deep link landed OUTSIDE the router base, matched no route, hit
main.js's `/:pathMatch(.*)*` catch-all and redirected to the default page. That
is why 67 of 70 tests failed while the app itself was fine, and why the only
test that "passed" was the redirect target — the failure mode and the success
signal were the same URL.

Both specs now read `OC.generateUrl('/apps/hrmq')` out of the live page: literally
the call main.js passes to createWebHistory, so the base cannot drift from the
router's, and an absent OC is a loud error rather than a wrong measurement.

No assertion is weakened: route identity, rendered-content and console-error
checks are unchanged and still unconditional. Nothing is skipped, no timeout is
raised, no suppression is added — the specs simply now address the router at the
base it actually uses, which is what makes them able to assert anything at all.

Also makes ci-seed.sh's HTTP import a FALLBACK. Run 30919961510 proved the app
provisions its own register on a fresh install ("BEFORE import: the hrmq register
is ALREADY present"), so the unconditional import re-posted an already-imported
configuration and answered HTTP 400 on every healthy run — training the reader to
ignore a 400 from the one step whose failure would matter if the app fix
regressed. It now runs only when the pre-probe finds no register. The merge-time
consistency check and the full verification still run every time.
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Quality Report — ConductionNL/hrmq @ 8ac2d81

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
build
check-manifest
check-widget-keys
composer ✅ 99/99
npm ✅ 813/813
PHPUnit
Newman ⏭️
Playwright

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

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 0f49dd0 into development Aug 4, 2026
34 checks passed
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