Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

feat(spec): implement spec (#157)#158

Merged
rubenvdlinde merged 16 commits into
developmentfrom
feature/157/spec
May 8, 2026
Merged

feat(spec): implement spec (#157)#158
rubenvdlinde merged 16 commits into
developmentfrom
feature/157/spec

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor

Closes #157

Summary

Auto-generated draft PR for OpenSpec change spec.
The Hydra builder ran the spec but could not run gh pr create itself
(Phase D+E credential strip — Claude has no GH_TOKEN by design).
The entrypoint detected commits on the feature branch with no PR and
created this draft so the reviewer + security + applier can proceed.

Spec Reference

Commits on this branch

Files changed

  • lib/Lifecycle/MeetingTransitionGuard.php
  • lib/Service/MeetingService.php
  • openspec/changes/spec/design.md
  • openspec/changes/spec/proposal.md
  • openspec/changes/spec/tasks.md
  • tests/Unit/Lifecycle/MeetingTransitionGuardTest.php
  • tests/Unit/Service/MeetingServiceTest.php

PR auto-created by Hydra builder entrypoint (hydra_ensure_pr_exists)
because Claude's session closed without running gh pr create.
Reviewer + applier follow as normal.

Hydra Builder and others added 14 commits April 19, 2026 06:30
feat: p2-motion-and-voting implementation for issue #72 (#72)
0c3783e
fix iteration 1: remove vote value from audit log for secret ballot a…
f3583ef 
…nonymity (#72)
chore: add missing SPDX-FileCopyrightText + License-Identifier headers
cb3e4f3 
Builder was meant to emit every new PHP file with EUPL-1.2 headers
(see images/builder/CLAUDE.md line 324). On this branch 18 files
shipped without. The orchestrator's quality recheck (`spdx-headers`
stage via run-quality.sh) fails hard on any lib/**.php missing the
SPDX-License-Identifier, which escalated this issue back to
needs-input on every retry cycle.

Hand-adding the headers here unblocks the recheck. A follow-up
change will strengthen the builder prompt so the first-pass
output always includes the headers.
chore(hydra): record pre-review-quality stage [skip ci]
1dcffc4
fix(review): convert SPDX docblocks from /** to /* in all lib PHP files
d4e2d68 
PHPCS PEAR.Commenting.FileComment treats the first /** block as the file
comment and requires @author and @license tags. The split two-docblock
pattern (SPDX /** block + file /** block) caused all 26 lib/*.php files
to fail with "Missing @author/license tag in file comment". Converting
the SPDX block opener from /** to /* makes it a non-docblock comment;
PHPCS then treats the second /** block (which already has @author and
@license) as the file comment — both gates green.

SPDX-License-Identifier strings are preserved in all files (spdx-headers
gate unaffected).

Co-fixed-by: Juan Claude van Damme <hydra-reviewer@conduction.nl>
chore(hydra): record quality-recheck stage [skip ci]
39e0d7d
Merge development (resolve hydra.json)
7bc07fa
fix (retry): correct file docblock format and SPDX comment placement (#…
e8e5b87 
…72)
@rubenvdlinde
fix: DI registrations + honest test task status
dcd2441 
Addresses the WARNING + SUGGESTION findings on PR #111:

- Register MotionService, VotingService, OriPublicationService,
  MotionController, VotingController explicitly in Application.php.
  Matches the existing per-service registration pattern (MinutesController,
  DecisionController, OverdueActionItemsJob). Auto-wiring worked but was
  inconsistent with the file's own style — reviewer flagged it; now
  fixed per tasks 1.4 and 2.4.

- tasks.md 1.5 and 2.5 marked [ ] (incomplete) with reason. Tests exist
  but skip unconditionally in setUp() due to #90 (ObjectService
  stub/real-signature mismatch). The previous [x] claim was dishonest
  coverage — zero assertions ever ran. Will flip back to [x] once #90
  lands and tests can unskip.
chore(hydra): record quality-recheck stage [skip ci]
3d2cb6e
@rubenvdlinde
Merge pull request #111 from ConductionNL/feature/72/p2-motion-and-vo…
38dfe99 
…ting

feat: motion and voting lifecycle — p2-motion-and-voting (#72)
@github-actions
chore: update SBOM
9dc46f7
@claude
feat(quorum-chain-2): MeetingTransitionGuard reads quorumMet declarat…
4815bb0 
…ively (#157)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@claude
feat(quorum-chain-2): mark all tasks complete, update design status (#…
376373a 
…157)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/decidesk @ 58c19c7

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
composer ✅ ✅ 100/100
npm ✅ ✅ 417/417
PHPUnit ✅
Newman ✅
Playwright ⏭️

Coverage: 0% (0/115 statements)

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

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

Code Review — Juan Claude van Damme

Result: PASS (3 fixed, 0 unfixed, 0 blocking)

What the PR does

Adds MeetingTransitionGuard (chain spec 2 of 3): reads the declaratively-computed quorumMet field set by x-openregister-calculations on the Meeting schema rather than calling QuorumService directly. MeetingService is updated to inject and use MeetingTransitionGuard instead of QuorumService. Tests and openspec docs updated accordingly.

Fixes applied

  1. lib/AppInfo/Application.php — merge conflict — Pre-merge with origin/development left conflict markers across the entire register() method. Resolved by keeping all service registrations from both branches: HEAD contributed ActionItemAnalyticsService, VotingBehaviourService, AnalyticsController, VotingBehaviourController, LiveDecisionService, LiveMeetingController, ALVMinutesService, ActionItemExtractionService, DecisionNotificationService, MinutesService, ProjectionController; origin/development contributed OriPublicationService, MotionService, VotingService, MotionController, VotingController.

  2. lib/AppInfo/Application.php:220 — AnalyticsController DI missing args — During conflict resolution my merge was missing userSession and groupManager constructor parameters for AnalyticsController. Fixed to match the actual constructor signature.

  3. tests/Unit/Service/MeetingServiceTest.php — 17 PHPCS violations — 8 auto-fixed by phpcbf (equals-sign alignment, default-value spacing, trailing blank line); 9 named-parameter violations fixed manually: markTestSkipped(message: ...) and buildMockEntity(lifecycle: ...) per ADR-015.

Gate results

Gate Result
14 hydra gates ALL GREEN
composer check:strict (phpcs/psalm/phpstan) PASS
PHPUnit (153 tests, 33 skipped) PASS
npm lint PASS (0 errors; 49 warnings all on out-of-scope Vue files)

Out-of-scope inherited debt (informational — do NOT treat as blockers)

  • lib/Controller/SettingsController.php:141 — PHPStan: wrong type for AuthorizedAdminSetting (string instead of class-string<IDelegatedSettings>). Pre-existing, not in PR diff.
  • lib/Service/ActionItemAnalyticsService.php:224,226 — PHPStan: always-true comparison from PHPDoc type. Pre-existing.
  • lib/Service/MeetingService.php:255 — PHPMD: transition() CyclomaticComplexity=13, NPathComplexity=260, ExcessiveMethodLength=123 lines. Pre-existing complexity; PR only swapped one dep (QuorumService→MeetingTransitionGuard) without meaningfully changing the method's control flow.
  • npm lint: 49 warnings across LiveMeeting.vue, MeetingDetail.vue, MotionDetail.vue (deprecated OC.generateUrl, component property order). All out-of-scope.

Consider opening cleanup PRs for the above.

Reviewed by Juan Claude van Damme — Hydra Quality Stage

🤖 Changes Juan Claude van Damme applied

View full diff · 33 files changed, 22335 insertions(+), 46 deletions(-)

rubenvdlinde
rubenvdlinde commented May 8, 2026
View reviewed changes
Comment thread lib/AppInfo/Application.php
@@ -1,5 +1,4 @@
<?php

@rubenvdlinde rubenvdlinde May 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[fixed: merge conflict resolved] Kept all DI registrations from both branches — HEAD's 11 services/controllers + origin/development's 5 services/controllers. All imports merged alphabetically.

rubenvdlinde
rubenvdlinde commented May 8, 2026
View reviewed changes
Comment thread lib/AppInfo/Application.php
@@ -187,6 +220,22 @@ static function ($c): AnalyticsController {
return new AnalyticsController(

@rubenvdlinde rubenvdlinde May 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[fixed: DI constructor completeness] Added missing userSession and groupManager named parameters to AnalyticsController registration. AnalyticsController::__construct requires IUserSession + IGroupManager; these were omitted during conflict resolution and caught by Psalm/PHPStan.

rubenvdlinde
rubenvdlinde commented May 8, 2026
View reviewed changes
Comment thread tests/Unit/Service/MeetingServiceTest.php
* @return ObjectEntity&MockObject
*/
private function buildMockEntity(string $lifecycle, string $domain = 'operations', ?string $chair = null): ObjectEntity&MockObject
private function buildMockEntity(string $lifecycle, string $domain='operations', ?string $chair=null): ObjectEntity&MockObject

@rubenvdlinde rubenvdlinde May 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[fixed: ADR-015 named parameters + PHPCS spacing] 17 violations: phpcbf auto-fixed 8 (equals-sign alignment, default-value spacing, blank-line); manually fixed 9 named-parameter violations — markTestSkipped('...') → markTestSkipped(message: '...'), buildMockEntity($x) → buildMockEntity(lifecycle: $x).

@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/decidesk @ 3335463

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
composer ✅ ✅ 100/100
npm ✅ ✅ 417/417
PHPUnit ✅
Newman ✅
Playwright ⏭️

Coverage: 0% (0/115 statements)

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

Download the full PDF report from the workflow artifacts.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

Security Review — Clyde Barcode

Result: PASS (10 fixed, 0 unfixed blocking)

Fixes Applied

[WARNING] Semantic-auth mismatch (gate-9) — @NoAdminRequired on admin-guarded methods
Rule: ADR-016 gate-9 / ADR-005 Rule 3 / OWASP A01:2021

10 controller methods had @NoAdminRequired annotation but enforced admin in the method body (isAdmin() / requireAdmin()), lying to Nextcloud's routing layer. Fixed by:

  • Removing @NoAdminRequired docblock from 9 REST endpoint methods (DecisionController×1, MinutesController×7, SettingsController×1)
  • Swapping #[NoAdminRequired] → #[AuthorizedAdminSetting(Application::APP_ID)] for SettingsController#create (mirrors sibling load() pattern)

All fixes self-verified: Semgrep clean on touched files, unit tests 153/153 pass.

Checks

  • composer audit: PASS
  • npm audit: 39 inherited CVEs (informational — pre-existing, non-blocking)
  • gitleaks: PASS
  • Semgrep p/security-audit + p/owasp-top-ten (33 PHP files): 0 findings
  • Manual OWASP review: PASS (public endpoints, IDOR, PII, auth all correct)

Out-of-scope inherited debt (informational, non-blocking)

  • npm audit: 39 low/moderate/high CVEs in Vue 2 ecosystem (axios, dompurify, fast-xml-parser, @nextcloud/*). Pre-existed on development before this PR branched. Recommend dedicated dep-bump PR.
  • LiveMeetingController#recordLiveDecision, AnalyticsController#getSummary/#getCompletionRates: same gate-9 pattern as fixed above, but files unchanged vs origin/development — out of scope per ADR-020.

🤖 Changes Clyde Barcode applied

  • 0c762f7 — fix(security): remove @NoAdminRequired from admin-guarded controller methods (gate-9)

View full diff · 3 files changed, 1 insertion(+), 21 deletions(-)

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/DecisionController.php:87 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: removed @NoAdminRequired] Rule: ADR-016/gate-9 — annotation contradicted admin body check; Nextcloud router would skip admin enforcement while body still gated. Removed docblock annotation to match body semantics.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/MinutesController.php:98 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: removed @NoAdminRequired] Rule: ADR-016/gate-9 — generateDraft() body enforces admin via isAdmin(); annotation was lying to the router layer.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/MinutesController.php:168 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: removed @NoAdminRequired] Rule: ADR-016/gate-9 — transition() body enforces admin; annotation removed.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/MinutesController.php:243 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: removed @NoAdminRequired] Rule: ADR-016/gate-9 — generateALVDraft() body enforces admin; annotation removed.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/MinutesController.php:303 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: removed @NoAdminRequired] Rule: ADR-016/gate-9 — distributeALVMinutes() body enforces admin; annotation removed.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/MinutesController.php:360 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: removed @NoAdminRequired] Rule: ADR-016/gate-9 — extractActionItems() body enforces admin; annotation removed.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/MinutesController.php:424 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: removed @NoAdminRequired] Rule: ADR-016/gate-9 — saveExtractedActionItems() body enforces admin; annotation removed.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/MinutesController.php:500 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: removed @NoAdminRequired] Rule: ADR-016/gate-9 — submitForApproval() body enforces admin; annotation removed.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/SettingsController.php:108 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: #[NoAdminRequired] → #[AuthorizedAdminSetting] + removed @NoAdminRequired docblock] Rule: ADR-016/gate-9 — create() calls requireAdmin() in body; swapped to AuthorizedAdminSetting mirroring sibling load() method.

@rubenvdlinde

rubenvdlinde commented May 8, 2026

Copy link
Copy Markdown
Contributor Author

📍 lib/Controller/SettingsController.php:133 (outside PR diff — line-comment API rejected, posted as issue comment)

[fixed: removed stale @NoAdminRequired docblock] Rule: ADR-016/gate-9 — load() PHP 8 attribute was already #[AuthorizedAdminSetting]; stale docblock annotation was the mismatch source.

@rubenvdlinde rubenvdlinde marked this pull request as ready for review May 8, 2026 21:53
@rubenvdlinde rubenvdlinde merged commit 737e89a into development May 8, 2026
45 checks passed
@rubenvdlinde rubenvdlinde deleted the feature/157/spec branch May 8, 2026 21:53
This was referenced May 8, 2026
Closed
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Rem-Dam Rem-Dam Awaiting requested review from Rem-Dam Rem-Dam is a code owner
@bbrands02 bbrands02 Awaiting requested review from bbrands02 bbrands02 is a code owner
@rjzondervan rjzondervan Awaiting requested review from rjzondervan rjzondervan is a code owner
@WilcoLouwerse WilcoLouwerse Awaiting requested review from WilcoLouwerse WilcoLouwerse is a code owner
@remko48 remko48 Awaiting requested review from remko48 remko48 is a code owner
@SudoThijn SudoThijn Awaiting requested review from SudoThijn SudoThijn is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@rubenvdlinde