Release: merge development into beta for the morning checkup build - #809
Merged
Conversation
Supersedes #526, which bumped phpstan/phpstan alone and could not pass: 60 errors, of which 38 were migration behaviour rather than defects. Those 38 are fixed once for the fleet in ConductionNL/.github#517, released as conduction/hydra-gates v1.8.2 (treatPhpDocTypesAsCertain: false, plus an ignore for PHPMD's @SuppressWarnings tag which 2.x parses as its own). This bumps hydra-gates to ^1.8.2 alongside phpstan ^2.0 — the lockfile pinned v1.8.1, so the shared config was not in effect here — and clears the remaining 22, which are genuine: 4 throws.unusedType @throws RuntimeException on four getObjectService() methods that ADR-083 turned into plain property reads. Their own comment already says 'a property read throws nothing, so the old catch was unreachable'; the tag was left behind. 11 alreadyNarrowedType is_object($saved) / method_exists($x, 'getObject'| 'jsonSerialize') on values from OpenRegister. saveObject() declares ': ObjectEntityInterface' — a REAL return type, not a docblock — and that interface declares getObject(): array and extends JsonSerializable. PHP raises a TypeError before any of these guards could run, so each fallback branch was unreachable. Removed the probes and kept the branch that always executed. 3 notIdentical.alwaysTrue '!== null' after isset() (already false for null), and after a '?? ""' chain that cannot yield null. 2 parameterByRef.type copyItemDocuments() declared array<int, string> for the same $skipped it passes straight into writeFileDefensively(), which declares array<string>. Aligned to the consumer. 1 argument.type str_replace() handed mixed values from decoded object payloads where it declares array<string>. Cast — an array replacement would silently become the literal 'Array'. 1 function.impossibleType class_exists() on OPENCONNECTOR_CANDIDATES. That probe IS the optional-integration feature; OpenConnector is a separate app whose classes exist only at runtime. Ignored in phpstan.neon, scoped to the rule AND the file, so a genuinely impossible class_exists() elsewhere still fails. Verified locally: phpstan 2.2.8 exit 0 (control: a deliberately broken file still reports 'Found 1 error', so it is analysing). phpcs exit 0. php -l clean. PHPUnit 1150 tests, 4546 assertions, OK.
…60820130433 chore(release): 1.0.1-unstable.20260820130433
…60820202311 chore(release): 1.0.1-unstable.20260820202311
…60820204542 chore(release): 1.0.1-unstable.20260820204542
…60820211107 chore(release): 1.0.1-unstable.20260820211107
`CnObjectDataWidget.editable` defaults to TRUE, so every property named in a data widget's `include` list becomes a text box the user can type into. That put lifecycle state and audit stamps — `status`, `lifecycle`, `submittedAt`, `approvedBy`, `openedAt`, `closedAt`, `publishedAt`, `enactedAt` — in front of users as editable fields. These are written by the backend when a transition lands (`TransitionEngine` stamps them through `saveObject()`), so an input for them is a control that can only ever fail or confuse: the guarded path is the lifecycle buttons, and `LifecycleValidationListener` rejects anything that is not a legal transition. Locked with per-field `overrides.<field>.editable: false` rather than `editable: false` on the widget: these panels mix process state with fields the user legitimately edits, and a blanket lock would make those read-only too. NOT fixed here: widgets that declare no `include` at all render EVERY schema property, and enumerating their fields in the manifest would drift the moment the schema changes. 52 such widgets fleet-wide expose 124 process fields. Closing those needs a server-side "system-owned" marker, which OpenRegister does not have — `readOnly:true` has no bypass for backend callers and would break the transition that writes the field. Filed as ConductionNL/openregister#2644. Inserted textually, one compact line per widget, so the diff is the change and nothing else: a full JSON re-serialisation reflowed hand-compacted lines and turned this into thousands of lines of churn. A verifier re-parses both files and asserts the only structural difference is the added overrides, and that no non-process field was locked.
…60820213054 chore(release): 1.0.1-unstable.20260820213054
…60820215610 chore(release): 1.0.1-unstable.20260820215610
…l copy The PHPUnit leg failed on "Guard coverage baseline", and the guard itself printed the reason: scripts/coverage-guard.php predates --deletion-neutral, so deleting well-tested dead code will still read as a coverage drop. Copy the canonical version from ConductionNL/.github at quality-config/coverage-guard.php to pick it up. That is exactly what this PR does to the code: base 1782/2324 statements (76.68%) head 1763/2305 statements (76.49%) FAIL: dropped by 0.19% Nineteen statements were deleted and all nineteen were covered. Nothing became less tested; the denominator shrank and took the numerator with it, so the ratio dipped. Reading the percentage alone would call this a regression — the counts are what say otherwise. Replaces the app's 434-line copy with the canonical 837-line one, which reports `deletion-neutral` among its capabilities. No local edits: it is a verbatim copy of ConductionNL/.github@main quality-config/coverage-guard.php, which the shared workflow expects apps to keep byte-identical. Every one of the fleet's PHP apps still ships the stale copy, so this will resurface elsewhere; fixing it here rather than suppressing the ratchet keeps the guard doing its actual job on real coverage losses.
…60820222118 chore(release): 1.0.1-unstable.20260820222118
…60820223950 chore(release): 1.0.1-unstable.20260820223950
fix(manifest): process and audit fields are no longer editable inputs
…60820230259 chore(release): 1.0.1-unstable.20260820230259
The coverage ratchet failed this PR, and after refreshing the guard to the
canonical deletion-neutral copy it still failed with the SAME numbers:
Deletion-neutral: attributed by method name
(head 13 file(s), base 13 file(s), 144 head method(s), 144 base method(s))
Surviving code, head: 76.49% (1763/2305 statements)
Surviving code, base: 76.68% (1782/2324 statements)
144 methods on both sides — so no method was deleted, and deletion-neutrality
had nothing to excuse. The 19 statements went from INSIDE methods that still
exist, and all 19 were covered. The guard is arithmetically right: removing
covered lines from a 76%-covered body lowers the ratio. My earlier assumption
that refreshing the guard would clear this was wrong.
The honest way back to parity is coverage, not suppression.
VotingBehaviourService was the obvious candidate: 129 statements, ZERO covered,
while being what answers 'how does this board member actually vote' — a
participation statistic shown to a governance body. It needed tests on its own
merits.
Five tests pin the three-hop traversal and the arithmetic:
- an empty body yields a zeroed but WELL-FORMED report (callers render every
key; a missing one is a broken page, not a zero)
- an OPEN round is excluded from the denominator — nobody is absent from a
vote that has not closed
- ballots tally per bucket, an unknown value lands in none, and an isProxy
ballot counts as GIVEN not received (inverting that misreports who
delegated to whom)
- participation rate is a percentage of closed rounds
- a motion with no resolvable id is skipped rather than fatal
Note on style: tests are not in phpcs's scope here (phpcs.xml declares only
<file>lib</file>), and the existing suite uses positional arguments in test
code — ActionItemAnalyticsServiceTest reports 21 of the same
RequireNamedParameters findings. This file matches that convention rather than
inventing a different one.
…60820233551 chore(release): 1.0.1-unstable.20260820233551
…60820235806 chore(release): 1.0.1-unstable.20260820235806
…60821001641 chore(release): 1.0.1-unstable.20260821001641
…60821021105 chore(release): 1.0.1-unstable.20260821021105
…60821022430 chore(release): 1.0.1-unstable.20260821022430
…60821024534 chore(release): 1.0.1-unstable.20260821024534
…60821025343 chore(release): 1.0.1-unstable.20260821025343
…60821030551 chore(release): 1.0.1-unstable.20260821030551
…60821031952 chore(release): 1.0.1-unstable.20260821031952
…60821033312 chore(release): 1.0.1-unstable.20260821033312
…60821035120 chore(release): 1.0.1-unstable.20260821035120
…60821040203 chore(release): 1.0.1-unstable.20260821040203
…60821041243 chore(release): 1.0.1-unstable.20260821041243
…stance (#781) * fix(security): close the raw board-evaluation answers to the whole instance `evaluation-response` declared no authorization block, so it fell back to the decidesk register baseline and every raw board self-evaluation answer was readable by ANY authenticated account through OpenRegister's generic object API. This does NOT de-anonymise anyone — the schema carries no member relation, and `responseToken` is an HMAC(participantId + evaluationId) under a server-side secret. What it defeats is the anonymity THRESHOLD deliberately built one level up: `BoardEvaluation` suppresses `dimensionScores` and `themes` (`suppressed = true`) when too few members responded, precisely so a small board cannot be read individually — while the underlying answers stayed open to everyone. You could not learn WHO said it; you could read WHAT was said in a cycle whose summary was withheld for exactly that reason. The block declares `create` and omits everything else: * `read` closed — the exposure above. Owner and admin only. * `create` OPEN to `authenticated`, and this is load-bearing: a block SHADOWS the register baseline entirely, so a block without `create` would stop members submitting at all. That is the same shape as the read-only block that closed create/update once before, which 1073 unit tests and 60 gates all passed and only the Newman collection caught. * `update` closed still permits a resubmission, because the author OWNS their response — the upsert is keyed on the opaque `responseToken` slug, so it never needs to read anyone else's row. * `delete` closed outright. `BoardEvaluationScoreService::closeCycle()` now reads the response set with `_rbac: false`. That is REQUIRED, not an optimisation: a chair does not own the other members' responses, so under caller RBAC the query would return nothing and the cycle would close with a vacuous score summary on a healthy 200 — which the comment directly above that call records having already happened once, for a different reason. The caller is authorised before the service is reached: `BoardEvaluationController::close()` runs `BoardEvaluationAccessGuard::requireChairOrSecretary()`, and it is the only caller. `BoardEvaluation` itself is deliberately left alone. Its `read: authenticated` was flagged as over-broad, but its aggregate is suppressed below threshold and both participant-id arrays are documented as carrying no link to response content. The schema holding the sensitive material was the one with no block at all. Schema version bumped 0.1.0 -> 0.2.0 — an annotation-only change never deploys. Sentinel 26 -> 27, with the `create` exception pinned to `['authenticated']` and nothing wider. 1150 tests / 0 failures, verified with a COPIED vendor: with vendor symlinked, `DecisionLifecycleVocabularyTest::testClassesUnderTestResolveToThisRepository` correctly failed because the autoloader was serving the other checkout's classes and the suite was not testing this code at all. * test(security): pin the `_rbac: false` the scorer depends on The coverage ratchet failed this PR honestly: base 75/131 -> head 75/132 statements on the touched files. The `_rbac: false` argument added one statement and nothing covered it. Covering it properly rather than adjusting the baseline, because the argument is exactly the kind that should be pinned: it is one token, and deleting it leaves a suite that still passes and a feature that silently reports zeros. `evaluation-response` now closes `read` to everyone but the response author, so under caller RBAC a chair sees NOTHING and the cycle closes with a vacuous score summary on a healthy 200 — the same silent failure the relation-filter comment above that call records having already happened once, for a different reason. The test CAPTURES the argument the service actually passed rather than pinning it with `->with()`, so the assertion describes the value and not merely that a call happened. Control run, because a guard test that cannot fail is not evidence: with `_rbac: false` removed the test fails with "Failed asserting that true is false"; restored, it passes. Full suite 1151 tests / 0 failures, phpcs clean. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…60821042457 chore(release): 1.0.1-unstable.20260821042457
Carries the evaluation-response read-scope fix (#781) into the beta build being reviewed this morning, at Ruben's request. Before #781 the raw anonymous board self-evaluation answers were readable by any authenticated account through OpenRegister's generic object API, which defeated the anonymity threshold BoardEvaluation applies to its aggregate. The schema now closes `read` and keeps `create` open so members can still submit, and BoardEvaluationScoreService reads the set with `_rbac: false` because the caller is already authorised by BoardEvaluationAccessGuard::requireChairOrSecretary(). No conflicts. Verified in the merged tree rather than assumed: evaluation-response is at version 0.2.0 with `{"create":["authenticated"]}`, the scorer carries `_rbac: false`, and the test pinning that argument is present.
rubenvdlinde
requested review from
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
August 21, 2026 04:34
Contributor
Quality Report — ConductionNL/decidesk @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-nav-ceiling | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| composer | ✅ | ✅ 104/104 | |||
| npm | ✅ | ✅ 538/538 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-21 05:29 UTC
Download the full PDF report from the workflow artifacts.
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
2 participants
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.
Carries the
evaluation-responseread-scope fix (#781) into the beta build being reviewed this morning.Why now: #781 landed on
developmentafter last night's fleet sync, so the beta environment would otherwise be reviewed without it. Requested explicitly rather than assumed — a mid-review beta change is normally worth avoiding.What it carries. Before #781, the raw anonymous board self-evaluation answers were readable by any authenticated account through OpenRegister's generic object API. That does not identify anyone — the schema holds no member relation and its
responseTokenis a server-secret HMAC — but it defeats the anonymity thresholdBoardEvaluationapplies to its aggregate, which suppresses scores when too few members responded precisely so a small board cannot be read individually.The schema now closes
readand keepscreateopen so members can still submit (a block shadows the register baseline entirely, so a block withoutcreatewould stop submission — the same shape as the read-only block that only Newman ever caught).BoardEvaluationScoreServicereads the set with_rbac: false, because a chair does not own the other members' responses and the caller is already authorised byBoardEvaluationAccessGuard::requireChairOrSecretary().No conflicts. Verified in the merged tree rather than assumed:
evaluation-responseat version0.2.0with{"create":["authenticated"]}, the scorer carrying_rbac: false, and the test pinning that argument present.On
developmentthis merged with 33 jobs green — Newman, Playwright, Hydra Gates and the Coverage guard included.