From 8eda634f8f6aadf5fc5178ac4d5844e464c62f03 Mon Sep 17 00:00:00 2001 From: Conduction Release Bot Date: Wed, 5 Aug 2026 22:53:45 +0200 Subject: [PATCH] =?UTF-8?q?ci(quality):=20valtimo=20had=20no=20quality=20w?= =?UTF-8?q?orkflow=20at=20all=20=E2=80=94=20add=20one,=20and=20gate=20the?= =?UTF-8?q?=20code=20it=20actually=20runs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/code-quality.yml | 133 +++++++++++++++++++++++++++++ Makefile | 77 ++++++++++++++++- ex_app/lib/main.py | 21 +++-- pyproject.toml | 56 ++++++++++++ requirements-dev.txt | 9 ++ 5 files changed, 283 insertions(+), 13 deletions(-) create mode 100644 .github/workflows/code-quality.yml create mode 100644 pyproject.toml create mode 100644 requirements-dev.txt diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml new file mode 100644 index 0000000..8941b95 --- /dev/null +++ b/.github/workflows/code-quality.yml @@ -0,0 +1,133 @@ +name: Code Quality + +# This file did not exist until 2026-08-05. 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 PR here ran +# ONE check. Roughly 25 jobs did not skip; they did not exist. That is worse +# than a failing gate: a failing gate is a signal, an absent one is silence +# that reads exactly like success. +# +# Its three siblings (openklant, opentalk, openzaak) already had this file. +# This is that file, adapted to what valtimo actually is, plus the two things +# none of the four had: the Python checks, and Hydra Gates switched on. + +on: + push: + branches: [main, beta, development, feature/**, bugfix/**, hotfix/**] + pull_request: + types: [opened, reopened] + branches: [main, beta, development] + workflow_dispatch: + +concurrency: + group: quality-${{ github.head_ref || github.ref_name }} + cancel-in-progress: true + +# Permission CEILING for the called quality pipeline. GitHub statically +# validates the called workflow's declared job permissions against this +# grant — even for jobs that are disabled — so it must cover the maximum +# any nested job declares: journeydoc-capture (contents+actions write), +# update-baseline / features-extract (contents write), and the Quality +# Report PR comment (issues / pull-requests write). +permissions: + contents: write + actions: write + issues: write + pull-requests: write + +jobs: + # ── The application itself ──────────────────────────────────────────────── + # valtimo is a Python ExApp sidecar. The application is ex_app/lib/main.py. + # Nothing in this repo has ever looked at it: phpcs.xml / psalm.xml / + # phpstan.neon are all pointed at phpcs-custom-sniffs/, which is correct for + # what they are, and leaves the actual app entirely ungated. + # + # Measured on 2026-08-05, first run of these checks against this repo: + # ruff found 3 lint findings (I001 unsorted imports, RUF005 list + # concatenation, RUF010 implicit str() in an f-string) plus reformatting; + # mypy found 0. All fixed in the commit that added this file, so this job + # starts green on a real scan of the file — not on an empty scope. + python-checks: + name: ${{ matrix.check.name }} + runs-on: ubuntu-latest + # Observed locally at well under a minute; bounded so a hung job cannot + # sit until the 6h default and be reported as "still running". + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + check: + - { name: "Ruff Lint", command: "ruff check ex_app/" } + - { name: "Ruff Format", command: "ruff format --check ex_app/" } + - { name: "Mypy", command: "mypy ex_app/" } + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + # Matches [project].requires-python and [tool.ruff].target-version in + # pyproject.toml, which are in turn measured from the Dockerfile: + # eclipse-temurin:17-jre-jammy + apt python3 == 3.10. + python-version: "3.10" + + - name: Install quality tooling + # requirements-dev.txt pins ruff and mypy exactly. requirements.txt is + # installed too so mypy resolves fastapi/httpx for real rather than + # falling back to ignore_missing_imports and checking less than it + # appears to. + run: | + python -m pip install --upgrade pip + pip install -r requirements-dev.txt + pip install -r requirements.txt + + - name: ${{ matrix.check.name }} + run: ${{ matrix.check.command }} + + # ── The shared Conduction quality pipeline ──────────────────────────────── + quality: + if: github.event_name != 'push' || github.event.created != true + uses: ConductionNL/.github/.github/workflows/quality.yml@main + with: + app-name: valtimo + # composer.json pins config.platform.php to 8.3 + php-version: "8.3" + # No package.json in this repo, so all npm-side checks are off + # (enable-npm gates the npm legs of security/license; enable-frontend + # gates Vue Quality and custom frontend checks). + enable-npm: false + enable-frontend: false + # The SBOM job invokes `composer CycloneDX:make-sbom`, which this repo + # does not ship — enable once cyclonedx/cyclonedx-php-composer is added + # to require-dev. + enable-sbom: false + # No openspec/specs and no docs/features.json yet — the features check + # would fail on every PR comparing "" against "[]". + enable-features-extract: false + # Hydra Gates was never evaluated in CI for any of the four ExApp + # sidecars, for a boring reason: `enable-hydra-gates` defaults to false + # and none of them passed it. The job's `if:` is + # `inputs.enable-hydra-gates && !cancelled()`, so it was the FIRST term + # that deleted it, not the Playwright dependency. + # + # Measured before switching it on (--full scan of the whole tree, + # 2026-08-05): 29 of 63 gates reported, 0 failures. So this starts green + # honestly. The COVERAGE line is the real output here — most of the + # suite is PHP/Vue/Nextcloud-shaped and has no subject matter in a + # Python sidecar, and the gates say so by name rather than passing + # quietly. + enable-hydra-gates: true + # Deliberately NOT pinned to a tag. 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 every one of those gates was dead for as long as the pin + # stood. This repo's gate surface is small and its diffs are tiny, so + # tracking main costs little and inherits gate fixes the day they land. + hydra-gates-ref: main + # Left at its default (false) on purpose. 34 of the 63 gates have no + # subject matter in a repo with no lib/, no src/ and no manifest; + # demanding full coverage here would fail every PR for a condition no + # PR can fix. The coverage block prints either way, which is the part + # that matters. + # hydra-gates-require-full-coverage: false diff --git a/Makefile b/Makefile index 51d098b..d5ee2f1 100644 --- a/Makefile +++ b/Makefile @@ -1,17 +1,41 @@ -.PHONY: build push test clean +.PHONY: help build push run clean lint format lint-fix format-fix mypy check check-strict APP_ID = valtimo REGISTRY = ghcr.io IMAGE = conductionnl/$(APP_ID)-exapp VERSION ?= latest +help: + @echo "$(APP_ID) ExApp" + @echo "" + @echo " make build Build the Docker image" + @echo " make push Push it to $(REGISTRY)" + @echo " make run Run the container locally (interactive; asserts nothing)" + @echo " make clean Remove the local image" + @echo "" + @echo " make lint ruff check ex_app/" + @echo " make format ruff format --check ex_app/" + @echo " make mypy mypy ex_app/" + @echo " make check lint + mypy" + @echo " make check-strict lint + format + mypy" + @echo "" + @echo "There is NO 'make test' target. This repo has no automated test" + @echo "suite of any kind, and a target that pretends otherwise is worse" + @echo "than its absence. 'make run' is what used to be called 'make test':" + @echo "an interactive 'docker run -it' that boots the container and" + @echo "asserts nothing." + build: docker build -t $(REGISTRY)/$(IMAGE):$(VERSION) . push: build docker push $(REGISTRY)/$(IMAGE):$(VERSION) -test: +# Renamed from `test`. It never tested anything — it starts the container +# interactively and makes no assertion, and cannot run in CI at all (-it needs +# a TTY). Calling that `test` is the same defect as `|| echo skipping`: a +# command whose name claims a verdict it never reaches. +run: docker run --rm -it \ -e APP_ID=$(APP_ID) \ -e APP_VERSION=0.1.0 \ @@ -22,3 +46,52 @@ test: clean: docker rmi $(REGISTRY)/$(IMAGE):$(VERSION) || true + +# ── Python quality ────────────────────────────────────────────────────────── +# The application is ex_app/lib/main.py. Until 2026-08-05 nothing in this repo +# looked at it: the static-analysis stack (phpcs/psalm/phpstan/phpmd) is aimed +# at phpcs-custom-sniffs/, and there was no CI workflow running any of it. +# Install the tools with: pip install -r requirements-dev.txt + +lint: + ruff check ex_app/ + +format: + ruff format --check ex_app/ + +lint-fix: + ruff check --fix ex_app/ + +format-fix: + ruff format ex_app/ + +mypy: + mypy ex_app/ + +check: + @E=0; \ + for CMD in lint mypy; do \ + echo; echo "=== $$CMD ==="; \ + $(MAKE) --no-print-directory $$CMD || E=1; \ + done; \ + echo; \ + if [ $$E -eq 0 ]; then echo "ALL CHECKS PASSED"; else echo "SOME CHECKS FAILED (see above)"; fi; \ + exit $$E + +check-strict: + @E=0; \ + for CMD in lint format mypy; do \ + echo; echo "=== $$CMD ==="; \ + $(MAKE) --no-print-directory $$CMD || E=1; \ + done; \ + echo; \ + if [ $$E -eq 0 ]; then \ + echo "ALL CHECKS PASSED - STATIC ANALYSIS ONLY."; \ + echo "This green covers ruff (lint + format) and mypy over ex_app/."; \ + echo "It says NOTHING about behaviour: this repo has no automated test"; \ + echo "suite - no pytest config, no test_*.py, no phpunit.xml, no tests/."; \ + echo "Do not add a test target until a real suite exists."; \ + else \ + echo "SOME CHECKS FAILED (see above)"; \ + fi; \ + exit $$E diff --git a/ex_app/lib/main.py b/ex_app/lib/main.py index 92752d2..26bd1e2 100644 --- a/ex_app/lib/main.py +++ b/ex_app/lib/main.py @@ -4,14 +4,15 @@ Valtimo is a less-code platform for Business Process Automation. See: https://docs.valtimo.nl/ """ -import os -import subprocess + import asyncio import base64 +import os +import subprocess from contextlib import asynccontextmanager import httpx -from fastapi import FastAPI, Request, BackgroundTasks +from fastapi import BackgroundTasks, FastAPI, Request from fastapi.responses import JSONResponse, Response # Environment variables set by AppAPI @@ -92,7 +93,7 @@ def start_valtimo() -> None: print(f"OIDC configured with Keycloak at {KEYCLOAK_URL}") # Start Valtimo (Spring Boot JAR) - cmd = ["java"] + java_opts + ["-jar", "/app/valtimo.jar"] + cmd = ["java", *java_opts, "-jar", "/app/valtimo.jar"] VALTIMO_PROCESS = subprocess.Popen( cmd, env=env, @@ -167,6 +168,7 @@ async def heartbeat(): @app.post("/init") async def init(background_tasks: BackgroundTasks): """Initialization endpoint called by AppAPI during deployment""" + async def do_init(): await report_status(0) print("Starting Valtimo initialization...") @@ -213,10 +215,7 @@ async def proxy(request: Request, path: str): method=request.method, url=url, content=await request.body(), - headers={ - k: v for k, v in request.headers.items() - if k.lower() not in ("host", "content-length") - }, + headers={k: v for k, v in request.headers.items() if k.lower() not in ("host", "content-length")}, params=request.query_params, timeout=60, ) @@ -225,17 +224,17 @@ async def proxy(request: Request, path: str): content=resp.content, status_code=resp.status_code, headers={ - k: v for k, v in resp.headers.items() - if k.lower() not in ("content-encoding", "transfer-encoding") + k: v for k, v in resp.headers.items() if k.lower() not in ("content-encoding", "transfer-encoding") }, ) except httpx.RequestError as e: return JSONResponse( - {"error": f"Proxy error: {str(e)}"}, + {"error": f"Proxy error: {e!s}"}, status_code=502, ) if __name__ == "__main__": import uvicorn + uvicorn.run(app, host=APP_HOST, port=APP_PORT) diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..24558fb --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,56 @@ +[project] +name = "valtimo-exapp" +version = "0.1.0" +description = "Valtimo BPM/case-management ExApp for Nextcloud" +license = "EUPL-1.2" +# Measured from the Dockerfile, not guessed: the runtime is +# `eclipse-temurin:17-jre-jammy` + `apt-get install python3`, and jammy's +# python3 is 3.10. Targeting py311 here would let ruff's pyupgrade rules +# rewrite this file into syntax the container cannot execute. +requires-python = ">=3.10" + +[tool.ruff] +target-version = "py310" +line-length = 120 +src = ["ex_app"] + +[tool.ruff.lint] +select = [ + "E", # pycodestyle errors + "W", # pycodestyle warnings + "F", # pyflakes + "I", # isort + "N", # pep8-naming + "UP", # pyupgrade + "B", # flake8-bugbear + "A", # flake8-builtins + "C4", # flake8-comprehensions + "SIM", # flake8-simplify + "TCH", # flake8-type-checking + "RUF", # ruff-specific rules + "C90", # mccabe complexity +] +ignore = [ + "E501", # line too long (handled by the formatter) + "B008", # function call in argument default — the FastAPI Depends() idiom +] + +[tool.ruff.lint.mccabe] +max-complexity = 15 + +[tool.ruff.format] +quote-style = "double" +indent-style = "space" +skip-magic-trailing-comma = false + +[tool.mypy] +python_version = "3.10" +warn_return_any = true +warn_unused_configs = true +disallow_untyped_defs = false +check_untyped_defs = true +ignore_missing_imports = true + +[[tool.mypy.overrides]] +module = ["httpx.*", "fastapi.*", "uvicorn.*"] +ignore_missing_imports = true diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..8494c6b --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1,9 @@ +# Quality tooling for the Python ExApp wrapper in ex_app/. +# +# Pinned exactly, on purpose. A floating linter adds rules on its own schedule +# and turns CI red with no commit in this repo to explain it; an exact pin makes +# every verdict change a reviewable diff. Bump deliberately. +# +# These are the versions the checks were first measured with (2026-08-05). +ruff==0.16.1 +mypy==2.3.0