Skip to content

ci(quality): valtimo had no quality workflow at all — add one, and gate the code it actually runs - #21

Closed
rubenvdlinde wants to merge 1 commit into
developmentfrom
ci/quality-enforcement
Closed

ci(quality): valtimo had no quality workflow at all — add one, and gate the code it actually runs#21
rubenvdlinde wants to merge 1 commit into
developmentfrom
ci/quality-enforcement

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

The gap

valtimo shipped seven workflows — beta-release, build-exapp, pull-request-from-branch-check, pull-request-lint-check, push-development-to-beta, release-workflow, unstable-release — and not one of them ran a quality check. A recent PR here ran 1 check. Roughly 25 jobs did not skip; they did not exist.

An absent gate is worse than a failing one. A failing gate is a signal; an absent one is silence that reads exactly like success. Its three siblings (openklant, opentalk, openzaak) have had code-quality.yml all along.

What was actually ungated — and it isn't the PHP

phpcs.xml, psalm.xml and phpstan.neon are all aimed at phpcs-custom-sniffs/, and say so in their own comments. That is correct: this is a Python ExApp sidecar and there is no lib/. Forcing PHP tooling at a Python codebase would be the wrong fix.

What was ungated is the application: ex_app/lib/main.py, 241 lines of FastAPI fronting a Spring Boot Valtimo process. Nothing in this repo has ever looked at it.

So this adds two things:

  1. python-checks — ruff (lint + format) and mypy over ex_app/, matching the pattern n8n-nextcloud and keycloak-nextcloud already use. pyproject.toml carries the config; requirements-dev.txt pins ruff==0.16.1 and mypy==2.3.0 exactly — a floating linter changes a repo's verdict with no commit in that repo to explain it.
  2. The shared quality pipeline, same inputs as the three siblings, plus enable-hydra-gates: true.

Measured before and after, identically conditioned

check before after
ruff check ex_app/ 3 findings — I001 unsorted imports, RUF005 list concatenation, RUF010 implicit str() in an f-string All checks passed
ruff format --check ex_app/ 1 file would be reformatted 1 file already formatted
mypy ex_app/ 0 errors, 1 source file checked 0 errors, 1 source file checked
make check-strict (target did not exist) exit 0
composer check:strict exit 0 exit 0 — lint, phpcs, phpmd, psalm, phpstan
hydra-gates --base origin/development (never ran in CI) exit 0, 5 changed file(s) in scope, 27 of 63 reported, 0 failures

All three ruff findings are fixed here, so the job starts green on a real scan of a real file — not on an empty scope. The before column was re-measured against the committed HEAD after the fix, so it is a positive control: the job demonstrably fails on the code as it stood.

Note on composer check:strict: the first attempt reused a vendor/ copied from the main checkout and psalm died on Cannot resolve stubfile path vendor/nextcloud/ocp/OCP/Capabilities/ICapability.php — that tree had been mutated by something else (OCP moved aside to OCP.bak). A borrowed vendor/ is not evidence about this repo; the exit 0 above is from a fresh composer install in this worktree.

Why Hydra Gates was skipping — and it was not the Playwright dependency

enable-hydra-gates defaults to false in ConductionNL/.github/.github/workflows/quality.yml, and none of the four ExApps passed it. The job's guard is:

if: ${{ inputs.enable-hydra-gates && !cancelled() }}

It was the first term that deleted the job. !cancelled() — the part with the long comment about needs: implying success() — was already doing its job correctly. There was nothing subtle here: the input was simply never set.

hydra-gates-ref is deliberately left at main rather than pinned. A pin is a silent expiry date: the fleet pinned v1.0.1 across 22 repos, the pin predated the fixes to 16 gates, and all 16 were dead for as long as the pin stood.

hydra-gates-require-full-coverage is left at its default false: 31 of 63 gates have no subject matter in a repo with no lib/, no src/ and no manifest. Demanding full coverage would fail every PR for a condition no PR can fix.

⚠️ One of the 29 gates was passing on nothing

gate-28 (license-triangle) called _pass 28 unconditionally after a comparison that only runs inside if [ -n "${_composer_lic}" ] && [ -d lib ]. This repo has no lib/, so it printed

[gate-28] license-triangle: PASS

having opened zero files, and the coverage accounting counted it as a gate that reported a result. Fixed in ConductionNL/.github#(companion PR); with that fix it reports:

[gate-28] license-triangle: NOT APPLICABLE — this repo has no lib/ directory, so there are
no per-file @license PHPDoc tags for composer.json's license to be compared against.
Nothing was inspected and nothing could be.

and the count drops 29 → 27 diff-scoped. Until that companion PR merges, this PR's CI will still show gate-28 as a green PASS. That green is the vacuous one described above — do not read it as coverage.

No test suite, and no pretending otherwise

This repo has no automated tests: no pytest config, no test_*.py, no phpunit.xml, no tests/. composer.json's check:strict already says so at length and asks that no test script be re-added until a real suite exists. That is respected — nothing here scaffolds a suite.

The Makefile's test target is renamed to run, because it never tested anything: an interactive docker run -it that boots the container, asserts nothing, and cannot run in CI at all (-it needs a TTY). There is now no test target, so make test fails loudly instead of exiting 0 having proved nothing. check-strict prints what its green does and does not cover.

Left for you

  • valtimo also has no branch-protection.yml, which all three siblings have. Not added here — that changes who can push where, which is your call.
  • The three siblings' code-quality.yml described these as "PHP-only ExApp". They are Python ExApps. Corrected in the sibling PRs.

…te the code it actually runs

valtimo shipped seven workflows — beta-release, build-exapp,
pull-request-from-branch-check, pull-request-lint-check,
push-development-to-beta, release-workflow, unstable-release — and not one of
them ran a quality check. A recent PR here ran ONE check. Roughly 25 jobs did
not skip; they did not exist. An absent gate is worse than a failing one: a
failing gate is a signal, an absent one is silence that reads exactly like
success. Its three siblings (openklant, opentalk, openzaak) have had
code-quality.yml all along.

WHAT WAS ACTUALLY UNGATED

Not the PHP. phpcs.xml, psalm.xml and phpstan.neon are aimed at
phpcs-custom-sniffs/ and say so in their own comments, which is right: this is
a Python ExApp sidecar and there is no lib/. What was ungated is the
application — ex_app/lib/main.py, 241 lines of FastAPI that fronts a Spring
Boot Valtimo process. Nothing in this repo has ever looked at it.

So this adds two things, not one:

  1. python-checks — ruff (lint + format) and mypy over ex_app/, matching the
     pattern already used by n8n-nextcloud and keycloak-nextcloud. pyproject.toml
     carries the config; requirements-dev.txt pins ruff==0.16.1 and mypy==2.3.0
     exactly, because a floating linter changes a repo's verdict with no commit
     in that repo to explain it.

  2. The shared ConductionNL/.github quality pipeline, with the same inputs as
     the three siblings, plus enable-hydra-gates: true.

MEASURED BEFORE AND AFTER, identically conditioned

  ruff check ex_app/     BEFORE: 3 findings (I001 unsorted imports,
                                 RUF005 list concatenation,
                                 RUF010 implicit str() in an f-string)
                         AFTER:  All checks passed
  ruff format --check    BEFORE: 1 file would be reformatted
                         AFTER:  1 file already formatted
  mypy ex_app/           BEFORE: 0 errors, 1 source file checked
                         AFTER:  0 errors, 1 source file checked
  make check-strict      AFTER:  exit 0

All three ruff findings are fixed here, so the job starts green on a real scan
of a real file — not on an empty scope. The BEFORE numbers were re-measured
against the committed HEAD after the fix, so they are a positive control: the
job demonstrably fails on the code as it stood.

  composer check:strict  exit 0 (lint, phpcs, phpmd, psalm, phpstan)

Run in a clean container with a fresh `composer install` IN this worktree. The
first attempt reused a vendor/ copied from the main checkout and psalm died on
"Cannot resolve stubfile path vendor/nextcloud/ocp/OCP/Capabilities/
ICapability.php" — that vendor tree had been mutated by something else
(OCP moved aside to OCP.bak). A borrowed vendor/ is not evidence about this
repo.

WHY HYDRA GATES WAS SKIPPING — and it was not the Playwright dependency

`enable-hydra-gates` defaults to false in the shared workflow and none of the
four ExApps passed it. The job's guard is
`if: inputs.enable-hydra-gates && !cancelled()`, so it was the FIRST term that
deleted the job. `!cancelled()` was already doing its job correctly.

Measured with a --full scan of the whole tree before switching it on:
29 of 63 gates reported, 0 failures. Switched on here.

hydra-gates-ref is deliberately left at `main` rather than pinned. A pin is a
silent expiry date — the fleet pinned v1.0.1 across 22 repos, the pin predated
the fixes to 16 gates, and all 16 were dead for as long as the pin stood.

NO TEST SUITE, AND NO PRETENDING OTHERWISE

This repo has no automated tests: no pytest config, no test_*.py, no
phpunit.xml, no tests/. composer.json's check:strict already says so at
length and asks that no test script be re-added until a real suite exists.
That is respected — nothing here scaffolds a suite.

The Makefile's `test` target IS renamed to `run`, because it never tested
anything: it is an interactive `docker run -it` that boots the container,
asserts nothing, and cannot run in CI at all (-it needs a TTY). There is now no
`test` target, so `make test` fails loudly instead of exiting 0 having proved
nothing. check-strict prints what its green does and does not cover.
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Companion gate-28 fix: ConductionNL/.github#172license-triangle reported PASS on repos with no lib/, having opened zero files. Until #172 merges, this PR's Hydra Gates output will still show [gate-28] license-triangle: PASS; that is the vacuous pass, not coverage. With #172 it reads NOT APPLICABLE and the diff-scoped count drops 28 → 27.

@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Closing: out of scope for now — the ExApp sidecar wrappers are not part of the 16 Nextcloud apps this sweep covers. Findings are recorded in the agent report; the branch ci/quality-enforcement is left in place (not deleted) so this can be reopened as-is.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Quality Report — ConductionNL/valtimo @ 2a854cf

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint ⏭️
stylelint ⏭️
build ⏭️
composer ✅ 69/69
npm
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️
Hydra gates

Quality workflow — 2026-08-05 21:06 UTC

Download the full PDF report from the workflow artifacts.

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