Release: merge development into beta - #18
Open
github-actions[bot] wants to merge 1324 commits into
Open
Conversation
This was referenced Apr 18, 2026
…on-save-validation
…ion source=bag save-path validation' (#210) from wip/bag-location-save-validation into development
…ed views, version history Bumps @conduction/nextcloud-vue to 1.0.0-beta.211 (published fresh — beta.210 predated PR #215/#216) and enables/configures the four just-merged nc-vue leaf features per ADR-Leaf-First: - Saved views: allowSavedViews on Cases, Bezwaren, Tasks, Voorstellen, Advice, Beroepen. - Multi-column sort: rides CnIndexPage/CnDataTable automatically, no procest-side sort code exists to touch. - Notes @mention: new CaseNotesTab.vue sidebar tab (wraps the library's CnNotesTab via leafTab('notes')) forwards the mention event to a new POST /api/notes/mention endpoint, which dispatches real NC notifications via a new MentionNotificationService + Notifier. - Version history: new VersionHistoryLeafTab registry entry (leafTab('version-history')) wired as a sidebar tab beside "audit" on all 21 detail pages.
Moves the completed change to openspec/changes/archive/2026-07-14-ncvue-w2-leaves-adoption/ and syncs its spec into the canonical openspec/specs/ncvue-w2-leaves-adoption/.
… sort, mentions, saved views, version history' (#211) from wip/ncvue-w2-leaves-adoption into development
…ugd taakveld-6 refinement Resolves the known limitation iv3-case-cost-reporting shipped: the taakveld list carried the pre-2023 code set. iv3_taakvelden.json bumps to iv3-bbv-v2, adding the 18 official 2023-refinement codes (6.71a-d, 6.72a-d, 6.73a-c, 6.74a-c, 6.81a-b, 6.82a-b) sourced directly from the Rijksoverheid Iv3-Informatievoorschrift 2023 1.0 PDF, marking their 4 pre-2023 parent codes (6.71/6.72/6.81/6.82) deprecated (still resolvable) and relabeling 6.2/6.4 per the same document. Iv3TaakveldList gains isDeprecated()/aggregationKeyFor()/geldigVanaf(); Iv3ReportService buckets quarterly reports by aggregation key so a case tagged with a deprecated code and a case tagged with one of its 2023 successors land in the same bucket, keeping trend reporting continuous across the transition.
…ld-2023-refinement
… 2023 BBV/Iv3 Wmo/Jeugd taakveld-6 refinement' (#212) from wip/iv3-taakveld-2023-refinement into development
The 15 "IRootFolder resolution gap" failures reported against LibresignSigningAdapterTest + CaseEmailServiceTest after PR #201 were not a code regression. Bisecting merges #202-#211 and rerunning both files against each candidate showed no failure — the actual cause was a contaminated vendor/ directory: nextcloud/ocp's post-install step symlinks OCP/ -> /var/www/html/lib/public when it runs inside a live NC dev container, and that symlink was rsync'd into a bare php:8.3-cli test container that has no such path. composer install silently no-ops on an already-"satisfied" vendor/, so the dangling symlink survived, and OCP\EventDispatcher\Event (needed by the Decidesk event stubs tests/bootstrap.php loads) failed to resolve — surfacing 100+ lines deep as a misleading "Class ... not found" that multiple agents mistook for an IRootFolder-related regression. A clean `rm -rf vendor && composer install` reproduces the CI recipe exactly and both files (and the full 1346-test suite) pass with zero code changes. This adds a guard to tests/bootstrap.php that detects the dangling symlink immediately after autoloading and exits with an actionable diagnostic, instead of letting it masquerade as an unrelated class-resolution bug.
… masqueraded as IRootFolder regression' (#213) from wip/fix-dev-test-failures into development
…subsidie vaststelling
Closes the follow-up iv3-case-cost-reporting documented: settled subsidie
amounts never reached case.kosten, under-stating the quarterly IV3 report.
VaststellingService::finalize() now appends an idempotent kosten entry
({bedrag, type: subsidy_disbursement, datum, source: subsidie_vaststelling,
vaststellingId}) to the case linked via subsidieUitvoering ->
subsidieAanvraag -> case, through the same ObjectService::saveObject()
write path as the vaststelling patch itself. Fail-soft on every
unresolvable hop (subsidieAanvraag.case is optional/SET_NULL): settling
never fails because of the cost enrichment; re-finalizing the same
vaststelling never duplicates the entry.
Iv3ReportService counts subsidy_disbursement toward totalCosts (never
leges income). case.kosten description updated + case schema 1.8.0 ->
1.9.0 (the field is a free-form JSON string; no JSON-schema enum exists
to extend — verified at HEAD). Also adds the 5 missing @SPEC tags on
VaststellingService's pre-existing public methods.
…ettlement-case-costs
…te case.kosten from subsidie vaststelling' (#214) from wip/subsidie-settlement-case-costs into development
… to Hermiq
Thin consumer of Hermiq's case-assistant-surface (hermiq#67): procest
carries NO LLM/prompt logic (fleet rule: AI lives in Hermiq).
- HermiqAssistantClient: single HTTP boundary to POST
/apps/hermiq/api/assistant/converse via IClientService, feature-gated on
IAppManager::isEnabledForUser('hermiq'), service-account Basic Auth,
http_errors:false so Hermiq's 400/403/404/422(guardrail_blocked)/503
mapping is relayed via HermiqAssistantException (status + errorCode).
- CaseAssistantService: fail-closed enrichment — the case loads through the
standard OR read path; OR-unavailable / unknown / unreadable all map to
ONE indistinguishable 404 before any Hermiq call (hermiq#57 lesson:
defaulting open on service failure is fail-open). Context is a WHITELIST
of CaseDetail-widget fields only (no documents/contacts/initiator PII),
description mb-truncated. Per-(user,case) session continuity via IConfig.
- Every exchange audited through the existing AiService sink
(recordAssistantAuditEntry -> recordAuditEntry) so /api/ai/audit and the
audit export cover the conversational surface too.
- UI: CaseAssistantPanel mounted for real on the manifest CaseDetail page
(custom widget + layout cell + widget-case-assistant slot -> registry),
availability-gated: renders nothing when Hermiq is absent. NC CSS vars.
- l10n en+nl pairs (English keys); vitest for the pure panel helpers.
- openspec change archived as 2026-07-14-case-assistant-via-hermiq; @SPEC
tags target the canonical openspec/specs/ home.
…hat panel delegating to Hermiq' (#215) from wip/case-assistant-via-hermiq into development
…tion lookup seams Adds the remaining two Dutch base-registration adapters, completing procest's BRP/BAG/BRK/HR/WOZ coverage, mirroring the just-merged BAG adapter pattern exactly (config-tier mode log|test|live, normalized DTO + one mapper each, controllers + routes mirroring BAG, not-configured graceful 200 shape, frontend fetch shims, dormant-by-default via IntegrationMode). BRK (Kadaster Haal Centraal BRK Bevragen API v2, X-Api-Key): lookup by kadastrale aanduiding (gemeentecode + sectie + perceelnummer + optional appartementsrecht volgnummer) and by kadastraalOnroerendeZaak id; DTO carries kadastrale gemeente/sectie/perceelnummer/oppervlakte/soortCultuurBebouwd, zakelijk-gerechtigden as REFERENCES only (no inline personal data), geo centroid. WOZ (Kadaster Haal Centraal WOZ Bevragen API, X-Api-Key): lookup by postcode+huisnummer, by BAG nummeraanduiding id (preferred composition path - no BAG address-search duplication), and by wozobjectnummer; DTO carries wozobjectnummer, most-recent vastgestelde waarde + waardepeildatum, grondoppervlakte, gebruiksdoel, nummeraanduidingId. Deliberately NOT bound to the public WOZ-waardeloket, which has no programmatic API (web-only viewer) - documented in design.md Decision 2. Tests: 66 PHPUnit (adapter request-building/validation/error-mapping, mapper normalization matrix, offline contract lanes, controller 400/401/200-graceful), 11 vitest shims. Full suite green, phpcs/phpmd/psalm clean on diff, vitest 279 green, build exit 0, l10n en/nl parity.
…s (Kadaster BRK/WOZ Bevragen)' (#216) from wip/brk-woz-register-adapters into development
Ships the DMN decision-table capability procest's README listed as roadmap:
domain experts configure permit/eligibility rules without a developer.
Model: OR schema `decisionTable` (register.d fragment) — inputs, outputs,
rules (positional input/output entries), hit policy. Definitions are data;
evaluation is a pure, deterministic, exhaustively-tested engine.
Engine (lib/Service/Dmn): ExpressionEvaluator is a SAFE bounded FEEL-subset
DSL (comparisons, ranges [1..10], sets in(a,b), booleans, wildcard -) with
NO php eval — every branch is a fixed parse + comparison. DecisionEngine
implements UNIQUE/FIRST/COLLECT in full; PRIORITY/ANY rejected with a typed
error. Errors (unknown/missing input, type mismatch, no/ambiguous match)
surface as typed exceptions, never a silent default.
REST: POST /api/decisions/{id}/evaluate (any authenticated user) + admin-
gated CRUD, following KccRoutingController.
Workflow hook (NOT orphaned): EvaluateDecisionHandler implements the
existing ActionHandlerInterface and is registered in ActionHandlerRegistry,
so a transition's automaticActions[] can invoke a decision by key and write
its outputs onto the case — proven end-to-end by EvaluateDecisionHandlerTest.
UI: DecisionTablesTab (list + structured header fields + JSON editor with
client-side structural validation), wired into the admin settings shell.
Tests: 64 PHPUnit (grammar matrix, hit policies, CRUD auth, evaluate
endpoint, workflow-hook end-to-end) + 14 vitest. Full unit suite green
(1437). l10n en+nl parity. NO new composer/npm deps.
Follow-up (documented): DMN-XML import/export, PRIORITY/ANY, full grid editor.
… workflow hook, editor UI' (#217) from wip/dmn-decision-tables into development
Archive the completed dmn-decision-tables change to openspec/changes/archive/2026-07-14-dmn-decision-tables/ and sync its delta into the canonical openspec/specs/dmn-decision-tables/spec.md (9 requirements). Repoint every code @SPEC anchor from the now-archived change dir to the canonical spec path (change dirs evaporate on archive; @SPEC must target openspec/specs/). Docblock/comment-only — no runtime change; 64 DMN PHPUnit tests + phpcs remain green.
… engine
Adds a second, adaptive process-handling model (CMMN — Case Management Model
and Notation) as a sibling to procest's structured BPMN-style workflow engine,
for government cases where the case worker decides next steps based on how the
case evolves rather than a predetermined transition graph.
Definitions are data (OR objects); the runtime is a pure, deterministic engine.
- NEW schema `caseModel` (register.d): CMMN case-plan definition — caseFileItems,
nestable stages, humanTasks, milestones, and entry/exit sentries, each plan
item flagged discretionary (optional, worker-activated) or mandatory.
- MODIFIED `caseType`: additive `handlingModel` enum (bpmn default | cmmn)
selecting which engine drives cases of that type.
- MODIFIED `case`: additive `casePlanState` (JSON) — the CMMN runtime's single
OR write path (plan-item states, milestones, case-file, event log).
- NEW lib/Service/Cmmn/: CaseModelEngine (plan-item lifecycle state machine +
sentry-driven cascades + discretionary enablement gating + milestone
achievement), PlanItemTransitions (exhaustive legal-transition table; illegal
transitions throw, never no-op), SentryEvaluator (AND within a sentry, OR
across a criteria array; reuses the {field,operator,value} condition shape),
CaseModelLoader (active-model-by-caseType, mirroring WorkflowTemplateLoader).
- NEW CmmnCaseController + routes: get plan, enable discretionary item,
complete/terminate human task, signal case-file event — auth-gated and
OR-RBAC group-authorized per the StatusTransitionController convention.
- NEW UI: CmmnCasePlanPanel (+ recursive CmmnPlanItemNode) — case-detail widget
grouped by stage with state badges and enable/complete/terminate actions; NC
CSS vars only; renders nothing for BPMN-managed cases. Registered as a
manifest widget slot on CaseDetail following CaseAssistantPanel.
BPMN/CMMN coexist: each engine owns exactly one case field (status/statusHistory
vs casePlanState) and refuses the other's caseTypes; no migration (both new
fields are additive/optional). CMMN milestones and task lifecycle are namespaced
separately from the existing milestoneDefinition/task features (design.md §5).
Tests: 48 new PHPUnit (state machine incl. every illegal transition, sentry
AND/OR/multi-criteria, discretionary gating, milestone achievement,
single-write-path, BPMN/CMMN mutual-refusal, REST auth 401/403/409, and an
end-to-end case-driven run proving the engine is wired to a real case, not
orphaned) + 14 new Vitest for the pure frontend helpers. Full unit suite green
in php:8.3 (1487 tests). CMMN-XML import documented as follow-up (design.md §7).
@SPEC openspec/specs/cmmn-adaptive-case/spec.md
…ive-case # Conflicts: # l10n/en.json # l10n/nl.json
# Conflicts: # src/dialogs/AddAssignmentDialog.vue # src/dialogs/BeschikkingComposerDialog.vue # src/dialogs/ConsultationCreateDialog.vue # src/dialogs/DsoCaseDetail.vue # src/dialogs/RolEditorDialog.vue # src/dialogs/VoorstelCreateDialog.vue # src/modals/AddCaseRelationModal.vue # src/modals/MandaatEditor.vue # src/modals/TermijnDefinitieEditor.vue # src/views/cases/components/BeschikkingDetailView.vue # src/views/cases/components/WooIntakeForm.vue # src/views/cases/components/beroep/BeroepEscalationPanel.vue # src/views/cases/components/bezwaar/BezwaarIntakeForm.vue # src/views/complaints/components/ComplaintCreateDialog.vue # src/views/settings/tabs/GeneralTab.vue
15 files conflicted; resolution took development's bytes for every one of them and then re-ran this repo's own formatter and the eslint autofix chain, so the merge cannot silently prefer this branch's reformatting over incoming logic.
build(lint): migrate to eslint 10 + @nextcloud/eslint-config 9
* ci: fast structural checks on every branch * ci: close the branch-trigger gap * ci: scope the JSON check — JSONC configs are not a defect * ci: scope the marker check to code — prose that documents a conflict is not one * ci: the JSON check reached a template's editor settings
…82) (#841) 65 publicly reachable methods across the ten ZGW API controllers carried no rate limit. All 65 declare themselves public with the LEGACY @publicpage ANNOTATION rather than the #[PublicPage] attribute, which is why the fleet sweep that closed ADR-082 did not see them: it line-anchored the attribute form and excluded docblock matches. This app already had 13 attribute-form endpoints throttled, which is why it looked finished. The annotation is a live declaration, proven on two other apps in this fleet - openregister GraphQLController::execute and opencatalogi CatalogiController:: index both answer 200 unauthenticated with only the annotation on them. Limits are chosen per endpoint rather than uniformly: OpenAPI index/spec 240/60 fetched by tooling and client generators; a tight ceiling breaks them rather than protecting anything reads, audit trails 120/60 zoek, download 60/60 most expensive query; moves bytes writes, publish*, uploadChunk, 30/60 uploadChunk is the cheapest way for notificatieCreate an anonymous caller to consume storage; a notification fans out to every subscribed channel publicResponsePost, cancel, 20/60 consultation stuffing; a destructive export cancel behind only a link token; a cheap request that buys a lot of server work AnonRateLimit only, not BruteForceProtection: these endpoints check no credential, and the latter without a paired registerAttempt() is the inert half of a two-half mechanism. AnonRateLimit applies only to anonymous callers, so authenticated ZGW client traffic is untouched - which matters here, because these are statutory wire APIs other systems integrate against. Verification: php -l clean on all 10 files; diff is 89 added lines and 0 removed; gate-82 reports 78 public methods, 78 throttled, 0 unthrottled.
…y watched (#844) * refactor(stuf)!: rename inkomend to inbound, and test the guard nobody watched TWO THINGS, ONE CAUSE. The endpoint was Dutch, and it was untested. Both are fixed here because the second is what made the first worth doing carefully: renaming a webhook route without a test suite means finding out from a municipality. THE GUARD WAS REAL BUT UNWATCHED inbound() verifies a WSSE UsernameToken and answers 422 on mismatch, and NOTHING tested it - no test in the repository referenced the endpoint at all. Delete the verifyWsse() call and every test stayed green. Its two sibling public routes already had that cover in StufSoapRequestDispatcherAuthTest, whose docblock states the standard: these are #[PublicPage] routes, nothing in Nextcloud's middleware will refuse a caller, so the refusal has to come from the app and has to be tested. Negative control run before pushing: with the guard deleted the suite gives 2 failures, and the informative one is that logInbound was called with a TAMPERED envelope - the message was accepted and recorded. Restored, 5/5 pass. The five arms pin: 422 with logInbound never reached (an endpoint that records first and refuses second has already accepted the message); 400 for an unresolvable sender with verifyWsse never called; 400 for an empty body; a POSITIVE control that a verified sender is acknowledged (without it, `return 422` on line one would pass every refusal test); and that the token is checked against THAT sender's stored credentials, so one configured system cannot impersonate another. WHY IT WAS UNTESTABLE `file_get_contents('php://input')` has no seam and OCP\IRequest exposes no raw-body accessor. Added a protected readRawBody() the test overrides. The production path is unchanged - same call, one indirection. THE URL IS A WIRE CONTRACT /api/stuf/inbound is canonical. /api/stuf/inkomend stays as an explicitly deprecated alias to the same method, because that URL lives in the UPSTREAM zaaksysteem's configuration, not ours: renaming it alone turns a working webhook into a silent 404 on somebody else's schedule. It is commented as a migration step with an end. DUTCH DELIBERATELY LEFT OUT OF THIS PR 'direction' => 'inkomend' and the ["uitgaand","inkomend"] enums are STORED DATA VALUES. Existing rows carry them, so renaming the code without migrating the data breaks every read - that belongs in the Dutch-to-English programme's data tranche. The sibling routes /api/stuf/zaken and /api/stuf/personen are Dutch and are wire contracts too. Verification: php -l clean; full unit suite 1904 tests, 8007 assertions, 0 failures on PHP 8.4 in the container (this box runs 8.2, the app needs ^8.3); no stale reference to stuf#inkomend, ->inkomend( or function inkomend in lib/, tests/ or appinfo/. Closes #843. * fix(routes): the legacy alias needs its own method, not a postfix CI caught this, and it was mine. The alias registered a SECOND routes.php entry under the same name, `stuf#inbound`, distinguished only by 'postfix' => 'legacy-dutch-alias'. Nextcloud's own RouteParser does support `postfix` - it appends to the generated route name. openregister's AppHost Routes::standard() does NOT: its assertNoDuplicateNames() keys on `name` alone and never reads `postfix`, so the two-entries-one-name form throws InvalidArgumentException: Duplicate route name "stuf#inbound" in AppHost Routes::standard($extra) at boot. That does not fail one endpoint, it takes the whole app's routing down - which is why it surfaced as procest's E2E SEED failing rather than as a routing test. The alias now routes to its own method, inboundLegacyPath(), which delegates to inbound(). The name is unique, the identifier is English, and the Dutch spelling survives only in the URL string - which is the wire contract held in the upstream zaaksysteem's configuration and the whole reason the alias exists. The constraint is written at both the route and the method so the next person does not rediscover it by breaking boot. Verification, redone against the RIGHT tree. The earlier full-suite run was made against the shared bind-mounted checkout, which is sitting on another workstream's branch (refactor/adr-083-inject-openregister, 8 commits behind development) where SamenwerkverzoekService gained a constructor argument its test was never updated for - 8 errors that belong to that branch, not to this one. Re-run against this worktree with vendor copied in: 1904 tests, 8007 assertions, 0 failures. StufControllerInboundTest still 5/5. Also verified directly: parsing appinfo/routes.php gives 388 entries and ZERO duplicate names, and the same check flags a planted duplicate - so the check is not vacuous.
… move rows (#845) * refactor(direction)!: English direction values, with the migration to move rows inkomend -> inbound, uitgaand -> outbound, intern -> internal, in the code and in the stored data. THE PART THAT NEEDED CARE: `intern` MEANS TWO DIFFERENT THINGS It is also a value of the statutory ZGW vertrouwelijkheidaanduiding enum (openbaar, beperkt_openbaar, intern, zaakvertrouwelijk, vertrouwelijk, confidentieel, geheim, zeer_geheim - ZgwRulesBase::VERTROUWELIJKHEID_LEVELS). Those are wire values of the standard this app consumes and emits, so they are exempt from the vocabulary rule. A WORD-BASED REWRITE WOULD HAVE CORRUPTED EVERY CONFIDENTIALITY FIELD IN THE INSTALL. What keeps them apart is that the migration is scoped to the `direction` COLUMN, not to the word. Two tests pin it and the negative control confirms they bite: setting COLUMN to vertrouwelijkheidaanduiding turns the suite red. THE MIGRATION RenameDutchDirectionValues, modelled on RenameDutchDeadlineColumns and reusing its information_schema discovery and the reasoning behind it. It differs in one way that matters: the sibling moves data between COLUMNS because MagicMapper never renames one; this step changes no schema at all - the column is already `direction` and only the strings inside it move. Scope, measured rather than assumed: - BOTH procest (17) and procest-default (2424) carry the three schemas with a direction property, so registers resolve by slug PREFIX; one exact slug would migrate half the rows and report success. - 105 shard tables on this install have a `direction` column, across pipelinq, decidesk, shillinq, scholiq and openconnector. Out of scope, and a test pins it - a procest step rewriting those edits another app's data. - portaalBericht stores `citizen_to_handler` in its own direction column, a different vocabulary sharing a column name. Only the three known strings are rewritten. Non-destructive, idempotent (no English target is also a source - pinned by a test), soft-deleted rows included, one unreadable table logs rather than aborting. Post-migration only, excluded from install like its siblings. THE READ THAT WOULD HAVE FAILED SILENTLY findOutboundByReferentienummer() filtered on 'uitgaand'. Querying only 'outbound' makes a pre-migration row return NULL rather than an error, and the caller reads null as "no outbound message to confirm" - a Bv01 confirmation dropped silently. It now falls back to the legacy value; findOne() takes scalar filters only, so this cannot be an IN. Marked for removal. Verification: php -l / JSON / XML valid on all 9 files; new suite 8 tests and 26 assertions with a passing negative control; full unit suite 1912 tests, 8033 assertions, 0 failures. Measured first: zero Dutch direction rows exist on this instance, so the step is a no-op here - which is why it reports its counters even when they are zero. * test(repair): cover the migration's real code paths, not just its constants The coverage guard caught this and it was a fair catch: the step added 92 statements and the tests covered 5, so coverage fell 0.05% against the merge base (31.43% vs 31.48%). Every assertion inspected a constant through reflection; nothing drove run(). Five tests added, and the one that matters drives run() end to end against mocked SQL: a procest shard table with a `direction` column gets exactly three UPDATEs, one per mapped value, and asserts on the statements issued - - the target is oc_openregister_table_17_928, - the column quoted in every statement is `direction`, - no statement mentions vertrouwelijkheidaanduiding, - another app's table (register 16) appears in information_schema and contributes NOTHING, so the register filter is exercised rather than assumed. The other four cover the paths a migration is judged on when it goes wrong: an install with no procest register reports "nothing to do" instead of passing silently and issues no statement at all; a failed register lookup logs and skips rather than throwing, so one broken install cannot abort the repair run; a table whose columns cannot be inspected is skipped rather than updated blind; and the step names itself for occ maintenance:repair. 13 tests, 48 assertions. The coverage delta itself is CI-measured - there is no xdebug or pcov in the dev container, so I could not confirm the percentage locally, only that the statements are now executed.
) * refactor(procest): translate 139 pre-existing Dutch property names Tranche 2, and the largest so far: 179 Dutch names measured, 139 renamed, 22 held back with a machine-recorded reason, 18 already covered. The map was DRAFTED from the tranche-1 token dictionaries rather than written by hand, then reviewed. The generator splits rather than guesses — resolved when every token of the RESULT is provably English, partial when any token is not, refused when the name is a schema slug, a lifecycle value, or would collide inside its own schema. It found 7 partials that needed a token (`afwijkend`, `deurwaarders`, `geadviseerde`, `inkomens`, `juridische`, `overschreden`) and 21 refusals, and I only had to read those 28. An early version called `aangezochtBevoegdGezag -> aangezochtCompetentAuthority` "resolved": it applied translations only to tokens the DUTCH WORDLIST knew, and `aangezocht` is a participle the wordlist does not carry, so it passed through as if it were English. It now applies any known translation and then verifies every RESULT token is English — which is the property that actually matters. FIVE THINGS THIS APP RENAMED THAT ARE NOT PROPERTY NAMES, each caught by a test and each now excluded by path: StUF/ZKN adapters `<zkn:omschrijving/>` became `<zkn:description/>` and getElementsByTagName('referentienummer') became 'referenceNumber'. Those are element names in a PUBLISHED STANDARD — Dutch by specification. ZGW mapping config `zgw_mapping_zaak` is a STORED app-config key. Email templates `{{omschrijving}}` is a placeholder, not an identifier. MandaatCsvParser REQUIRED_COLUMNS are the headers of the CSV a user uploads. One of four was translated — the same shape as the DSO defect in tranche 1. triggerTerugvordering Both a property and a service METHOD. The applier renames method CALLS but not the DECLARATION, so the test moved and the service did not. And two more `X ?? X` tautologies, from dual-spelling fallbacks written with parentheses — `$zioData['zaak'] ?? ($zioData['case'] ?? '')`. The guard added for softwarecatalog only matched the unparenthesised form. psalm names these precisely (ParadoxicalCondition), which is the only reason they were not shipped: collapsing the pair silently stops accepting the old spelling. VERIFIED against a control run of the same tree: PHPUnit 1899 tests with IDENTICAL errors and failures on both sides (4 errors, 2 failures, all pre-existing); phpstan [OK] both; psalm "No errors found!" both; vitest 349/349; eslint 0; prettier clean; build OK; 383 routes resolved, 0 broken; 0 slugs changed; 0 surviving uses in src/. 139 migration entries appended; the map has 452 entries with no duplicate and no identity pairs. * fix(procest): shorten a variable the rename pushed past phpmd's limit phpmd caps a variable name at 20 characters and `$escalationRecommended` is 21 — the rename lengthened it. The ARRAY KEY it feeds is the property and is unaffected; only the local moves, to `$escalationAdvised`. phpmd is green on development, so this one was mine. Also merges development's 2 commits (the ZGW volume-ceiling fix and a CI change). The merge is clean and brought in no code using the old Dutch names — re-running the applier over the merged tree reports 0 substitutions, which is the check that matters after a merge: development can always add fresh code using names this branch has already moved. The remaining PHPUnit cell failure is the coverage ratchet at 0.01% — 46789 statements on both sides, 4 fewer COVERED. Nothing was added; four statements stopped being reached. Re-measuring against the refreshed merge base. * fix(procest): pin the 13 properties an EXCLUDED adapter still reads The coverage ratchet was right and I had read it as noise. 46789 statements on both sides, **4 fewer COVERED** — nothing added, four statements stopped being reached. Chasing those four found a real defect. Excluding the StUF/ZGW adapters protected their published wire element names — `<zkn:omschrijving/>`, `getElementsByTagName('referentienummer')` — but those same files also read OUR properties off our own objects: `$case['zaakIdentificatie']`, `$data['externIdentificatie']`, `$row['synchronisatieStatus']`. The rename moved the objects to English and left the adapters reading keys that no longer exist. A missing subscript is `null`, so nothing threw; the only trace anywhere was four statements dropping out of coverage. THE RULE, now enforced by the applier: **a file excluded from the rename pins every property it reads.** Half-excluding a file is worse than either choice made whole. 13 names pinned — zaakIdentificatie, externEntiteit, externIdentificatie, laatsteSynchronisatie, synchronisatieStatus, fout, zaakIdentificatieStrategie, bronId, authenticatie, gebruikersnaam, wachtwoordKluisRef, ontvangenOp, omschrijving — leaving 126 renames. The tree is RE-DERIVED from `origin/development` with the corrected map rather than patched. Reverting `description` -> `omschrijving` and `error` -> `fout` in place would have hit every pre-existing English `description` and `error` in the app; re-deriving produces exactly the tree the corrected map would have made in the first place, and needs no history rewrite. Also carries the phpmd fix (`$escalationRecommended` is 21 chars against a 20-char cap; the array key it feeds is unaffected). VERIFIED against a control run of the same tree: PHPUnit 1899 with IDENTICAL errors and failures (4/2, all pre-existing); psalm "No errors found!"; phpstan [OK]; vitest 349/349; prettier clean; 383 routes resolved, 0 broken; 126 migration entries, no duplicate or identity pairs. The two remaining src/ hits are `fout` and `zaakIdentificatieStrategie` — both pinned, both still declared by the schema, so those reads are correct. * fix(procest): restore five StUF element names the vocabulary pass translated A PRODUCTION DEFECT already on `development`, found while chasing four missing covered statements. #832 renamed five StUF element names, and an element name is not ours to translate: StufZknMessageResponder::extractFields() passes each entry of `fieldNames` straight to getElementsByTagName(). `toelichting` had become `notes` and `einddatum` had become `endDate`, so those two tags never matched and the fields were silently dropped from every inbound zakLk01. StufFieldMappingService's mapping tables key on the ELEMENT name on the LEFT and the internal property on the RIGHT. Three ZKN keys (`toelichting`, `einddatum`, `registratiedatum`) and two BG keys (`geslachtsnaam`, `voornamen`) had been translated, so those fields never mapped either. Restored all five, verified by diffing every mapping key against the last commit before the vocabulary programme: **0 keys now differ from the specification.** I NEARLY BROKE A SIXTH THE OTHER WAY. `DEFAULT_ZENDER` looks like the same shape — `'organisation' => 'Procest'` next to `'applicatie' => 'Procest'`, one translated and one not. It is NOT an element list: StufResponseBuilder hardcodes `<stuf:organisatie>` and reads the array by key, so the English key is correct and "fixing" it would have emitted an empty element. The difference is only visible in the CONSUMER, which is the thing to read before deciding. VERIFIED: PHPUnit 1899 with identical errors and failures to a control run of the same tree (4/2, all pre-existing), phpstan [OK], psalm "No errors found!". * fix(procest): the excluded StUF tests kept feeding the OLD config keys The coverage cell was failing for -7 statements and nothing else was red. The clover diff put all of it in one file: StufMessageBuilder, 179 covered on development and 171 here. Cause: I excluded the whole StUF directory from the rename to protect the wire element names, and that exclusion covered the TESTS as well. The source moved on — it reads `$endpoint['freeMessagesTemplates']`, `['senderOrganisation']`, `['recipientApplication']` — while the fixtures went on supplying `vrijeBerichtenTemplates`, `zenderOrganisatie`, `ontvangerApplicatie`. Six keys. Every read returned null, the free-message template branch was never entered, and THE TESTS STILL PASSED, because they assert on the envelope they get rather than on the branch being taken. The only signal was eight statements quietly leaving coverage. That is the ratchet earning its place: no test failed, no analyser complained, and the code under test was not being reached. An endpoint CONFIG key is ours. Only the XML tag names belong to the specification, and those are untouched — verified separately: every mapping key in StufFieldMappingService matches the pre-programme spec names, 0 differ. VERIFIED: PHPUnit 1899 with 4 errors and 2 failures, identical to a control run of the same tree, and the eight statements are exercised again. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…alive (#846) Finishes what #844 started on the StUF surface. Both old URLs keep answering: POST /api/stuf/cases canonical /api/stuf/zaken deprecated alias POST /api/stuf/persons canonical /api/stuf/personen deprecated alias Same reasoning as /api/stuf/inkomend: the URL lives in the SENDING zaaksysteem's configuration, not ours, so renaming it alone turns a working SOAP receiver into a silent 404 on somebody else's schedule. Each alias needs its OWN method because AppHost Routes::standard() rejects duplicates by `name` and ignores `postfix` - that form throws at boot and takes the whole app's routing down. Verified: 390 route entries, 0 duplicate names. `zaken` MEANS TWO DIFFERENT THINGS AND ONLY ONE OF THEM MOVED The StUF $service token is INTERNAL - the sending endpoint is resolved from the envelope's zender, and the token's only other use is log context. It never reaches the wire, so it is now SERVICE_CASES / SERVICE_PERSONS. Deliberately untouched because they are statutory: ZrcController / ZgwService / ZgwRulesDispatcher $resource === 'zaken' is the ZGW REST resource AcController scopesContain(...,'zaken') is a ZGW autorisaties scope That distinction is the whole risk in this change and a test asserts it. Fault messages moved to English - 'Authenticatie mislukt', 'Ongeldig XML bericht', 'Onbekend berichttype'. The StUF fault CODE (StUF001) is statutory and unchanged; only the free-text faultstring moved. Verification: a new 4-test suite asserts each alias dispatches with the CORRECT service constant, not merely that it dispatches - passing the wrong service would still return 200 and route StUF-ZKN traffic through the person handler. Negative control: wiring casesLegacyPath() to persons(), the plausible copy-paste slip, makes it fail; restored, green. php -l clean; full unit suite 1921 tests, 8690 assertions, 0 failures.
…ck (#848) procest carried 34 Dutch names after tranche 2 — the largest remaining set in the fleet. This takes the 16 that are plain properties. The other 18 need their own passes: 13 are also SCHEMA SLUGS and want the RenameDutchSchemaSlugs treatment softwarecatalog established, 4 are enum/lifecycle VALUES needing a data migration, and one is held back. `omschrijving` is that one, and it is worth naming precisely, because renaming the property renames THREE external contracts at once: - a StUF WIRE element. The outbound builder emitted `<zkn:description />` where the Key2Zaken integration expects `<zkn:omschrijving />`. The adapter already does this correctly at the boundary — StufMessageParser maps `//zkn:omschrijving` onto an internal `description` — so the wire spelling is exactly what must NOT move. - a CSV IMPORT COLUMN HEADER (`mandaatNummer,omschrijving,rolNaam,...`). Existing import files would have stopped loading with "Missing required CSV columns". - an email TEMPLATE VARIABLE, `{{omschrijving}}`. Six tests caught it. Renaming the property still wants doing, but each of those three positions has to be pinned to the old spelling first, which is a change of its own rather than a line in a rename map. `triggerTerugvordering` was also a method, so its declaration moved by hand — to `recoveryTrigger`, which is what the schema's own title ("Recovery Trigger") already called it, rather than the dictionary's `triggerRecovery`. Verified against a control built with `git archive origin/development` (not `git stash` — that reverts uncommitted work only): PHPUnit 1921 tests both sides, same 4 errors and 2 failures, none unique to this change and the count unchanged so nothing stopped running. phpstan clean, psalm "No errors found", phpmd clean, vitest 349, eslint 0 errors, l10n and manifest validators pass. Stale field references unchanged at 11. Dutch names 34 -> 18. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
Comment on lines
+32
to
+36
| uses: ConductionNL/.github/.github/workflows/release-beta.yml@main | ||
| with: | ||
| app-name: procest | ||
| channel: dev | ||
| secrets: inherit |
The app pinned the exact prerelease 2.2.0-vue3.16, which is now deprecated: the Vue 3 line was folded into the mainline 2.x release series and ships as 2.3.0 on the `latest` dist-tag. A caret range replaces the exact pin so future 2.x releases roll out without a per-app edit. Drops the `overrides.@conduction/nextcloud-vue.eslint` entry. That override existed only because the old prerelease declared `eslint: ^8.56.0 || ^9.0.0` and could not see the app's eslint 10; 2.3.0 declares `|| ^10.0.0` itself, so the peer resolves without help. No API change: 252 components in and 252 out, no export removed, one added (the BSN validators). Peer ranges are otherwise identical.
…oves them (#849) * refactor(procest): translate 14 schema slugs, and the migration that moves them A schema slug is what OpenRegister's ImportHandler matches an incoming schema against (`SchemaMapper::findBySlugInIds()`). Changing it in the register JSON renames NOTHING: the import finds no match, creates a second schema, and every stored object stays on the old one. Nothing raises — it presents as an app with no records. So this adds `RenameDutchSchemaSlugs`, registered FIRST in post-migration, ahead of both import triggers (InitializeSettings for the procest register, RegisterOriRegister for ORI). The six ORI schemas move too. Open Raadsinformatie / VNG ODS-O is a standard, and a standard's own spelling belongs in a MAPPING — which is configuration, and out of scope for translation. A SCHEMA is English. procest has `LoadDefaultZgwMappings` for ZGW and nothing equivalent for ORI, so nothing consumes the standard's spelling from these schemas today; giving ORI a real mapping is separate work and worth doing. `bezwaar` is HELD BACK, and the reason is the interesting part. Renaming it onto `objection` produced a DUPLICATE JSON KEY. That is legal JSON: the file parsed, every check passed, and the consistency check I wrote passed too — because it read the already-deduplicated structure. Python, PHP and JS all keep the LAST key, so the `bezwaar` schema's `x-openregister-lifecycle` and `x-openregister-calculations` simply vanished. Nine tests caught it; no linter could have. **A file that parses is not a file that kept your data.** They are also not duplicates. PanelIndependenceChecker resolves `bacAdviceRequest.bezwaar -> bezwaar (lifecycle record) -> bezwaar.case -> objection (filed on that case)`, and SettingsService carries separate `bezwaar_schema` and `objection_schema` keys. Naming the lifecycle record in English is a design decision, not a translation. `rename-slugs.js` now refuses when the target is already a schema, and grew the schema-MAP-KEY pass it was missing: a slug lives in three places — the `slug` value, the `/components/schemas/` key, and the register's `configuration.schemas` key — and softwarecatalog #518 had to fix the last two by hand. `subsidieaanvraag` is deferred: the property should become `grantApplication`, but a seed caseType carries `subsidieaanvraag` as its `identifier` VALUE, which other cases reference. The applier refused it for the wrong reason (it saw an object slug) and was right to. Verified against a control built with `git archive origin/development`: PHPUnit 1926 vs 1921 tests (+5 new), identical 4 errors and 2 failures, none unique to this change. phpstan clean, psalm "No errors found", phpmd clean, 0 phpcs errors on the new files, 386 routes resolve, vitest 349, eslint 0 errors, l10n and manifest pass, no duplicate JSON keys anywhere, no dangling $refs, no key/slug mismatches, stale field references unchanged at 11. Dutch names 18 -> 7. * refactor(procest): leave the ORI schemas alone — decidesk already owns this Reverts six of the fourteen slug renames. procest should not have an ORI schema register at all, so renaming its schemas cements a structure that is going away and mints names that collide conceptually with the ones that are already canonical elsewhere. decidesk already implements the intended architecture, and it is not a plan — it is on development today: Popolo-shaped schemas (Person, Membership, Post, Meeting, Vote, VotingRound, AgendaItem, GovernanceBody, Minutes, Transcript), extended with schema.org through `x-schema-org`, plus OriController and OriSerializer that map them onto ORI. The canonical structure is Popolo; ORI is the mapping over it; and a mapping is configuration, so its vocabulary may stay in the standard's own language. That leaves procest's `ori` register as a duplicate of decidesk's, and the fix is to remove it and consume decidesk's instead — a design change, not a rename. Recorded rather than attempted here. Reverted with the same tooling in reverse, then the twelve resulting column-map entries were removed by hand: the forward six, two of which the second pass had turned into identity mappings (`'stemming' => 'stemming'`), and the reverse six that pointed English at Dutch. Two column-map invariant tests caught exactly that — `testEveryEntryIsSnakeCase` and `testNoTargetIsAlsoASource`. Eight slugs remain in this PR, all procest's own vocabulary: avgClassificatie, catalogus, dwangsomBerekening, ingebrekestelling, kanaal, termijnDefinitie, termijnInstance, voorstel. Re-verified against the same control: PHPUnit 1926 vs 1921 (+5 new), identical 4 errors and 2 failures, none unique. phpstan, psalm, phpmd clean; 386 routes resolve; vitest 349; prettier clean; no duplicate JSON keys, no dangling $refs, no key/slug mismatches. * test(procest): cover the slug step's shared helper and its name The coverage guard failed the stable34 cell by 0.01% — a repair step needs a database, so most of it is unreachable from a unit suite. Same answer as softwarecatalog #518: move what is a decision rather than DDL into the injected collaborator. `placeholders()` is trivial and earns its place anyway: the step builds an IN list three times, and a mismatch between placeholder count and bound parameters surfaces only at runtime, inside a repair step, on somebody else's install. Tested for 0 and for a negative count too. Also worth recording: CI reports 'Tests: 1921' in that job and I briefly read it as my five new tests not running. It is the MERGE BASE run — the coverage job checks out head, then the base, to compare. Head reports 1926. Colour codes hid the first line from a plain grep. * test(procest): cover the step the slug test constructs PHPUnit's strict coverage marks a test RISKY when it executes a class the @Covers annotation does not name, and one risky test fails the whole cell. testShippedStepNamesItself reflects RenameDutchSchemaSlugs into existence, so the annotation has to name it. Only CI sees this — there is no coverage driver in the container, so the strict checks never fire locally. Same fix as softwarecatalog #516. * test(procest): read the slugs where they can be tested too Coverage guard still 0.01% under. slugsFrom() is the sibling of schemaIdsFrom() and belongs beside it regardless: both read a database row defensively, because a null column must yield an empty string rather than a TypeError inside a repair step, where an exception aborts the upgrade. 60 repair tests pass; phpstan and psalm clean. * test(procest): drive the slug step through a mocked connection Coverage guard still 0.01% under, so the step itself needs exercising rather than another predicate extracted. The interesting part is what the mock throws. The step catches OCP\DB\Exception specifically, so a RuntimeException from a mock escapes the try/catch and the test measures the mock instead of the step — which is exactly how it failed first time round. The mirror of that trap, a mock throwing a type the step DOES catch, is how a broken repair step once read as a green no-op. So this feeds an EMPTY RESULT instead and asserts the step reports 'nothing to do' and issues no statement. 62 repair tests pass. * docs(procest): mark the slug step's methods @SPEC exclude gate-16 (spec-coverage) failed with six changed methods missing @SPEC — the only gate on this PR that development does not also fail. procest already has the right form for this: RenameDutchColumns carries `@spec exclude` with the reason that no canonical spec covers the Dutch-to-English vocabulary migration, and pointing it at an existing spec would report conformance to a requirement that says nothing about it. The same is true here, so the same tag with the same reason. * fix(procest): the ZGW adapter keeps its Dutch resource names Three files should never have been touched by the slug rename, and gate-16 is what led me to them — it flagged changed methods in ZgwService/ZgwMappingService that I had no business changing. `catalogus` and `kanaal` are TWO different things in this app: procest's own schema slugs, which move, and ZGW RESOURCE names, which do not. `ZgwService::$resourceKeys`, `ZgwMappingService`'s plural map (`'catalogussen' => 'catalogus'`) and `LoadDefaultZgwMappings`' mapping registry are all the second kind. Those schemas ARE the mapping onto ZGW, a mapping is configuration, and the standard's vocabulary stays in the standard's language — the same rule that kept pipelinq's `zgwResourceType` out of its value map. The applier cannot tell the two apart: it matches a quoted string, and both spellings are `'catalogus'`. `LoadDefaultZgwMappings` had already gone half-broken from it — the key renamed to `'catalog'` while the template beside it still read `{{ catalogus | zgw_extract_uuid }}`. A mapping pointing at a variable that no longer exists resolves to empty, not to an error. All three reverted to development. PHPUnit 1931 tests, same 4 errors and 2 failures as development, none unique; 386 routes resolve; phpstan and psalm clean. * docs(procest): @SPEC exclude on the slug test's methods too gate-16 still named one changed method. lib/ is clean — every changed method there carries a @SPEC — so the remaining one is in the new test file, which had none. Same exclude, same reason. * docs(procest): @SPEC exclude on the slug test class as well gate-16 still named one changed method after every method in lib/ and in the test carried a tag. The class docblock was the one thing left without one. * docs(procest): @SPEC on the one method gate-16 was actually naming Three rounds of adding @SPEC to lib/ and to the test file did not move gate-16 off 1, because the method it meant was in src/: gate-16 covers non-trivial FRONTEND methods too, and `BeschikkingDetailView::hasVerzending` changed when the property it reads went from `kanaal` to `notificationChannel`. I found it by running the gate's own script — `check_spec_coverage.py` with HYDRA_GATE_BASE_REF=origin/development — which names the method and prints `# count=0` when clean. Reading its message and guessing at the cause cost three pushes; running it took one. Tests are NOT in scope, so the annotations I added there were unnecessary (harmless, and left as documentation). Gate now reports count=0 locally. vitest and prettier clean. * fix(register): point five relation $refs at the renamed schema keys The slug rename moved `catalogus` -> `catalog` and `voorstel` -> `proposal`, but five relation properties refer to their target by BARE schema key rather than by a `#/components/schemas/...` path, and the rename tool only rewrote the path form. The refs were left pointing at keys that no longer exist. Nothing in PHP or JS notices: a dangling $ref is not a parse error and not a lint finding -- the relation simply stops resolving at render time, so the picker comes up empty. gate-54 is the only instrument that sees it, and only when it reads its own log file: the helper writes findings to the path it is given and discards stdout, so its exit code is 0 either way. gate-54 goes 6 -> 1, and the remaining one is development's own. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…#847) * refactor(deps): inject OpenRegister instead of looking it up (ADR-083) 2 file(s) reached OpenRegister through $this->container->get(...) on an UNCONDITIONAL path — no availability check, no degrading catch. The dependency was announced nowhere: not in the constructor, not in the use block, not in any type. It appeared mid-method, as a string. Now constructor-injected and typed, so the dependency is visible to a reader and to tooling. Behaviour is unchanged: the same object, from the same container, resolved at construction instead of at first use. ContainerInterface is dropped only where nothing else used it. Deliberately NOT converted, because they are correct as written (ADR-083 rule 1's exception): lookups behind isInstalled()/getInstalledApps(), and lookups whose catch degrades rather than rethrows. Verified per file: php -l clean, and gate-66's lookup check reports zero remaining findings for each file changed. gate-66 for this app: 3 -> 1. * style: put the injected parameter on its own line The transformer appended the new promoted parameter AFTER the whitespace-only line preceding the closing paren, producing private readonly ObjectService $objectService,) { php -l accepts that; phpcs does not, and it reads badly. Fixed here and in the transformer, which now backs up over trailing whitespace before inserting when the constructor has no defaulted parameters to sit in front of. No behavioural change — parameter list is identical. * refactor(deps): type-hint OpenRegister's published contract (ADR-084) Five services take OCA\OpenRegister\Contract\ObjectServiceInterface instead of the concrete class, bound in the composition root, with hydra-gates bumped to v1.8.0 so composer installs the interface into vendor/. This is what makes the ADR-083 conversion mockable. A leaf app cannot load a class from another Nextcloud app, so a typed constructor parameter had no satisfiable double: TypeError: __construct(): Argument #N ($objectService) must be of type OCA\OpenRegister\Service\ObjectService, class@anonymous given The 199 files that name ObjectService only as a CONTAINER KEY are untouched. Those are availability-guarded lookups — the ADR-083 rule-1 exception — and the string must go on naming the concrete service, because that is the key the alias resolves TO. Rewriting it would have made the container look up something nobody registered. Two of these five files were nearly missed. The first transformer masked string literals with a regex, and in DsoCaseService the comment `// King's Day.` opened a string that did not close until `date('N')` 240 lines later — so every type position between them looked like it was inside a string and the file was silently skipped. A skipped file is indistinguishable from a clean one, which is why it took a count to notice: 3 converted where 5 were due. The transformer now takes its comment and string ranges from PHP's own token_get_all(), and reports every file it declines to convert together with the reason. * fix(adr-084): complete the conversion the first transformer under-did The first pass through this app used a transformer with three defects, each of which failed SILENTLY -- a skipped file is indistinguishable from a clean one. 1. It masked string literals with a REGEX. An apostrophe in a comment (`// King's Day.`) opened a string that did not close for 240 lines, so every type position between them looked quoted and the file was skipped. Now the comment and string ranges come from PHP's own token_get_all(). 2. It compared PHP's BYTE offsets against Python's CHARACTER indices. One file differed by 94 -- em dashes in prose comments -- so spans after the first non-ASCII byte were misaligned and a docblock was judged "not in a comment". Everything is handled as bytes now. 3. It missed short-form RETURN types (`): ?ObjectService`), which neither the parameter nor the fully-qualified pattern matched. And it dropped the concrete import even where `ObjectService::class` or `instanceof ObjectService` still needed it. That one is not merely incomplete, it is silent damage: `::class` does not require the class to exist, so the lookup would have resolved to this app's own namespace. An invariant check now enforces the rule -- a file may import the contract, or both, but never the contract alone while still naming bare ObjectService -- and reports 0 for every app in this rollout. * test: construct the two services with the contract they now require ADR-083 added a fifth constructor parameter and the tests still passed four: ArgumentCountError: Too few arguments to DsoCaseService::__construct(), 4 passed and exactly 5 expected Both sites use named arguments, so each is one line: an ObjectServiceInterface mock. That clears 6 of the 8 PHPUnit errors on this branch. NOT fixed here, deliberately. testCreateZaakFromVergunningaanvraagCallsObjectService builds its double from a LOCAL interface declared in this test file, DsoCaseObjectServiceStub, whose own docblock describes exactly the problem ADR-084 solves -- a \stdClass mock generates positional-only signatures and dies on "Unknown named parameter", so somebody hand-rolled a typed interface to get named arguments working. It is an eleventh double, and being an interface rather than a class it was missed by the fleet count of ten. Swapping it for the contract is not mechanical: it stubs find() to return an ARRAY, while the contract returns ?ObjectEntityInterface. That is the same shape as openbuild's stub failure, but the fix there was to make the stub satisfy the contract; here the test is asserting on a raw array it never had to justify. It needs a decision about what the test should actually assert, not a rushed signature change. * fix: saveObject() returns an entity, and two methods were returning it as array The last PHPUnit error on this branch was not a test problem. It was the test faithfully modelling a production defect. `createZaakFromVergunningaanvraag(): array` did $created = $objectService->saveObject(...); return $created; and `ObjectServiceInterface::saveObject()` returns an ObjectEntityInterface. That worked only while the service was reached through an untyped container lookup, which returned whatever it returned and told nobody. The test's local stub returned raw arrays because that is what production expected. Fixed on the production side, using the shape this app already had: a saveObjectAsArray() helper next to findObjectAsArray(), same array-or- jsonSerialize() normalisation, applied at the two sites that RETURN the result. The third (syncPermitApplicationStatus) discards it and is unchanged. The test now mocks the contract and returns what the contract promises — entities whose jsonSerialize() carries the payload — and rebuilds the service with that double, because the container path setUp() configured is dead now that the dependency is injected. DsoCaseObjectServiceStub is deleted. It was an ELEVENTH hand-rolled double, and being an interface rather than a class it was missed by the fleet count of ten. Its own docblock stated the problem ADR-084 exists to solve: "a \stdClass-based mock generates positional-only signatures and fails at call time with Unknown named parameter; this typed interface lets PHPUnit generate a mock whose method signatures accept the named arguments". Someone hand-rolled a contract to make mocking work. Now there is a published one. * fix: the contract lives in Contract\, not Service\ phpstan caught a defect in the rollout transformer: PHPDoc tag @var for property $objectService with type OCA\OpenRegister\Service\ObjectServiceInterface is not subtype of native type OCA\OpenRegister\Contract\ObjectServiceInterface The docblock rewrite matched `@var \OCA\OpenRegister\Service\ObjectService` and appended `Interface` to the CLASS name while leaving the NAMESPACE alone, so the declared type named a class that does not exist. The native type next to it was correct, which is why only phpstan noticed — PHP itself never reads the docblock, and the tests pass either way. That is the fifth silent failure from this transformer, and the same shape as the others: it produced plausible output that no runtime check disagreed with. * ci: adopt development's Code Quality workflow — the branch had the pre-fix one These branches were cut from ADR-083 branches predating the 2026-08-14 correction, so they carry an older code-quality.yml whose push trigger has no refactor/** — which is this branch's prefix. development's version says why that matters: An ALLOW-LIST of branch prefixes is a gate with a hole in it, and the hole is SILENT: a branch matching nothing gets no CI at all, and its last visible status is whatever it inherited — indistinguishable, on every dashboard, from a branch that passed. Observed here: softwarecatalog#519 settled at FOUR checks (CodeQL and Analyze only) and read as green, having previously reported 43. shillinq#556 did the same at three, which is shillinq#557. Takes merge-hygiene.yml with it, the companion added in the same change, which runs the fast structural checks on ** so an unlisted prefix is not completely unguarded. This restores coverage via the PUSH path. It does not explain why the pull_request runs stopped, which is tracked separately. * test: seed the Samenwerkverzoek doubles from the contract, not a local stub procest's PHPUnit went 8 errors -> 3 after the arity fix, and all three were the same thing: "Zaak not found", "Samenwerkverzoek not found". A bare createMock(ObjectServiceInterface::class) returns null from find(), so the service correctly concluded the object did not exist. The seeding was there — it just went to the wrong place. The tests configured a SamenwerkObjectServiceStub mock and handed it over through the CONTAINER, and the container path is dead now that the dependency is injected. So the service was built with setUp()'s empty double while a fully-seeded one sat unused. Two changes per test: mock the contract, and rebuild the service with that double instead of relying on the container. find() and saveObject() now return ENTITIES via a small entity() helper, because that is what the contract promises and what the service normalises with jsonSerialize(). The old stub returned bare arrays — which the untyped container lookup allowed, and which is the same mis-shaped double this ADR keeps turning up. * test: complete the ADR-083 constructor changes and make the stubs satisfy the contract Three distinct pre-existing defects, all of which only became visible once the tests could actually construct their subjects. 1. ARITY, completely this time. The first pass added `objectService` only, because that is the parameter ADR-084 was about. ADR-083 added others in the same commit — softwarecatalog's ContactpersonenController gained THREE (objectService, magicMapper, organisationService) — so a call could be fixed for one and still be short by two. Every required parameter is now supplied, by NAME so it fills the right slot regardless of the existing arguments. 2. IMPORTS for the types those arguments name. `createMock(MagicMapper::class)` without a `use` resolves the short name against the TEST's own namespace, and `::class` does not require the class to exist — so it silently mocks a class nobody declared. Same trap as the container-key strings in lib/. 3. The ObjectEntity STUB now implements ObjectEntityInterface. Once ObjectServiceInterface is mocked its return types are enforced: Method find may not return value of type MockObject_ObjectEntity, its declared return type is "?OCA\OpenRegister\Contract\ObjectEntityInterface" A hand-rolled double that does not declare the interface cannot be handed back. This is ADR-084's argument arriving in the tests: ten apps had such a double, and none of them was checked against anything until now. softwarecatalog also gains a MagicMapper stub, because ADR-083 injected OpenRegister's MagicMapper into a controller and this app has no way to load it. That stub is debt of exactly the kind ADR-084 removed for ObjectService — noted in the file so it stays visible rather than becoming furniture. php -l on every touched file, reverted on failure. * refactor: drop the dead catch ADR-083 left behind phpstan, on this branch: Dead catch - Throwable is never thrown in the try block. getObjectService() used to resolve the service from the container, which could fail, so it wrapped the call and rethrew a RuntimeException. ADR-083 made the dependency injected, and the body became `return $this->objectService;` — a property read, which throws nothing. The catch survived the conversion and was unreachable. Removing it is not a loss of safety: absence is now a CONSTRUCTION failure on the route that needed the data, which is precisely what ADR-083 rule 1 asks for, rather than a RuntimeException raised deep inside a call. * fix(tests): remove constructor arguments that belong to a different class The arity fixer keyed constructors by FILE STEM, and two classes with the same stem in different namespaces collide. procest has lib/Service/ChecklistService.php no constructor arguments lib/Service/Inspection/ChecklistService.php three so a correct `new ChecklistService()` was given three arguments belonging to the other class: Error: Unknown named parameter $settingsService Constructions are now resolved through the FILE'S OWN `use` imports to a fully-qualified name before their arguments are judged. The reconcile pass was itself wrong on its first run, in a way worth recording: its parameter parser only accepted types beginning with a capital, so `string $appName` on every Nextcloud controller was invisible and a valid `appName:` argument looked like one the constructor did not have. It removed 33 files' worth of arguments in procest; with scalar types accepted it removes 8. The tell was reading a diff that deleted `appName: 'procest'` — a line no plausible fix would touch. php -l on every touched file, reverted on failure. * chore(phpmd): record why DsoCaseService is one collaborator over ADR-083 replaced a lazy container lookup with a typed ObjectServiceInterface dependency — the point of the ADR, since the dependency becomes visible to readers and to tooling. That took the class to 13 collaborators, one over phpmd's threshold. The container cannot simply go: IGroupManager is still resolved through it. Swapping one for the other would leave the count unchanged. Suppressed with the reason attached rather than silently, and consistent with this app's own convention — 29 classes here already carry it. * fix: repair references to the local $objectService ADR-083 deleted phpmd, on this branch: UndefinedVariable $objectService ADR-083 replaced $objectService = $this->container->get('OCA\OpenRegister\Service\ObjectService'); with an injected property and rewrote most usages to $this->objectService — but not all. What remained read a local that no longer exists. In PHP that is not a parse error and not a test failure unless the line executes; it is null at runtime, and the call it feeds gets null instead of the service. Rewritten only where the enclosing function has no assignment to that local AND does not take it as a PARAMETER — several helpers legitimately receive it, e.g. CreditLimitGuard::sumOutstandingCents(object $objectService, ...), and those are untouched. php -l on every touched file, reverted on failure. * Revert "fix(tests): remove constructor arguments that belong to a different class" The reconcile pass removed VALID arguments. PHPUnit, after it: Error: DsoObjectRepository::__construct(): Argument #1 ($settingsService) not passed Error: ZgwRulesBase::__construct(): Argument #1 ($logger) not passed Error: Too few arguments to ProcestCaseReader::__construct(), 0 passed It was written to fix ONE real problem — two classes sharing a file stem, where `new ChecklistService()` was given three arguments belonging to lib/Service/Inspection/ChecklistService.php. That problem is real and is fixed by hand in the next commit. The pass itself judged every construction in the file, and where its constructor parse came back empty it treated every named argument as unknown and deleted it. A tool that removes code needs to be certain, not merely unable to confirm; reverting is cheaper than making it certain, since exactly one call site actually needed changing. * fix(tests): ChecklistService takes no constructor arguments OCA\Procest\Service\ChecklistService has an empty constructor. The three arguments passed here belong to OCA\Procest\Service\Inspection\ChecklistService, a different class with the same FILE NAME — which is what an earlier fixer keyed constructors by, and how they arrived: Error: Unknown named parameter $settingsService This is the one call site that genuinely needed changing; the sweep written to find it removed valid arguments elsewhere and has been reverted. * fix(tests): drop constructor arguments belonging to a same-named class Several classes here share a FILE NAME with another in a different namespace — lib/Service/ChecklistService.php and lib/Service/Inspection/ChecklistService.php, BelplanRoutingService, HearingService and others. The arity fixer keyed constructors by that file name, so some constructions received arguments from the wrong class: Error: Unknown named parameter $settingsService Classes are now resolved through the file's own `use` imports, and the pass refuses to act unless it is certain: a constructor that exists but parses to nothing is treated as a PARSE FAILURE and skipped, never as "takes no arguments". It also reads only DEPTH-0 named arguments. A nested construction has its own constructor: new ProcestToolProvider( caseReader: new ProcestCaseReader( settingsService: $settingsService, <-- the INNER call's parameter logger: $logger, Judging those against the outer constructor is what made the first attempt delete valid arguments; that attempt was reverted, and this is its replacement. * fix(tests): restore inherited constructor args, and expect saveObject's real order Two remaining PHPUnit causes in this app. 1. 21 errors: ArgumentCountError on ZgwRulesBase::__construct(). ZgwZrcRulesService has NO constructor of its own — it INHERITS ZgwRulesBase(logger, settingsService, fieldValidator). An earlier sweep read "no __construct in this file" as "takes no arguments" and dropped the two inherited ones. Both mocks were already built two lines above; they had simply stopped being passed. 2. 2 TypeErrors: the doubles declared function (string $r, string $s, array $obj) which is the OLD positional saveObject($register, $schema, $object). ObjectServiceInterface::saveObject() takes $object FIRST, and the caller uses named arguments, so the double receives them in the contract's order. The with() expectation had the same assumption and asserted on the third argument. This is the contract doing its job: a signature nobody could see is now one the doubles have to agree with.
2.3.0 is released and the `vue3` and `latest` dist-tags have CONVERGED on it — the separate 2.2.0-vue3.N line is over. The exact 2.2.0-vue3.16 pin left this app five releases behind and made a caret range impossible to express, because the -vue3.N prereleases do not order the way callers expect. A caret also unblocks local library development: the sibling-checkout guard validates ../nextcloud-vue against this app's declared range, so an exact pin refused every sibling that was not byte-identical, newer ones included. Verified rather than assumed: npm install resolves 2.3.0, npm ci reproduces it (run explicitly, because npm install does not re-run postinstall hooks for an already-present version — that is how vue-demi ends up on the wrong shim), the production build compiles clean with 0 errors, and the app renders in the browser with cache disabled. All 17 apps in this sweep render. Note for reviewers: 2.3.0 declares the same peerDependencies as 2.2.0-vue3.16 — dexie, marked, dompurify, gridstack, @vueuse/core, @nextcloud/capabilities and friends. Those app-level declarations are REQUIRED by the library, not redundant, and must not be pruned as "unused".
* fix(samenwerkverzoek): honour the ObjectServiceInterface contract All six PHPUnit matrix cells are red on `development`. Two separate defects against OpenRegister's published ObjectServiceInterface, and the first one is a production bug, not a test artefact. 1. `initiateSamenwerking()` and `respondToSamenwerking()` are both declared `: array`, but `saveObject()` returns an `ObjectEntityInterface`. Returning it straight out is a TypeError on every call — including from DsoController::respondSamenwerking(), which is why it surfaced as an ERROR rather than a failed assertion. Both now normalise through an `asArray()` helper, mirroring the helper of the same name already in CaseCollaborationService, and staying defensive so either shape works. 2. `testInitiateSamenwerkingCreatesObject` pinned the OLD argument order ($register, $schema, $object). The contract takes $object FIRST, and the caller uses named arguments, so the double receives ($object, $extend, $register, $schema). The sibling test at line 231 had already been corrected and carried a comment saying exactly this — this one was missed. It now also asserts the register and schema it was silently ignoring. Verified in a PHP 8.4 container with OpenRegister's real Contract interfaces preloaded, which reproduces CI exactly (a plain standalone run cannot resolve them and errors identically on both sides, so it cannot discriminate): pristine development 1931 tests, 2 errors, 1 failure, 1 risky this branch 1931 tests, 0 errors, 0 failures Exactly the three SamenwerkverzoekServiceTest cases move to passing, and nothing is introduced. * refactor(samenwerkverzoek): drop the asArray() helper — it tripped phpmd The helper I added took `mixed $value`, and that pushed SamenwerkverzoekService's CouplingBetweenObjects from 12 to 13, which is the hydra-gates threshold. phpmd is GREEN on this branch's merge-base (34a35dc) and red on the branch, so it was mine. `ObjectServiceInterface::saveObject()` returns an `ObjectEntityInterface`, which extends `JsonSerializable`, so the defensive is_array() branch was never reachable. Calling `->jsonSerialize()` at the two sites is both simpler and adds no type reference at all. Same behaviour: full suite still 1931 tests, 0 errors, 0 failures.
…in l10n (#854) * feat(procest): translate enum values to English, Dutch kept in l10n 1786 substitutions across 267 files, property-anchored. The Awb outcome vocabulary is three distinct results and stays three distinct words: `gegrond` -> upheld, `ongegrond` -> dismissed (failed on the merits), `niet-ontvankelijk` -> inadmissible (not considered at all). Collapsing any two would erase a legal distinction. Likewise the Awb 7:11 dispositions: upheld_maintain / upheld_revoke / upheld_amend. WHAT IS DELIBERATELY NOT TRANSLATED * The ZGW and StUF adapter layers. Their vocabulary is the standard's, it is Dutch by statute and it travels on the wire. The first pass rewrote BOTH sides of LoadDefaultZgwMappings -- `'openbaar' => 'openbaar'` became `'public' => 'public'` -- which destroys the very translation the mapping exists to perform. 17 files reverted. * `vertrouwelijkheidaanduiding` (openbaar … zeer_geheim). It is the ZGW confidentiality designation, so the guard's copy must keep spelling it the way the wire sends it; translating one side broke three InformatieobjectAccessGuard tests. * The sentiment TRIGGER lists. `SERIOUS_TRIGGERS = ['klacht', …]` scans Dutch citizen messages for Dutch keywords -- `complaint` will never appear in Dutch text, so complaint detection would have stopped silently, half-translated. * A hash seed. `hash('sha256', 'rapport' . $seed)` is INPUT, not vocabulary; changing it changes every id the mock ever produced. * `tussenrapportage`. It is a schema KEY, and a slug rename needs the ImportHandler migration -- a different and more dangerous operation than a value rename. Held for a slug batch. l10n: 244 entries, and the `.js` twins REGENERATED. Nextcloud loads the .js at runtime while the l10n checker reads the .json, so a json-only edit passes the checker and still shows English to Dutch users. en.js had drifted to 779 entries against 3246 in json; both now match, verified by executing the file through a fake OC.L10N.register. Data migration: RenameDutchValues, with logic in RenameDutchValueDecisions and storage behind ValueMigrationPort so it can be tested without a real IDBConnection. Also fixes pre-existing debt: RenameDutchColumns' docblocks described shillinq. PHPUnit failing set is IDENTICAL to development (46 both sides, 0 introduced); PHPCS 0 errors; 0 orphaned defaults; 0 identity entries introduced. * test(e2e): pin the Dutch value vocabulary a Dutch UI must still show The value migration translated the enum values and l10n carries the Dutch word back. Nothing tested that end to end, and the failure is quiet: a Dutch user simply reads English. It has to be e2e. Nextcloud loads `l10n/<lang>.js` at RUNTIME while the l10n checker reads `l10n/<lang>.json` — two artefacts, and they had already drifted here (en.js held 779 entries against 3246 in en.json). A test over the JSON proves nothing about what the browser got. Two traps hit while writing it, both now guarded: * `/apps/procest/l10n/nl.js` answers HTTP 200 with the SPA SHELL — Nextcloud serves the app's HTML for any sub-path, so `res.ok` passes on HTML. The path is resolved from `OC.appswebroots` instead, which is how Nextcloud resolves it and which differs between apps/ and custom_apps/. The body is then asserted to begin `OC.L10N.register`. * Scraping the text would not notice a bundle that fails to PARSE, which at runtime registers nothing. It is evaluated, and what it REGISTERS is what gets asserted. Shown to discriminate before being committed: against the instance's pre-migration bundle it fails on `Dismissed` and `Partly upheld`; against this branch all four resolve. It also asserts the three Awb outcomes stay three DISTINCT Dutch words — collapsing `gegrond`/`ongegrond`/`niet-ontvankelijk` would erase a legal distinction while still satisfying a per-key check. * fix(procest): move the display-label keys and restore two bilingual fallbacks Three defects CI found that the local run could not, because the local vendor lacks OpenRegister's published contract and stopped 44 tests before they reached this code. 1. DISPLAY-LABEL MAPS. 64 keys across 17 Vue components map a stored value to its label — `gegrond: t('procest', 'Upheld')`. The keys are BARE identifiers, so the quoted-token pass never saw them: the value moved to `upheld` while the lookup stayed keyed `gegrond`, and every one of those components would have rendered a raw value or nothing at all. Renamed only where the value is a t() call or a string literal — the signature of a label map. A form model (`advies: null`, `advies: this.form.advies`) does not match and is untouched, which matters because `advies` is a property name, not a value. Same shape in `STATUS_TRANSITIONS`, where the keys stayed `concept`/`definitief` while the values became `final`/`archived`, so `isTransitionAllowed('draft', 'final')` could not resolve its own start state. 2. TWO BILINGUAL FALLBACKS collapsed into self-comparisons: `$zioData['case'] ?? ($zioData['zaak'] ?? '')` became `['case'] ?? ['case']`. psalm reported both as ParadoxicalCondition. The sibling line `['document'] ?? ['informatieobject']` is untouched and shows the intent. 3. TRIGGER-WORD and Dutch-prose SPECS reverted: kccTriggerWords and emailTemplatePreview assert on Dutch keyword/template content, and the pass had moved their inputs while leaving the expectations behind. vitest 349/349. PHPUnit failing set still IDENTICAL to development (0 introduced; the 44 errors are the local vendor gap, absent in CI). * style: prettier the two label maps Format-only; renaming the bare keys left two t() calls prettier rejoins. vitest still 349/349. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
This repo carried min-release-age=0 and none of the three settings that make a supply-chain cooldown work together: min-release-age was 0, fleet minimum is 2 (unit: DAYS) min-release-age-exclude[] absent — @conduction/* was not excluded engines.npm ^10.0.0 — npm 10 does not implement it Any one alone is a configuration that looks like protection and is not. The option does not exist in npm 10 (`npm config get min-release-age` answers `undefined`), so the window was read by NOTHING. The exclusion is the dangerous omission: without it the cooldown does not fail loudly, it silently resolves BACKWARDS. Installing @conduction/nextcloud-vue on release day under a cooldown with no exclusion resolves an old version and exits 0 — a live risk to the fleet's ^2.3.0 migration, because a green install of months-old first-party code is indistinguishable from a correct one. Verified with the gate's OWN script: 3 failure(s) exit 1 before, 0 failure(s) exit 0 after, "checked 3 setting(s)" on both sides. Pattern matches opencatalogi, nldesign and softwarecatalog, which already ship exactly this and are green. Piloted on decidesk#502: Frontend Build, Frontend Tests and Security (npm) all pass.
…ool that missed them (#856) * refactor(procest): translate the last two schema slugs, and fix the tool `bezwaar` -> `objectionProceeding`, `tussenrapportage` -> `interimReport`. 56 substitutions across 30 files, plus both /components/schemas/ map keys. WHY objectionProceeding. procest declares BOTH `bezwaar` and `objection`, and they are two entities with ZERO shared properties: `objection` is the SUBMISSION (contestedDecision, grounds, requestedRelief, receivedDate, isTimely) and `bezwaar` is the Awb PROCEEDING around it (case, a ref to that objection, status, awbReference, receiptDate, adjournedOn, suspensionStart). An earlier attempt renamed the first onto the second and produced a DUPLICATE JSON KEY -- legal, parses, and every parser keeps only the last, silently dropping one schema's lifecycle and calculations. Both are in RenameDutchSchemaSlugs::SLUG_MAP, which is not optional: OpenRegister's ImportHandler matches by SLUG, so a renamed slug without the migration makes the import CREATE A SECOND SCHEMA and strands every stored object behind one nothing reads. TWO TOOL DEFECTS FIXED, both of which this batch reproduced: * rename-slugs.js handled `$ref` only in its PATH form. A $ref may name its target by BARE schema key, and procest#849 shipped five dangling ones that had to be repaired by hand. I fixed the instances then and NOT the tool, so it happened again here -- 3 more. Fixed at the source. * its EXCLUDE named only its own step, so the slug pass walked into RenameDutchValueDecisions.php -- a DIFFERENT migration's map -- and turned `'tussenrapportage' => 'interimReport'` into `'interimReport' => 'interimReport'`. That migration's own identity test caught it. Now matched by prefix AND by content (any file declaring a VALUE_MAP / COLUMN_MAP / SLUG_MAP), with every skip logged. Verified: 0 dangling $refs across 29 register files, no duplicate JSON keys, PHPUnit failing set IDENTICAL to development (0 introduced). * style: prettier the three files the slug rename reflowed Format-only. * fix(test): the deepLink map is keyed by SLUG, so its keys moved too `searchableSchemas.spec.js` keys expectedTemplates/expectedRoutes by schema slug, as bare identifiers the slug pass cannot reach. The manifest's deepLink now says `objectionProceeding` while the spec still asked for `bezwaar`. The URL values are unchanged on purpose: `/bezwaren/:id` is a published ROUTE, it resolves at request time, and breaking one fails silently. Only the keys are slugs. vitest 349/349. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…858) `USE_LOCAL_LIB` was opt-OUT here, so unset — its normal state — meant "alias whatever `../nextcloud-vue` checkout happens to be on disk into a build that can ship". That was not theoretical in this repo. With the sibling present, `npm run build` failed outright: Module not found: Error: Can't resolve 'stream' in '.../procest/node_modules/sax/lib' Compiling the sibling's SOURCE also pulls in the sibling's own dependency graph, which wants node core polyfills this app deliberately does not configure (the resolve.fallback here stubs `path` and nothing else). The same command with the sibling disabled succeeds, which is what identified the cause. So the alias is now opt-in, and opting in is not enough on its own: the sibling must satisfy this app's declared range, checked with semver rather than by eyeballing a major. The check fails CLOSED — if it cannot run, the sibling is refused rather than trusted. Measured, all three states, exit code and behaviour: USE_LOCAL_LIB unset exit=0, sibling not used USE_LOCAL_LIB=true exit=0, prints "IGNORING sibling @conduction/nextcloud-vue@2.0.5" and builds the npm dist USE_LOCAL_LIB=false exit=0, sibling not used The opt-in case is the positive control: the sibling is 2.0.5 against a declared ^2.3.0, so the guard is shown refusing, not merely staying quiet.
…er a file-wide no-undef (#859) * chore(security): enable the npm supply-chain cooldown on npm 11 Sets `min-release-age=2` and `min-release-age-exclude[]=@conduction/*`, raises `engines.npm` to ^11.0.0, and regenerates the lockfile under npm 11. The .npmrc comment here has described a cooldown for months and it has never been in effect. `min-release-age` does not exist in npm 10 — `npm config get min-release-age` answers `undefined` — and every Node 22 release bundles npm 10, so the setting was read by nothing. Most repos also had it at 0, which disables it outright. @conduction/* is exempt because without the exemption the cooldown does not fail loudly, it silently resolves backwards: measured 2026-08-15, an install of @conduction/nextcloud-vue on release day picked 2.0.7 instead of 2.3.0 and exited 0. The lock is regenerated under npm 11 and iterated to a fixed point. Where the tree changed rather than its metadata, that is npm 10 -> 11 reconciling a lock shaped by the older resolver, not the cooldown — verified by regenerating with the cooldown enabled and disabled and getting identical trees. Verified: npm ci exit 0 under npm 11.19.0, @conduction/nextcloud-vue resolves to 2.3.0, gate-84 conformance passes. * ci: re-run against the merged shared workflow `gh run rerun` replays the workflow version resolved when the run was created, so a reusable workflow referenced as @main is NOT re-resolved — every re-run after ConductionNL/.github#469 merged still executed Node 22 with npm 10.9.8, where `min-release-age` does not exist and `npm ci` cannot read an npm-11 lockfile. Only a new run picks up the merged workflow. This empty commit is that trigger. * fix(lint): clear the last tranche-A suppression — require.context under a file-wide no-undef `require.context()` is a WEBPACK build-time API the bundler rewrites at compile time, so eslint is right that no runtime `require` exists and the code is right too. The file-wide `no-undef` suppression that recorded this also switched the rule off for every OTHER identifier in the file, so a genuine typo there would have been silent. Scoped to `/* global require */`. Same shape found in 9 apps across the fleet; all cleared the same way. Verified: eslint 0 errors, build exit 0, tests pass.
…seType property (#860) gate-54 relation-dialect, check (d): `zaaktypeInformatieobjecttype.statustype` carries `x-relation-filter: { "caseType": "@object.zaaktype" }`, but this schema has no `zaaktype` property — it has `caseType`. Commit 75f578d ("translate Dutch vocabulary to English, with migration", #832) renamed `zaaktype` -> `caseType`; the two sibling tokens on the same register (procest_register.json:1055 and :1274) were updated to `@object.caseType`, this one was missed. So this finding is INTRODUCED by that rename, not legacy debt. Effect of the defect: ADR-062 rule 6 drops an unresolved token rather than filtering on nothing, so the status-type picker on a zaaktype-informatieobjecttype row has been offering EVERY statusType in the register instead of only the ones belonging to this row's case type. This is NOT a data migration: `x-relation-filter` is picker-scoping metadata naming a sibling property. No property is renamed and no stored value changes — #832 already migrated the data. Both sides verified by hand (the gate only checks the value side): filter key `caseType` -> statusType.caseType OK token `@object.caseType`-> zaaktypeInformatieobjecttype.caseType OK gate-54, the gate's own vendored helper (check_relation_dialect.py, blob 3d4c81ca == ConductionNL/.github@main), over its own enumeration (git ls-files lib/Settings, register*.json + register.d/*.json): BEFORE: 1 finding over 22 files AFTER: 0 findings over 22 files
…861) Adopts the canonical script from ConductionNL/.github (quality-config/coverage-guard.php). The whole-project comparison fires on measurement noise. doriath#240 was a PR whose entire diff was `webpack.config.js` — no PHP at all — and the guard failed it: identical denominator (13723), both runs reporting exactly `Tests: 948, Assertions: 3051, Skipped: 1`, and six covered statements of run-to-run xdebug variance between them. The measured `--against` floor cancels driver variance (xdebug vs pcov), as its header says. It does not cancel run-to-run variance within one driver, and the ratchet has no tolerance. Scoping the comparison to the PHP a change actually touches keeps full strength where a regression matters and makes the noise unreachable by construction — a diff with no PHP cannot fail. New `changed-files` capability; the shared workflow PROBES for it rather than assuming, so an un-updated copy keeps the previous behaviour instead of silently accepting and ignoring the flag. Script only — no behaviour change until the workflow passes `--changed-files`. Byte-identical to the canonical copy (md5 5be122aad209da030c79b22a133232fb).
* fix(gate-57): delete eighteen orphaned write capabilities
gate-57 orphaned-write-capability reported 18 findings over 396 lib/Service
files — the largest set in the fleet. All eighteen had zero callers; all
eighteen are removed.
Two whole classes were unreferenced and are deleted:
- HoorzittingCalendarSync (sync) — nothing constructs it.
- Inspection\ChecklistService (createRun, submitRun) — a second, parallel
implementation of a capability the live InspectionChecklistService already
serves through InspectionChecklistController. Deleting only the two flagged
methods surfaced dispatchFollowUps() as the next orphan and would have left
a dozen unreachable private helpers, so the class goes as a whole; the two
files that cited it as their answer-convention reference now cite the
schemas and Support\ChecklistPayloadReader instead.
Four removals are security-relevant, and each is a case where wiring would
have OPENED a surface rather than closed a gap:
- TenantJwtService::createTokenFromSaml (+ createToken, its only caller) —
a tenant JWT minter taking roles straight from an assertion. Procest has no
token-issuing route; the middleware only validates tokens the external
broker mints. Both go together so the base minter does not become the next
finding.
- TenantLifecycleControlService::archiveAndDelete — dropped a tenant's
database schema. No caller, no retention timer, nothing references the
service at all.
- Stuf\StufVaultService::storeSecret — wrote a plaintext credential into app
config. All three vault consumers only ever resolveSecret().
- AdvisoryBodyService::issueSecureToken — minted a public consultation-access
token. The read half (/api/public/consultations/{token}) is live but can
never be entered, because nothing ever minted one. Reported as a capability
gap rather than closed by adding an unguarded minter.
The rest were writers for paths that do not exist, or log-only stubs:
AdvisoryBodyService::sendExternalNotification (a log line; real delivery is
the schema's x-openregister-notifications config, ADR-031),
Bezwaar\BeroepService::recordJudgment, Cmmn\CaseModelLoader::clearCache and
WorkflowTemplateLoader::clearCache (request-scoped memos),
ConflictOfInterestService::clearConflict, DoorverbindingService::
createContextSnapshot, EmailArchivalService::markComplete,
ParaferingNotificationService::notifyParaferingReminder,
TenantConfigurationService::updateBranding + ::updateLocale, and
VergaderingCaseService::createForVergadering.
EmailPdfRetryJob's comment is updated rather than left stale: it now says the
success branch must re-add markComplete() when the Docudesk adapter lands,
because that branch is the only caller it will ever have.
Before: 18 findings over 396 files. After: 0 findings over 394 files.
Measured with the helper from ConductionNL/.github@main (43ecb0c8), not the
stale vendored copy.
* fix(phpstan): remove the four symbols the gate-57 deletions left unread
Deleting eighteen orphaned write capabilities left their private collaborators
behind, and PHPStan is the only leg that notices:
BeroepService::VALID_OUTCOMES unused const (read only by recordJudgment)
VergaderingCaseService::AGENDA_DEADLINE_DAYS unused const (read only by createForVergadering)
TenantLifecycleControlService::$schemaProvisioner never read, only written
TenantLifecycleControlService::$provisioning never read, only written
All four are removed at source rather than baselined — procest's phpstan.neon
says so in its own header ("No baseline. Fix at source"), and a suppression
would leave the schema-drop collaborator injected into a service that
deliberately no longer exposes a schema drop.
TenantLifecycleControlServiceTest now asserts, by reflection, that the
constructor does not take TenantSchemaProvisioner — re-adding the parameter and
its setUp() argument makes the test print
"must not be injected with the schema provisioner" rather than pass silently.
The assertion carries an anti-vacuity guard on the parameter list it reads.
phpstan, same command, same tree, same vendor, php 8.3.33:
before: [ERROR] Found 4 errors after: [OK] No errors (exit 0)
PHPUnit tests/Unit/Service/TenantLifecycleControlServiceTest.php: OK (5 tests, 12 assertions)
… for it
`StufRegisterAccess::getObjectService()` was a bare
`$container->get('OCA\OpenRegister\Service\ObjectService')` — ADR-083 rule 1's
exact shape: the dependency is declared NOWHERE a reader or a gate can see it,
so the app looks constructable without OpenRegister and then fails at the first
StUF write with a container exception instead of a stated reason.
OpenRegister is genuinely OPTIONAL here (StUF outbound is one integration among
several), so the rule's second remedy applies: establish availability first and
keep the lookup. `OpenRegisterSharingGateway` in this same app already does
exactly that, so this adopts the shape the app had rather than inventing one —
inject `IAppManager`, guard on `isInstalled('openregister')`, catch a failed
resolve, log, return null.
## The null had to be handled, not just returned
Widening the return type without touching the callers would trade a container
exception for a fatal on `null->saveObject()`, so both call sites now decide:
* `findAll()` returns `[]` — it is a READ, an empty list is the honest answer
for an absent register, and it is already what the method returns when the
lookup throws two lines below.
* `saveObject()` THROWS. Returning the payload unsaved would look like a
successful write to every caller, and an outbound StUF audit record that
silently did not persist is worse than a loud failure.
gate-66: 1 -> 0 (616 files checked). Nothing constructs this class manually —
lib/ and tests/ have no `new StufRegisterAccess(` — so the added constructor
parameter is autowired and breaks no caller.
## Hydra Gates stays red, and what is left is not mine to decide
* gate-53 (8) — every one is `removals-invariant`: a `menu-layout` removal
orphans a route that still exists, so ADR-044 (no functionality loss) fails.
`BezwaarBeroepGroup`, `Bezwaren`, `Beroepen`, `SubsidiesGroup`, `CaseMap`,
`Voorstellen`, `Advice`, `BesluitvormingAgenda`. Fixing it means either
restoring menu entries or deleting the routes — a decision about what this
app's menu should offer, not a lint fix.
* gate-7 (27), gate-25 (116), gate-26 (78) — untouched here.
…ailability fix(gate-66): declare the OpenRegister dependency instead of reaching for it
…un (#864) * test(e2e): seed the Workflow Board so the two kanban tests actually run Both tests in kanban-board-keyboard-status-transition.spec.ts opened the board, found no `.case-card`, and skipped with 'No cases on the Workflow Board to exercise the move control' That reason was TRUE, and that is exactly why it had to be fixed as a FIXTURE change rather than a timing one. The board is data-dependent: WorkflowBoard.fetchData() builds one column per non-final statusType and groups cases into a column by resolving `case.status` to that statusType's name. With no statusTypes and no cases in the target register there is nothing to render, so on CI these two assertions had never executed at all — an invisible pass under L8, reported as "not applicable" rather than as a gap. The file now seeds the same shape workflows/case-lifecycle.spec.ts seeds — seedStateMachine() for a caseType, two non-final statusTypes, one final one and an active workflowTemplate, then seedCase() for the card — and asserts with no escape hatch. Two non-final statusTypes is the minimum the move control needs: CaseCard renders its NcActions only when `otherColumns.length > 0`. That fixture is known-good rather than newly invented: case-lifecycle.spec.ts:185 ("the workflow board renders a column per status type with real case rows") passes in CI using precisely it. Assertions are scoped by RUN_PREFIX (`.case-card` filtered on the seeded title, the seeded column by name) so the tests never drive data they did not create, and afterAll deletes exactly what this run made. Expect the skip count to fall by two. Whether those two then pass or fail is a genuine verdict either way, and either is worth more than a permanently-true skip. * fix(e2e): keep the seeded machine, and drop serial mode — both were measured Two corrections to the first commit, each found by running it rather than by reading it. 1. `mode: 'serial'` turned a FAILURE into a REASON-LESS SKIP. Measured locally: with serial on, a failure in the first test marks the second "did not run", and the report records that as outcome "skipped" with NO annotation at all — a skip carrying no reason whatsoever, which is strictly worse than the false-reason skips this file removes. The two tests share only the beforeAll fixture and neither depends on the other's side effects, so serial bought nothing. Off serial the same run reports 2 failed, 0 skipped. 2. Child-first cleanup left a DANGLING case and reddened another test. The `case` schema is archival: a user-driven DELETE is refused with 403 ArchivalImmutableException and the record persists — cases-crud.spec.ts:225 asserts exactly that and passes. `fixtures.ts#deleteObject` never inspects the response, so `cleanupRunObjects(…, ['case'])` reports success and removes nothing. The caseType and statusTypes ARE deletable, so deleting them orphaned a case that was still live. In CI run 31964165472 that showed up four hops away: the dashboard's grouped aggregations (aggregations/procest/case/grouped?groupBy=status and =caseType) still return the orphaned case's group keys, the chart widget resolves each key by id, and two of those lookups 404 — reddening spec-coverage/ui-pages.spec.ts:55 "dashboard mounts without procest console errors". That test was doing its job. afterAll now preserves the surviving case's referential integrity and sweeps only what is actually deletable. The residue is RUN_PREFIX-tagged and consistent, and CI builds a throwaway instance per run. workflows/case-lifecycle.spec.ts carries the same latent defect and is invisible only because `workflows/` sorts after everything that would notice. Recorded on the fleet board, not changed here.
…mment
gate-26 (visual-coverage) reads the e2e corpus through
`source_scope.js_comment_mask`, which blanks comments and keeps string
literals, so a component named only in a comment is INVISIBLE to it
(.github#358 — "a comment is not a baseline"). Four procest screens were
in exactly that state: a spec really navigated to the route and asserted
its surface, but nothing in the executable text said which component that
route renders, so the gate — and a reader — could not tell.
Adds tests/e2e/helpers/page-components.ts: route constants whose
IDENTIFIER is the rendering component's file stem, and uses them in the
specs that already drive those routes. Every substituted string resolves
to the byte-identical URL, so this changes no behaviour and adds no
assertion — it only makes the existing coverage legible.
Screens closed (measured with the gate's own check_visual_coverage.py,
full-tree mode, before -> after): 78 -> 74 uncovered.
src/views/MyWorkCards.vue spec-coverage/my-work.spec.ts
src/views/CasesOnMapView.vue spec-coverage/workflow-operations.spec.ts
src/views/workflow-board/WorkflowBoard.vue spec-coverage/workflow-operations.spec.ts
src/views/admin/VerwerkingenOverview.vue spec-coverage/avg-verwerkingenlogging.spec.ts
+ visual/procest.visual.spec.ts
`/substitution` and `/substitution-admin` are deliberately NOT given
constants here: the only spec that drives them
(handler-vervanging-waarneming.spec.ts) is mid-flight in PR #863, and a
constant nobody imports would be the "declaration nobody reads" this
helper's own docblock warns against.
…863) * fix(e2e): de-race 9 test.skip() gates on the non-waiting isVisible() procest was NOT in this workstream's brief — S37's fleet sweep scored it ZERO, because its subset grep matched the literal `isVisible()` with empty parens and every procest gate is written `isVisible({ timeout: N })`. Re-reading the source found nine, and procest skips 38 of 128 tests. `locator.isVisible()` is an IMMEDIATE predicate — its `timeout` option is IGNORED, so `{ timeout: 10000 }` bought nothing here. Called on the tick after a `goto`, it asks "is this here right now", before the SPA has issued an XHR. 🔑 AND THE STATED REASONS ARE FALSE. The gates skip with "Workflow Board surface not deployed in target instance" "Case Type Management admin section not present in the deployed build" while `/workflow-board` is a declared route in `src/manifest.json` carrying the title "Workflow Board", and "Case Type Management" is a real heading rendered by `src/views/settings/AdminRoot.vue`. Both surfaces ship. The instrument looked too early and then blamed the deployment. A skip whose stated reason is untrue is an invisible pass — worse than a failure, because the count looks deliberate and the reason looks investigated. Gates de-raced (9): - workflow-editor-canvas 5, of which 3 are in the shared `openFirstCaseTypeWorkflowTabOrSkip()` helper that gates both of the file's tests - handler-vervanging-waarneming 3 (+1 optional-branch probe) - kanban-board-keyboard-status-transition 3 Adds `tests/e2e/helpers/becomes-visible.js`, a polling probe on `waitFor`. The `test.skip()` calls are KEPT: the fix is not to unskip, it is to make the gate tell the truth. A skip that survives this is skipping for the reason it states.⚠️ This may turn silent skips into REAL FAILURES. That is the point, and any such failure will be reported as a finding rather than re-skipped. Verified against the COMMITTED tree, not the working tree: `git archive HEAD` extracted to a clean directory, `playwright --list` run there. eslint on the 3 changed specs: base 5 problems, branch 5. Zero introduced. * docs(e2e): CORRECTION — I claimed procest's skip reasons were false. They are TRUE. The measurement contradicted me, so the claim goes, not the measurement. I asserted in the previous commit and in the PR body that these skips were invisible passes, reasoning from source that: - `/workflow-board` is a declared route in `src/manifest.json`; - `WorkflowBoardView` resolves in `src/registry.js`; - `WorkflowBoard.vue` renders an UNCONDITIONAL `<h2>Workflow Board</h2>`; - "Case Type Management" is a real heading in `AdminRoot.vue`. Every one of those facts is true. **The conclusion drawn from them was not.** Measured with these polling gates in place (job 95202301447, E2E `success`, `enable-playwright: true`): BASE Running 128 tests → 90 passed 0 failed 38 skipped PR Running 128 tests → 90 passed 0 failed 38 skipped Byte-identical, and the skip list diffs to line-number shifts only. Nor is this the "suite measuring an unbuilt app" trap — the job's own log shows `warm bundle /apps/procest/js/procest-main.js -> 200 application/javascript 3163201`. The elements genuinely do not appear within 10–15 s. **The skips are honest.** 🔑 SOURCE CODE PROVES A SURFACE IS DECLARED; IT DOES NOT PROVE IT RENDERS. A route can be declared, registered, wired to a component with an unconditional heading — and still never mount. The remaining suspect is the server-side URL the specs navigate to (`/index.php/apps/procest/workflow-board`) versus where the SPA actually mounts that page, which is an app/routing question and out of this workstream's scope.⚠️ Note the symmetry, which is why this correction matters more than the wording: this workstream exists because a SKIP carried a stated reason nobody had verified. I then wrote a CODE COMMENT carrying a stated reason nobody had verified. Same defect, opposite sign — and a comment asserting "these reasons are false" would have sent the next reader hunting a bug that is not there. The code change is unaffected and still correct: a polling gate is strictly better than a non-waiting one regardless of which way the answer comes out. What changed is that the 38 skips are now **believable** — each survived a real poll — instead of merely unexamined. * docs(e2e): correct the correction — the reasons differ PER GATE, and the count cannot show it I got procest wrong twice, in opposite directions, and the resolution is more useful than either answer. 1. First I asserted the skip reasons were FALSE, reasoning from source: the route is declared, the component resolves, the `<h2>` is unconditional. 2. Then the tally came back byte-identical (128/90/0/38) and I over-corrected to "the reasons are all TRUE, the elements genuinely do not appear". **Both were wrong, because the question is PER GATE and a skip COUNT cannot answer it.** The kanban test has two gates in series: 1. heading "Workflow Board" → "Workflow Board surface not deployed in target instance" ← REASON IS FALSE 2. `.case-card` → "No cases on the Workflow Board to exercise the move control" ← REASON IS TRUE Gate 1 is false, proven by a SIBLING TEST PASSING IN THE SAME RUN: `spec-coverage/workflow-operations.spec.ts:18` reaches the board by identical navigation — `navToRoute()` is literally `page.goto('/index.php/apps/procest' + route)` plus the same dialog dismissal — and asserts `.workflow-board__header h2` visible. The board renders. So my "app/routing" theory in the previous commit was also wrong, and is withdrawn. Gate 2 is true, proven by another passing test: `workflows/case-lifecycle.spec.ts:185` is the only spec that sees case cards, and it **seeds two cases itself** first. The specs in this file seed nothing, so there are no cards. 🔑 THE SKIP MOVED FROM A FALSE REASON TO A TRUE ONE, AND THE COUNT — the only thing CI reports — COULD NOT SHOW IT. De-racing gate 1 hands control to gate 2. A stable skip count is compatible with every reason underneath it changing. 🔑 What settled both gates was not reading the app. It was finding a PASSING TEST that already exercised the same thing. That evidence is cheap, and it is stronger than any amount of reasoning from source.⚠️ Skip reasons are NOT auditable from CI here: the `list` reporter drops them, and the uploaded `playwright-report` artifact is a 648 KB `index.html` with its data folder missing — an empty shell that shows no tests when opened. ➡️ FOLLOW-UP, EVIDENCED, DELIBERATELY NOT DONE HERE: to actually recover these tests, seed cases the way `case-lifecycle.spec.ts` does instead of hoping the shared fixture has some. That is a fixture change, not a timing change, and it deserves its own reviewed commit. Comment-only. No code change. * docs(e2e): the skip REASONS are recoverable, and they settle this — nothing was false Third and final correction, and this one is backed by the recorded reasons rather than by inference. I should have found this instrument on day one. Playwright's HTML report embeds its data as base64 in index.html (`<script id="playwrightReportBase64">data:application/zip;base64,…`), which decodes to a ZIP of JSON carrying every `{"type":"skip","description":…}`. The `list` reporter drops reasons, so the CI LOG cannot answer this — but the `playwright-report` ARTIFACT can, and procest uploads one. Extracted for base and branch. Identical sets: 12 Sub-cases tab not present in the deployed build (deploy mismatch). 9 Related cases sidebar tab not present in the deployed build (…). 9 No case types in the deployed/seeded register — … 3 No cases on the Workflow Board to exercise the move control 3 No cases on the Workflow Board to exercise the drag path 3 OR addresses register not installed (…) 🔑 "Workflow Board surface not deployed in target instance" — the reason I twice argued about — DOES NOT APPEAR ON EITHER SIDE. That gate was already passing before this change; the non-waiting probe happened to win. The gates that fire are the SECOND ones in each chain, and their reasons are TRUE (these specs seed nothing; the only spec that sees case cards seeds them itself). So my previous commit's claim — "the skip moved from a false reason to a true one" — is ALSO wrong, and withdrawn. Nothing moved. **These nine gates were latent, not false. procest had no invisible pass.** The value of this change is that each surviving skip now reports a real absence instead of depending on a probe that could not have known. ➡️ THE REAL SUSPECTS ARE NOT `isVisible()`. The two largest clusters (21 of 39) come from `deelzaak-support.spec.ts` and `related-case-linking.spec.ts`, gating on `(await tab.count()) === 0` after a bare `waitForTimeout(1000)`. `count()` does not wait either — same defect, different method, invisible to every `isVisible` grep — and the reason blames the DEPLOYMENT, which is the tell. Recorded as unverified: "Sub-cases" is declared in `src/manifest.json`, and this file has already been wrong once about declaration implying rendering. Comment-only. No code change.
test(e2e): name the screen a spec drives in executable code (gate-26 78 -> 74) + measured verdict on gates 7/25/53/62
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated PR to sync development changes to beta for beta release.
Merging this PR will trigger the beta release workflow.