Release: merge beta into main - #837
Merged
Merged
Conversation
* 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>
phpstan.neon now includes the shared base shipped in conduction/hydra-gates (quality-config/phpstan-base.neon) and keeps only what is genuinely local to this app. Requires hydra-gates v1.7.1 — v1.7.0's base declared bare relative paths, which PHPStan resolves against the file that declares them, so the run aborted before analysing anything. composer.lock is updated accordingly; no other package moved. Verified with phpstan dump-parameters before and after: level, paths, excludePaths, bootstrapFiles and scanDirectories resolve byte-identically, the same number of files is analysed on both sides, and the finding count is unchanged.
Moves the pin from 2.2.0-vue3.9 to the current vue3 dist-tag. The lockfile was regenerated with npm 10.8.2 to match the npm version CI runs (engines: npm ^10.0.0); npm ci was verified from a clean node_modules. Verified locally: npm ci, build (including the check:vue-demi prebuild guard), 9 test suites / 120 tests, eslint (0 errors), stylelint — all pass. Co-authored-by: Ruben van der Linde <juan.claude@conduction.nl>
…501) Lock-only. composer.json is untouched: the ^1.0 constraint is correct and stays floating. v1.7.3 removes two conditional paths from the shared phpstan-base.neon (%cwd%/vendor-bin and %cwd%/lib/Resources/template). A conditional path in a shared base has no spelling that is safe on both PHPStan majors: plain is validated and ABORTS on PHPStan 2.x, the '(?)' marker is parsed as a NEON entity after a %...% expansion and crashes 2.x, and quoting it stops 1.x from stripping the marker so the exclusion silently matches nothing. This app is on PHPStan 1.12.x, so it is not broken today, but it carries the landmine until it moves to PHPStan 2. It has neither vendor-bin nor lib/Resources/template, so no phpstan.neon change is needed.
#500) * fix(repair): guard the Dutch column rename on what the schema declares RenameDutchCatalogColumns is registered, live on development and unguarded. It renames a shard column precisely when the English column is ABSENT — which is today's state for every in-scope schema — so on the next version bump it would move the data out of `naam` while the register still declares `naam`. MagicMapper re-adds an empty `naam` on the following sync and every read returns null. That is the step's own header warning with the two halves swapped. The precondition it needed was stated only in prose, in appinfo/info.xml: "Must run AFTER the register sync that adds the English columns". Nothing performs that sync. No file under lib/ reads the register JSON at all — the only references are tests, root debug scripts and comments — and InitializeSettings, ordered immediately before this step, writes config keys and imports nothing. The register is imported by a human through OpenRegister's configuration UI, on their own schedule. The old test could not tell the two states apart: "renamed, mapper behind" and "not renamed at all" are byte-identical in a column list. The schema's DECLARED properties are what separates them, so that is what is consulted now. renameIsSafe() requires both halves — the destination declared AND the source no longer declared — which makes the step correct in either merge order and a genuine no-op until the register moves. A schema whose declared properties cannot be read is skipped rather than migrated on an assumption. Declared properties are read from oc_openregister_schemas.properties, verified first-hand against a live instance rather than inferred: a json column, object-typed on all 21 softwarecatalog schemas, keyed by the camelCase property name. sanitizeColumnName() mirrors MagicMapper's transform step for step, because the comparison is only meaningful if both sides spell the name the same way. Tests pin the guard's full truth table, the column-name transform, and — the regression test for this defect — that against the register this repo actually ships, every rename defers. That last one reads the shipped file rather than a fixture, so it pins what the register says rather than what I believed it said, and it carries two positive controls so an empty loop cannot pass for free. Proven able to fail: inverting the guard reddens exactly those three tests and no others. Refs #492. * fix(repair): bind the schema id as a string and split the table loop out Two real failures from the first push, both mine. phpstan: IDBConnection::executeQuery() declares array<string> for its parameter list, so passing [$schemaId] as an int array is a type error. Bound as a string; the driver casts back for the numeric comparison. phpmd: the two new branches took run() to a cyclomatic complexity of 11. Extracted the per-column work into migrateTable(), which returns counters rather than mutating shared state, so run() stays a readable table loop and the two levels cannot disagree about what was done. Suppressing the warning was the alternative and would have hidden a method that had genuinely grown past the point of being read in one go.
#502) * fix(auth): four admin actions were reachable by any authenticated user Hand-read all thirteen no-admin-idor findings rather than treating the count as a work queue. One is a real IDOR, four are semantic-auth mismatches, two are genuinely guarded downstream, five take no object reference at all, and one is scoped through its receiver. The four semantic-auth ones are the serious half. SettingsController has a consistent convention — every mutator is admin-required and only readers carry the no-admin annotation — and these were the exceptions: - testEmailConnection: caller-supplied smtpHost/smtpPort reach a DSN and the server opens an outbound TCP connection to whatever was named. For a non-admin that is an SSRF and internal port-scan primitive. - updateEmailTemplate: the only write in the whole class carrying the annotation. It writes app configuration, and the stored HTML is rendered into real outbound mail, so any authenticated user could rewrite the templates every recipient receives — and mint unbounded email_template_* config rows besides. - syncOrganisations: triggers a register-wide write sync with a caller-chosen batch size. - exportArchiMate: exports the WHOLE register while its sibling exportOrgArchiMate exports one organisation — and the sibling has carried verifyOrgExportPermission all along. The broader endpoint was the unguarded one. The annotation is kept here deliberately: that helper grants organisation-admins too, which is the tier the admin UI relies on and which removing the annotation would drop. The three email-template reads move with their write. No frontend code calls any of those routes — the settings UI reads templates from the bulk settings payload — verified with a positive control on a route that IS called, so nothing breaks. getSbomImportStatus is the real IDOR: a caller-supplied moduleVersieUuid reached SbomImportService::getStatus(), whose lookup runs with rbac and multitenancy both off. Guarded with authorizeManage()'s read tier — the same two building blocks minus the editor-group requirement, since reading a status is not managing an import. Checked rather than assumed: the module schema carries a real read ACL, so this is a genuine scope and not the default-open case an authorization-less schema would give. Refuses 404, not 403, so it cannot become an existence oracle. Everything else carries a reason-bearing exemption naming the code path that makes it safe, not a state of the world. One finding is deliberately LEFT OPEN. getGebruikenForDeelnemer forces its organisation filter after getParams(), so a caller cannot forge it — but it passes an ARRAY where the app's canonical sibling passes a SCALAR, and the query runs with rbac and multitenancy off. Whether OpenRegister honours array-containment matching on a related-object array property is unverified, and if it silently ignores the array form that scope is vacuous. It could not be settled here: the available instance has zero gebruik rows, so a live A/B would have returned empty under both forms and proved nothing. It gets the fail-closed guard its canonical sibling already has, and keeps the finding. Exempting it would have manufactured the coverage. Nothing here makes anything return 200 that previously errored: every change is deny-only. exportArchiMate's dead $organization filter parameter is a real bug found on the way and is NOT repaired here — that is a feature repair sitting behind a missing guard, and it belongs after the guard lands. Refs #492. * fix(sbom): guard inside the try, not ahead of it My own regression, caught before it landed. authorizeRead() reaches SbomImportService::resolveParentModuleUuid(), and OpenRegister's real ObjectService::find() RE-THROWS DoesNotExistException for a well-formed but non-existent uuid rather than returning null — importSbom()'s docblock already records this and wraps its whole body for exactly that reason. Placing the guard ahead of the try would have converted this endpoint's clean 404 into a 500 for precisely the callers the guard was added for: a non-admin passing an unknown id. The guard now sits inside the try, so the not-found dialect is the same whichever call raises it. * test(auth): pin the auth posture of the six hardened endpoints The security-change-has-tests gate failed on the previous push, correctly: that change moved auth posture in lib/ and touched no test. This is the test, and it pins the near-miss rather than the diff. It parses the controller source with Nextcloud's OWN annotation regex, copied byte for byte from ControllerMethodReflector::reflect(). That is the point. While writing the fix, three of these methods documented their own hardening with the sentence "the endpoint must not declare @NoAdminRequired" — and that token, at the start of a comment line, MATCHES that regex. Nextcloud would have gone on treating the endpoint as non-admin-required: the sentence explaining the removal would have undone the removal, and the change would have read as a security fix while being a no-op. A test searching for the attribute form, or stripping comments first, would pass straight over it. Five tests. The first is a positive control asserting the parser CAN find the annotation where it legitimately remains (getSyncStatus) — without it, a typo in the regex or the docblock walk would make every absence assertion pass over an empty array and the file would be green while asserting nothing. exportArchiMate is pinned in BOTH directions, because the two pull opposite ways: it must KEEP the annotation, since verifyOrgExportPermission grants organisation-admins and removing it would drop that tier, AND it must call that helper. The right fix for its neighbours was the wrong fix for it. getSbomImportStatus is pinned for guard presence and for guard POSITION — inside the try, because authorizeRead() reaches ObjectService::find(), which re-throws for an unknown uuid, so guarding ahead of the try would turn a clean 404 into a 500 for exactly the callers the guard was added for. Proven able to fail, prediction written first: re-planting the prose form of the annotation reddens exactly testAdminActionsDoNotDeclareNoAdminRequired and nothing else. Reverted byte-identically. The positional assertion first failed on its own account, and the reason is worth keeping: it anchored on the bare name, which the controller's own explanatory comment uses ABOVE the try. A positional assertion over a corpus that includes prose measures the prose. It anchors on the call now.
…oo (#505) CSS and SCSS were enforced by nothing: @nextcloud/stylelint-config carries no indentation rule, so styles had drifted to 4 tab-lines against 76 space-lines while .editorconfig says tabs. @nextcloud/prettier-config (useTabs: true, tabWidth: 4) covers CSS/SCSS as well as JS/Vue. eslint-config-prettier is spread LAST in eslint.config.js so the two formatters cannot demand opposite things. l10n/ is in .prettierignore: 38 generated l10n/*.js translation bundles. Co-authored-by: Ruben van der Linde <juan.claude@conduction.nl>
This repo carries `@nextcloud/prettier-config` and a `format` script, but nothing ever ran that script in CI. The shared `quality.yml` has NO prettier job of its own — it mentions prettier ZERO times (eslint 9, stylelint 10) — and `frontend-checks` is the only opt-in that can invoke a repo's own npm scripts. So prettier was active in developers' editors and inert everywhere else: exactly the state the fleet's old `.prettierrc` was deleted for. Appending "format" to the existing `frontend-checks` array adds one `Frontend Check (format)` job. Every pre-existing entry and its order are unchanged. Measured on this tree before enabling, with the same prettier version and the same resolved config CI uses; the per-app scope and result are recorded in the comment above the input. Centralising the config never stopped drift; the gate does.
Coverage 74 -> 76 files. The two newly-linted files produced 3 findings: 2 rule-empty-line-before (auto-fixed) and one no-duplicate-selectors — .filesListDragDropNoticeTitle declared twice in css/main.css. The duplicate is merged with its declaration order preserved (margin sets all four sides, the later margin-left overrides the left one), so the computed style is unchanged. larpingapp carries the identical pair: this stylesheet was copy-pasted between apps and nothing linted it, because css/ sat outside the glob in 10 of the 11 apps that have one.
ReviewController::aggregate was the app's one unthrottled #[PublicPage] endpoint. IntakeController already carried 5/3600. 120/60 rather than IntakeController's 5/3600, deliberately: that endpoint accepts a SUBMISSION, this one answers a page render, and a catalogue page listing many subjects will legitimately call it repeatedly. Copying the intake limit here would have broken the catalogue. No brute-force counter: no credential, and the aggregate scores are already published. COUNT CORRECTION: the fleet sweep first reported 3-5 unthrottled endpoints here. There is 1. The earlier grep counted prose mentions of `#[PublicPage]` in docblocks as attributes; line-anchoring the pattern gives the real number. Same error class as the hermiq count in this sweep. VERIFICATION: none. The test suite crashes on this instance BEFORE any change -- identical stack trace with and without the diff -- so there is no regression signal available locally, and equally no green to claim. CI is the first real run.
…mits fix(security): rate-limit the public review aggregate endpoint
Same migration as the pilot (ConductionNL/larpinq#325) and the template (ConductionNL/nextcloud-app-template#146). eslint.config.mjs is the canonical copy; only the app-specific blocks at the end differ. Requires Node 22 — @nextcloud/eslint-config@9 declares engines.node ^22.14 || ^24 || >=26 and imports findPackageJSON from node:module, first available in 22.14. The shared workflows already default to 22 (ConductionNL/.github#450). Stale eslint-8-era direct deps and overrides are removed, and the two peers the config needs are declared at the right major (vue-eslint-parser ^10.3.0, @typescript-eslint/parser ^8.67.0). An overrides entry resolves nc-vue's OPTIONAL eslint peer against eslint 10 — optional means npm will not install it, not that a mismatched version is accepted. Coverage: all 73 .vue files parse, 0 fatal errors. 422 PRE-EXISTING VIOLATIONS ARE RECORDED, NOT FIXED ------------------------------------------------------ v9 enables rules this app has never run. --fix and prettier resolved the mechanical ones; 422 findings across 64 files remain and are recorded in eslint-suppressions.json using eslint's own bulk suppressions (--suppress-all), NOT by loosening any rule. Every rule keeps the severity @nextcloud/eslint-config gives it, the debt is counted per file, and --prune-suppressions shrinks it as it is paid down. A NEW violation still fails the run — verified on openconnector by appending a console.log after suppressing. 159 no-console 134 jsdoc/require-param-type 38 @typescript-eslint/no-unused-vars 20 vue/prefer-define-options 18 @nextcloud/no-deprecated-library-props 17 no-unused-vars 9 vue/custom-event-name-casing 9 no-useless-assignment 4 vue/multi-word-component-names 4 vue/prefer-separate-static-class 3 vue/slot-name-casing 3 @nextcloud/l10n-enforce-ellipsis TWO AUTOFIXES ARE DELIBERATELY WITHHELD (recorded as debt instead) ----------------------------------------------------------------- - @nextcloud/l10n-enforce-ellipsis rewrites '...' to the typographic '…' INSIDE translatable strings. That changes the translation KEY and orphans every l10n/*.json entry for it; on openconnector it turned the l10n parity check red while it was green on development. The migration must not silently drop translations, so the source strings are left alone. - vue/prefer-define-options rewrites 'export default {…}' in a plain <script> into 'defineOptions({…})' inside <script setup>, carrying props/data/computed across the block boundary. @vue/compiler-sfc then rejects the result outright (defineOptions() cannot declare props), which broke the build in softwarecatalog and docudesk. It is a semantic refactor, not a lint fix. TEST GLOBALS ARE DECLARED, NOT SUPPRESSED ----------------------------------------- Spec files that live under src/ have no framework globals, so no-undef reported every describe/it/expect as undefined — 1203 findings in openregister from just 7 identifiers. Declaring the environment removed ~1900 phantom findings fleet-wide. Suppressing them instead would have buried any REAL no-undef, which is the rule that catches a typo'd identifier. VERIFIED -------- npm run lint PASS npm run stylelint PASS npm run format PASS npm run build PASS
* refactor(softwarecatalog): translate Dutch vocabulary to English, extending the existing migration
Applies the fleet Dutch->English pass: 93 property names plus identifiers,
comments and docblock shapes. Extends RenameDutchCatalogColumns (6 pairs -> 84)
rather than adding a second step.
THE PIPELINE ABORTED BEFORE TOUCHING ANYTHING, WHICH IS THE POINT
`relation` already has BOTH `bron` and `source`, and their descriptions say the
same thing — a pre-existing duplicate from a partial earlier rename. Renaming
`bron` onto `source` would collide and lose data; inventing a third name would
be worse. Consolidating them is a DATA decision for the owners, so `bron` is
excluded with that reason recorded in .exclude-swc-tr.json.
TWO MORE EXCLUSIONS, FOR FLEET CONSISTENCY. This app already SHIPPED
`beschrijving_kort -> short_description` and `beschrijving_lang -> description`.
My dictionary would have produced `descriptionKort` and `descriptionLang` — HALF
DUTCH, and forking the app against its own migration. It got as far as the
registers (31 occurrences) before I noticed, because `kort` and `lang` carry
none of the Dutch orthographic markers the residual guard looks for. Guard
widened; both names excluded; branch rebuilt from clean.
`versies` was the same story — `standaardVersies` became `standardVersies`.
Added to the dictionary, guard widened again.
TWO DEFECTS I CAUSED BY HAND, BOTH BYPASSING THE TOOL'S GUARDS
- shortening `$moduleVersionSchemaId` with a plain regex renamed the parameter
but not its NAMED ARGUMENTS, so three call sites broke. The AST tool moves
both together; my regex did not.
- the `name` alias block in DataMapper normalised the incoming key `name` onto
the then-canonical `naam`. The canonical IS `name` now, so the block became
`isset($data['name']) === false && isset($data['name']) === true` — a
contradiction that can never run. Removed rather than left as dead code.
VERIFIED in the container against a CONTROL run of clean development:
phpstan [OK] · psalm 0 no-cache (baseline 0) · phpmd 0 (baseline 0) · phpcs 4
errors, IDENTICAL to the baseline.
NOT VERIFIED LOCALLY: the test suite. softwarecatalog's bootstrap loads every
installed app, and `nldesign` in this container is missing its vendor — the run
dies before reaching a test, on the baseline too. CI must confirm the suite.
* fix(softwarecatalog): three CI failures — a dead annotation, stale specs, and a filter VALUE
phpstan passed locally and failed in CI. Cause: I "fixed" phpcs by changing
/** @var */ to /* @var */, and a SINGLE-ASTERISK comment is not a docblock, so
phpstan ignores it entirely. The annotation was dead the moment it satisfied the
other tool. Restructured instead: read every value BEFORE the write that narrows
the inferred array shape, which needs no annotation and cannot rot.
FRONTEND SPECS were stale rather than wrong: they pass `standaard` as an
UNQUOTED object key (`{ standaard: [...] }`), which the quoted-string rename
cannot see. The register calls it `standard`, so the specs were simply behind.
AND ONE REAL DEFECT THE MANIFEST WAS ABOUT TO SHIP
"filter": { "gemmaType": "standaard" } -> "standard"
`gemmaType` is a STORED DATA VALUE on GEMMA element objects, not a property
name. Renaming it makes the filter match nothing and the Standaarden list page
silently goes empty — no error, no test, just an empty page.
Found by diffing every filter VALUE against the baseline rather than reading the
diff. That sweep now also covers lib/Settings: three filter values moved
in total, and only this one was data — the other two are `@objectId` /
`@object.name` tokens that did not exist in the baseline at all.
phpstan [OK] and phpcs at its baseline of 4 with both fixes in place together,
which was the thing that failed before.
* fix(softwarecatalog): reflection-by-name in tests, and a half-translated method
PHPUnit failed on ReflectionException for 28 private methods. The AST renamer
correctly renamed them and their `$this->` call sites, but the tests invoke them
BY NAME as a string:
new ReflectionMethod(Service::class, 'normaliseCurrentStandaarden')
A string is invisible to the AST, and method names are not in the property map,
so nothing moved it. Fixed by diffing private-method declarations against the
baseline and updating every test that names an old one — 5 files.
That diff also exposed a HALF-TRANSLATION the guard let through:
`updateGeregistreerdDoor` had become `updateGeregistreerdBy` — `door` mapped to
`by` inside a compound whose other half stayed Dutch. `geregistreerd` was not in
the dictionary and carries none of the markers the residual check looks for
('ee' followed by n or s; this is 'eer'). Token added, guard widened for `-eerd`
endings, and the name is now `updateRegisteredBy`.
phpstan [OK], phpcs at its baseline of 4.
* fix(softwarecatalog): the rename corrupted a test's own Dutch/English fixtures
RenameDutchCatalogColumnsTest exercises `renameIsSafe()` with LITERAL pairs —
`('naam', 'name', [...declared...])` — to prove the migration only moves data
when the register has actually moved. Those literals are FIXTURE DATA, not app
vocabulary.
The blanket rename turned every `'naam'` into `'name'`, collapsing each scenario
into `('name', 'name', ...)`. The test then asserted TRUE and FALSE about
identical inputs, which is why it failed — and had it not failed, it would have
been worse: a migration-safety test that no longer distinguishes the safe case
from the data-loss case, still reporting green.
Restored from development verbatim. Its private-method references were checked
against the AST renames and needed none.
This is the fixture form of the rule the rest of this work follows: a Dutch
string that is DATA — a CSV header, a stored enum value, a config key, a test
input proving behaviour about Dutch names — does not move with the vocabulary.
* fix(softwarecatalog): translate a user-facing Dutch validation message
ReviewService returned 'waardering must be between 1 and 10' — half Dutch, half
English, and the test asserted on the substring the rename had already moved.
A user-facing MESSAGE is app text, not a wire contract: it is displayed, not
parsed by anything. So it translates, unlike the CSV headers, ZGW resource keys
and stored enum values elsewhere in this branch that were deliberately held.
* fix(softwarecatalog): restore the GEMMA facet dimension vocabulary
The wire dimension list between the Vue store and FacetController is
['referentiecomponent', 'standaard', 'applicatieservice', 'domein']. My rename
translated exactly ONE of the four, leaving a half-Dutch vocabulary and breaking
the round-trip the store spec asserts.
These are not schema fields — facets.js documents that explicitly: 'the
module/dienst schema has no field named referentiecomponent/standaard/domein/
applicatieservice (the real fields are referentieComponenten, standaardVersies)'.
They are DERIVED dimension names on the wire, so they stay as a set or move as a
set, and moving them is a client+server+URL-key change that is not this branch's
job.
Restored across FacetController, services/facets.js, both specs and the _gf_
URL keys.
* fix(softwarecatalog): a half-Dutch field name, and a test reading the wrong dimension
`referentieComponenten` had become `referenceComponenten` — `referentie`
translated, `Componenten` did not — and it SHIPPED into the register, so
FacetService read a field that no longer exists and array_column() got null.
`componenten` was not in the dictionary and carries none of the markers the
residual guard looks for. Token added, guard widened, and the field is now
`referenceComponents` across the register, the code and the migration.
The other failure was mine from the previous commit: restoring the wire
dimension name to `standaard` left FacetServiceTest still reading
`$result['standard']`. The dimension is the result KEY, so the test had to move
back with it.
This is the third half-compound this app produced (`kort`/`lang`, `versies`,
now `componenten`). The guard is a heuristic over Dutch orthography and these
words have none of it — each one is a dictionary gap, found by a failing test
rather than by the guard. Worth saying plainly: the guard reduces the class, it
does not close it.
phpstan [OK], phpcs at its baseline of 4.
* fix(softwarecatalog): make the facet dimension key consistent everywhere
Restoring the wire dimension to `standaard` in FacetService::DIMENSIONS left the
same key spelled `standard` in three other places: the result array the service
builds, its @return array{} shape, and three assertions in FacetServiceTest. The
dimension name IS the result key, so all four have to agree — the tests were
reading a key nothing produced.
Worth separating two things that look identical and are not:
- the facet DIMENSION `standaard` is a wire name between the Vue store and
FacetController, documented as NOT a schema field. It stays Dutch until the
whole set of four moves together.
- the register PROPERTY `standaard` IS a schema field and does move, which is
why the migration still carries `'standaard' => 'standard'`.
Both are correct at the same time; conflating them is what produced this churn.
phpstan [OK].
* fix(softwarecatalog): e2e specs seeded objects with removed property names
Playwright failed where the merged-PR baseline passes it, so this was mine. The
specs create catalog objects through the API with bodies keyed by PROPERTY
names — `naam`, `beschrijving`, `waardering`, `versie` — which the registers no
longer declare, so the seeds failed and the assertions after them cascaded.
WHY THE PIPELINE MISSED THEM. Its rename rewrites QUOTED occurrences, which is
right for PHP and JSON. TypeScript object literals use UNQUOTED keys
(`waardering: 3`) and property access (`obj.naam`), and neither is quoted. 10
spec files updated across all three positions.
Pairs were rebuilt from THIS BRANCH'S migration map rather than the shared
.compose.json — that file is one path reused by every app's run, and reading it
after another app had overwritten it is exactly what made the decidesk Newman
check report a false clean.
DELIBERATELY LEFT: 8 remaining matches are not property keys — `/standaarden`
URL ROUTES (the manifest keeps them), a `standaard-detail` test label, local
variables and comments. Renaming a route would break navigation for a cosmetic
gain.
* fix: the frontend still used property names the registers had renamed
THE PIPELINE HAD A HOLE AND THIS IS WHAT FELL THROUGH IT.
Its property pass rewrites QUOTED occurrences, which is right for PHP and JSON.
Javascript does not quote object keys, and property access has no quotes at all:
{ waardering: 3 } an object key POSTed to the API
obj.naam a read
"naam" the quoted form the old pass already handled
So Vue components kept posting and reading names the registers no longer
declare. This is a PRODUCTION defect, not a test artifact: on softwarecatalog
the review dialog never closed because the POST 400d silently, and the only
thing that noticed was an e2e assertion that a dialog should be hidden.
Fleet-wide it was 100 source files — procest 57, softwarecatalog 37, pipelinq 6.
decidesk had none, which is why it went green first time and hid the class.
Fixed by a new pass (rename-frontend.js) covering unquoted keys, property access
and quoted forms. Its pairs come from THIS repo diff of lib/Settings crossed
with the app own migration map — never from the shared .compose.json, which is
one path reused by every app run and was stale enough to make an earlier check
report a false clean.
Frontend files only; no PHP touched, so the PHP gates are unaffected. `node
--check` clean on every changed .js/.ts.
* fix(softwarecatalog): prettier and eslint after the frontend rename
Two consequences of the frontend pass, both mechanical:
- prettier: renaming changed line widths in two vitest specs. Reformatted
with the app's own @nextcloud/prettier-config; `prettier --check` over
CI's exact glob ("**/*.{js,ts,vue,css,scss}") is clean.
- eslint object-shorthand: `waardering: rating` became `rating: rating`,
which is the one ERROR among 164 warnings. Now `rating`.
node --check clean on every changed .js/.ts.
* fix(softwarecatalog): the frontend pass re-renamed the facet dimension
The migration map legitimately contains `standaard => standard` — that is the
register PROPERTY. The facet DIMENSION happens to share the spelling, and the
frontend pass, which drives off that same map, renamed both.
So the dimension I had deliberately restored two commits ago was renamed again
by a later, more thorough pass. Both changes were individually right; together
they were wrong.
Restored in the four files that deal only in dimensions (services/facets.js,
store/modules/facets.js and their specs). Everything else keeps the property
rename.
VERIFIED locally with the app's OWN runner — CI uses jest, not the vitest config
in the repo, and vitest excludes src/** entirely, so `npx vitest` reported 'no
test files found' and would have looked like a pass:
npx jest -> 9 suites, 120 tests, all passing
prettier --check over CI's exact glob -> clean
* fix(softwarecatalog): two more stored VALUES the rename should not have touched
Playwright went from 12 failures to 5; these are the last two causes, and both
are the same class — a Dutch string that is DATA, not vocabulary.
gemmaType === 'standaardversie' a STORED value on GEMMA element objects.
Renamed, the compliance matrix filter
matched nothing and the view rendered empty.
COLUMN_SOURCE.STANDAARDVERSIE the enum compared against that value, so it
has to hold the same string.
DIMENSIONS ['... 'standard' ...] the e2e spec's copy of FacetController's
dimension list, which is Dutch on the wire.
That makes three separate places in this app where appears: a
register PROPERTY that moves, a facet DIMENSION that does not, and a gemmaType
VALUE that does not. Same spelling, three different contracts.
VERIFIED with the app's own runner: jest 9 suites / 120 tests green, prettier
clean over CI's glob.
* fix(softwarecatalog): finish the translation — 22 names were half English, half Dutch
The rename this PR ships emitted a new name whenever SOME token was translatable
and let the rest through untouched, so it produced names like
`accountantsverklaringRequired` and `afstandToArbeidsmarkt` — English grammar
around a Dutch word. 22 of the names it introduced still carried one. Merging
that is worse than not renaming: the schema ends up in a third language nobody
can search for.
The number is measured with real wordlists — 274,937 English words and 164,174
Dutch, a token counting as Dutch when the Dutch list has it and the English list
does not. Two earlier instruments were wrong in opposite directions: matching
against the translation dictionary found almost nothing, because the names that
broke are built from words the dictionary never knew; matching against a
hand-written English vocabulary flagged ordinary words like `transaction` and
`income`. Control on the real one: `opbrengst`/`dienst`/`termijn` flag,
`transaction`/`settlement`/`allocation` pass.
CASE was wrong too. A dictionary value containing an underscore turned a
camelCase name into snake_case mid-schema. Style now comes from the schema name
that was REMOVED, never from the migration map's left-hand side — that side is a
COLUMN name and is always snake_case, so asking it whether the original was
snake_case answers yes for every multi-word name. My first attempt did exactly
that and rewrote `adviesAuthority` as `advice_authority`.
NO SECOND MIGRATION: the branch is unmerged, so these names have never existed
in a database. The correction rewrites them to the final name everywhere,
including the migration map's RIGHT-hand side, so the map points the original
Dutch column straight at the correct English one. Every rename is registered in
its snake spelling as well — the map is keyed on column names, and without that
a camelCase correction never reaches it and the repair step would migrate data
into a column the schema no longer declares.
VERIFIED: PHPUnit identical to a control run of the branch without these
corrections, phpstan [OK], psalm 0, eslint 0 errors, build OK, 0 surviving uses
of any renamed property in src/, every register file parses.
NOT IN SCOPE, measured rather than assumed: 62 property names that were ALREADY
Dutch on development and which the first pass never touched. A separate tranche,
not a defect in this PR.
* style(softwarecatalog): re-run prettier after the name corrections
The renames changed identifier lengths, so prettier's wrapping no longer matched
in 31 files and `Frontend Check (format)` went red. `development` is
fully prettier-clean, so this is drift the correction introduced, not
pre-existing.
Ran the project's own `format:fix`. `git diff -w` is attribute wrapping only and
no import line moved — worth checking, because a formatter that reorders
side-effect CSS imports changes behaviour while looking cosmetic. eslint 0
errors, build OK.
* fix(softwarecatalog): shorthand properties and a computed the rename split in two
Three vitest tests went red and development is fully green, so this was mine.
Both causes are the same shape: the rename updated one half of a pair.
1. SHORTHAND OBJECT PROPERTY. complianceMatrix.js reads `data.standardGemma`
(renamed, correct — that is the schema property) into a local variable that
kept its old name, and pushes it with shorthand:
const standaardGemma = … data.standardGemma …
unresolved.push({ moduleUuid, standaardGemma, evidenced, record })
A shorthand property has no `name:` for a key-rewrite to match, so the object
went on emitting `standaardGemma` while every consumer had moved to
`standardGemma`. The variable name IS the key here.
2. A COMPUTED AND ITS READERS. ComplianceMatrixView.vue declares
`standaardversies()` and reads `this.standard_versions` — the rename rewrote
the reads and not the declaration. That is not a test problem: the reads
resolve to undefined and `.length` throws when the view renders. No unit test
covers it; it was found only by chasing why one spec assertion failed.
`standaardversies` is a FUNCTION PARAMETER, not a schema property, so it is now
`standardVersions` in camelCase rather than the snake_case a column rename would
have produced — and every call site agrees, which they did not before: one spec
call passed `standard_versions` while three passed `standaardversies`.
VERIFIED: vitest 226/226, eslint 0 errors, build OK, format clean, PHPUnit
identical to control. `adminApi.spec.js` fails to collect with "window is not
defined" on this branch AND on development when run in isolation — pre-existing,
not touched here.
* fix(softwarecatalog): one of four facet dimensions was translated, breaking the contract
`Frontend Tests (unit)` was red on CI and green locally, because this repo runs
BOTH runners: `test` is jest over `src/**.spec.js` and `test:unit` is vitest over
`tests/vitest`. My verification only ran vitest, so two failing jest specs were
invisible to me and obvious to CI.
The defect they caught: `FACET_DIMENSIONS` in src/services/facets.js is
`['referentiecomponent', 'standaard', 'applicatieservice', 'domein']`, and the
vocabulary pass translated exactly ONE of the four to `standard`. Those strings
are not ours to rename one at a time — `FacetController` and `FacetService` both
declare the same four as the query parameters and response keys of this app's own
facet endpoint. So the frontend began sending `standard[]` to a backend that only
reads `standaard[]`, and **facet filtering by standard silently returned
everything**: no 400, no console error, both sides behaving exactly as written.
Same shape as the DSO defect in openconnector — one member of a wire contract
translated, the rest untouched, which is hard to see in review precisely because
the surrounding lines still look right. Reverted, with a comment saying why the
four move together or not at all. Renaming all four plus the backend is a real
option, but it is an API change and belongs with the pre-existing tranche, not
smuggled into a vocabulary PR.
The two spec files are restored to development, since their sources are now
functionally identical to it.
Also in this commit, from the earlier round: a SHORTHAND object property
(`unresolved.push({ moduleUuid, standaardGemma, … })` — the variable name IS the
key, and a key-rewriter matching `name:` cannot see it) and a Vue computed whose
readers had been renamed without the declaration, which throws on `.length` at
render and no unit test covered.
VERIFIED: jest 120/120, vitest 226/226, eslint 0 errors, build OK, format clean.
`adminApi.spec.js` fails to collect with "window is not defined" here AND on
development in isolation — pre-existing.
* fix(softwarecatalog): the e2e suite carried its own copy of the wire names
Two more places where the rename moved one half of a pair, both caught by CI's
Playwright job and neither visible to jest, vitest, eslint or the build.
1. gemma-faceted-search.spec.ts keeps its OWN `DIMENSIONS` list, a duplicate of
the four the backend declares, and the pass had translated `standaard` there
too. With the source reverted to match FacetController/FacetService, the spec
was the only thing left asserting `standard`. Realigned, with the same comment
the source now carries: the four move as a set or not at all.
2. crud-persistence.spec.ts POSTs a module version with SHORTHAND —
`{ data: { versie, status } }` — so the local variable's name is the key. The
rename updated the READ two lines below (`r.version === versie`, property
access has a dot to match on) and could not see the write. The schema declares
`version` now, and MagicMapper DISCARDS an undeclared property with a log line
rather than a 4xx:
[MagicMapper] Discarding 1 property the schema "Application version"
does not declare: versie. They are NOT stored anywhere.
So the create "succeeded", the row came back without a version, and only the
later assertion noticed. Now written as `version: versie` explicitly, with a
note about why shorthand is wrong here.
That is the third shorthand-property defect in this programme — after procest's
BAG/WOZ query parameters and this app's own `unresolved.push({ moduleUuid,
standaardGemma, … })`. A key-rewriter matches `name:`; shorthand has no colon.
VERIFIED: jest 120/120, vitest 226/226, eslint 0 errors, format clean.
---------
Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
# Conflicts: # src/dialogs/SuiteWizard/Step1Details.vue # src/views/LifecycleRoadmapView.vue # src/views/suites/SuitesIndexView.vue
build(lint): migrate to eslint 10 + @nextcloud/eslint-config 9
* ci: fast structural checks on every branch * ci: close the branch-trigger gap * ci: scope the JSON check — JSONC configs are not a defect * ci: scope the marker check to code — prose that documents a conflict is not one * ci: the JSON check reached a template's editor settings
…514) These methods declare themselves public with the legacy @publicpage ANNOTATION rather than the #[PublicPage] attribute, which is why the fleet sweep that reported this app fully throttled did not see them: that sweep line-anchored the attribute form and excluded docblock matches. The annotation is not a docblock mention. It is a live declaration, proven against the running server on two other apps in this fleet: openregister GraphQLController::execute @publicpage only -> 200 anon opencatalogi CatalogiController::index @publicpage only -> 200 anon AnonRateLimit only, not BruteForceProtection: these endpoints check no credential, and brute-force protection without a paired registerAttempt() is the inert half of a two-half mechanism. AnonRateLimit also leaves authenticated server-to-server traffic untouched, so no integration can be throttled by this change. Health endpoints get a deliberately generous 240/60 - monitoring polls them on a short interval, and a ceiling that trips on a normal probe cadence turns the health check into the outage it was meant to detect. Verification: php -l clean on every changed file; the diff is purely additive with zero lines removed; gate-82 (.github#460) goes to 0 findings on this tree.
5 file(s) reached OpenRegister through $this->container->get(...) on an UNCONDITIONAL path — no availability check, no degrading catch. The dependency was announced nowhere: not in the constructor, not in the use block, not in any type. It appeared mid-method, as a string. Now constructor-injected and typed, so the dependency is visible to a reader and to tooling. Behaviour is unchanged: the same object, from the same container, resolved at construction instead of at first use. ContainerInterface is dropped only where nothing else used it. Deliberately NOT converted, because they are correct as written (ADR-083 rule 1's exception): lookups behind isInstalled()/getInstalledApps(), and lookups whose catch degrades rather than rethrows. Verified per file: php -l clean, and gate-66's lookup check reports zero remaining findings for each file changed. gate-66 for this app: 23 -> 8.
…ames (#513) Tranche 2. Measured at 28 Dutch property names against real wordlists; 12 move here and 13 are held back with a reason each, which is the more useful half of this commit. HELD BACK, and why — these need a decision, not a rename: organisatie, contactpersoon, dienst, gebruik, moduleVersie, bioMaatregel Each is ALSO a schema slug. A quoted 'organisatie' in PHP is a schema reference in one place and a property key in another, and there are 107 of them; nothing in the token tells the two apart. The property rename needs the SCHEMA rename decided with it. domein One of the FOUR facet dimensions the endpoint declares as its query parameters and response keys. Renaming one of four is exactly what broke filtering in tranche 1 — they move together, as their own change. bron The `Relation` schema declares BOTH `bron` and `source`, two properties for one concept. A rename would silently merge two columns. The duplication is the defect and needs deciding. omschrijving The COLUMN_MAP is FLAT and already maps `omschrijving` -> `description`. The remaining occurrence sits in a schema that already declares `description`, so it would need `summary` — and one source cannot have two targets in a flat map. alg, bomRef, tooi, *Url JOSE header, CycloneDX field, TOOI register, and `url`. Not Dutch. THREE DEFECTS IN MY OWN TOOLING, all found by tests rather than by review: 1. The unquoted-object-key pattern NEVER FIRED. Its `(?!:)` guard — meant to skip PHP `::` — sat immediately after the name, where the next character is the very colon being matched, so it could never pass. Every `beschrijvingKort:` payload key was left behind while its VALUE was renamed, so the wizard POSTed a key the schema no longer declares. Moved inside the lookahead. 2. Running the applier twice REWROTE THE MIGRATION MAP'S OWN LEFT-HAND SIDE. The map is a .php file under lib/, so `'afkorting' => 'abbreviation'` became `'abbreviation' => 'abbreviation'` — a no-op that also destroys the only record of the column's old name. It took a pre-existing entry with it. The map and its test are excluded from the rename now. 3. A DESTRUCTURED PARAMETER is shorthand and has no colon either, so `buildOrganisationCoverage({ gebruiken })` kept its parameter while one call site moved to `usages:` — the function then read an undefined key and returned an empty coverage array. The app's own test suite caught two of these, including a positive control asserting the register still declares Dutch columns. That control's threshold (`> 20`) was a snapshot of how much Dutch remained, not a property of the guard; it is `> 0` now, which is what it was actually for, with a note that the day it fails is the day the test has nothing left to guard. VERIFIED against a control run of the same tree: PHPUnit 684 tests, 1 error on BOTH (a pre-existing missing Symfony class), 0 failures; phpstan [OK]; psalm "No errors found!"; jest 120/120; vitest 226/226; eslint 0; prettier clean; build OK; 0 broken routes; 0 surviving uses in src/. 10 migration entries appended, map verified free of duplicate and identity entries. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…the schema decide (#516) * refactor(softwarecatalog): one Dutch name, two English targets — let the schema decide Tranche 3a. Two things the flat column map could not express, and the mechanism that fixes both. REMOVED A DUPLICATE. The `Element` schema declared `bron` alongside `source` — two properties for one concept, which is why tranche 2 refused to rename it: a rename would have silently merged two columns. `bron` is the dead one (0 references in lib/, 0 seed objects, not required, against 173 for `source`), so it is gone rather than renamed. `omschrijving`/`description` looked like the same case and is NOT. They are two real properties: on `organisatie`, `omschrijving` is the BRIEF description — its own title already said "summary" — and `description` is the detailed one. So `omschrijving` becomes `summary` HERE while it stays `description` on every other schema, and a flat one-source-one-target map cannot say that. Choosing either globally puts data in the wrong column for the other side. THE MECHANISM. A map value may now be a LIST of candidate targets, and the schema's own declared columns decide which applies — via renameIsSafe(), the predicate that already governed the single-target case, so there is one rule about when a column may move, not two. Candidate order is authoritative where a schema declares more than one: most specific first. GETTING THERE COST TWO WRONG TURNS, both caught by the ruleset: - Extracting the predicates as STATIC helpers tripped phpmd's `StaticAccess` rule — the ruleset wants a collaborator, not statics — and `hasCollision` could not move anyway because it reads COLUMN_MAP. - Inlining the resolver instead traded one violation for two: the class fell to exactly 50 but `migrateTable()` then tripped CyclomaticComplexity at 10. The class was already sitting ON phpmd's 50-point ceiling before this change, so any addition trips it. The predicates now live in an injected RenameDutchCatalogDecisions — instance methods, no static access, defaulted in the constructor so DI needs no wiring — and the step is back under the limit with the new capability included. VERIFIED against a control run of the same tree: PHPUnit 684 with 1 error on both sides (pre-existing missing Symfony class), phpmd 0 violations, phpstan [OK], psalm "No errors found!", jest 120/120, vitest 226/226, eslint 0, prettier clean. The duplicate-property detector now reports 0 pairs. * test(softwarecatalog): cover the decisions collaborator in @Covers CI marked two tests RISKY — 'executed code that is not listed as code to be covered' — because the class-level @Covers still named only RenameDutchCatalogColumns while the predicates now live in the injected RenameDutchCatalogDecisions. PHPUnit's strict coverage mode is right: the annotation claimed a narrower surface than the test exercises. My local run could not see it. There is no coverage driver in the dev container, so the strict-coverage checks never fire there — the same local-vs-CI layer gap that has caught me before, and the reason the ratchet cell is the one that keeps finding things. * style(softwarecatalog): add the @copyright tag gate-1 requires The new collaborator carried SPDX-FileCopyrightText but not the @copyright docblock tag, which is what spdx-headers actually reads. gate-1 went from passing on development to failing on the PR — one new gate, and the only one this branch introduced. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
21 lib classes and 17 test classes take OCA\OpenRegister\Contract\ObjectServiceInterface instead of the concrete class, bound in the composition root, with hydra-gates bumped to v1.8.0 so composer installs the interface into vendor/. This is what makes the ADR-083 conversion mockable: a leaf app cannot load a class from another Nextcloud app, so a typed constructor parameter had no satisfiable double. Test doubles now mock the contract, which does load. Files naming ObjectService only as a CONTAINER KEY are untouched — those are availability-guarded lookups (the ADR-083 rule-1 exception), and the string must go on naming the concrete service because that is the key the alias resolves TO.
The first pass through this app used a transformer with three defects, each of which failed SILENTLY -- a skipped file is indistinguishable from a clean one. 1. It masked string literals with a REGEX. An apostrophe in a comment (`// King's Day.`) opened a string that did not close for 240 lines, so every type position between them looked quoted and the file was skipped. Now the comment and string ranges come from PHP's own token_get_all(). 2. It compared PHP's BYTE offsets against Python's CHARACTER indices. One file differed by 94 -- em dashes in prose comments -- so spans after the first non-ASCII byte were misaligned and a docblock was judged "not in a comment". Everything is handled as bytes now. 3. It missed short-form RETURN types (`): ?ObjectService`), which neither the parameter nor the fully-qualified pattern matched. And it dropped the concrete import even where `ObjectService::class` or `instanceof ObjectService` still needed it. That one is not merely incomplete, it is silent damage: `::class` does not require the class to exist, so the lookup would have resolved to this app's own namespace. An invariant check now enforces the rule -- a file may import the contract, or both, but never the contract alone while still naming bare ObjectService -- and reports 0 for every app in this rollout.
…them (#518) * refactor(softwarecatalog): translate eight schema slugs, and migrate them A schema slug is not a name the code merely mentions — it is what OpenRegister's ImportHandler matches an incoming schema against (`SchemaMapper::findBySlugInIds()`). Changing a slug in the register JSON therefore renames nothing: the import finds no match, creates a SECOND schema, and every stored object keeps pointing at the old one. Nothing raises. The data is stranded behind a schema nothing reads, which presents as an app that has no records. So this adds `RenameDutchSchemaSlugs`, which renames the slug on the existing row first, and registers it FIRST in post-migration — ahead of InitializeSettings, which is what triggers the import via SettingsService::initialize(). The shard table is named for register and schema IDs, neither of which moves, so the rows come along untouched. Five of the eight are also declared PROPERTY names, which is why tranche 2 held them back: a quoted 'gebruik' is a schema reference in one file and a property key in another. Both halves move here, in one operation, and the applier now refuses unless the two maps agree on the target. Targets are each schema's own English title, not invented — `beoordeeling` was already titled "Assessment", `kwetsbaarheid` "Vulnerability". Three things this turned up: - The slug lives in THREE places per schema, not one: the `slug` value, the `/components/schemas/` map KEY, and the register's `configuration.schemas` map key. The five dual names had their map key renamed as a side effect of the property pass; the three slug-only ones did not, leaving `$ref`s pointing at schemas that no longer existed. - A `$ref` carries a PATH (`#/components/schemas/dienst`), so a key/value pattern never matches it — 77 in one file. - One jest expectation embedded the slug in a URL rather than quoting it bare, so no pattern reached it. The test caught itself. NOT in scope, deliberately: the `<slug>_schema` app-config keys. That family spans ~40 sites in SettingsService alone, including differently-prefixed (`voorzieningen_contactpersoon_schema`) and compound (`koppeling_gebruik_schema`) forms. Migrating a subset would resolve some object types and leave others silently "not configured" — how the ratings feature died once already. The objectType->key map keeps its Dutch keys, so resolution is unaffected. `organisatie` is also out of scope and says why in the step's docblock: this app declares BOTH an `organisatie` schema and an `organization` one, sharing not one property, and renaming the first onto the second collides on the flat config key. Verified against a control: phpstan clean, psalm "No errors found", phpmd clean, phpcs 908 violations in 12 sources both before and after on the same 61 files, jest 120/120 (control 120/120), vitest 226 passing with the same one pre-existing suite failure as development, webpack build clean, register internally consistent (no dangling $refs, no key/slug mismatch, both registers' schema lists resolve). PHPUnit could not run locally — the shared instance's openregister checkout is on a feature branch and its ObjectService fails to resolve at bootstrap — so CI is the authority for that suite. Dutch names 12 -> 7. * fix(softwarecatalog): keep the user-facing strings English, not identifiers The slug rename reached inside a translation KEY, turning "Failed to add contactpersoon: {error}" into "...contactPerson...". That is an identifier in a sentence meant for a person to read, and it is the failure mode the l10n checker exists for: the key used in source no longer existed in en.json, so the string would have rendered untranslated for every locale. The l10n gate was the only instrument that saw it. phpstan, psalm, phpcs, jest and vitest were all green on the same tree. Fixed as prose rather than as an identifier — "contact person" — and the three neighbouring strings that carried the same Dutch word were fixed with it, along with three PHP log messages. Their now-orphaned keys are removed from en.json; each was confirmed unused in source first. * refactor(softwarecatalog): merge the two organisation schemas into one The app declared two: `organisatie` (the catalogue's own, in the voorzieningen register) and `organization` (ArchiMate, in vng-gemma). They shared NOT ONE property — the first held relations and lifecycle (contactpersonen, deelnames, participants, status, publicationDate), the second held identity and statutory identifiers (name, summary, description, oin, tooi, rsin, pki, image) plus the ArchiMate round-trip `xml`. Two halves of one entity, not two entities. They are one `organization` now: the nine identity properties are folded into the catalogue schema, the ArchiMate schema entry is removed from the register JSON, and the slug is translated. Measured before touching anything: `organisatie` holds 60 rows, the ArchiMate `organization` holds 0, and a third orphaned `organization` (id 47, in NEITHER register, so out of scope for a register-scoped step) also holds 0. `required` is deliberately NOT the union of the two. The ArchiMate schema required name+summary; the catalogue records delegate their identity to Nextcloud Contacts through `contactsUid` and carry neither. Unioning would have marked all 60 existing rows invalid, so required stays [contactsUid, type] and `name` is documented as the optional mirror. `retireArchimateOrganization()` frees the name on an existing install by parking the absorbed schema under `archimateOrganizationLegacy` — renamed, not DROPped, because a retired row costs nothing and keeps the decision reversible. It only proceeds when that schema holds NO rows; where it holds data the merge is a decision about that data and the step refuses loudly rather than guessing. A failure to COUNT is also treated as non-empty, so an unchecked assumption cannot look like an empty schema. Two consequences worth naming: - SettingsService had two branches for the two object types, and the merge inverts their order: `amef_organization_schema` points at the ArchiMate schema — the one now absorbed and empty — so preferring it would resolve every organisation lookup to nothing. The merged voorzieningen schema wins; AMEF is the last resort. - phpstan reported three NEW errors that are not new: the baseline carried them keyed on the old identifier, so the rename changed the message and the entries stopped matching. Re-keyed, not re-suppressed. Verified against a control: phpstan clean, psalm "No errors found", phpmd clean, phpcs 0 errors on the new step, jest 120/120, vitest 226, l10n and the manifest/vue-demi validators pass, register internally consistent (no dangling $refs, no key/slug mismatch, both registers' lists and their configuration.schemas maps agree). Dutch names 7 -> 6. Note: `isFullyConfigured()` and `$slugToKey` already referenced an `organization` slug the voorzieningen register did not have. Both resolve correctly now, which may also be what the E2E seed has been failing on — but that job is red on development and cannot run locally, so CI decides. * fix(softwarecatalog): resolve the config key through one map, not by derivation CI's PHPUnit matrix went red on all six cells. Three separate causes, and all three are the same shape: something DERIVED the app-config key from the object type, and the slug half of that pair was just translated. - `getRegisterIdForObjectType()` builds `$objectType . '_schema'`. With slugs English and the stored keys still Dutch that lookup misses, and a miss is not an error — it returns null and the caller reads "not configured". Ratings and every catalog type resolved to no register. - `FacetService::fetchBaseObjects()` did the same and returned an empty facet set, which renders as a page with no filters rather than a failure. - `catalog-ratings.json` still declared the schema map key `beoordeeling`. The by-hand fix for map keys covered the main register file only; the register.d fragments were not swept. Found systematically this time. All three now go through `SettingsService::LEGACY_SCHEMA_KEY`, the one place that knows slug and stored key diverge. FacetService reads it as a CONSTANT rather than calling the service: it takes SettingsService as a collaborator and every test mocks it, so a method call there returns the mock default and misses in exactly the tests meant to catch this. Also: one assertion hardcoded the old name inside a REGEX literal (`/module.*dienst|dienst.*module/`) — a position no rename pattern reaches. The control I used before this was WRONG. `git stash` reverts uncommitted work only, and this branch already had three commits, so what I compared against was my own branch. It reported 7 pre-existing failures where a real control from `origin/development` reports none. Rebuilt with `git archive origin/development`. Verified against that real control, full unit suite both sides: 684 tests, 1 error, 25 skipped on BOTH — same count, so nothing silently stopped running, and the one error (Symfony HeaderUtils missing under the unit bootstrap) is identical on development. * test(softwarecatalog): move the slug step's decisions where they can be tested The stable34 cell failed on the coverage guard, not on a test: coverage dropped 0.08% against the merge base because the new repair step is ~150 statements a unit suite cannot reach — it needs a database. Same answer as #516: the parts that are decisions rather than DDL move to an injected collaborator, and the step keeps only the part that talks to the database. Four pure predicates now, each with the failure it prevents written down: - `plan()` — which slugs may be renamed. Carries its own earlier renames forward, so two entries aiming at one target cannot both read as safe and collide at the database instead. - `mayRetire()` — only a schema PROVEN empty may be retired. A negative count means the count could not be taken, and an unreadable table must never be mistaken for an empty one. - `schemaIdsFrom()` — the registers' `schemas` JSON column, read defensively. Null, malformed JSON and non-numeric entries all yield no ids rather than a fatal, because an exception here aborts an upgrade. - `isShardTableFor()` — `LIKE %_table_%_3` also matches `_table_3_13`, and counting another schema's rows would make an empty schema look occupied and refuse a merge that was safe. Full unit suite 691 tests (was 684 on development, +7 new), same single inherited error (Symfony HeaderUtils missing under the unit bootstrap), 25 skipped. phpstan, psalm and phpmd clean. * style(softwarecatalog): document the injected decisions parameter phpcs wanted a @PARAM for the new constructor argument. Both new files are now 0 errors; InitializeSettings' line length and the columns test's 41 errors are identical on development. * test(softwarecatalog): move the organisation-pair lookup into the decisions The coverage guard went 0.08% -> 0.01% after the first extraction, still just under the merge base. Picking the two organisation schemas out of the schema rows is another pure decision, so it moves too — and it earns its test: a MISSING second schema is an ordinary outcome (the merge already ran, or this install never had it), not an error path. 692 tests, same single inherited error. phpstan, psalm clean; 0 phpcs errors on all three files. * fix(softwarecatalog): the e2e seed asserts schema SLUGS, so they moved too The seed verifies that the import produced the schemas the fixtures need, by slug. Those slugs are exactly what this branch renamed, so the check failed with "schemas missing after import: organisatie, contactpersoon, moduleVersie, kwetsbaarheid". That list is the only place outside the register JSON that names them, and it is checked AFTER the import — which is why it caught the rename here rather than in a spec, and why it is worth a comment saying so. The seed's OTHER check, on `<slug>_schema` app-config keys, is left alone on purpose: those keys are deliberately still Dutch and `_fixtures.ts` reads the same ones, so both stay consistent. Note the failure MOVED rather than appeared: development fails this job earlier, at the config-mapping check, because `isFullyConfigured()` and `$slugToKey` looked up an `organization` slug the voorzieningen register never had. Getting past that to a later assertion is the merge doing its job. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…ate them (#520) * refactor(softwarecatalog): translate the stored enum values, and migrate them Values are the other half of this programme, and the quieter half. Renaming one in the schema changes the DECLARATION; every row already written still holds the Dutch string, and a filter on the new value then returns NULL rather than an error — so the feature reports "nothing found" instead of failing. `RenameDutchCatalogValues` rewrites the stored rows, scoped by COLUMN and idempotent. Scoped by column, never by the string alone: `intern` is a connection's integration type here and a statutory ZGW confidentiality value elsewhere, and `Concept` is a lifecycle state on one column and an ordinary word on the next. Held back, each for a reason rather than a hunch: - `roles` — its members MIRROR NEXTCLOUD GROUP NAMES that ContactpersonenController checks with `isInGroup('gebruik-beheerder')`. Renaming the enum while the groups keep their names desynchronises the two, and the groups are instance data an administrator created. - `digikoppeling`, licence names, and the abbreviations (SLA, DVO, IaaS, BBN1-3) — proper names. Only the Dutch WORDS inside a licence move: `Licentie` -> `License`, `versie` -> `version`. - `samenwerkingtype` as a member of its own enum — the property name leaked into the value list. That is a data defect, not a translation. Two tooling defects this shook out, both found by tests rather than by reading: - The enum rewriter matched its property block with a regex that tolerated ONE level of nested braces, so any property nesting deeper was silently skipped — `service.type` was. The test asserting the old value is gone caught it; nothing else would have. Now brace-matched. - The assignment rewriter built its replacement and then rewrote the first quote character, emitting `'value"` and breaking seven PHP files. Verified against a control: PHPUnit 696 vs 684 (+12 new), the single remaining error identical on development (Symfony HeaderUtils missing under the unit bootstrap). phpstan, psalm, phpmd clean; 0 phpcs errors on the new files; jest 120; vitest 226; l10n and prettier clean; no duplicate JSON keys. Dutch enum values 45 -> 8. * test(softwarecatalog): cover the decisions class the value test uses testPropertiesSnakeToRealColumnNames calls RenameDutchCatalogDecisions to check the map's properties snake down to the columns the migration will UPDATE. PHPUnit's strict coverage marks a test RISKY when it executes a class @Covers does not name, and one risky test fails the whole cell. Fixed before CI reported it — procest #849 hit exactly this an hour ago, and softwarecatalog #516 before that. It only ever shows up in CI: there is no coverage driver in the container. * test(softwarecatalog): plan the value rewrites where they can be tested The coverage guard failed stable34 by 0.03%: the value step is DB code a unit suite cannot reach. Working out WHICH rewrites a table needs is not DB code though — it is a decision, and it moves to the collaborator with the rest of them. It earns the test on its own merits. Shard tables are per-schema, so most carry only a few of the mapped columns, and an UPDATE against a column the table lacks is an error rather than a no-op. 698 tests (was 684 on development), the single remaining error identical there. phpstan and psalm clean. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
ADR-083 added a constructor parameter; the test constructions still passed the
old argument count:
ArgumentCountError: Too few arguments to __construct(),
N passed and exactly N+1 expected
Each site gains one argument BY NAME, which fills the right slot whether the
preceding arguments were written positionally or by name — so the same edit
works for both shapes, and a call short by more than this one parameter still
errors, correctly.
Every touched file is re-parsed with php -l and reverted on failure, and a
re-scan reports 0 remaining sites in each app.
phpstan caught a defect in the rollout transformer:
PHPDoc tag @var for property $objectService with type
OCA\OpenRegister\Service\ObjectServiceInterface is not subtype of native
type OCA\OpenRegister\Contract\ObjectServiceInterface
The docblock rewrite matched `@var \OCA\OpenRegister\Service\ObjectService` and
appended `Interface` to the CLASS name while leaving the NAMESPACE alone, so the
declared type named a class that does not exist. The native type next to it was
correct, which is why only phpstan noticed — PHP itself never reads the docblock,
and the tests pass either way.
That is the fifth silent failure from this transformer, and the same shape as
the others: it produced plausible output that no runtime check disagreed with.
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>
The catalog register's slug moves with the app's identity, and a repair step renames the existing row BEFORE the import so the rename lands on the register that already holds the data. Why the repair step is not optional. OpenRegister resolves a register by SLUG and by nothing else, and its not-found branch is not an error path — it is the "create a new one" path. Shipping the renamed slug in the register JSON alone would therefore rename nothing: the import finds no match, CREATES A SECOND, EMPTY REGISTER, and every stored object stays behind on the old row, reachable by nothing. Nothing errors; the app just looks new. Why it moves no data. An object is bound to its register by NUMERIC id — every shard table's `_register` column holds the id, and the tables are named `oc_openregister_table_<registerId>_<schemaId>`. The slug appears nowhere in the physical layout, so this is a one-column UPDATE on one row (id 11 on the dev instance, which it keeps). The step is idempotent, non-destructive, and never throws (it runs under <install>, where an escaping exception aborts the install and the app never enables). When both the old and new slugs already exist it REFUSES and renames neither — merging two registers is a decision about data, not a rename. It also re-points a stored app-config `register` value that still holds an old slug, guarded on the VALUE rather than the key. `vng-gemma` deliberately stays exactly as it is: it holds VNG GEMMA reference data, is not this app's own store, and its name is answered to elsewhere. Also fixes an inconsistency the app-id rename left behind: `x-openregister.app` has said `stackiq` since #708 while the register row still said `voorzieningen` — which is precisely the fork this step exists to prevent. Installs that have imported since then may already carry an empty `stackiq` register, and the step refuses rather than merging on those.
…ace moved (#728) * fix(decidiq): contract delegation was silently off — the event namespace moved `ContractApprovalService` pinned `\OCA\Decidesk\Event\DecisionRequestedEvent`. The decision app renamed to `OCA\Decidiq` with no compatibility alias, so `isDelegationConfigured()` began returning false on instances where the app was installed, and contract approvals stopped delegating. Nothing reported it, and nothing could: `class_exists` going false is exactly what an uninstalled optional app looks like, which is the case the guard was written for. The feature reads as "not configured" rather than "broken". Measured on a running instance: OCA\Decidesk\Event\DecisionRequestedEvent MISSING OCA\Decidiq\Event\DecisionRequestedEvent EXISTS The constant is now a LIST, newest first, resolved to the first that exists. The old spelling stays until no supported install ships it — dropping it would break the integration in the other direction during a staggered upgrade, which is the window this broke in to begin with. An app cannot move another app's class name; it can only follow it. So the test asserts that property rather than a literal: the list names the current namespace, tries it first, and still names the old one. Pinning one spelling fails a test now instead of silently disabling a feature later. NOT changed: `SOURCE_APP = 'softwarecatalog'`. That is this app's id AS THE DECISION APP KNOWS IT and is echoed back on the conclusion event, so it moves only when both sides move together. Found by a fleet sweep for stale cross-app namespaces after the same defect was confirmed in dossiq; openregister (→ Keepiq) and decidiq (→ Filinq) carried it too and are fixed in their own repos. * fix(psalm): resolve the event class ONCE, and narrow it where it is used Psalm rejected the previous commit: `Type null cannot be called as a class` at `new $eventClass(...)`. It was right, and the shape it caught is worth naming. The method called `isDelegationConfigured()` — which resolves the class and throws away the answer — and then resolved AGAIN at the call site with no guard. Two lookups of the same thing, only the first of them checked. The second had a `?string` reaching `new`, so an instance with no decision app installed would have gone from a clear "delegation is not available" to a fatal on a null class name. Now it resolves once, narrows, and fails closed on the null — the same refusal as before, from the value actually used.⚠️ I did not run psalm locally before pushing; I ran phpunit, phpcs and phpstan, and phpstan does not flag this. One tool green is not the gate green, and the tools disagree by design. psalm "No errors found", phpcs 0, phpstan [OK] on the touched file.
#724) The final step had no `task`, so the guided tour stopped without telling the user where to go next. It now closes on the documentation, per the fleet rule that a walkthrough's last step points somewhere. The CTA targets the `Documentation` nav item that already exists in this app's menu, so it lands on a real destination rather than a URL invented for the copy. The same step also carried voice defects the shared writing skill bans: "Nicely done" is praise rather than voice, the em-dash is stripped fleet-wide, and "reopen this tour anytime from the … menu" is housekeeping in the one line a user is most likely to act on. The title now states what the user actually has, and the body says what to do with it.
…es this site (#726) * fix(docs): publish from development, to the worker that actually serves this site Two silent failures, both of which had to be fixed before this site could update at all. 1. The workflow triggered on `documentation`, a branch that exists but nobody updates. Green and idle for months while the live site aged. 2. `worker-name` was never passed, so the callee derived it from `cname`. Since the app-id rename `cname` is the NEW host, while the worker that actually holds the custom domains is still named after the OLD app id. The derived name points at a worker that does not exist — deploying it CREATES a second worker while both custom domains keep routing to the original. Every deploy green, reaching nobody, with the live-site verification added in ConductionNL/.github#555 as the only thing that would ever have noticed. Measured today: both hostnames still serve the pre-rename title while docs/docusaurus.config.js has carried the new one since the rename. Nothing has carried a build to the edge. * fix(docs): ship the og:image file the config already names The docs build FAILS, and has been failing — it was simply never run, because the workflow triggered on a branch nobody updates. Making the trigger correct surfaced it on the first run. The app-id rename updated `docusaurus.config.js` to point og:image at the new filename and left the actual PNG under its old name, so the AI-baseline validator's last check fails: ✗ og:image URL resolves to a file in the build and `npm run build` exits 1 via postbuild. Nothing could have published even with a correct trigger and a correct worker. Renames the asset to the name the config has been asking for. No references to the old filename remain. Verified locally: npm ci --legacy-peer-deps && npm run build now exits 0 with all 10 AI-baseline checks passing.
GitHub is the only host for this org — Codeberg was a mirror and is no longer used, including for issues. Many of these links also carried a PRE-RENAME repo name: this repo is now ConductionNL/stackiq (was softwarecatalog), nldesign is thematiq, OpenConnector is integriq, and decidesk is decidiq. Converted (24 files): - 21 PHP @link docblock tags -> github.com/ConductionNL/stackiq (13 files) - 4 PHP @link tags naming OpenConnector -> github.com/ConductionNL/integriq - 8 README dependency links (openregister, opencatalogi, nldesign->thematiq, tilburg-woo-ui, launchpad) - docs/static/llms.txt org link -> github.com/ConductionNL (its label already said "GitHub") - .forgejo/workflows/tests-live.yml: the live `git clone` of openregister was still pointing at codeberg.org — CI would clone from a dead host. - 4 hydra change records ("repo" field, issue-less only) Issue / PR numbers deliberately NOT mapped. Codeberg numbers do not correspond to GitHub ones, so rewriting only the host would point at a real but unrelated GitHub item. The two decidesk PR-160 links are replaced with plain text. Deliberately left alone (see PR body): - 32 "configuration" URLs in lib/Settings/softwarecatalogus_register.json. These are RUNTIME-fetched register config pointing at opencatalogi/.../publication_register_magic.json — a file that does not exist in ConductionNL/opencatalogi on ANY branch. Repointing the host would swap a dead Codeberg URL for a dead GitHub one and merely look fixed. - 4 hydra records whose codeberg "repo" sits next to an issue number/URL. - The beta-surface-alignment openspec requirement that MANDATES the word "Codeberg" in docs/GOVERNMENT-FEATURES.md, and that doc line. - .forgejo runner labels and the CODEBERG_TOKEN secret reference.
This repository's only issue forms lived under `.forgejo/issue_template/`. GitHub is the fleet's only host, so those forms are invisible to everyone filing an issue here. Two approved fleet changes make this urgent: 1. `.forgejo/` is being removed fleet-wide. Without this port that removal would delete the only issue forms this repo has, leaving contributors with a blank issue box. 2. The shared library's `DEFAULT_FORGE` moves from `codeberg` to `github`. The in-product "Request a feature" deep-link then targets a GitHub Issue Form named exactly `feature-request.yml`. If that file is absent GitHub silently drops every pre-filled field instead of erroring, so the app context (app, page, surface, object, spec-ref) would be lost without a single visible failure. Copies all four templates to `.github/ISSUE_TEMPLATE/`, keeping the filenames identical. `.forgejo/` is deliberately left untouched; its removal is a separate later change. Conversion is lossless: Forgejo's issue-template schema is derived from GitHub's, and every construct used here (markdown/input/textarea/dropdown blocks, `render: shell`, `validations.required`, `labels`, `assignees`, `title`) is valid GitHub issue-form syntax. Nothing was dropped or reworded. The top-level `type: "Feature"` in feature-request.yml was verified against the ConductionNL org issue types, where "Feature" exists and is enabled. No `config.yml` was added: `.forgejo/issue_template/` has no equivalent.
'quality / E2E Tests (Playwright)' has been red on development. The product
is fine; the assertion was ambiguous.
Each component row renders the NAME and its purl, so getByText('lodash')
substring-matched two spans — <span>lodash</span> and
<span>pkg:npm/lodash@4.17.21</span> — and Playwright's strict mode failed
the assertion rather than choosing one. Adding the purl column is what made
a previously-unique string match twice, so this broke without either the
component or the test being edited.
The express assertion had the identical defect and never ran: the lodash
line failed first and masked it. Fixing only the reported line would have
surfaced express as the next failure. Verified against the fixtures —
cyclonedx-1.5-valid.json is express/pkg:npm/express@4.19.2 and vue.
The absence assertion is deliberately left as a substring match: for
'is it gone' the looser match is the STRONGER claim, since it also fails if
the purl survives, and toHaveCount tolerates multiple matches so strict mode
never applies there.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…stackiq application id (#723) * fix(repair): move OpenRegister schemas from softwarecatalog onto the stackiq application id OpenRegister resolves a REGISTER by slug alone, but a SCHEMA by the PAIR (application, slug) via SchemaMapper::findByApplicationAndSlug(). This app passes appId: Application::APP_ID = 'stackiq', while every schema it already owns still carries application = 'softwarecatalog'. The pair matches nothing, and ImportHandler's not-found branch is not an error path — it is the create-a-new-one path. The next import therefore builds a second, EMPTY schema set under the new application id while every stored object stays bound to the old rows. Nothing errors; the app renders empty collections. Measured on a live install: 21 schemas under softwarecatalog, zero under stackiq, and zero slug collisions — all 21 move cleanly. A DIFFERENT COLUMN from RenameDutchSchemaSlugs (that step rewrites openregister_schemas.slug, this one openregister_schemas.application), and the third store keyed by app id after oc_appconfig and oc_preferences. Registered after RenameDutchSchemaSlugs so the collision check sees the final slugs, and before InitializeSettings because that triggers the register import. The step refuses rather than merges where a slug already has a twin under the new application id, distinguishes a FAILED READ from an EMPTY RESULT, never deletes a schema and never throws — it runs under <install>, where an escaping exception aborts the install. * fix(repair): run RenameDutchSchemaSlugs before the schema re-point on install E2E failed on one test: the SBOM component table showed "lodash" twice where the second import should have replaced the first. Development is 95/0; this branch was 94/1, and the pre-merge commit on this branch was green — so the merge introduced it. Cause is a missing prerequisite in <install>. MigrateSchemaApplicationId says of its own position: POSITION IS LOAD-BEARING IN BOTH DIRECTIONS. After RenameDutchSchemaSlugs, so the collision check is judged against the slugs the rows will actually carry; before InitializeSettings, which is what triggers the register import. <post-migration> honours that — MigrateRegisterSlug, RenameDutchSchemaSlugs, then MigrateSchemaApplicationId. <install> ran the first and third and skipped the second, so on the fresh-install path the collision check was judged against pre-rename slugs. That path is not an edge case: an app-id rename presents to Nextcloud as a fresh install, which is the whole reason this block repeats the steps. The failure mode follows from the step's own contract — it refuses rather than merges, leaving two rows where it sees a twin. Two schema rows is two component rows, which is the duplicate "lodash" the test caught. I had also claimed in the merge commit that the ordering constraint was against MigrateRegisterSlug. It is not; it is against RenameDutchSchemaSlugs, a different class rewriting a different column.
GitHub is the only host this organisation publishes to. No local checkout has a Codeberg git remote, so nothing is pushed there and no workflow under .forgejo/ has ever run for this repository. Issue templates: the 4 templates under `.forgejo/issue_template/` were already ported to `.github/ISSUE_TEMPLATE/` and were verified present there before deletion (including `feature-request.yml`, which the in-product "Request a feature" deep-link targets by that exact filename). Release workflows: the deleted `.forgejo/workflows/` release jobs (release-beta.yml release-stable.yml ) are superseded by `.github/workflows/release.yml`, which is the live release path for this repository. .github/workflows/ is untouched — that is the live CI. Any CODEBERG_TOKEN reference lived only inside the deleted files and goes with them. Removes 12 file(s) under .forgejo/.
* fix(docs): pass secrets to the reusable documentation workflow A called workflow receives no secrets from its caller unless they are passed explicitly or inherited. Without `secrets: inherit` the callee sees an empty `secrets.CF_API_TOKEN`, its "Publish to the Cloudflare Worker" step skips itself on its own guard, and the run finishes green having written only gh-pages — which nothing serves. The live docs site never changes and no check goes red to say so. Measured on planninq run 32715324775: all three jobs green, GitHub Pages deploy success, Worker publish skipped, warn step reporting the Worker was not updated. * fix(docs): map the Cloudflare secrets explicitly instead of inheriting all `secrets: inherit` handed the reusable documentation workflow every secret this repo holds — the Nextcloud signing cert and key, the appstore token, the deploy keys — for the sake of two Cloudflare values. It also would not have worked. The org secrets are CLOUDFLARE_API_TOKEN / CLOUDFLARE_ACCOUNT_ID and `inherit` passes secrets under their original names, while the callee reads CF_API_TOKEN / CF_ACCOUNT_ID — so the publish step would still have skipped itself and the run would still have gone green over an unchanged live site. Maps the two names explicitly instead, so nothing else crosses the boundary. Depends on ConductionNL/.github#568, which declares both as optional secrets on the callee: an explicit mapping only compiles for names the callee declares. * fix(docs): map the Cloudflare secrets from the names that actually exist This branch introduced the mapping reading `secrets.CLOUDFLARE_API_TOKEN` / `secrets.CLOUDFLARE_ACCOUNT_ID`, which are not secrets anywhere in this org. Mapping from a non-existent secret is not an error - it yields an empty string - so the callee`s "Publish to the Cloudflare Worker" step would have skipped itself on its own guard and the run would still have gone green, exactly the failure this PR set out to fix. Caught before merge; measured on planninq run 32760529026, where the same spelling did land. The real org secrets are CF_API_TOKEN / CF_ACCOUNT_ID, the same names the callee declares and the same ones ConductionNL/.github deploy-docs.yml reads directly. Only the mapping values change; the keys stay. The comment claimed the names differ on each side. They do not, and that claim is what produced the wrong values. Replaced with the reason that still holds: `secrets: inherit` would hand the callee every secret this repo holds for the sake of two Cloudflare values.
Closes #739. Running the repo's own lint fixer produced a file the repo's own format check rejected, so fixing lint the documented way landed a red `Frontend Check (format)`. It is not a rule disagreement, and eslint-config-prettier cannot prevent it. That package only turns rules OFF, and the culprit is not a formatting rule: `import-extensions/ban-inline-type-imports` rewrites an import, and its AUTOFIXER emits its own text — import type {Page} from '@playwright/test'; — which is not what prettier wants (`{ Page }`, no semicolon). Any autofixer that constructs source can do this; disabling formatting rules does not stop it. So `lint-fix` now re-runs the formatter afterwards, which makes the two tools ordered instead of competing: eslint decides what the code SAYS, prettier decides how it LOOKS, in that order, always. That matches the config's own stated doctrine — "exactly one of them is allowed an opinion and prettier is it". Verified both directions on a clean tree: `eslint --fix` alone leaves prettier --check REJECTING, and the chained script leaves it passing. WORTH KNOWING, and the reason this went unnoticed: the two tools do not cover the same files. lint -> eslint src (src only) format -> prettier "**/*.{js,ts,vue,css,scss}" (everything, incl. tests/) `tests/e2e/**` is format-checked but never linted, so an eslint --fix run there is outside the workflow CI exercises — which is exactly where I hit this. Whether to widen eslint's scope to tests/ is a real decision with a findings backlog behind it, so it stays in #739 rather than riding along here.
…m repo root (#743) The 2026-08-24 fleet structure audit found softwarecatalog carrying 63 root files beyond the nextcloud-app-template baseline — the largest root in the fleet, against a fleet median of ~40. Most were one-off investigation scripts and status notes from finished work. Removed (39 files, ~6.5k lines): - 27 one-off scripts: check_*.php, debug_*.php, find_objects_*.php, cleanup_*.php, enhance_archimate_service.php, and the test_*/test-* shell and PHP scripts superseded by test_archimate_unified.sh or by the Newman/Postman suite. - test_small_archimate.xml — an unreferenced fixture; the kept ArchiMate suite reads lib/Settings/GEMMA_release.xml instead. - 11 point-in-time status docs: AMEF_TESTING.md, ARCHIMATE_IMPORT_FIX.md, ARCHIMATE_PROJECT_STATUS.md, CIRCLE_TEST_DOCUMENTATION.md, FIX_VERIFICATION_SUMMARY.md, INTEGRATION_TEST_RESULTS.md, KOPPELINGEN_GEBRUIK_REFACTOR.md, README_DEBUG.md, WORKFLOW-UNSTABLE-RELEASE.md, Openregister.md, aanvullende-informatie.md is retained (issues.md links it). Deliberately KEPT, because each is live-referenced: - test-setup.sh — invoked as `bash stackiq/test-setup.sh` by .claude/commands/test.md and six persona test skills. - compare_archimate.php — documented in docs/ARCHIMATE_QUICK_TEST.md and docs/ARCHIMATE_TESTING_GUIDE.md (4 call sites). - compare_archimate.py — invoked by test_archimate_unified.sh:260. - test_archimate_unified.sh — the consolidated ArchiMate suite. - issues.md — NOT scratch. 3,659 lines tracking 137 VNG IGS issues and 1,026 acceptance criteria; README.md calls it "the master file", it is listed in .distignore, and nine .claude commands/skills read it to drive the persona test suite. - BUG_FIX_ORGANISATION_USER_ASSIGNMENT.md — named by open task 4.1 of the active openspec change organisation-parent-hierarchy-rbac-fix. test_archimate_unified.sh's header comment is updated so its "this replaces" list reads as history rather than pointing at files that no longer exist. Root file count: 96 -> 57. Refs ADR-099 Decision 2 (the repository root is a closed set).
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Part of the 2026-08-25 fleet structure audit (ADR-100 Decision 2: the repository root is a closed set; generated files are never tracked). Ignore rules added: .stale/ /.e2e-state/ .phpunit.cache `.stale/` was missing from ALL 19 fleet repos and is the one that matters most operationally: agent scratch there grew unbounded and filled the dev disk once already. Refs ConductionNL/hydra ADR-100. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Every `@self.configuration` in this register pointed at https://codeberg.org/Conduction/opencatalogi/raw/branch/master/ apps-extra/opencatalogi/lib/Settings/publication_register_magic.json which returns **HTTP 404**. Verified with curl, not assumed from the host name. Three separate things had rotted at once: host ...... codeberg is no longer where opencatalogi lives path ...... the old monorepo layout `apps-extra/opencatalogi/...` is gone; the app is at its repo root now filename .. `publication_register_magic.json` was renamed to `publication_register.json` So repointing the host alone would still have 404'd. The replacement is the live file, confirmed HTTP 200 on main, master and development; `main` is the repo's default branch and is what this now uses. This matters because a `configuration` URL is FETCHED at register-import time. A 404 there does not announce itself as a broken link — the import simply does not get the configuration it asked for, which is the quiet-failure shape this codebase has been bitten by before. 32 occurrences, all byte-identical, replaced in one pass. JSON re-parsed clean afterwards.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Found by gate-96 (manifest-copy-style, ConductionNL/.github#581). Two getting-started tour steps, both already real user copy. The second takes a colon rather than a period, because the clause after the dash explains what "the heart of your catalogue" means and belongs to the same sentence: "Organisations are the heart of your catalogue: they own the contracts, modules and compliance records you track." The welcome step also loses the comma splice the dash was hiding, joining with "and" instead of leaving two sentences fused. Verified: gate-96 0 findings over 153 strings, check:manifest PASS, test:l10n PASS, check:schema-l10n PASS.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
* feat(nav): a Flows surface in this app, on the shared page types ADR-110 Decision 4. A flow is app-specific — it operates on this app's objects — so the authoring surface belongs here rather than behind a deep link to another app's list. The ENGINE stays single (ADR-065): these pages are a scoped view onto OpenRegister's one native flow store, not a per-app store. Two manifest pages and one settings entry, no component files: `type: "flows"` and `type: "flow-detail"` are shipped page types in @conduction/nextcloud-vue 2.19.0, scoped by `config.app`. Note the layout of the diff: entries are appended textually rather than by reserialising the manifest. A `json.dump` round-trip rewrote pipelinq's file as a 3,950-line diff for a 20-line addition — correct output, unreviewable change. * build(deps): @conduction/nextcloud-vue 2.19.0 for the flows page types Required by the manifest change: `type: "flows"` / `type: "flow-detail"` are rejected by the compiled validator in earlier versions, and CI installs with `npm ci` — so the LOCK is what decides, not the `^2.x` range. Several of these locks were pinned many minors back, which is why some lockfile diffs are large: npm restructures the nested tree (mostly @esbuild platform binaries under @nextcloud/vue) to satisfy 2.19.0's peers. No direct dependency other than @conduction/nextcloud-vue changes. * fix(icons): register Sitemap, or the Flows entry renders with no icon An icon name a manifest uses but src/icons.js does not register renders as NOTHING — not a fallback (ADR-077 rule 3). The Flows menu entry this PR adds uses `Sitemap`, and this app never registered it, so the entry would have shipped with an empty icon slot. Caught by gate-60 icon-vocabulary. I had checked `Sitemap` was registered in dossiq and carried the assumption to the fleet; each app keeps its own icons.js, and six of the twelve did not have it. The six failing gate runs were exactly those six apps. Both halves are required: the import alone is dead code, the registry entry alone does not resolve.
…Codeberg workflows, keeps beta's version)
Release: merge development into beta
beta is the release candidate, 1026 commits ahead of main. Every conflict resolved to beta's side, including its refactors: where beta had removed a file the removal stands rather than resurrecting a stale copy from main. Conflicts: 4 (0 took beta's content, 4 removed per beta's refactor).
Contributor
Quality Report — ConductionNL/stackiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue-demi | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| composer | ✅ | ✅ 130/130 | |||
| npm | ✅ | ✅ 720/720 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ✅ | ||||
| Hydra gates | ❌ |
Quality workflow — 2026-08-30 13:45 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stable release.
betawas 1026 commits ahead ofmain.Conflicts were resolved to beta's side, including its refactors: where beta had removed a file, the removal stands rather than resurrecting a stale copy from main. No conflicts.
git merge -X theirssettles content conflicts but leaves modify/delete ones unmerged — beta deleted the file, so there is no "theirs" blob to take. Those were resolved by honouring the deletion.Verified before pushing: the commit has exactly two parents, and no conflicted path was left unresolved. Files main keeps that beta never had (archived openspec docs, whitespace-only differences) are preserved — "beta wins" governs conflicts, not additions.
A failing
… / releasecheck here is the App Store publish step, not a quality gate: 7 apps have no signing key and thematiq's certificate carries its old app id. The GitHub release and tag are still created.