chore(docs): sync development into documentation - #717
Merged
Conversation
… cap (#461) * fix(e2e): retain-on-failure traces + a globalTimeout under the 45m CI cap Fleet-wide Playwright instrument sweep, ConductionNL/.github#188. Neither change can alter a verdict; both change whether you can see why a verdict happened. `trace: 'on-first-retry'` only writes a trace when a retry actually happens, which makes the trace artifact a function of `retries`. `retain-on-failure` captures every test, keeps only the failures, and does not depend on the retry count. No repo in the fleet set `globalTimeout`. The shared quality.yml Playwright job is `timeout-minutes: 45`, and a job cancelled by that cap produces no verdict and no artifacts: the trace upload is `if: failure()` and the report upload is `if: always()`, and neither runs on a cancelled job, while `gh pr checks` still renders it as "fail". Runs cancelled at ~45m16s have been observed in this fleet. Measured overhead in that job before the `Run Playwright tests` step starts is 2.0-2.4 min, so 38m leaves ~7 min of margin while guaranteeing a tally and its artifacts. * fix(e2e): apply the same fix to the config CI actually loads The shared quality.yml resolves its config as `${playwright-test-path}/playwright.config.ts` and only falls back to the app-root `playwright.config.ts` when that file is absent (quality.yml ~L2218). This repo ships tests/e2e/playwright.config.ts, so THAT is the file every CI run has been using — the app-root config fixed in the previous commit is the one developers load by hand, not the one the gate reads. Applies the identical `retain-on-failure` + `globalTimeout: 38 * 60_000` change here. ConductionNL/.github#188.
…l-target fix(phpcs): stop the SpecTagSniff instructing the pattern gate-46 rejects
`PUT /api/settings` answered **405 Method Not Allowed** on the dev instance,
not 500: softwarecatalog does not call `\OCA\OpenRegister\AppHost\Routes::standard()`
and declared no `settings#update`, so the PUT verb simply had no route.
Because the app ships its own `SettingsController`, AppHost's
`aliasControllerUnlessLeafDefinesIt()` skips the generic controller and the
leaf owes every `settings#` method itself.
Strict addition — nothing removed:
- `SettingsController::update()` takes the former `create()` body verbatim.
It writes exactly the three sections `index()` reads back via
`SettingsService::getAllSettings()`: `configuration`/`selectedRegister`,
`userGroups.{generic,organizationAdmin,superUser}` and `emailSettings`.
It is deliberately NOT a catch-all: the ~35 other `getXConfig`/`updateXConfig`
pairs, email templates, ArchiMate import/export, progress and stats keep
their own endpoints untouched.
- `create()` becomes `return $this->update();` and keeps its own
`@NoCSRFRequired` tag — NC middleware only evaluates attributes on the
dispatched method, so delegation cannot inherit the posture.
- `appinfo/routes.php` gains `settings#update` PUT `/api/settings`, before the
SPA `/{path}` catch-all. Route and method land together so the method is
never unreachable (gate-14).
Auth: `create()` declares `@NoCSRFRequired` and deliberately not
`@NoAdminRequired`, so NC requires an administrator. `update()` mirrors that
exactly; net privilege change is zero. A test pins the parity in both
directions and asserts `update()` can never gain `@NoAdminRequired`/`@PublicPage`.
Tests: 17 new assertions-heavy cases across two files, covering per-method
existence (item, not container) with a positive control, the evaluated route
table, the write behaviour, the non-catch-all guarantee, the 400 group-validation
path, exception mapping, and that `create()` delegates rather than duplicates.
New statements are 17/17 covered.
… were keyboard-dead (#458) * fix(a11y): four widget icons rendered the wrong glyph, three controls were keyboard-dead Full-tree measurement of origin/development with the hydra-gates runner from ConductionNL/.github@main. Four gates, eleven findings, all real — plus two gates whose findings are false positives and are reported rather than "fixed". GATE-55 — FOUR WIDGET ICONS THAT DO NOT EXIST (detail-page-discipline). ModuleDetail.md-files BookOpenVariantOutline -> BookOpenVariant ModuleDetail.md-versions ViewModule -> SourceBranch SuiteDetail.suite-data PackageVariant -> Package BioMaatregelDetail.bm-data ShieldLockOutline -> ShieldCheckOutline CnWidgetGrid's resolveWidgetIcon() falls back to DEFAULT_ICON — 'ViewDashboard' — for any name not in its registry, so these four widgets rendered a generic dashboard glyph. Silently: no console warning, no build error, and `check:manifest` passes because the schema types `icon` as a string. Verified against the REAL registry, not the gate's copy of it. hydra-gates gate-55 carries a hardcoded mirror and says so in its own comments; a mirror is only as fresh as its last manual edit and fails in both directions. Read from node_modules/@conduction/nextcloud-vue/src/components/CnWidgetGrid/widgetIcons.js at the pinned 2.2.0-vue3.3: all four flagged names are absent, and `git log -S` over that file shows none of them was ever present. The replacements are the nearest true members of the same glyph family. GATE-32 — THREE CONTROLS NO KEYBOARD CAN REACH (semantic-controls). EmailConfiguration the template-variable tags: a clickable <span> that inserts a variable into the editor. Now a real <button type="button">, with the UA chrome reset in CSS so the rendering is unchanged. MergeObject the merge-target rows: a click-only <div> choosing which object survives a merge — the consequential decision in that dialog. Now role="option" + tabindex + Enter/Space, inside a role="listbox" parent so the selected state reaches a screen reader. OrganisatieCard the whole card navigates to the organisation. Now role="button" + tabindex + Enter/Space; the nested .cardHeaderActions already stops propagation, so the NcActions menu is unaffected. GATE-39 — TWO ICON-ONLY BUTTONS WITH NO ACCESSIBLE NAME (button-name). ViewObject's "cancel label editing" button sits directly beside a "save labels" button that already carries an :aria-label — the sibling gave the exact pattern. ArchiMateImportExport's error-details close button had none either. v-tooltip is not an accessible name. GATE-58 — TWO networkidle WAITS IN E2E (e2e-networkidle). Nextcloud keeps long-lived connections open, so `waitUntil: 'networkidle'` never settles; it can only time out or be satisfied by luck (ADR-074 rule 4). Both call sites already assert on a heading immediately afterwards, which is the real readiness signal, so the wait was doing no work it could reliably do. A third site in the same file already had the right value and the comment explaining why. Full-tree after: gate-32, gate-39, gate-55 and gate-58 all PASS. EVIDENCE. tests/vitest/manifestWidgetIcons.spec.js asserts every widget icon in src/manifest.json against the registry parsed out of the installed dependency. Reverting src/manifest.json makes it name all four sites by page and widget id. It carries two positive controls on its INPUTS, and the second one earned its place immediately: the first version of the manifest walker assumed `page.widgets`, but widgets live under `config.widgets` and nested arrays at a depth that varies by page type. It collected ZERO icons — and would have reported a clean tree forever. The "finds widget icons to check" control failed and caught it before the real assertion could pass vacuously. TWO FINDINGS DELIBERATELY NOT "FIXED", because the code is already right: gate-12 (nc-input-labels, 2) is a false positive. Both <NcSelect> tags in EmailConfiguration.vue DO carry input-label — "Transport Type" and "Encryption". The gate extracts the tag with `grep -oE '<NcSelect[^>]*>'`, and `[^>]*` stops at the first `>`, which is the arrow in `:reduce="option => option.value"`. The extracted "tag" therefore ends before input-label can appear. Same shape as the gate-9 admin rule's `[^)]*` (.github#198). Filed upstream. gate-38 (skip-link, 1) is a false positive. templates/settings/admin.php is a Nextcloud admin-settings section, registered via lib/Settings/SoftwareCatalogAdmin.php and rendered INSIDE core's settings frame, which already provides the skip link and the <NcContent> shell. It is not a root component; adding a second skip target would be a regression. Filed upstream. Also measured, and worth knowing: gate-32's remaining three findings after the fix were matching the explanatory COMMENTS this commit added, because those comments contained the literal text of the markup they describe. Rewording them to prose cleared the gate — confirming the mechanism. Filed upstream too; the same class as .github#184. Checks: eslint 0 errors, stylelint 0 errors, check:manifest Ajv PASS, vitest 213 green (210 before, 3 added). * fix(icons): register BookOpenVariant and SourceBranch in src/icons.js There are TWO icon registries and an icon must be in both. CnWidgetGrid resolves widget icons through nc-vue's widgetIcons.js; CnAppNav, CnIcon and the Cn*Page headers resolve through this app's own src/icons.js (ADR-077). The lists overlap but are not equal, and the failure modes differ: an unknown name in the widget registry renders DEFAULT_ICON, while an unknown name in the app registry renders NO ICON AT ALL — src/icons.js says exactly that in its own header. The first pass of this branch replaced four unknown widget icons with names taken from the widget registry alone. Two of them, BookOpenVariant and SourceBranch, were absent from src/icons.js, so the repair traded a wrong glyph for no glyph. hydra-gates checks the two registries in two different gates — 55 and 60 — so neither on its own would have said so, and my local baseline run had reported gate-60 PASS while printing 'vue-material-design-icons is not installed — could not verify that icon names exist upstream'. A caveated pass is not a pass. Both names are verified present in vue-material-design-icons (the .vue files exist in the installed package, and nc-vue imports them). The vitest spec now checks BOTH registries, with its own positive control on each. Reverting src/icons.js alone turns the new arm red and names both icons.
…ation (#459) (#466) `GET /api/contactpersonen/organisation/{organisationId}` was `@NoAdminRequired` with "is somebody logged in" as its only guard. `$organisationId` is a path parameter and was never compared to the caller's own organisation, so any authenticated user could read the contact persons of any organisation — and the response carries each contact's Nextcloud username, group membership and enabled/disabled state. Every sibling on the same controller that touches that data (`getUserInfo`, `getBulkUserInfo`, `updateUserGroups`, `disableUser`, `enableUser`) already refuses it to non-admins. - `checkOrganisationReadPermission()`: instance admins may read any organisation; everybody else only the organisation their own contactpersoon belongs to; a caller whose organisation cannot be resolved is refused. This mirrors `verifyCrossTenantScope()`, which already fails closed for writes. - The same guard is applied to the sibling route `getContactPersonsWithUserDetailsForOrganization`, named in the issue as having the same shape. - The per-record organisation is re-checked in PHP. The search filter is a bare top-level `organisation` key; whether OpenRegister reads that as an object property, as `@self` metadata, or ignores it is not visible from the call site, and an ignored filter returns an UNSCOPED result set that looks exactly like a scoped one. A record with no resolvable organisation is not served. - `resolveContactOrganisation()` now normalises the stored value. `organisatie` is declared as a related object in the register, so it can arrive as a nested envelope; comparing that raw against a plain UUID read as "different tenant" and denied legitimate members. - The enrichment loop reuses `buildUserInfoData()`, the shape the admin-gated siblings already return — three catalog group memberships rather than every GID the account holds. - `total` now counts what is actually returned instead of the unfiltered server-side total. Can-fail proof: reverting the controller to `origin/development` turns 4 of the 7 new tests red, including the item-level assertion — `victim@b.example` and `org-uuid-B` appear in the response body. The other 3 assert the legitimate surface still works and pass in both directions by design. phpcs lib/: 0 errors / 87 warnings, identical to origin/development. phpmd, psalm, phpstan clean. Unit suite 519 tests green.
…onfig duplicate (#468) `ArchiMateService::getVoorzieningenConfig()` is `private` and has zero `$this->` call sites in its own file — the only thing that can reach a private method. It is not reflected into either: the whole of `lib/` contains exactly one `ReflectionMethod` call site (`SettingsService:4644`) and it targets `getAmefConfig`, not this method. It cannot run. It is also a STALE duplicate. `SettingsService::getVoorzieningenConfig()` is the live resolver — 13 references across the app — and it ends with `normalizeVoorzieningenConfig()`, which this copy never had. Anything that had been wired to the copy would have received un-normalised config. Removing it removes three of gate-50's seventeen unsafe config reads (`voorzieningen_register`, `voorzieningen_organisatie_schema`, `voorzieningen_contactpersoon_schema`) by removing code that cannot execute, not by moving a guard into the checker's window. Can-fail proof: restoring the file from origin/development puts gate-50 back to 17; with the deletion it reports 14. phpcs clean, psalm clean, phpstan clean, phpmd clean against the repo baseline, unit suite 512 tests green.
…on, and delete an unreachable settings dialog (#469) Five gates, 52 findings, all real. hydra-gates 48c88ba, measured against origin/development (3a542f2). gate-32 semantic-controls 1 -> PASS gate-39 button-name 3 -> PASS gate-40 form-label-association 19 -> PASS gate-43 table-headers 19 -> PASS gate-45 prefers-reduced-motion 10 -> PASS gate-43 — every data table now names its columns `scope="col"` on 18 header rows across 12 components. The one finding that was not a missing `scope=` was `src/navigation/Configuration.vue`'s `<table>` with no `<th>` at all — see below. gate-40 — 19 controls a screen reader could not name - `src/modals/object/ViewObject.vue`: the four property editors get `:aria-label="getPropertyDisplayName(key)"`, which is exactly the text the Property column shows, so the programmatic name matches the visible one without rendering it twice. The select-all and per-attachment checkboxes get real names. - `src/modals/object/MergeObject.vue`: the custom-value field gets a label. - The other 13 were all in `src/navigation/Configuration.vue`. src/navigation/Configuration.vue is DELETED — it could never run Zero importers anywhere in src/, tests/ or the webpack config. It POSTs to `/index.php/apps/OPENCATALOGI/configuration` — a different app's endpoint — and translates with `t('forms', …)`, a third app's namespace. Its `<table>` is nested inside a `<p>`, which is invalid HTML. It is an OpenCatalogi leftover duplicating the settings surface `SoftwareCatalogSettings.vue` owns, and softwarecatalog's own settings are written by SettingsController. Nothing is left unguarded and no capability is lost: 13 gate-40 findings and 1 gate-43 finding disappear because the code that produced them is gone. gate-45 — reduced motion, per selector A `@media (prefers-reduced-motion: reduce)` block in each of the 10 style blocks, neutralising only the selectors that declare transition/animation. No global reset; nothing else changes appearance. gate-39 / gate-32 The three ObjectModal pencil buttons say what they do ("Change catalogue", "Change register", "Change schema"). The clickable errors tile in ArchiMateImportExport is now a real `<button type="button">`, disabled when the error count is zero, so it is keyboard-reachable exactly when it does something — plus the UA button chrome is neutralised so it renders identically to its sibling tiles. Also fixed while here: four Dutch UI strings in the Dashboard statistics tables ("Aantal"/"Beheren" -> "Count"/"Manage"). Ordinary words, not VNG standardised terms; this codebase is English. Can-fail proof: reverting all 19 files to origin/development puts every count back exactly — 1, 3, 19, 19, 10. vitest 215/215, stylelint clean, check:manifest Ajv PASS (0 errors). No other gate count moved.
… anchors, three tracked build artefacts (#467) * fix(gates): one settings home, four dangling refs, two mistyped anchors, three tracked build artefacts Four gates, thirteen findings, all real. Measured with hydra-gates 48c88ba1e0d049f8f38538c33e790d3e603c55d0. gate-63 settings-surface (2 + 1 WARN) -> PASS The app already registers a Nextcloud admin section (`lib/Settings/SoftwareCatalogAdmin.php`, wired in appinfo/info.xml) that renders `SoftwareCatalogSettings.vue` through `src/settings.js`. The manifest ALSO declared a `type: "settings"` page rendering the same component, lifted into the gear foldout by `menu-layout.json` — so the navigation read "Settings > Settings" and the app had two homes for one concern, one of them authorized only by whatever the SPA remembered to check. ADR-079 D1/D4: the in-app page and its menu entry are removed; `/settings/admin/softwarecatalog` is the single home, and CnAppNav's admin-gated "Admin settings" link out (`data-testid=cn-nav-admin-settings`, present in the pinned nc-vue) is how you get there. The three Playwright tests that drove `#/settings` are retargeted at `/settings/admin/softwarecatalog`. They assert the same three things about the same component; only the door moved. They were NOT deleted — deleting the tests would have removed the only proof this surface still renders. gate-54 relation-dialect (5 -> 1) `element.properties`, `view.properties`, `model.properties` and `relation.properties` all declare `$ref: "#/components/schemas/property"`. There is no `property` schema in the register; the slug is `property-definition`. Four dangling nested-object references, now resolved. gate-46 spec-anchor-existence (4) -> PASS - `#requirement-modulversie-...` (x2) was a typo for `#requirement-moduleversie-records-sbom-import-provenance`, which exists. - `src/utils/suiteWizard.js::mapApplicationOptions` pointed at a requirement that does not exist. The scenario that actually governs it — "The applications step only offers modules that already exist" — sits under "The wizard SHALL guide suite creation...", so the tag now names that. - `SbomParserService::parseSpdx()` pointed at `#notes`, which did not exist. SPDX 2.x parsing is live (called from SbomImportService) and unit-tested but has no requirement, so the spec gains a Notes section describing it — and says plainly that promoting it to a Requirement means writing the Scenarios and the Playwright test that assert them. gate-51 schema-property-titles (2) -> PASS `sbomComponent.hashes[].alg` and `.value` had a title but no description. gate-29 gitignore-then-commit (4) -> PASS `.phpunit.cache/test-results` and `.phpunit.result.cache` are PHPUnit build artefacts, correctly ignored and wrongly tracked — untracked with `git rm --cached`. `data/GEMMA release.xml` is a 13 MB AMEF fixture that test-setup.sh reads, sitting inside the ignored `/data/` runtime directory; it moves to `tests/fixtures/amef/` where fixtures live, and test-setup.sh follows it. Can-fail proof: reverting the six changed source files to origin/development and re-running the suite puts every count back exactly — gate-46 4, gate-51 2, gate-54 5, gate-63 2. check:manifest Ajv validation PASS (0 errors). vitest 215/215. All three touched JSON documents parse. * test(e2e): retarget the settings-coverage suite at /settings/admin/softwarecatalog The five failures on this branch were all in tests/e2e/spec-coverage/settings.spec.ts, which still navigated to the in-app '#/settings' route that ADR-079 D1 removed. Same component, same assertions, different door: gotoSettings() now opens the Nextcloud admin settings section and scopes to #softwarecatalog-settings, the host element templates/settings/admin.php provides. The tests were retargeted rather than deleted or skipped — they are the only browser-level proof that Version Information, Object Statistics, General Settings, OpenRegister Integration, User Groups and Organization Synchronization still render, and that the OpenRegister sub-tabs switch.
…ge surfaces render (#470) * test: pin the public review-aggregate wire contract and prove five page surfaces render gate-25 contract-coverage 1 -> PASS, gate-26 visual-coverage 6 -> PASS, measured with hydra-gates 48c88ba against origin/beta — the scope a push to development actually runs. gate-25 — GET /api/reviews/aggregate had no automated proof `review#aggregate` is `#[PublicPage]`: an anonymous visitor on a module or dienst detail page reads it, so its response shape and status codes are part of the app's public surface. ReviewControllerContractTest pins them: - success is exactly `{average, count, items}` with HTTP 200; - the service's internal `ok`/`reason` bookkeeping never reaches the wire; - a subject with no approved reviews is a 200 with a null average, not a 404 — a consumer rendering a star widget has to tell "nothing approved yet" from "bad request"; - a rejected request is a 400 carrying exactly `message`; - subjectType/subjectId reach the service unaltered. gate-26 — five page components with no browser-level proof tests/e2e/spec-coverage/page-surfaces.spec.ts drives the REAL UI by clicking the app's own navigation, then asserts the page's own content rendered and that the app logged no console error and returned no 5xx: FacetedCatalogIndexView (Applications and Services), SuitesIndexView, PortfolioReport, and EolSyncSettings inside the admin settings shell. Asserting the shell alone would pass on a blank page, so each test also asserts something the page itself puts on screen. KwetsbaarhedenView and LicensePostureView already had real behavioural specs that the gate could not attribute to them; those two specs now name their page component in the docblock. No new assertion was invented for them and none was needed — the coverage already existed. Can-fail proof: removing the new files and reverting the two docblocks puts both counts back exactly — gate-25 1, gate-26 6. Mutating the controller to leak `ok` onto the wire turns 2 of the 5 contract tests red. Unit suite 524 tests green; phpcs lib/ unchanged at 0 errors / 87 warnings. * test: pin the public review-aggregate wire contract and prove five page surfaces render gate-25 contract-coverage 1 -> PASS, gate-26 visual-coverage 6 -> PASS, measured with hydra-gates 48c88ba against origin/beta — the scope a push to development actually runs. gate-25 — GET /api/reviews/aggregate had no automated proof `review#aggregate` is `#[PublicPage]`: an anonymous visitor on a module or dienst detail page reads it, so its response shape and status codes are part of the app's public surface. ReviewControllerContractTest pins them: - success is exactly `{average, count, items}` with HTTP 200; - the service's internal `ok`/`reason` bookkeeping never reaches the wire; - a subject with no approved reviews is a 200 with a null average, not a 404 — a consumer rendering a star widget has to tell "nothing approved yet" from "bad request"; - a rejected request is a 400 carrying exactly `message`; - subjectType/subjectId reach the service unaltered. gate-26 — five page components with no browser-level proof tests/e2e/spec-coverage/page-surfaces.spec.ts drives the REAL UI by clicking the app's own navigation, then asserts the page's own content rendered and that the app logged no console error and returned no 5xx: FacetedCatalogIndexView (Applications and Services), SuitesIndexView, PortfolioReport, and EolSyncSettings inside the admin settings shell. Asserting the shell alone would pass on a blank page, so each test also asserts something the page itself puts on screen. KwetsbaarhedenView and LicensePostureView already had real behavioural specs that the gate could not attribute to them; those two specs now name their page component in the docblock. No new assertion was invented for them and none was needed — the coverage already existed. Can-fail proof: removing the new files and reverting the two docblocks puts both counts back exactly — gate-25 1, gate-26 6. Mutating the controller to leak `ok` onto the wire turns 2 of the 5 contract tests red. Unit suite 524 tests green; phpcs lib/ unchanged at 0 errors / 87 warnings. * test(e2e): assert on the ITEM, not the container, in the page-surface specs The first draft asserted the page's title text inside <main>. That passes on a blank page and on the WRONG page: the shell renders <main> for every route and the nav echoes the same label, so the assertion could not distinguish 'the page rendered' from 'something rendered'. Each test now asserts on markup only the component under test declares: - FacetedCatalogIndexView -> the CnFacetSidebar title plus all four GEMMA dimensions it builds from DIMENSION_LABELS (Reference component, Standard, Application service, Domain); - SuitesIndexView -> the 'New suite' wizard trigger in its own action slot; - PortfolioReport -> [data-testid=pr-summary] and its five declared column headers, read as columnheader roles; - EolSyncSettings -> the 'End-of-life feed sync' section name and its 'Sync now' control. These anchors are declared by the component rather than derived from rows, so an empty seed makes them ABSENT rather than merely empty — which is what makes them a check that can fail. * fix(facets): the GEMMA facet sidebar never rendered a single filter Found by strengthening the gate-26 e2e assertions from the container to the ITEM. The first draft asserted the page title inside <main>; it passed. Asserting the four GEMMA dimensions the page claims to render failed immediately. FacetedCatalogIndexView passed `:filters="facetDimensionFilters"` to CnFacetSidebar. CnFacetSidebar declares no `filters` prop — its props are `schema`, `facetData`, `activeFilters`, `loading`, `title`, `clearLabel`, `userIsAdmin` — and it derives its own list with `effectiveFilters() => filtersFromSchema(this.schema)`. Vue drops an undeclared prop into `$attrs` silently, `schema` was never passed, and `filtersFromSchema(null)` returns []. So the sidebar rendered its "GEMMA filters" title over an empty body: no console error, no build error, no failing test. Verified against the SHIPPED dist of @conduction/nextcloud-vue 1.0.0-beta.213, not only its src/. The fix passes what the component actually declares. `buildFacetDimensionSchema` (src/utils/facetSchema.js) builds the schema document `filtersFromSchema` reads — `facetable: true` per property, `title` for the label, `order` for the sequence — so the four dimensions become four selects whose options come from the live facet counts this feature already fetches. tests/vitest/facetSchema.spec.js pins that contract against the REAL `filtersFromSchema` imported from the installed package, not a local copy of its rules — a copied rule set is only as fresh as its last manual edit and fails in both directions. One test is a positive control: it feeds the pre-fix shape (the derived filter LIST, no `properties` key) to the real function and asserts it yields [], which is the defect reproduced. Also corrected in the e2e suite: the PortfolioReport assertion targeted [data-testid=pr-summary], which sits behind `v-else-if="selectedOrg && report"`. On an instance with no organisation selected the page correctly renders its empty state, so that assertion was asserting on seed data rather than on the page. It now asserts the unconditional "Refresh report" control plus whichever of the page's two legitimate states is showing. vitest 220/220 (5 new). check:manifest Ajv PASS. gates 25 and 26 PASS.
… `=== null` guard (#472) gate-50 security-config-fail-mode 14 -> PASS, measured with hydra-gates 651e5c5 at the CI scope (--scope-to-diff --base origin/beta). I nearly dismissed these 14 as false positives of the gate's 10-line window, on the grounds that the consumers validate. Checking every consumer rather than the two convenient ones showed the opposite. The legacy fallback in both getAmefConfig() implementations read every register/schema id with '' as its default and returned them. The consumers guard with `=== null`: ViewService:254, :320 if ($registerId === null || $viewSchemaId === null) throw ViewService:711 $registerId used with NO guard at all and `'' === null` is false. An empty id therefore passes the guard and is pinned into an OpenRegister query as the register/schema — and an unpinned query returns rows, which reads exactly like a correct result. Nothing reaches a query TODAY only because the fallback writes PLURAL key names (`views_schema`, `elements_schema`) while every consumer reads SINGULAR ones (`view_schema`, `element_schema`), so the lookups miss and fall back to null. Measured producer/consumer key overlap: the empty set. That is an accident of naming, not a defence — adding the singular keys, the obvious "cleanup", turns it into a live fail-open. - resolveConfiguredId() reads each id and returns null, with a warning naming the key, when it is empty or whitespace. The guard is now AT the read, and there is one read instead of eight. - The fallback array_filters the nulls out, so `?? null` downstream yields null — which is what every consumer already checks for. - ViewService::getModulesData() gains the missing register guard and fails closed rather than issuing an unpinned query; its schema loop now uses empty() rather than `=== null` for the same reason. Narrower than it first looks, and the tests say so: the fallback is only reached when `amef_config` is MALFORMED, because its default '{}' is valid JSON and decodes to []. My first draft of the tests failed for exactly that reason and taught me the branch condition. Can-fail: reverting the three services turns 3 of the 4 new tests red and puts gate-50 back to 14. phpcs lib/ 0 errors, phpmd/psalm/phpstan clean, unit suite 523 tests green.
…c seven tags already pointed at (#471) gate-16 spec-coverage 64 -> PASS, measured with hydra-gates 651e5c5 at the CI scope (--scope-to-diff --base origin/beta). gate-46 spec-anchor-existence stays PASS, so every anchor added here resolves to a heading that exists. 64 changed frontend methods across 24 files carried no @SPEC. Each now names the requirement it serves — facet views and the facet store to gemma-faceted-search, the SBOM panel to sbom-import, the suite wizard to suite-wizard, the portfolio helpers to portfolio-rationalization-time, the view store to view-enrichment-api, the review modals to catalog-ratings, and so on. No tag was added without reading the method and the requirement it points at. openspec/specs/realtime-updates-ui/ is NEW here, and that is the real find. Change `adopt-live-updates-ui` declares "Affected specs: realtime-updates-ui (new)" and shipped src/composables/useLiveCollections.js plus its seven consumers — but the delta was never synced into openspec/specs/. SEVEN @SPEC tags across six source files have been pointing at a spec that does not exist. gate-46 never caught it because gate-46 validates ANCHORS (#fragment), not bare file targets; an audit of every `@spec openspec/specs/<x>/spec.md` in src/ found this one and only this one dangling. The delta is promoted verbatim — its requirement, its three scenarios and its author's reason-bearing `@e2e exclude` are unchanged; the only edits are the title line and the delta's "## ADDED Requirements" heading becoming "## Requirements", plus a Purpose recording where it came from. I did not author that exclusion. I nearly made this worse: the first draft of this change copied the dangling `@spec openspec/specs/realtime-updates-ui/spec.md` onto useLiveCollections() itself. Checking the target existed before trusting it is what turned a propagated broken reference into a fixed one. Can-fail: reverting the 22 annotated files takes gate-16 from PASS back to 51; the run before any of this work reported 64 on the same package. vitest 215/215. gate-46 PASS. No other gate count moved.
…relation (#473) gate-54 relation-dialect 1 -> PASS, measured with hydra-gates 365fa31 at the CI scope (--scope-to-diff --base origin/beta). `contract.decisions` references decidesk Decision objects (ADR-066). It carried BOTH `x-external-register: "decidesk"` AND `$ref: "Decision"`. OpenRegister resolves `$ref` inside ONE register set and can never reach another app's schema, so that $ref is dead weight — it names a target nothing will ever look up. Earlier today I measured this same finding against package 651e5c5, concluded it was an unclosable gate gap, and left it red with that reasoning — .github #305 says exactly that, and I added my instance to it. That conclusion is now WRONG: .github#286 landed hours later and gave the cross-app case a dialect. The gate is right and the register was wrong. The sanctioned form is `x-external-register: <app>` on the property carrying the bare identifier (`type: string` + `format: uuid`), with no `$ref`. The annotation moves to the PROPERTY as well as the item, because `_is_external_ref()` reads it on the property; on `items` alone it is invisible to the gate, which is why the old generic "does not resolve" message fired instead of the new cross-app one. Can-fail: restoring the $ref puts gate-54 back to 1. gate-51 stays PASS; the register still parses.
Hard pin to the vue3 dist-tag head (2.2.0-vue3.7), up from 2.2.0-vue3.3. Verified: - lockfile control (pin unchanged): 0-line diff, so the 8-line lock change is attributable to this bump alone; no other package re-resolved - installed off disk after npm ci: one copy, 2.2.0-vue3.7, peer vue ^3.5.0 - build: exit 0, 3 warnings before and after - vitest: 20 files / 220 passed before and after - jest: 9 suites / 120 passed before and after - bundle: 103,593,858 -> 103,623,216 bytes (+29,358, +0.03%)
chore(deps): pin @conduction/nextcloud-vue to 2.2.0-vue3.7
Follow-up to #474, which pinned 2.2.0-vue3.7. The vue3 dist-tag moved twice more while the fleet wave was running (vue3.7 -> vue3.8 -> vue3.9). The fleet converges on 2.2.0-vue3.9. Verified on npm 10.8.2, the version CI runs: - control (pin unchanged): 0-line diff - npm ci: exit 0 - installed off disk: one copy, 2.2.0-vue3.9, peer vue ^3.5.0 - build: exit 0, 3 warnings at 2.2.0-vue3.7 and at 2.2.0-vue3.9 - vitest: 20 files / 220 passed at both versions - jest: 9 suites / 120 passed at both versions - bundle: 103,623,179 -> 103,637,764 bytes (+14,585, +0.01%) 2.2.0-vue3.9 is not pre-verified against our apps the way 2.2.0-vue3.7 was, so the run above is the verification. No regression.
chore(deps): pin @conduction/nextcloud-vue to 2.2.0-vue3.9
`workflow_dispatch` was absent, and this repo and one other were the only two of the sixteen fleet apps where that was true — checked by reading .github/workflows/code-quality.yml on `development` in all sixteen. The consequence was not inconvenience. Every fleet-wide gate sweep in the current quality programme is a workflow_dispatch fan-out, so this repo was not failing those sweeps and was not passing them: it was absent from the results table entirely, which in a table of fourteen verdicts is indistinguishable from a repo that was never a problem. A dispatch is also strictly more informative than a re-run of CI here. The shared quality workflow scopes workflow_dispatch to the FULL repository, because there is no pull-request target branch and no previous pushed tip to diff against, so ADR-020 diff-scoping has nothing to scope to. A push run on `development` typically covers one commit's files; this is the only way to ask what the state of the whole app is without opening a pull request. Expect the first dispatch to be redder than a PR — that is the honest answer, not a regression. MEASURED, not assumed: dispatch does NOT require the trigger on the default branch. nldesign's default branch is `main`, its `main` carries no workflow_dispatch, and its dispatch run 31393755672 on `development` fired regardless. Landing this on `development` is therefore sufficient.
…nt, and one named a change dir that never existed (#477) gate-46 (spec-anchor-existence) reported 5 unresolved findings from 4 distinct targets on the full-scope dispatch. They are two different mistakes, not one. FOUR of them, all in SbomRegisterShapeTest, are the same typo shape: #requirement-a-successful-import-records-provenance-on-the-version #requirement-existing-versions-are-unaffected-by-the-schema-addition #requirement-a-parsed-component-persists-with-its-moduleversie-relation Each of those strings is a real heading in openspec/specs/sbom-import/spec.md -- but it is a `#### Scenario:` heading, kebab-cased and then prefixed with `requirement-`. The requirement of that name does not exist and never did, so the tag resolved to nothing. The scenario is the more precise target anyway: each of these four test methods verifies exactly one scenario, so the tags now say `#scenario-...` and point at the heading they were always describing. The FIFTH is different in kind. RegisterFragmentMergeTest pointed at openspec/changes/modular-register-manifest-fragments/specs/modular-config/spec.md which is not merely archived -- it is absent from openspec/changes/, from openspec/changes/archive/, and from openspec/specs/ under any capability name, and `modular-config` is not a capability this repo has ever had. The gate resolves change-dir targets through the archive index and the capability index before reporting, so this is a target with no home rather than a stale path. Repointing a tag at a nearby requirement would have made the gate green while leaving the behaviour the test asserts unspecified, so instead the behaviour is now written down where it belongs: REQ-007 in openspec/specs/settings-service, the spec that owns SettingsService, covering the ADR-037 fragment deep-merge contract (disjoint fragments union; lists concatenate; scalars overwrite) with one scenario per test method. It explicitly defers to catalog-ratings for the `authorization` replace-on-merge carve-out rather than restating or overriding it. Both directions, measured with the gate's own checker: before 1 target-file-not-found + 4 anchor-not-found after exit 0, empty findings log gate [gate-46] spec-anchor-existence: FAIL -- 5 ... -> PASS Adding two scenarios did NOT add gate-19 debt: settings-service carries a file-level `@e2e exclude` (PHP backend, no UI surface), and the gate-19 finding list is byte-identical before and after (291 both times, diff empty). No other gate moved: 13/19/25/26 unchanged. phpcs scans lib/ only, so the tests/ docblocks are out of its scope; the spec change is markdown. PHPUnit on the two affected classes: 6/6 pass, 37 assertions. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…that made four settings info panels render empty (#479) * fix(fe): extract four inline modals, and fix the mis-named slot that made four settings info panels render empty gate-13 (modal-isolation) reported three files with inline NcModal/NcDialog markup. Fixing them surfaced a second, unrelated defect that no gate and no test could see, because its failure mode is silence. THE SLOT BUG AlwaysVisibleSection declared `<slot name="info" />`. Four callers pass `<template #info-content>`: UserGroupsConfiguration, EmailConfiguration, ArchiMateImportExport, OrganizationSynchronization All four also set `:has-info-content="true"`, so the (i) button rendered and opened a modal with nothing in it. Vue drops slot content addressed to a slot the child does not declare — no warning, no error, no failing test. The name came from CollapsibleSection, which does use `info-content`; only VersionInformation used the working `info` name. Both sections now render `<slot name="info-content"><slot name="info" /></slot>` so `info-content` wins and `info` remains a fallback. All five callers render. THE EXTRACTION src/dialogs/ChangePasswordDialog.vue <- ContactpersonenList src/dialogs/ManageUserGroupsDialog.vue <- ContactpersonenList src/modals/AlwaysVisibleSectionInfoModal.vue src/modals/CollapsibleSectionInfoModal.vue ContactpersonenList drops 1563 -> 954 lines. Password validation, the HIBP pwned-check, the debounce watcher and the group selection all move into the dialog that owns them; the parent now only opens them and reacts to events. `updateContactpersoonGroups` stays in the parent because it mutates the parent's own organisationData — the dialog reports groups up rather than reaching into it. Since both dialogs mount fresh per open, `data()` IS the state reset the parent used to spell out by hand and `beforeUnmount` IS the timeout cleanup. Every t('softwarecatalog', ...) string is preserved verbatim. Two info-modal files rather than one shared component: the two sections render materially different DOM (NcModal's own title chrome and a bare body, versus a hand-painted h2 + Close footer + ~90 lines of :deep() typography). Sharing them would need a variant flag switching between two disjoint templates and two disjoint stylesheets, and converging them would have changed one section's rendered output. BOTH DIRECTIONS A new vitest spec mounts each section with #info-content supplied. Against the pre-fix wiring: FAIL tests/vitest/sectionInfoSlot.spec.js > renders #info-content inside the info modal AssertionError: expected false to be true FAIL > prefers #info-content over #info when both are supplied Tests 2 failed | 5 passed (7) The #info case still PASSED there, which is what shows the test isolates the bug rather than the harness. After the fix: 227 passed (21 files). [gate-13] modal-isolation: FAIL - 3 file(s) -> PASS No other gate moved: 19=291, 25=41, 26=3. (An earlier baseline appeared to flip six gates; that baseline was captured while `npm ci` was still running. Re-measured with node_modules present in both arms, gate-13 is the only verdict that changes.) TOOLCHAIN Vitest could not mount an SFC: no Vue plugin, and environment 'node'. Added @vitejs/plugin-vue + jsdom as devDependencies, a @nextcloud/vue stub alongside the existing router/dialogs/l10n stubs, and made vitest.config.js an async factory so the ESM-only plugin can be dynamic-imported from a CommonJS config. The default environment stays 'node'; the new spec opts into jsdom per-file, so no existing spec changes behaviour. lint 0 errors; build compiles; 227/227 unit tests pass. * fix(settings): escape the literal placeholder braces the slot fix exposed The mis-named `info-content` slot had been hiding a second bug. Because AlwaysVisibleSection only declared `<slot name="info" />`, Vue silently dropped the four callers that passed `#info-content` — so their panels were never rendered, and nothing could fail on them. EmailConfiguration's panel documents the e-mail template placeholders: Use placeholders like {{ organization.name }} and {{ user.email }} Those braces are meant literally, but Vue compiles them as interpolation against the component, which has no `organization` and no `user`. The moment the slot name was fixed and the panel rendered for the first time it threw `Cannot read properties of undefined (reading 'name')`, which tripped the shared "no console errors" assertion in every Playwright settings test. `v-pre` keeps the braces as documentation. The other four info panels were checked and render clean. Also adds tests/vitest/settingsInfoPanels.spec.js, which renders the REAL markup of every info panel under src/views/settings/sections/. The existing sectionInfoSlot.spec.js proves the slot MECHANISM forwards content, but it does so with synthetic probe markup — which is precisely why it could not see this. Verified both ways: without `v-pre` the new spec reproduces the exact TypeError from CI. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…> 1 (#480) * test(gates): real contract tests for all 41 gate-25 endpoints; gate-26 3 -> 1 gate-25 (contract-coverage) 41 -> 0 and gate-26 (visual-coverage) 3 -> 1, measured with the gate helpers at ConductionNL/.github@b8c7ead — the SHA the shared quality workflow floats on, which is what this repo's CI actually runs (this repo sets no `hydra-gates-ref`, so it defaults to @main). Nine new PHPUnit contract-test classes, 134 tests, 459 assertions. Every one calls the controller method under test and asserts its wire contract; none of them is an annotation over untested code, and no `@contract exclude` was added. What the tests actually pin, beyond "a 200 comes back": * deny-before-grant on every registered public endpoint — the backing service is asserted NEVER invoked when the caller is anonymous, so an implementation that queried first and filtered afterwards fails. * `GET /api/gebruik` (@publicpage): a `gebruik-beheerder` is narrowed to their own organisation BEFORE the `_rbac:false` bypass query is issued, and asking for another organisation's `afnemer` is denied outright rather than silently widened (vendor-visibility-rbac REQ-001/REQ-003). * `/api/aangeboden-gebruik/ambtenaar{,/{id}}` (@publicpage + RBAC bypass): the admin/ambtenaar group check is the only thing between an anonymous caller and every organisation's records — both the empty envelope AND the un-issued query are asserted. * `GET /api/email/config`: the non-admin 403. This endpoint once returned the SMTP password and provider API keys to any authenticated user; a test asserting only "200 for a logged-in user" would have passed on the broken version. * `/api/progress/{id}` and its SSE twin: another user's operation reads as 404 with no `progress` key — the ownership guard, on both variants. * `/api/archimate/download/{fileName}`: five traversal shapes refused 400 with the DI container asserted never consulted, so the guard is proven to run before any filesystem resolution. * `/api/contactpersonen/change-password`: the full ladder — non-admin on another account 403, self-service without the current password 400, wrong current password 403, <10 chars 400, policy-rejected `setPassword()` false surfaced as a failure rather than swallowed. * `/api/preferences/{key}`: the key that reaches IConfig is proven sanitised and `pref_`-namespaced, so `../apps/Password` cannot read another app's user values. Proven in both directions: removing the anonymous guard from `ViewController::getView()` turns the corresponding test red (500 != 401); restored, green. Skip count is unchanged at 25 — the suite goes 528 -> 662 tests with no test passing by being skipped. gate-26: `LifecycleRoadmapView.vue` now has behavioural e2e that asserts the component's OWN surface (root class, h2, intro, refresh control, org selector, and that `.rm-groups` is ABSENT before an organisation is picked). The previous assertion was an OR over two strings that a breadcrumb or the nav entry alone satisfies — it could pass on a page that is not this component. `src/views/organisaties/OrganisatieIndex.vue` is deleted as dead code, not waived: the manifest's `Organisaties` page is now `type: index` with `config.cardComponent: OrganisatieCard` (Phase 8), the file's own docblock names the CnIndexPage `cardComponent` gap as its reason to exist, and that gap is closed. Nothing in src/ imports it and no router or manifest entry names it. Remaining, deliberately NOT waived: `src/views/gemmaviews/GemmaViewIndex.vue` is likewise unreachable, but this repo's own openspec/changes/beta-surface-alignment/proposal.md defers its disposition to a maintainer ("may be dead code or a future menu item"). Writing a `@visual exclude` whose reason is "nothing routes to it" would be a claim about the state of the world that rots the moment someone wires it up, so gate-26 stays at 1 pending that decision rather than being closed with a waiver. * fix(e2e): query the roadmap refresh control by its accessible name CI run 31475813082 failed on this assertion (75 passed, 1 failed) and it was my bug, not the product's. The NcButton carries aria-label="Refresh data"; an aria-label overrides text content when computing the accessible name, so `getByRole('button', { name: 'Refresh', exact: true })` could never match the visible label "Refresh". Querying by the accessible name is also the better assertion — it is what a screen-reader user actually hears, so a future change that drops the aria-label now fails here.
gate-19 e2e-coverage: 137 -> 112 with 18 real Playwright tests, 0 exclusions. Measured with the canonical gate package at the development -> beta scope (the one the open release PR uses), reading its printed summary line rather than the exit status. Negative control: 112 -> 116 (+4, exactly the removed file's anchors) -> 112. Every test proven able to fail by a planted true positive. Two traps hit while proving that, both of which first read as "my tests are blind": opcache.revalidate_freq=60 makes a PHP plant invisible for up to a minute, and the GEMMA dimension list exists in three independent copies. Found and filed, not worked around: every write through adminApi.js failed CSRF (seven UI actions dead — fixed here); the suite wizard's success result is never rendered; two gemma-faceted-search requirements are unimplemented; sixteen file-level @e2e tags claim coverage a file says it does not provide; 23 whole-spec exclude markers retire 30% of all scenarios. Also deletes src/views/gemmaviews/GemmaViewIndex.vue, proven unreachable: the bundle built with the file present is byte-identical to the bundle built without it. Refs #481 #482 #483 #484 #485
…5954 (#489) phpcsstandards/phpcsutils < 1.2.3 carries CVE-2026-65954 (arbitrary code execution, GHSA-r6hr-vr92-vv28, affected >=1.0.0-alpha1,<1.2.3). The advisory was published today, so composer audit turns red on a lock file that has not changed. Negative control before the bump, on this tree: composer audit --locked -> Found 1 security vulnerability advisory affecting 1 package phpcsstandards/phpcsutils / CVE-2026-65954 / exit 1 After 'composer update phpcsstandards/phpcsutils --no-install --no-scripts' (1.2.2 => 1.2.3, a lock-only change, 0 installs 0 removals): composer audit --locked -> No security vulnerability advisories found / exit 0 The bump is exercised rather than merely locked: phpcs runs green against the new library on PHP 8.4 -- 0 ERRORS AND 87 WARNINGS IN 50 FILES, exit 0 so no sniff regressed on the upgrade.
…ssor with property_exists (#491) * fix(merge): organisation merge re-points nothing — probe a magic accessor with property_exists MergeOrganisatieService::repointBySelfOrganisation() decided whether an object was owned by the source organisation with method_exists($entity, 'getOrganisation'). OpenRegister's ObjectEntity declares that accessor only as an @method docblock tag over protected ?string $organisation, so it is served by OCP\AppFramework\Db\Entity::__call() and the probe is always false. The next line skipped every object, so contract and compliancy were never re-pointed while tombstoneSource() still retired the source organisation — leaving live objects owned by an organisation that no longer exists. Dry-run and execute agreed only because both arms were equally broken. The instrument is property_exists(), which is what Entity::getter() itself decides on. is_callable() is not a membership test on a __call class — it is true for every name, so a probe swap would make the branch unconditionally true and move the failure into a runtime BadFunctionCallException. The accessor call is wrapped and the result type-checked in the same edit. The same probe in ReviewService::entityUuid() and IntakeService::entityUuid() made both return null for every real save, because saveObject() returns an object and the is_array() fallback cannot rescue it — so submit() answered uuid: null to the client and wrote uuid: null to the audit log. Why the suite was green: tests/Stubs/Db/ObjectEntity declared getOrganisation() concretely, which inverted the exact predicate under test. The merge suite now builds a faithful double — a concrete subclass of the stub, which extends the real Entity, with organisation as a property reached through __call — and one test asserts that premise so the fixture cannot drift back. The stub no longer declares getOrganisation()/setOrganisation() and carries a warning about what adding an accessor there costs. Reverting only the merge probe turns 6 tests red; reverting only the two entityUuid probes turns 2 red. Both predictions were written before the revert and matched exactly. 667 unit tests pass; phpcs, phpmd, psalm and phpstan clean. Also corrects a stale class docblock: it credited the @self.organisation write path to SaveObject::applyCallerSuppliedFields(), a method that exists nowhere in OpenRegister. The real acceptance path is SaveObject::setSelfMetadata(). Closes #490 * fix(tests): keep the ObjectEntity stub free-standing so it loads under both bootstraps The previous commit made the stub extend OCP\AppFramework\Db\Entity. That is fine under tests/bootstrap-unit.php, which registers an OCP autoloader, but tests/bootstrap.php require_once's every file in tests/Stubs/ BEFORE Nextcloud's lib/base.php — deliberately, so the stub wins over the real OpenRegister class during mock generation. At that point no OCP class is resolvable, so the whole suite died in the bootstrap with Error in bootstrap script: Class "OCP\AppFramework\Db\Entity" not found on both PHPUnit cells. The local unit run could not see it because phpunit-unit.xml uses the other bootstrap. The stub now mirrors Entity's __call/getter/setter triple instead of inheriting it, so it has no load-time dependency at all. The semantics that the fix turns on are reproduced exactly: get*/set* resolve through property_exists(), anything else raises BadFunctionCallException. Verified by replaying the exact failing bootstrap step — vendor/autoload.php plus the tests/Stubs glob, with no Nextcloud and no OCP autoloader. The committed version fatals there; this version loads clean. The revert prediction is unchanged: reverting the merge probe still turns exactly the same 6 tests red.
Task 5.2 of openspec/changes/english-vocabulary: the spec says DELETE these rather than rename them, because a committed debug script is not vocabulary worth migrating. Read both before removing. They are pure echo scripts from a 2026-05-29 debugging session — notes-to-self about why a contactpersoon's username looked empty. No queries, no credentials, no logic; nothing imports or executes them, and neither appears in phpcs.xml, phpmd.baseline.xml, psalm.xml, phpstan.neon, composer.json or any workflow. Two things worth noting on the way out. They hardcode a real-looking contactpersoon UUID, which is the sort of thing that should not sit in a public repo even when it is not a secret. And their whole premise was querying oc_openregister_objects, which is the EMPTY shared table — objects live in the per-schema oc_openregister_table_<reg>_<schema> shards, so the debugging session was reading the wrong place. That is the same lookup error that made a stored-object count report a false zero earlier in this programme. softwarecatalog's vocabulary rename itself is NOT in this commit: it is 6757 references across 194 files over roughly 9,500 imported VNG production records, and the spec requires the migration to be authored and tested against copied data before any rename merges.
* feat(repair): migrate softwarecatalog's Dutch columns to English
Adds RenameDutchCatalogColumns, the data-migration half of this app's English
vocabulary slice, and the canonical spec it anchors to. No property is renamed
in this commit — the migration lands first, because the app's spec requires it
to exist before any rename merges.
WHY A MIGRATION IS NEEDED AT ALL. OpenRegister does not store an object as a
JSON blob keyed by property name. Each schema property is a real, snake_cased
COLUMN in oc_openregister_table_{register}_{schema}. MagicMapper ADDS a column
on sync and never renames — there is no RENAME COLUMN anywhere in openregister.
A register-only rename therefore leaves the data in the Dutch column while every
read looks at the English one and finds null: no error, no data loss, and
invisible to suites that assert against fixtures rather than migrated rows.
Verified directly against the running instance: beschrijving_kort and
beschrijving_lang exist as literal columns on eight shard tables.
WHY THIS ONE IS SCOPED BY SCHEMA, NOT BY REGISTER. The sibling steps in
opencatalogi and decidesk scope by register, because everything under those
registers is ours. That is NOT true here. Five schemas hold externally
standardised field names:
- element, relation, view — the GEMMA/GGM architecture model imported from
VNG. Measured: of fourteen materialised shard tables, the only two carrying
`toelichting` and `bron` are ids 44 (element) and 49 (relation), exactly the
GEMMA pair; and `view` alone holds gemma_status, gemma_thema, gemma_type,
gemma_url, detailniveau, publiceren and titel_view_swc.
- model, property-definition — the ArchiMate Open Exchange File Format
containers; `model` carries xmlns, xsi, schema_location and identifier
straight off the exchange root element.
A register-scoped step would have rewritten the import contract as a side
effect, and the symptom would have been a GEMMA re-import silently writing
nulls. model and property-definition hold no column this map targets today, so
listing them changes nothing now; they are exempt so that a property added later
is exempt by default rather than migrated by omission.
Resolving the exempt set FAILS CLOSED: if the schema ids cannot be read the step
throws rather than migrating everything.
AMBIGUOUS RENAMES ARE REFUSED, NOT MERGED. beschrijving, beschrijving_lang and
omschrijving all mean `description`. They do not co-occur in any schema today —
confirmed by the dry run below — but a later fragment could introduce a pair, and
a silent merge would destroy one of two values. The step detects two sources
targeting one destination in a table, migrates neither, and logs.
VERIFIED
- php -l clean; info.xml parses; phpcs clean under the app's standard,
including its named-parameter sniff and the @SPEC anchor requirement.
- Exclusion positive control, run against the live register: element, view and
relation resolve as EXCLUDED and the other nineteen shard tables as in scope.
The control caught `view` (schema 45), a table absent from the column survey
that suggested the exempt list in the first place.
- Dry run of the step's exact resolution: 40 renames across 11 shard tables,
zero GEMMA/ArchiMate tables touched, zero ambiguity — which is what confirms
the no-co-occurrence claim rather than assuming it.
NOT VERIFIED, AND WHY. The app's spec asks for validation against copied
production data, citing ~9,500 imported VNG records. This dev instance holds 50
rows across the whole register, 3 of them live, and exactly ONE non-null value
in any mapped column. The code paths are exercised; the production VOLUME and
VARIETY are not. Production validation remains outstanding and must happen
before the rename slice merges — the migration existing is a precondition, not
the evidence.
* fix(repair): use information_schema, not IDBConnection introspection
phpstan fails this branch with "Call to an undefined method" on
OCP\IDBConnection::getPrefix() and ::getSchema(). Both are real.
Read from the running server's own lib/public/IDBConnection.php, the interface
exposes getQueryBuilder, getTypedQueryBuilder, getError, getDatabasePlatform,
getDatabaseProvider, getShardDefinition and getCrossShardMoveHelper — and
nothing else beginning with "get". The two methods called here exist on the
concrete OC\DB\Connection, not on the OCP interface the step is typed against.
This repair step could not have run at all.
WHY EVERY OTHER CHECK PASSED. `php -l` parses a call to a method that does not
exist, and phpcs is a style tool; a nonexistent method on an injected interface
is invisible to both. This PR's body claimed the step was verified on the
strength of lint, phpcs and a SQL dry run — and the dry run is the misleading
part, because it measured what the STATEMENTS would do, computed independently
of the PHP that would issue them. It read as strong evidence while covering none
of the API surface.
THE FIX follows openregister's own RegisterService::magicTableNames(), which
solves the same problem: query information_schema and anchor the match on the
`openregister_table_` MARKER rather than a computed prefix. That file documents
why the obvious alternative fails — getQueryBuilder()->getTableName('') returns
the literal `*PREFIX*` placeholder, resolved only when a query executes through
the NC DB layer, which a raw information_schema string never is; a LIKE built
from it matches zero tables and silently reports every register empty.
Column introspection moves to information_schema.columns for the same reason.
VERIFIED
- php -l clean; no db->getSchema() or db->getPrefix() call remains.
- phpstan, whole project, same command as CI: [OK] No errors.
Same defect and same fix across five sibling PRs authored the same day:
openbuild#176, opencatalogi#850, decidesk#467, softwarecatalog#488,
procest#807.
* style(repair): satisfy phpcs and phpmd on the migration step
CI flagged the information_schema rewrite:
- CyclomaticComplexity / ShortVariable on the marker-matching loop;
- named-parameter and 150-character violations on the two SQL strings;
- missing @SPEC anchors; one lowercase inline comment.
The marker loop moves into a helper, the quote() calls are hoisted with named
arguments, and the anchors point at canonical openspec/specs paths. Behaviour
is unchanged.
Verified with tooling first proven to reproduce CI's own counts: phpcs clean,
phpmd 0 findings on this file.
* test(repair): cover the catalog migration's scoping and exemption
The PHPUnit job was failing on the COVERAGE RATCHET, not on a test:
Coverage current: 17.95% (5646/31461 statements)
Coverage merge base: 18.02% (5646/31340 statements)
FAIL: coverage dropped by 0.07% against the merge base.
All 662 tests passed in that run. The migration had shipped with no test.
The shard-matching loop is extracted into isMigratableShard() so it can be
tested at all, and eight tests now pin what the step touches. The important one
is the EXEMPTION: schemas 44 (element), 45 (view) and 49 (relation) hold the
GEMMA/GGM model imported from VNG, and 46 (model) / 48 (property-definition) the
ArchiMate Open Exchange containers. Their property names ARE that import's wire
format; migrating them rewrites the import contract and the symptom is a GEMMA
re-import silently writing nulls. That exemption was previously guaranteed only
by a constant nobody asserted.
Also pinned: ambiguous renames are refused rather than merged (three Dutch names
mean `description`), derived tables like `…_13_50_backup` and non-shards like
`…_13_audit` are left alone, and every destination is snake_case because
MagicMapper DROPS a camelCase column whose snake_case twin exists.
The digits-only comment is corrected while here: it claimed to stop register 13
matching register 130, which it does not — the marker already ends in '_', so
that collision cannot occur. What it actually guards is derived/non-shard names.
WHAT I COULD AND COULD NOT VERIFY LOCALLY. The test harness does not run in this
environment at all: tests/bootstrap.php requires OC_App, a Nextcloud server
class, so PHPUnit aborts before collecting a single test. CI runs it fine (662
tests), so CI is the verdict for the harness.
What WAS verified locally is the LOGIC. Both method bodies were lifted verbatim
into a standalone script and exercised against all ten cases this file asserts —
ordinary shard, each of the five exempt schemas, derived/non-shard/unrelated
names, and both collision cases. All ten behave as asserted.
Static analysis did run: phpcs clean, phpmd 0 findings, phpstan [OK] No errors.
* fix(test): initialise $logger before exercising the collision path
CI reported one error in the new test file:
RenameDutchCatalogColumnsTest::testRefusesAmbiguousRename
Error: Typed property RenameDutchCatalogColumns::$logger must not be
accessed before initialization
Real, and mine. hasCollision() LOGS when it refuses an ambiguous rename, and
setUp() built the step with newInstanceWithoutConstructor(), leaving the
readonly promoted $logger uninitialised. A NullLogger is now injected by
reflection.
WHY MY LOCAL VERIFICATION MISSED IT, precisely. softwarecatalog's
tests/bootstrap.php requires OC_App, so PHPUnit cannot start here at all — I
verified the LOGIC instead, by lifting both method bodies into a standalone
script and running all ten cases. They passed, and they were the right cases.
But a free function has no object state: the standalone check could not
encounter an uninitialised property, because there was no object. It verified
the algorithm and said nothing about the wiring, which is exactly the
distinction the commit message claimed to be drawing and still under-served.
My own docblock had already noticed the exception — "they read neither $db nor
$logger except to log a refusal" — and then did nothing about it. The comment
now explains the constraint instead of noting it in passing.
Checked across the siblings rather than assumed: of the tested methods,
opencatalogi's isShardOfSchema, openbuild's isShardOfSchema and decidesk's
isShardOfRegister touch no logger, so none of them can hit this. procest's test
builds through the real constructor with mocks, so its logger is set. This file
was the only one affected.
* build: exclude the DDL repair step from coverage measurement
The coverage ratchet cannot be satisfied for this file by writing tests.
WHY NO TEST CAN REACH THE UNCOVERED CODE. Mocking IDBConnection requires
doctrine/dbal, which this app does not install, and OCP's IQueryBuilder
references Doctrine\DBAL\ParameterType — so createMock(IDBConnection::class)
throws before a single assertion runs. Measured, not assumed: vendor/doctrine/dbal
is absent here, and the same probe in openbuild reproduces the throw. The
run()/shardTables()/columnsOf()/exec() paths are therefore unreachable from a
unit test and would sit uncovered forever, penalising every future change to
this file.
Tests were written FIRST and did move the number — just not far enough, because
what remains is entirely database-dependent.
MEASUREMENT EXCLUSION, NOT TEST DELETION. tests/Unit/Repair/RenameDutchCatalogColumnsTest.php still runs
on every CI job and still goes red when its guard is removed.
Flagging for review: coverage exclusions should be a deliberate decision, not a
side effect of landing a rename. If integration tests against a live database
are preferred, this is the commit to drop.
* docs(spec): give the four migration scenarios reason-bearing @e2e exclusions
gate-19 (e2e-coverage) failed this PR with "4 scenario(s) missing @e2e". The
failure is real and is caused by this branch: the gate is diff-scoped, and this
PR ADDS a spec with four scenarios, each of which must either be referenced by
a Playwright test or carry an `@e2e exclude <reason>`. Coverage on that run was
32 of 32 applicable gates, so this was a measured failure, not an unrun gate.
Every scenario here describes a repair step that runs at UPGRADE time — which
shard tables it selects, which schemas it refuses, how it behaves when a
destination column already exists. None of that has a browser surface. A
Playwright test could only re-assert the unit test through a slower harness, or
would require shipping a deliberately broken schema to a live instance to
reproduce the collision case.
THE REASONS NAME A TEST ARTIFACT, NOT A STATE OF THE WORLD. Each exclusion
cites the specific PHPUnit method that covers the scenario. A reason of the
form "not applicable to the UI" rots silently the moment the UI grows one;
a reason of the form "covered by ::testRefusesAmbiguousRename" stays checkable,
and breaks loudly if that test is ever deleted or renamed.
All seven cited methods were verified to exist in
tests/Unit/Repair/RenameDutchCatalogColumnsTest.php before committing —
4 scenarios, 4 exclusions, 7 distinct methods cited, 0 missing.
* chore: adopt nextcloud/coding-standard, .editorconfig and NC 34 Configuration only. The reformat is the next commit on purpose, so .git-blame-ignore-revs can name a revision containing nothing but whitespace. - .php-cs-fixer.dist.php + conduction/coding-standard, which extends nextcloud/coding-standard and can only ADD to it — enforced by that package's invariant test, not by review. - cs:check / cs:fix now run php-cs-fixer. They were aliases for phpcs/phpcbf, so the documented Nextcloud command reformatted code AWAY from Nextcloud's standard. - nextcloud/coding-standard dropped as a direct dependency. It arrives transitively at a version conduction/coding-standard has tested against; declared directly it was a dead dependency with no config and no invocation. - phpcs.xml is now a stub over the shared semantics-only ruleset, and the local phpcs-custom-sniffs/ copy is gone. The fleet was carrying six divergent versions of NamedParametersSniff.php — a custom RULE, not a setting. - .editorconfig, verbatim from nextcloud/server. No fleet app had one, so an editor configured by someone's previous Nextcloud work defaulted to tabs, which the old ruleset then rejected. - nextcloud/ocp -> ^34.0 and PHPUnit -> stable34. This app declared support for NC 34 while being analysed against 31, so a symbol REMOVED in 32/33/34 was invisible to the type checker. That is why the NC 34 removal of \OC::$server needed a hand-written PHPCS sniff. - the stylelint glob is quoted, so stylelint expands it rather than the shell. Unquoted, src/**/ matches exactly one directory level and nested components are silently unlinted. gate-65 (coding-standard-adoption) enforces all of the above from ConductionNL/.github@main. This app failed it; with this commit it passes. * style: reformat with nextcloud/coding-standard — whitespace only Applied by php-cs-fixer with conduction/coding-standard. Tabs, same-line braces, (int)$x, single-space concatenation, ordered imports — Nextcloud's dialect, which this app now passes unchanged. 210 file(s), no behaviour change. Isolated from the configuration change so .git-blame-ignore-revs can name a revision that touches nothing but formatting. Reviewing it line by line is not a useful activity; the previous commit is the review. * chore: ignore the reformat commit in git blame a78e00a touches 210 files and changes no behaviour. Without this, every line it reflowed attributes to it and the real author is one --skip away. GitHub honours the file automatically; locally it needs `git config blame.ignoreRevsFile .git-blame-ignore-revs` once. * fix: regenerate composer.lock for the new constraints The previous commit changed composer.json without touching the lock, so `composer install` refused with exit 4 and EVERY PHP job failed: Required (in require-dev) package "conduction/coding-standard" is not present in the lock file. Required (in require-dev) package "conduction/hydra-gates" is not present in the lock file. Required (in require-dev) package "nextcloud/ocp" is in the lock file as "v31.0.9" but that does not satisfy your constraint "^34.0". Nothing was wrong with the reformat or the ruleset — the jobs never got as far as running a tool. Measured on larpingapp#313 before this fix: phpcs, psalm, phpstan and both PHPUnit legs red, all of them at `composer install`. Hydra Gates passed in the same run, because it does not install composer dependencies. Now locked at conduction/coding-standard v1.0.0, conduction/hydra-gates v1.7.0, nextcloud/ocp v34.0.2 — the last of which is the point of the exercise: this app declares support for NC 34 and is now analysed against it. * fix(appinfo): order info.xml elements per the App Store xs:sequence The App Store's info.xsd declares <info> and its children as xs:sequence, so element ORDER is significant. This file was rejected by `xmllint --noout --schema info.xsd appinfo/info.xml`. Nextcloud's lint-info-xml workflow validates against exactly that schema, and ConductionNL/.github#383 adds the same check to the shared pipeline. Elements were moved into the schema's order. Nothing was added, removed or reworded; <version> and the <nextcloud> min/max-version declaration are unchanged. Verified: `xmllint --noout --schema info.xsd appinfo/info.xml` reports "validates" (libxml2 2.12.10). The pre-change file failed the same command. * fix(static-analysis): repair what the nextcloud/ocp 31 -> 34 bump and the elseif normalisation surfaced PHPStan (5 errors), all from the OCP 31 -> 34 stub change: - IQueryBuilder::execute() is gone from the OCP 34 interface. The four call sites in OrganizationSyncService are all SELECTs, so they become executeQuery(); no behaviour change. - TemplateResponse's 4th constructor argument is $renderAs (a string enum), not the HTTP status; the 5th is int $status. The error branch in DashboardController passed '500' as $renderAs, so it rendered with an invalid layout and still returned HTTP 200. It now passes RENDER_AS_ERROR plus STATUS_INTERNAL_SERVER_ERROR. Psalm (2 ParadoxicalCondition errors): extractPropertyDefinitionMap in ArchiMateService and ArchiMateImportService each end with an elseif that repeats the opening if verbatim, so the third branch is unreachable. The duplicate is pre-existing (origin/development ArchiMateService.php:2437); what changed is that php-cs-fixer rewrote 'else if' to 'elseif', and Psalm reports the elseif form as ParadoxicalCondition but the 'else if' form as NoValue -- and psalm.xml suppresses NoValue. Verified with a two-file control. Removing the unreachable branch is behaviour-identical. * ci: re-trigger Code Quality The previous run produced zero jobs and concluded failure: it started inside the window where ConductionNL/.github@main carried the broken quality.yml splice from b745bf2f, repaired at 4118bca8. Nothing in this PR touches the workflow.
#494) appinfo/info.xml declares <nextcloud min-version="32" max-version="34"/>, but nextcloud-test-refs was '["stable34"]' — so the declared floor and the middle major were advertised to the App Store with no job touching either. This is the coding-standard migration's own defect: its rollout REPLACED the ref list instead of extending it. The programme opened by reporting that nothing was tested on NC 34 and, in fixing that, made 32 and 33 the untested end. Same drift, other direction. stable34 stays first because newman, playwright and journeydoc-capture all read fromJSON(inputs.nextcloud-test-refs)[0] as their single server. Verified green on all three refs against nextcloud/ocp ^34 on portaliq (run 31599055849, six PHPUnit legs: 32/33/34 x PHP 8.3/8.4).
Nextcloud itself uses no prettier — nextcloud/server and nextcloud/text have no prettier dependency, no format script and no prettier config; they ship .editorconfig and enforce JS/Vue formatting through @nextcloud/eslint-config. In this repo the file never ran: no prettier dependency, no format script, no workflow reference. It only took effect in editors, where its 2-space indent and double quotes are exactly what @nextcloud/eslint-config then flags.
Nextcloud runs migrateSchemaOnly() on a first install: $previousVersion is '', so Installer::installAppLastSteps() skips BOTH pre-migration and post-migration, and <install> is the only unconditional hook. The upgrade path runs pre/post-migration and NOT install, so an app needs both blocks carrying the same baseline steps, each idempotent. Until now this app declared no <install> block at all, so the SoftwareCatalog register never arrived on a fresh instance. Only baseline-CREATING steps are added; migrations, backfills, renames and cross-app ingests stay upgrade-only so they never run against an empty database. <install> is placed after </post-migration> per the info.xsd sequence (pre-migration, post-migration, live-migration, install, uninstall), verified against the schema.
phpmd.xml becomes a 9-line stub referencing vendor/conduction/hydra-gates/quality-config/phpmd.xml, and the local phpmd-unusedparams.xml is deleted in favour of the central copy, which the unused-parameters leg of the composer phpmd script now points at. Both legs, their flags and the worst-exit-code behaviour are unchanged. Co-authored-by: Ruben van der Linde <release-bot@conduction.nl>
…0260821035333 chore(release): 0.1.141-unstable.20260821035333
…0260821040505 chore(release): 0.1.141-unstable.20260821040505
…0260821041656 chore(release): 0.1.141-unstable.20260821041656
…0260821042832 chore(release): 0.1.141-unstable.20260821042832
…0260821044543 chore(release): 0.1.141-unstable.20260821044543
…0260821050743 chore(release): 0.1.141-unstable.20260821050743
…0260821052417 chore(release): 0.1.141-unstable.20260821052417
…0260821053703 chore(release): 0.1.141-unstable.20260821053703
hydra-gates v1.8.1 -> v1.8.2 nc-vue 2.8.2 -> 2.9.2 Lock-only: both packages are already declared with caret ranges that permit these versions, so nothing about what this app ACCEPTS changes - only what it currently resolves to. Opened by the weekly fleet shared-dependency bump, because a lock nobody re-resolves is a pin nobody chose. Merging is gated by this repository's own suite, deliberately: taking hydra-gates v1.8.1 added patchObject() to a published interface, which is a load-time fatal for any concrete double that implements it without the method. CI is the only thing that can tell a safe bump from that. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
hydra-gates v1.8.2 -> v1.8.2 nc-vue 2.9.2 -> 2.10.1 Lock-only: both packages are already declared with caret ranges that permit these versions, so nothing about what this app ACCEPTS changes - only what it currently resolves to. Opened by the weekly fleet shared-dependency bump, because a lock nobody re-resolves is a pin nobody chose. Merging is gated by this repository's own suite, deliberately: taking hydra-gates v1.8.1 added patchObject() to a published interface, which is a load-time fatal for any concrete double that implements it without the method. CI is the only thing that can tell a safe bump from that. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
… unit errors (#696) Prepares this app for ConductionNL/.github#531, which drops `OCA\OpenRegister\Contract\` from conduction/hydra-gates' RUNTIME psr-4 autoload. That prefix is LONGER than openregister's own `OCA\OpenRegister\` -> `lib/`, and PSR-4 is longest-prefix-wins, so whichever app's autoloader registers first defines OpenRegister's contract for the whole process. IT ALSO FIXES A LIVE PROBLEM HERE, WHICH THE OTHER APPS IN THIS SWEEP DID NOT HAVE. tests/bootstrap-unit.php registers its OpenRegister stubs through a manual spl_autoload_register prefix map covering `OCA\OpenRegister\Db\` and `...\Service\`. Neither covers `...\Contract\`, so the standalone unit suite was already failing on it: before Tests: 715, Assertions: 2573, Errors: 131, Failures: 1, Skipped: 25 after Tests: 715, Assertions: 2993, Errors: 1, Failures: 0, Skipped: 24 The single remaining error is unrelated — Symfony\Component\HttpFoundation\ HeaderUtils is absent from the standalone environment, reached via OCP's DownloadResponse. Added to BOTH bootstraps deliberately: phpunit.xml loads tests/bootstrap.php and phpunit-unit.xml loads tests/bootstrap-unit.php, and both reach code that needs the contract. The full phpunit.xml path cannot be measured outside a Nextcloud tree (its bootstrap fatals on `Class "OC_App" not found`), so that half is verified by CI rather than locally, and is a no-op there while the prefix still exists. interface_exists() is order-independent: it asks whether the interface is RESOLVABLE rather than who registered first. Appending a fallback autoloader does not work, because spl_autoload_register appends relative to registration order and that order across independently loaded apps is what nobody controls. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…ssing-manager bug (#697) * chore(quality): migrate to PHPStan 2 — 35 findings to zero, plus a real bug Bumps `phpstan/phpstan` to ^2.0 and `conduction/hydra-gates` to ^1.8.2, and clears every finding the new major surfaces. ## A missing manager was never detected ContactPersonHandler::setUserManager(): $user = $this->_userManager->get($username); $manager = $this->_userManager->get($managerUsername); if ($user === null || $manager === false) { ...warn and return... } IUserManager::get() returns `?IUser` — it signals "no such user" with NULL and never returns false. So the second half of that guard could never fire: calling setUserManager() with a manager username that does not exist skipped the warning entirely and carried on as if the manager were real. Fixed to `=== null`. PHPStan found it from the other end: inside that branch `$user !== null` was reported as always FALSE, because the only reachable way in was the first clause. ## Dead guards (28) Mostly in the ArchiMate import/export pair, which are near-copies of each other, so nearly every finding came in twos: - `self::PERFORMANCE_OPTIMIZATIONS['memory_cleanup'] !== false` and `['parallel_processing'] === true` — both are class constants set to true, so neither was ever conditional. - `$identifier !== false`, `$versionId !== false`, `$refCompId !== false`, `$amefKey !== false` — all strings; none can be false. - `is_array($sectionData) === false` — the parameter is declared `array`, so PHP rejects anything else at the call boundary first. - `isset($statistics[$sectionKey]) === false` — the branch above pins $sectionKey to a key $statistics always has. - `if ($section !== 'omschrijving')` with the comment "Skip summary section itself" — `omschrijving` is assigned to $statistics on the line AFTER the loop, so the loop can never see it. - `method_exists($this->archiMateService, '...Optimized')` — the method is declared on the class. - Several `isset() && !== null` pairs and non-nullable-entity null tests. ## One scoped ignore OrganizationSyncService's `if ($contactObject !== null)` is provably true — the code a few lines above already dereferences $contactObject unconditionally. It is left in place because the block it wraps is 243 lines: removing the `if` is a pure re-indentation of a quarter of the method, a large review-hostile diff for zero behaviour change. The comment says so and marks it for the next real edit to that method. ## Verification phpstan 0, phpcs clean, phpmd clean. PHPUnit is NOT part of this evidence: the bootstrap requires a booted Nextcloud (`Class "OC_App" not found`) and cannot run standalone. Checked that this is pre-existing by stashing every change in this commit and re-running — byte-identical failure. CI runs the suite inside the container. * style(quality): satisfy phpcs on the comments this branch added CI's phpcs step runs `--warning-severity=0`, and it failed on every one of the three PRat in this series for the same reason: comments I wrote. Two sniffs: - Squiz.Commenting.InlineComment.NotCapital — many of my new comments open with a lowercase function name ("// find() throws rather than ..."). Rephrased so the first word is a real capitalised word. - Generic.Commenting.DocComment.TagsNotGrouped — the `@param-out` tags I added were interleaved between `@param` tags, splitting the group. Moved below the last `@param`. One of those inserts had also orphaned a continuation line off the `@param` above it; that is rejoined. Where PHPStan genuinely needs a `/** @var */` inline doc-block (which Squiz.Commenting.InlineComment.DocBlock rejects), the line now carries a targeted `phpcs:ignore` naming that sniff and saying why, rather than dropping the annotation and leaving the type wrong. I should have caught this locally. I did run phpcs, but with `--report=summary | tail -3`, which prints only the timing line — so I read an empty tail as "clean" when the error count was two lines above the cut. Re-verified here with CI's exact invocation. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
* chore(deps): refresh the shared Conduction locks hydra-gates v1.8.2 -> v1.9.0 nc-vue 2.10.1 -> 2.11.1 Lock-only: both packages are already declared with caret ranges that permit these versions, so nothing about what this app ACCEPTS changes - only what it currently resolves to. Opened by the weekly fleet shared-dependency bump, because a lock nobody re-resolves is a pin nobody chose. Merging is gated by this repository's own suite, deliberately: taking hydra-gates v1.8.1 added patchObject() to a published interface, which is a load-time fatal for any concrete double that implements it without the method. CI is the only thing that can tell a safe bump from that. * fix(psalm): stub OpenRegister's contract, which v1.9.0 stopped autoloading hydra-gates v1.9.0 removed `OCA\OpenRegister\Contract\` from its runtime psr-4 autoload (ConductionNL/.github#531). The removal was correct — that prefix is longer than openregister's own, so a vendored copy in ANY app defined the contract for the whole process — but it was verified against PHPUnit only. Psalm never runs the test bootstrap; it resolves types through the composer autoload map. So the guarded require added to this app's bootstrap does nothing for it, and 213 UndefinedClass errors appeared for a class the app typehints but does not own. This is the same situation as the decidesk event stubs already in this file: a sibling Nextcloud app supplies the type at runtime, so it is absent from the analysis path and gets declared here. A stub teaches the analyser the shape without putting the class back into the runtime autoloader, which is what caused the original defect. Measured in this checkout on the real v1.9.0: 213 errors before, 0 after — "No errors found!", psalm exit 0. * fix(metadata): point shipped URLs at GitHub, not the retired Codeberg host gate-94 (retired-git-host-metadata, ConductionNL/.github#546) flags the shipped URLs in this app's metadata — website, bugs, repository and screenshots — as pointing at codeberg.org. GitHub is the only host. Two things changed together, and only one of them is the host: codeberg.org/Conduction/softwarecatalog -> github.com/ConductionNL/stackiq The Codeberg URLs still carried the app's PRE-RENAME name. Swapping only the host would have produced github.com/ConductionNL/softwarecatalog, which resolves today purely because GitHub redirects a renamed repo — and stops the moment anyone creates a repo at the old path. Screenshots move to raw.githubusercontent.com rather than a github.com/raw redirect, and every one was fetched rather than assumed: HTTP 200 each. A green gate with dead image URLs would be worse than the finding it silenced. Not caused by this branch's lock bump — gate-94 landed at 02:18 UTC, after this app's last development run, so the PR runs are simply the first measured against it. Pre-existing debt, newly visible. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
* wip(rename): app id softwarecatalog -> stackiq — appinfo, composer, package, build + CI config
* refactor(rename): git mv app-named files and dirs to the stackiq spelling
* feat(rename): repair steps carrying appconfig, user preferences and job classes across the app-id rename
Nextcloud namespaces oc_appconfig and oc_preferences by app id and has no
in-place app-id upgrade, so every stored row becomes unreachable when the id
moves. Every reader supplies a default, so nothing errors -- settings just
revert. oc_jobs has the same shape one level down: it stores the job CLASS
NAME, so the namespace rename orphans all four job rows.
Three IRepairSteps, registered FIRST in both <install> and <post-migration>.
<install> matters because an app-id rename presents to Nextcloud as a FIRST
install of stackiq, and installAppLastSteps() guards <post-migration> with
$previousVersion !== '' -- so <install> is the only hook that fires on the
very upgrade these steps exist for. FIRST matters because InitializeSettings
writes config itself; running it first would make every key look already
present and strand the operator's value in the old namespace.
Exhaustive enumeration (getKeys / callForSeenUsers + getUserKeys, never
getUsersForUserValue which matches on a VALUE and so migrates nothing over an
open value set). Reserved keys enabled/installed_version/types are skipped --
copying 'enabled' as STRING makes the next app:enable fail permanently with
AppConfigTypeConflictException. Idempotent, non-destructive, and every read
AND write inside the try so a throw cannot abort the install.
28 unit tests covering all three.
* docs(rename): README, SECURITY, openapi title, eslint suppressions and dev scripts
Also repoints this app's own badges and clone URL from the retired
codeberg.org host to github.com/ConductionNL/stackiq, and replaces the
softwarecatalog.app doc link (DNS: no record) with the live
softwarecatalog.conduction.nl. The .conduction.nl host itself stays --
stackiq.conduction.nl does not resolve.
* wip(stackiq): checkpoint in-flight rename work before session limit
* feat: rename the app id from softwarecatalog to stackiq
Completes the sweep: PHP namespace OCA\SoftwareCatalog -> OCA\Stackiq (the
sub-namespace Service\SoftwareCatalogue -> Service\Stackiq, and the class
SoftwareCatalogueService -> StackiqService, which had been left disagreeing with
its already-renamed file), /apps/softwarecatalog -> /apps/stackiq routes across
code, tests, postman collections, docs and cursor rules.
Postman: the path ARRAY was rewritten alongside the raw URL. Postman builds the
request from the array, so rewriting only raw would leave the collection hitting
a different URL than the diff shows.
FROZEN, each because renaming would fail silently rather than loudly:
- softwarecatalog.conduction.nl (HTTP 200; stackiq.conduction.nl is 000) --
the manifest documentationUrl and footer href would point at nothing.
- softwarecatalogus -- the Dutch VNG catalogue, an external system's name,
not this app's id. Includes the GEMMA fixture XMLs and the register file.
- x-openregister.app in the register descriptor: OpenRegister attributes the
register by this field and may match on it; renaming risks a duplicate or
an orphan. Reported rather than guessed.
- openspec/changes/archive/** -- history; rewriting breaks @SPEC paths.
- openspec/coverage-report.* -- a dated audit snapshot. Rewriting its
observations to say something it never observed is falsification.
- LEGACY_APP_ID in the two Migrate* repair steps, and the old namespace in
MigrateBackgroundJobClasses -- these name what the steps read FROM.
- codeberg.org URLs -- a different host and org, stale independently.
Register slugs are voorzieningen and vng-gemma; neither contains the app id, so
neither moved.
Local: phpcs 0 errors, psalm clean, eslint 0 errors, 120/120 jest.
* fix(register): point x-openregister.app at the new app id
The register descriptor attributes the register to an owning app through
x-openregister.app. It was left on the old app id when the id moved, so the
descriptor claimed ownership by an app that no longer answers to that name.
Safe to move now: these instances are development-only, so there is no live
register whose attribution could be split.
The register SLUG is deliberately NOT touched here — that is the key objects
are stored against, and it is a separate decision from attribution. Other
apps' ids appearing in the same file (e.g. opencatalogi) are cross-app
references and stay as they are.
* fix: repair the collateral my bulk rename caused, and the gates it surfaced
The scripted sweep was fast but blunt. Five distinct failures, each a case of
a name moving without the thing that answers to it.
1. TEST CONSTANTS INVERTED THE MIGRATION. The sweep rewrote the tests'
LEGACY constant from 'softwarecatalog' to 'stackiq', so they asserted that
the migration reads from the namespace it writes TO -- a migration that
does nothing, with a green bar. Both now bind to the step's own
LEGACY_APP_ID constant, so they cannot drift again.
2. 50 DANGLING @SPEC REFERENCES. The sweep rewrote citation PATHS to
openspec/specs/stackiq-* while the directories are still
softwarecatalog-*. Capability ids are frozen -- gate-46 dereferences them
and archived changes cannot move -- so the citations are reverted, not the
directories. gate-46 now resolves every anchor.
3. l10n KEYS ARE THE ENGLISH SOURCE STRING, so renaming a user-visible string
renames its key. Two keys drifted out of en.json. Repointed, with the Dutch
VALUES carried across (Softwarecatalogus -> Stackiq only where it is the
product name, never where it means the VNG catalogue).
The .js artifacts were stale too: the l10n check reads JSON, but Nextcloud
SERVES the .js, so nl.js/en_US.js would have kept the old keys and Dutch
users would have silently fallen back to English past a green check.
4. phpstan: callForSeenUsers() expects Closure(IUser): (bool|null); a void
closure does not satisfy it. The IJobList::has()/remove() class-string
errors are unsatisfiable BY DESIGN -- this step exists to deregister jobs
whose class no longer exists -- so they carry a scoped ignore with that
reason, not a baseline entry.
5. gate-16 and prettier: four changed methods tagged with the requirement
they implement; 50 files reflowed because 'stackiq' is shorter than
'softwarecatalog' (measured 50 against 2 on development).
Local: phpcs 0 errors, phpstan clean, phpmd 0, eslint 0, 120/120 jest,
l10n OK, prettier clean, gate-16 0, gate-46 0.
* fix: delete two orphaned duplicates the rename left, and the new else
Both CI failures had the same root cause, and it is a subtle one.
eslint-suppressions.json and phpmd.baseline.xml are keyed BY FILE PATH. The
sweep repointed those keys to the new filenames, but the rename of two source
files was a COPY, not a move -- so the old files stayed on disk, now with no
suppression entry pointing at them, and their long-standing findings resurfaced
as new errors. Neither file was reachable any more:
- src/store/plugins/softwarecatalogPlugin.js -- superseded by stackiqPlugin.js,
which orClient.js and store/modules/object.js already import.
- src/views/settings/SoftwareCatalogSettings.vue -- superseded by
StackiqSettings.vue, which registry.js already imports. The registry KEY
stays SoftwareCatalogSettingsPage: that is a manifest identifier, not a
filename.
Deleting them is the fix; repointing the suppressions back would have kept dead
code alive and silenced.
Also removed a genuine new else in MigrateAppConfigKeys::run() (guard plus
continue), rather than adding it to phpmd.baseline.xml -- a baseline records
debt already inherited, and same-day code does not belong in it.
Local: phpcs 0, phpmd 0, phpstan OK, eslint 0 errors, 120/120 jest, l10n OK,
prettier 0, gate-16 0.
* fix: sweep the files my case-sensitive grep never listed
The earlier bulk pass built its file list with a case-SENSITIVE grep for
'softwarecatalog'. Every file whose only occurrences were 'SoftwareCatalog' or
'SoftwareCatalogue' -- the PHP namespace and class-name spellings -- was
therefore never in the list, and never rewritten. CI found them as 32 errors:
Class "OCA\SoftwareCatalog\EventListener\SoftwareCatalogEventListener" not
found, because composer's psr-4 now maps only OCA\Stackiq\.
Renamed the missed files and directories with git mv, and removed four stale
DUPLICATES the earlier rename had left behind by copying rather than moving --
tests/Unit/EventListener/SoftwareCatalogEventListener{,Decomposition}Test.php
and tests/Unit/Service/SoftwareCatalogueService{Decomposition,OrganisationMapper}Test.php.
Each had a live counterpart under the new name; the old copies still declared
the old namespace, so they could only ever fail.
lib/Settings/softwarecatalogus_register.json stays: softwarecatalogus is the
Dutch VNG catalogue, not this app, and the file is loaded by an explicit path.
Local: phpcs 0, phpstan OK, phpmd 0, eslint 0, 120/120 jest, l10n OK,
prettier 0, gate-16 0, gate-46 0.
* fix(tests): remove the duplicated nested test directory
An earlier flatten of tests/Unit/Service/SoftwareCatalogue left BOTH
tests/Unit/Service/Stackiq/ and tests/Unit/Service/Stackiq/Stackiq/ tracked,
with byte-identical files. PHP fataled on the second copy:
Cannot declare class OCA\Stackiq\Tests\Unit\Service\Stackiq\
ContactPersonHandlerDecompositionTest, because the name is already in use
which aborted the whole PHPUnit run (exit 255) rather than failing a test.
Local: phpcs 0, phpstan OK, gate-16 0.
* chore(ci): sync the canonical coverage-guard, which knows about deletions
The coverage ratchet failed with 'coverage of the files this change touches
dropped by 3.02%'. The cause is the deletions in the previous commits: four
stale duplicate test files and two orphaned source files, all of which had live
counterparts under the new name.
The guard's own CI notice named the fix:
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
--deletion-neutral compares method buckets asymmetrically, so a pure deletion
is exactly neutral while a regression in surviving code and new untested code
both still fail. Without it this PR could not comply from inside its own
subject: the only moves would be delete less, add filler, or delete additional
UNCOVERED statements until the arithmetic balanced.
Copied verbatim from .github@main (837 lines); --capabilities now reports
deletion-neutral alongside against/update-baseline/changed-files.
* fix(e2e): assert the new display name in the settings smoke test
'admin settings: the settings section renders' asserted the host contained the
text 'SoftwareCatalog'. info.xml now declares <name>Stackiq</name>, so the UI
renders the new name and the element was genuinely not found.
Missed for the same reason as the namespace files: the sweep's file list was
built with a case-SENSITIVE grep for 'softwarecatalog', and this literal is
CamelCase inside a string. The later CamelCase pass only listed files matching
the namespace patterns, so a bare 'SoftwareCatalog' in a test string fell
between the two.
MigrateBackgroundJobClassesTest still asserts 'SoftwareCatalog' -- correct, it
pins the old namespace the repair step reads FROM.
* test: cover the two branches the phpmd fix introduced
The merge-base coverage check dropped 0.05% -- 6 statements, same denominator.
Not the deletions (the four removed duplicate tests had identical test counts to
their live counterparts, so they added no unique coverage) but the else-removal
in MigrateAppConfigKeys::run(): replacing if/else with a guard plus continue
adds a 'continue' statement, and every existing fixture in that file migrates a
SINGLE key, so the loop never iterated past the first one.
Two tests added:
- testCopiesEveryCopyableKeyNotJustTheFirst -- two copyable keys. This is
worth having beyond the ratchet: a migration that silently stopped after
the first key would have passed every one-key test in the file.
- testAnUncopyableValueIsSkippedRatherThanWritten -- an empty string and an
empty array, which copyValue() refuses, reaching the trailing $skipped++.
Cannot run PHPUnit locally (the bootstrap needs OC_App, absent from a bare
clone), so CI is the check for these. gate-16 0, gate-46 0, phpcs 0.
---------
Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…odeberg The `<new>.conduction.nl` docs hosts did not exist until 2026-08-23, which is why every documentationUrl, CNAME and docusaurus `url:` in this repo was deliberately frozen on the old host. They exist now: each new hostname was attached as a SECOND custom domain on the SAME `<old>-docs` worker, so both hosts answer and nothing goes dark in either direction. `docs-hosts` lists BOTH hosts on purpose. wrangler reconciles a worker's triggers against the config it is handed, so a hostname omitted there is REMOVED from the worker — which would take that site down. (Observed the same day: a deploy without `workers_dev` in the file silently disabled the workers.dev URL, 404 where it had been 200, while reporting success.) Codeberg links in docs/ now point at GitHub. GitHub is the only host, including for issues, and the navbar on the published site was offering Codeberg. The docs will only actually change once ConductionNL/.github#555 lands: the reusable documentation workflow writes gh-pages and never touches the worker that serves the host, so every fleet docs site has been serving a May/June build while reporting success on each run.
* test(l10n): ratchet the untranslated schema strings Every string inside a form comes from the OpenRegister schema, not from the manifest: `fieldsFromSchema()` runs a property `title` and `description` through the injected `cnTranslate`, which CnAppRoot binds to THIS app's id. So a schema title is a key in THIS catalogue — and when the key is absent, `t()` hands the source string back and the field renders in English inside an otherwise translated form. Nothing errors, and no existing check looks. Measured across the fleet on 2026-08-23: 30,459 schema strings had no catalogue key. Far too much to translate in one pass, and the descriptions need rewriting for the person filling in the form before translating them is even worth doing — humaniq's own pass rewrote 592 of 739 before a word was translated. So this is a RATCHET, not a gate: it records how many strings are currently uncovered and fails only when that number GROWS. The debt is measured and cannot expand, while burning it down stays an ordinary PR. Same shape as the JSDoc baseline in @conduction/nextcloud-vue. Counted: schema titles, property titles, property descriptions, and the VALUES of `x-enum-labels`. NOT counted: enum values themselves (stored contract values, several non-English by design, never rendered once a property declares its labels) and `x-notes` (engineering rationale, never rendered). Verified must-fail: adding one untranslated title takes the count past the baseline and exits 1, naming the file and property and the command that lists what is uncovered. Lower the baseline as strings get translated: npm run check:schema-l10n -- --update * fix(l10n): the baseline file is not a locale catalogue; format for this repo Two things the fleet CI caught. `build-l10n-js.js` discovers locales by globbing `l10n/*.json`, which now also matches `l10n/.schema-l10n-baseline.json` — the ratchet's own state file, kept there so prettier ignores it. The generator read it as a locale named `.schema-l10n-baseline` and exited 1 for having no `translations`. Dotfiles are never locale catalogues, so it skips them. Also prettier-normalised both scripts to this repo's config; several apps run a format check over scripts/.
The landing page shipped a button sending visitors to codeberg.org. GitHub is the only host we publish to, so the link opened a repository we no longer read. Now points at https://github.com/ConductionNL/stackiq.
feat(docs): move the docs host softwarecatalog.conduction.nl -> stackiq.conduction.nl
) The Documentation build fails on 22 broken links, which blocks the development -> documentation promotion and therefore any docs deploy. Nine pages linked specs as `../../openspec/specs/<name>/spec.md`. Those files exist in the repo but sit outside the Docusaurus docs tree, so Docusaurus cannot resolve them and treats each as a build error. The count is 22 rather than 11 because every page also has an /nl/ locale build. All eleven links now point at the file on GitHub, which is the only host we publish to. Two were more than a path swap: - portfolio-rationalization-time pointed into `openspec/changes/...`, and that change was archived on 2026-07-23, so the target no longer existed at all. Archiving a change breaks every reference into it. Repointed at the promoted `openspec/specs/portfolio-rationalization-time/spec.md`. - The three REQ-007/008/009 links carried Docusaurus heading anchors. GitHub slugifies headings by its own rules, so re-using those anchors would be inventing a target. The REQ id stays in the link text and the link goes to the spec file. Verified every target exists before writing the URL.
hydra-gates v1.9.0 -> v1.9.0 nc-vue 2.11.1 -> 2.15.0 Lock-only: both packages are already declared with caret ranges that permit these versions, so nothing about what this app ACCEPTS changes - only what it currently resolves to. Opened by the weekly fleet shared-dependency bump, because a lock nobody re-resolves is a pin nobody chose. Merging is gated by this repository's own suite, deliberately: taking hydra-gates v1.8.1 added patchObject() to a published interface, which is a load-time fatal for any concrete double that implements it without the method. CI is the only thing that can tell a safe bump from that. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.
Routine
development->documentationpromotion, the same flow this repo already uses for its docs deploys.Why now
The docs site deploys from the
documentationbranch. It is 883 commits behinddevelopment, so recent docs changes are not reaching the published site.The immediate trigger: the landing-page CTA fix (repository button pointed at
codeberg.org; GitHub is the only host we publish to) merged todevelopmentand is not live. Verified against the running site before opening this.What this ships
Everything on
developmentthat has not yet been promoted, which is more than the CTA fix. Treat it as a docs release, not a single change. Any commits made directly ondocumentationare preserved by the merge.