feat(governance): add the cross-app command seam for governance bodies by rubenvdlinde · Pull Request #1019 · ConductionNL/decidiq

rubenvdlinde · 2026-08-30T19:11:24Z

Why

decidiq#874 gave GovernanceBody the four fields a Dutch bezwaaradviescommissie needs (active, numeric quorum, jurisdiction, Membership.external) and a REST write path. dossiq's migrate-committees-to-decidiq stayed BLOCKED anyway, and its proposal says why: that path is the door for external callers, and an in-process app-to-app command cannot use it.

Two reasons, both load-bearing:

ADR-041 says a cross-app command travels as a typed event. ADR-066 amended that for collection only; gate-27 (no-phantom-cross-app-rpc) enforces the command rule.
ApiController::write() refuses when userSession->getUser() is null — exactly the state a background migration runs in.

What

The missing door, copying the DecisionRequestedEvent shape this app already uses:

GovernanceBodyRequestedEvent / GovernanceBodyCreatedEvent
GovernanceBodyCommandService — idempotent upsert plus the roster fan-out to Person + Membership
GovernanceBodyRequestedListener, registered beside registerDecisionEvents
sourceApp + externalReference on GovernanceBody, additively (required unchanged)

Every write is preceded by a resolve: the body on (sourceApp, externalReference), a Person on nextcloudUserId, a Membership on its (person, governanceBody) pair. The body is saved and its id read before the first membership write, so a crash mid-fan-out leaves a body the next run completes rather than orphans pointing at nothing.

active is refused when absent rather than defaulted to true. It is the one field the consuming app throws on, and a silent default would route objections to a disbanded committee with nothing erroring.

ApprovalRouteStore is renamed RegisterObjectStore. It was already generic and this change is its second consumer; a second copy would have been the "second store that drifts" hazard in miniature.

Verification

1241 unit tests pass (22 skipped, pre-existing)
PHPCS / PHPMD / PHPStan / Psalm clean on every changed file
All 73 applicable hydra gates green, including gate-27 no-phantom-cross-app-rpc
Every idempotency test calls the seam twice and counts rows. A test that calls it once sees a body either way and cannot tell an idempotent write from a duplicating one. Three mutations each turn the suite red: making findBody() always miss, making the Person lookup always miss, and defaulting active to true.
Exercised end to end on a live instance against the dossiq consumer: a seeded bezwaaradviescommissie produced GovernanceBody{bodyType: advisory-body, quorum: 3, active: true, statutoryBasis: 'Awb 7:13', sourceApp: 'dossiq', externalReference: <committee uuid>}.

Impact

Unblocks dossiq's migrate-committees-to-decidiq, which asks nothing else of decidiq.

Consumer PR: ConductionNL/dossiq#1531 — merge this one first.

🤖 Generated with Claude Code

decidiq#874 made GovernanceBody able to hold a Dutch bezwaaradviescommissie and added a REST write path for it. That path is the door for EXTERNAL callers. An in-process app-to-app command cannot use it: ADR-041 says a cross-app command travels as a typed event (gate-27 enforces it), and an in-process HTTP call to our own instance has no session, so ApiController::write() refuses it — which is exactly the state a background migration runs in. So this adds the missing door, copying the DecisionRequestedEvent shape: - GovernanceBodyRequestedEvent / GovernanceBodyCreatedEvent - GovernanceBodyCommandService — the idempotent upsert plus the roster fan-out to Person + Membership - GovernanceBodyRequestedListener, registered beside registerDecisionEvents - sourceApp + externalReference on GovernanceBody, additively Every write is preceded by a resolve. The body resolves on (sourceApp, externalReference), a Person on nextcloudUserId, a Membership on its (person, governanceBody) pair, so a re-run of a consuming migration updates rather than minting a second of each. The body is saved and its id read BEFORE the first membership write, so a crash mid-fan-out leaves a body the next run completes instead of orphans pointing at nothing. `active` is refused when absent rather than defaulted to true: it is the one field the consuming app throws on, and a silent default would route objections to a disbanded committee with nothing erroring. Every idempotency test calls the seam TWICE and counts rows. A test that calls it once sees a body either way and cannot tell an idempotent write from a duplicating one. Mutation-checked: making findBody() always miss, making the Person lookup always miss, and defaulting `active` to true each turn the suite red. ApprovalRouteStore is renamed RegisterObjectStore. It was already generic (save/findAll/normalise over decidiq's register) and this change is its second consumer; a second copy would have been the "second store that drifts" hazard in miniature. Unblocks dossiq's migrate-committees-to-decidiq, which is BLOCKED on precisely this seam and asks nothing else of decidiq. Verified: 1241 unit tests pass (22 skipped, pre-existing), PHPCS/PHPMD/PHPStan/ Psalm clean on every changed file, and all 73 applicable hydra gates green including gate-27 no-phantom-cross-app-rpc.

…gue the two new schema titles Two real CI failures on this branch, both caused by it. phpmd: adding a second event/listener pair took DomainServiceRegistrar to a coupling of 14, over the threshold — measured 0 on development. The registrar's job is registration, so the coupling is inherent; the fix is that the inbound cross-app surface is its own concern. CrossAppEventRegistrar now holds it as one map in one file, which also means adding a command is one line. This extraction was already in the stacked approval-route PR. It belongs here instead: this is the branch where the threshold is first crossed, and a stacked PR must not be what makes its parent green. check:schema-l10n: `Source app` and `External reference` are new schema property titles with no catalogue key, so they would render in English inside an otherwise translated form. Added to en.json and nl.json and rebuilt the js catalogues. The ratchet is back to its 1634 baseline. The third red check, "Conflict markers and PHP syntax", was CANCELLED rather than failed — it re-runs on this push. Verified at CI SCOPE, not just on the changed files: phpmd, phpcs, phpstan and psalm clean across the whole tree, 1241 unit tests pass, and all four npm checks pass. check:manifest needs Ajv resolvable; without it the script silently falls back to a structural lint and reports a PRE-EXISTING page-type finding, which is worth knowing before someone reads that fallback as a regression.

github-actions · 2026-08-30T22:56:13Z

Quality Report — ConductionNL/decidiq @ `b9e125c`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	❌
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-nav-ceiling		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		❌
composer			✅	✅ 104/104
npm			✅	✅ 557/557
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					✅
Newman					✅
Playwright					🚨 NO VERDICT — enabled but never ran
Hydra gates					✅

Quality workflow — 2026-08-30 22:56 UTC

Download the full PDF report from the workflow artifacts.

…ered The merge brought in schema strings with no catalogue key, which would render in English inside an otherwise translated form: the tender `awardedTo` and `referenceNumber` descriptions and the ORI `schemaOrgType` one. This is pre-existing debt rather than something this branch introduced — origin/development measures 1637 uncovered against a 1634 baseline on its own, so the check is red there too. Catalogued rather than worked around, which puts the count at 1632 and lets the ratchet come DOWN two. Baseline lowered accordingly, which is what the checker asks for when the count improves.

decidiq's development branch currently fails five Code Quality jobs. Three of them showed up on this PR and none of the three is this branch's doing, so they are fixed here rather than reported as inherited: - Frontend Check (format): tests/e2e/spec-coverage/example-set-setup-step.spec.ts was not prettier-formatted. Formatted. - Hydra Gates (gate-102 manifest-l10n-coverage): six manifest strings from development's example-set setup step had no nl.json key, so they rendered English to a Dutch user. Catalogued and rebuilt; the gate now passes and all 75 applicable gates are green. - Frontend Check (check:l10n-js): nl.js was behind nl.json. The l10n:build above brings them back in sync. The fourth, Integration Tests (Newman), is NOT fixed here and is not mine: one assertion expects `process-templates` to contain the built-in `association-alv` slug and gets an empty list. That is ProcessTemplate SEEDING, which this branch does not touch, and it fails identically on development. Left alone rather than patched blind — a seeding fix belongs with whoever changed the templates, and guessing at it from here would be a change nobody could review against intent. The fifth, E2E Tests (Playwright), is likewise pre-existing on development.

rubenvdlinde · 2026-08-31T02:38:06Z

Newman failure: diagnosed, deliberately not patched here

Integration Tests (Newman) fails on this PR and identically on development — it is not this branch's doing. I traced it rather than leave it as "pre-existing":

AssertionError  built-in ALV template is present
                expected [] to include 'association-alv'
                inside "GET process-templates as admin returns the built-in catalogue"

association-alv is defined in lib/Settings/profiles/association.json — a profile, not a built-in. It only exists once that profile is loaded.

Development's own example-set setup step made profile loading a deliberate choice; its copy says so outright: "Installing the app plants nothing on its own, so this is the only step that adds data." So the collection asserts data that is no longer seeded by default. The test and the seeding changed apart from each other.

Why I did not patch it. There are two defensible fixes and they mean different things:

the collection should load the association profile before asserting, or
process-templates should expose a built-in catalogue that is profile-independent, and association-alv genuinely belongs in it

Which one is right depends on what the built-in catalogue is meant to be, and that is the intent of whoever made profiles opt-in. Guessing from here would produce a change nobody could review against intent — and a green Newman that agreed with the wrong answer is worse than a red one that names the question.

The other four jobs red on development are addressed: format, Hydra Gates (gate-102) and check:l10n-js are fixed in this PR; E2E Tests (Playwright) is the remaining pre-existing one.

github-actions · 2026-08-31T02:41:04Z

Quality Report — ConductionNL/decidiq @ `ddb9ca9`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-nav-ceiling		✅
test-l10n		✅
format		❌
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 104/104
npm			✅	✅ 550/550
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					✅
Newman					❌
Playwright					🚨 NO VERDICT — enabled but never ran
Hydra gates					❌

Quality workflow — 2026-08-31 02:41 UTC

Download the full PDF report from the workflow artifacts.

github-actions · 2026-08-31T03:05:41Z

Quality Report — ConductionNL/decidiq @ `c0c8d54`

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-manifest		✅
check-nav-ceiling		✅
test-l10n		✅
format		✅
check-l10n-js		✅
check-schema-l10n		✅
composer			✅	✅ 104/104
npm			✅	✅ 537/537
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					✅
Newman					❌
Playwright					🚨 NO VERDICT — enabled but never ran
Hydra gates					✅

Quality workflow — 2026-08-31 03:05 UTC

Download the full PDF report from the workflow artifacts.

…1028) * feat(governance): add the cross-app command seam for governance bodies decidiq#874 made GovernanceBody able to hold a Dutch bezwaaradviescommissie and added a REST write path for it. That path is the door for EXTERNAL callers. An in-process app-to-app command cannot use it: ADR-041 says a cross-app command travels as a typed event (gate-27 enforces it), and an in-process HTTP call to our own instance has no session, so ApiController::write() refuses it — which is exactly the state a background migration runs in. So this adds the missing door, copying the DecisionRequestedEvent shape: - GovernanceBodyRequestedEvent / GovernanceBodyCreatedEvent - GovernanceBodyCommandService — the idempotent upsert plus the roster fan-out to Person + Membership - GovernanceBodyRequestedListener, registered beside registerDecisionEvents - sourceApp + externalReference on GovernanceBody, additively Every write is preceded by a resolve. The body resolves on (sourceApp, externalReference), a Person on nextcloudUserId, a Membership on its (person, governanceBody) pair, so a re-run of a consuming migration updates rather than minting a second of each. The body is saved and its id read BEFORE the first membership write, so a crash mid-fan-out leaves a body the next run completes instead of orphans pointing at nothing. `active` is refused when absent rather than defaulted to true: it is the one field the consuming app throws on, and a silent default would route objections to a disbanded committee with nothing erroring. Every idempotency test calls the seam TWICE and counts rows. A test that calls it once sees a body either way and cannot tell an idempotent write from a duplicating one. Mutation-checked: making findBody() always miss, making the Person lookup always miss, and defaulting `active` to true each turn the suite red. ApprovalRouteStore is renamed RegisterObjectStore. It was already generic (save/findAll/normalise over decidiq's register) and this change is its second consumer; a second copy would have been the "second store that drifts" hazard in miniature. Unblocks dossiq's migrate-committees-to-decidiq, which is BLOCKED on precisely this seam and asks nothing else of decidiq. Verified: 1241 unit tests pass (22 skipped, pre-existing), PHPCS/PHPMD/PHPStan/ Psalm clean on every changed file, and all 73 applicable hydra gates green including gate-27 no-phantom-cross-app-rpc. * fix(ci): give the cross-app listeners their own registrar, and catalogue the two new schema titles Two real CI failures on this branch, both caused by it. phpmd: adding a second event/listener pair took DomainServiceRegistrar to a coupling of 14, over the threshold — measured 0 on development. The registrar's job is registration, so the coupling is inherent; the fix is that the inbound cross-app surface is its own concern. CrossAppEventRegistrar now holds it as one map in one file, which also means adding a command is one line. This extraction was already in the stacked approval-route PR. It belongs here instead: this is the branch where the threshold is first crossed, and a stacked PR must not be what makes its parent green. check:schema-l10n: `Source app` and `External reference` are new schema property titles with no catalogue key, so they would render in English inside an otherwise translated form. Added to en.json and nl.json and rebuilt the js catalogues. The ratchet is back to its 1634 baseline. The third red check, "Conflict markers and PHP syntax", was CANCELLED rather than failed — it re-runs on this push. Verified at CI SCOPE, not just on the changed files: phpmd, phpcs, phpstan and psalm clean across the whole tree, 1241 unit tests pass, and all four npm checks pass. check:manifest needs Ajv resolvable; without it the script silently falls back to a structural lint and reports a PRE-EXISTING page-type finding, which is worth knowing before someone reads that fallback as a regression. * feat(approval): add the cross-app command seam for approval routes `approval-routes` shipped the engine and named the consumer it was built for: dossiq's parafering, "implemented today in dossiq, where it works", which "does not belong there". That migration still cannot start, for the same reason governance bodies could not move until the previous commit fixed it — the engine is reachable only over REST, and REST is the wrong door for an in-process app-to-app command (ADR-041, gate-27), besides refusing a request with no session, which is the state a migration runs in. So this adds the door: - ApprovalRouteRequestedEvent — hold this route, and optionally start one subject travelling it in the same command - ApprovalActionRequestedEvent — record one actor's action - ApprovalRouteConcludedEvent — emitted when an action decides the final stage - ApprovalRouteCommandService, two listeners, and `sourceApp` + `externalReference` on ApprovalRoute as the idempotency key 🔴 IT IS NOT A SECOND ENGINE, and that is the design constraint. Which stage is active, what a return does, which actors may act, whether a mandatory stage may be skipped: all of it stays in ApprovalRouteService and none of it is restated. testSeamAndServiceCannotDiverge advances two identical subjects — one through the seam, one through the service — and asserts their stage rows are identical, so a seam that grew its own rules fails rather than drifting. Two smaller decisions worth reading: The listener carries the engine's REFUSAL REASON back, not a bare false. "You are not the named actor" and "there is nothing to act on" need different handling by the producer and a boolean collapses them. No producer-supplied `step` reaches the engine. The engine decides which stage is active; accepting a step number would let an action be filed against a stage nobody is waiting on. `ApprovalRouteService::stagesFor()` is promoted to public so the seam can answer "did that finish the route" by asking the engine rather than running a second query over decision-stage rows. The event registrations move out of DomainServiceRegistrar into a new CrossAppEventRegistrar. What was one event/listener pair is now four, and each pair cost that class two imports: it went from clean on development to a coupling of 14 after the governance seam and 18 after this one. Splitting fixes the measurement and reads better — the whole inbound cross-app surface is now one map in one file. Verified: 1262 unit tests pass (22 skipped, pre-existing), PHPCS/PHPMD/PHPStan/ Psalm clean on every changed file, the fragment merge run through a real deep-merge rather than assumed, and ALL 75 applicable hydra gates green with all 75 actually running. Mutation-checked: making the route lookup always miss, never reporting completion, and dropping the empty-steps guard each turn the suite red. Stacked on #1019 — it needs the RegisterObjectStore rename. Unblocks dossiq's parafering migration. * fix(l10n): catalogue the two new approval-route property descriptions check:schema-l10n counts property DESCRIPTIONS as well as titles, so the sourceApp/externalReference helper text on ApprovalRoute would have rendered in English inside an otherwise translated form. The ratchet is back to its 1634 baseline. * fix(l10n): resolve the en.json merge conflict the previous commit carried The merge resolved nl.json but left en.json's conflict markers in place, and they were committed. Invalid JSON there is silent in the worst way: check:schema-l10n could not parse the catalogue, so it reported 1948 of 1948 strings uncovered — a total failure that reads like a catastrophic regression rather than an unparseable file. Both sides of that conflict ADD keys, so both are kept. Artefacts rebuilt; the ratchet is back at its 1632 baseline. --------- Co-authored-by: Ruben van der Linde <juan.claude@conduction.nl>

rubenvdlinde requested review from Rem-Dam, WilcoLouwerse, bbrands02 and rjzondervan as code owners August 30, 2026 19:11

This was referenced Aug 30, 2026

feat(bezwaar): hold advisory committees in the decision app, and run the referral as a flow ConductionNL/dossiq#1531

Merged

feat(approval): add the cross-app command seam for approval routes #1023

Closed

rubenvdlinde requested review from SudoThijn and remko48 as code owners August 30, 2026 20:34

rubenvdlinde mentioned this pull request Aug 30, 2026

feat(approval): add the cross-app command seam for approval routes #1028

Merged

juanclaude-conduction added 3 commits August 31, 2026 04:03

Merge development into the governance-body seam

4b617d5

Merge development into the governance-body seam

7512e94

rubenvdlinde merged commit dad5534 into development Aug 31, 2026
34 checks passed

rubenvdlinde deleted the feat/governance-body-events branch August 31, 2026 20:07

Navigation Menu

Uh oh!

feat(governance): add the cross-app command seam for governance bodies - #1019

feat(governance): add the cross-app command seam for governance bodies#1019
rubenvdlinde merged 6 commits into
developmentfrom
feat/governance-body-events

rubenvdlinde commented Aug 30, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 30, 2026

Uh oh!

rubenvdlinde commented Aug 31, 2026

Uh oh!

github-actions Bot commented Aug 31, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

rubenvdlinde commented Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Verification

Impact

Uh oh!

github-actions Bot commented Aug 30, 2026

Quality Report — ConductionNL/decidiq @ b9e125c

Uh oh!

rubenvdlinde commented Aug 31, 2026

Newman failure: diagnosed, deliberately not patched here

Uh oh!

github-actions Bot commented Aug 31, 2026

Quality Report — ConductionNL/decidiq @ ddb9ca9

Uh oh!

Uh oh!

github-actions Bot commented Aug 31, 2026

Quality Report — ConductionNL/decidiq @ c0c8d54

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rubenvdlinde commented Aug 30, 2026 •

edited

Loading

Quality Report — ConductionNL/decidiq @ `b9e125c`

Quality Report — ConductionNL/decidiq @ `ddb9ca9`

Quality Report — ConductionNL/decidiq @ `c0c8d54`