chore(deps): Nextcloud 35 support (max-version 34 -> 35) - #2065
Merged
Merged
Conversation
The reason recorded for capping at 34 was that "NC 35 does not exist —
there is no tag, no branch". nextcloud/server now carries stable35 and
nextcloud/ocp has shipped v35.0.0, so it has expired.
Checked rather than assumed, against the v34.0.4 / v35.0.0 stub surfaces:
- Of the 11 OCP classes removed in 35 — OCP\Remote\* (IInstance,
IInstanceFactory, ICredentials, IUser, Api\{IApiCollection,IApiFactory,
ICapabilitiesApi,IUserApi}), Collaboration\AutoComplete\AutoCompleteEvent,
Files\Cache\{CacheInsertEvent,CacheUpdateEvent} — integriq references
none, by fully-qualified name across lib/ and tests/.
- The migration break that NC 35 introduced (ISchemaWrapper returns
OCP\DB\Schema\ITable where Doctrine\DBAL\Schema\Table used to be, which
is what ConductionNL/openregister#3775 exists to fix) cannot reach this
app: it ships one migration, Version2Date20260908000000, and that file
type-hints no table.
- symfony/console and symfony/http-foundation are the two packages NC 35
core ships that an app can collide with. Both are already required at
^7.4 here; stable35's 3rdparty declares ^7.4.15.
DO NOT MERGE BEFORE ConductionNL/openregister#3777. openregister is a hard
dependency and still declares max-version="34", so on NC 35 it refuses to
install and every integriq entity — each one an OpenRegister object — has
nowhere to live. This is correct about integriq and premature about the
stack until that PR lands.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit moves appinfo/info.xml to NC 32-35 while nextcloud-test-refs still pinned '["stable34", "stable32", "stable33"]', so stable35 was advertised to the App Store and exercised by nothing — gate-65 rule 11, and the only mechanical failure this branch would have introduced. Deleting the override rather than appending "stable35". Both produce the same matrix today and the same single-server, so the difference is the next bump: the gate fires on the manifest/matrix disagreement itself, not on the diff that caused it, so an appended list re-arms the trap for NC 36 and arms it on development rather than on the PR doing the bump. openregister#3777 took the same decision and its reviewer preferred it to the appended list he had originally asked for. The ordering the old list encoded is inert now. E2E, newman and journeydoc-capture used to read `fromJSON(nextcloud-test-refs)[0]`, which is why stable34 had to lead — openregister declares min-version="32" and an older leg failed ~70s in on missing tables (#1172). They now consume the workflow's `single-server` output, and the floor is carried by info.xml's min-version="32" instead of by a hand-written array. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rjzondervan
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rubenvdlinde
as code owners
September 18, 2026 08:25
Contributor
Quality Report — ConductionNL/integriq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ❌ | ||||
| psalm | ✅ | ||||
| phpstan | ❌ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ❌ | ||||
| check-schema-l10n | ❌ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 141/141 | |||
| npm | ✅ | ✅ 545/545 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ❌ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it on any branch from the Actions tab, or locally with npx playwright test. |
||||
| Hydra gates | ❌ | ||||
| Hydra gates (axe) | ✅ |
Quality workflow — 2026-09-18 08:29 UTC
Download the full PDF report from the workflow artifacts.
The stable35 leg this branch adds reported 8 errors, all one cause: TypeError: Cannot assign MockObject_IAppContainer to property OCP\AppFramework\App::$container of type OC\AppFramework\DependencyInjection\DIContainer NC 35 declares `private DIContainer $container`; NC 32-34 leave the property untyped. Reflection assignment still enforces a declared type, so mocking IAppContainer and reflecting it in works on the older half of the declared range and throws on the newer one. Five tests in ApplicationStorageMigratedTest and three in ApplicationWorkflowEngineOperationsTest hit it. Rather than naming either class, AppContainerInjection reads the declared type back off the property: NC 35 gets a DIContainer double, NC 32-34 get the IAppContainer they always got, and one test body stays correct across the whole range. DIContainer implements IAppContainer, so the get()/query() surface these tests stub is present either way. The declared type is used only when it is loadable. DIContainer is an OC\ class rather than OCP\, and this suite already stubs several of those; if it cannot be autoloaded, a double of it could not be built anyway, so the helper falls back to the previous behaviour instead of failing on a class that is not there. NOT verified locally: integriq has no vendor/ installed here and no phpunit to run, so this rests on CI. openregister carried the identical fix for the identical NC 35 change (its BuiltinProviderDiFactoryTest), verified there against real ocp 32/34/35 trees. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`quality / Frontend Check (format)` is red on both the NC35 branch and the development->beta release PR. These sixteen spec files are the reason: they arrived with the intake, outbound, migration and gateway features and were never formatted, because development's own CI is bypassed on this gate. Formatting only — no assertion, fixture or expectation changed. `prettier --check` over the whole set is clean afterwards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Small, unrelated-to-each-other fixes that the phpstan run names: FileMigrationSource::readFile() asked IRootFolder::get() for a node and called getContent() on whatever came back. A path resolving to a folder reached getContent() anyway and landed in the generic Throwable arm, which reports "could not be read" — true, but it hides that the real answer is "that path is a directory". It now narrows to File and says so, with its own catch arm ahead of the Throwable one, because without that the generic catch swallows the specific message. DigikoppelingBrokerResolver and SubscriptionRoster drop an elvis operator each, where the shorthand made a null and an empty string indistinguishable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two ratchets move in opposite directions, and the phpmd one deserves the explanation. phpstan-baseline.neon SHRINKS, 417 entries to 362: the three lib fixes in the previous commit clear their own entries, and the rest go because the merged code no longer trips them. phpmd.baseline.xml GROWS, 324 entries to 367. Those 43 are not new violations written here — they are pre-existing ones in code already merged to development through #2050-#2063, in files like OutboundSecurityService, EmlParser, CompoundFileReader and ImapMailboxTransport. Mostly StaticAccess (26), ErrorControlOperator (12) and BooleanArgumentFlag (11). Development's phpmd gate is bypassed, so they were never baselined when those PRs landed and they surface on the first branch that runs the gate honestly. Baselining someone else's already-merged debt is what a baseline is for. Baselining your OWN new violations is what it is not for, and that distinction is why this commit exists separately: fixing 43 findings across eleven feature files nobody asked me to touch would turn an NC35-support branch into a refactor of other people's work, and suppressing them silently inside a bulk commit would hide that a choice was made at all. They should be worked off. They belong on development's debt list rather than in this branch's diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
l10n/en.json goes from 1158 keys to 2478. The backend catalogue is maintained by hand — there is no scanner for it, unlike the frontend set — so it had fallen behind the schema strings the newer features introduced, which is what `check:schema-l10n` reports. en.js is rebuilt from en.json by `npm run l10n:build`, per this repo's rule that the JSON is the source and the JS is generated; editing the JS directly is discarded by the next build. .schema-l10n-baseline.json drops to 0 outstanding, which is the point of the exercise: the gate now asserts the real state instead of an allowance. The other 35 locales are deliberately untouched — the decision is that integriq ships nl and en for now, and forcing values into 35 locales to satisfy parity is exactly the placeholder-shaped filler the l10n rules forbid. Disabling the parity gate for the unshipped locales is its own change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Squiz.Operators.ComparisonOperatorUsage.ImplicitTrue` flags `?:`,
because the left operand is tested for truthiness rather than compared.
Fourteen sites, plus the two singleton findings in the same pass: a
166-character line and an inline comment opening on a lowercase spec
slug.
The rewrite is to statements, not to ternaries. That is forced rather
than stylistic: `Squiz.PHP.DisallowInlineIf` bans the ternary too, so
`json_encode($x) ?: '{}'` cannot become
`json_encode($x) !== false ? ... : '{}'` — it has to be an assignment
and an explicit `=== false` check. I wrote four of them as ternaries
first and phpcs immediately flagged all four.
Most are `json_encode()` falling back to '{}' or '[]', where the check
is now against `false` specifically rather than against anything falsy —
`json_encode()` can legitimately return the string "0", which the elvis
form would have replaced with the fallback.
`RecordOwnershipService::forObject()` gets the largest change: the
"empty means absent" narrowings move out of the constructor call and
into named statements, which also makes it legible that an absent source
id is a different fact from an empty one.
phpcs errors: 450 -> 418. ImplicitTrue, LineLength and InlineComment are
now clear; RequireNamedParameters (328) and DisallowInlineIf (90) remain.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Squiz.PHP.DisallowInlineIf` bans the ternary outright. Three files were
done by hand where the shape needed judgement, and nineteen more by a
converter restricted to two provably safe shapes.
The converter got one thing wrong, and it is worth recording because the
mistake is not obvious. For an assignment it emitted:
$var = <else-branch>;
if (<cond>) { $var = <then-branch>; }
which reads as equivalent and is not: the ternary SHORT-CIRCUITS and this
does not. Two of the nineteen were genuinely broken by it —
`GatewayRegistry::register()` called `GatewayDescriptor::fromArray($entry)`
on every path including the one where `$entry` is already a descriptor,
and `MappingVersionService` called `trim($requested)` before the null
check that exists to stop exactly that, which is a deprecation on PHP 8.1+.
Both are fixed, the first with a real if/else and the second by inverting
the condition so the call sits inside it.
I checked the remaining seventeen individually rather than assuming: each
has a literal or a plain variable as its default, which cannot have a side
effect, so the flattened form is safe there. The `return` conversions were
never at risk — `if (C) { return A; } return B;` short-circuits correctly.
Two helpers come out of repeated narrowing: `InboundMessage::arrayOr()`
and `CallReplayService::requestOf()`, both for "a stored payload is
whatever was written, narrow it before use".
phpcs errors: 418 -> 390. DisallowInlineIf 90 -> 53. RequireNamedParameters
rises 328 -> 337 because the new statements are themselves internal calls.
Unit suite: 3413 tests, 1 failure — `EndoflifeDateLiveSyncTest`, which
fetches https://endoflife.date over the network and references none of the
changed files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lues These are the ones no converter could take: each ternary sits inside a call argument or an array value, so it has to be hoisted to a named statement before the expression it feeds. Mostly that improves the reading. `GraphMailboxTransport` had two ternaries on the same `$isHtml` question producing two different fields, now one branch setting both; `MailboxSourceHandler` narrowed the same nullable cursor three separate times, now once; `CallRecorder` and `MessageRecorder` each computed the same outcome string in two places. One bug caught in my own work, worth naming because neither phpcs nor `php -l` sees it: replacing a ternary with a bare variable leaves the variable UNDEFINED until you remember to add the hoist. `MessagingChannelAdapter` shipped `$timestamp,` with no assignment and both tools reported clean — an undefined variable is a runtime fault, not a syntax or style one. Found by reading back what I had written and then swept for across every changed file. phpcs errors: 390 -> 364. DisallowInlineIf 53 -> 27. Unit suite unchanged: 3413 tests, same single network failure in EndoflifeDateLiveSyncTest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DisallowInlineIf is now at zero across lib/. These last thirteen were the awkward ones: ternaries spanning several lines, two inside exception constructors where the hoist has to precede `parent::__construct()`, and two in `ChannelReportingCapabilities` that were whole return statements and became plain early returns. Two mistakes of my own in this batch, both caught by reading back rather than by a tool. A condensed edit for the two `is_scalar(...) ? ... : gettype(...)` sites produced a syntactically broken line in `DisappearancePolicy` — `php -l` did catch that one. The other was the same undefined-variable trap as the previous commit: `$receivedAt`, `$lastSeen`, `$targetSuffix` and `$declaredText` were referenced before I added their hoists, which neither phpcs nor `php -l` reports. The sweep over every changed file is clean. phpcs errors: 364 -> 350, all of which are now RequireNamedParameters. Unit suite unchanged: 3413 tests, same single network failure in EndoflifeDateLiveSyncTest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`CustomSniffs.Functions.NamedParameters` requires every argument in a call to internal code to be named, and phpcs only emits a placeholder (`method(paramName: $value)`) — it does not know the real names. So this needed a resolver: parse every method signature under lib/ (553 classes, 2106 distinct method names), then map each flagged call site to its callee and rewrite positionally. Resolution is deliberately conservative, because naming an argument wrongly is not a style defect — PHP raises "Unknown named parameter" and the call dies. `$this->` / `self::` / `static::` resolve against the current class; `Foo::` and `new Foo` against that class, walking `extends` for an inherited constructor; a bare name only when its signature is unambiguous across the whole codebase. Anything else is skipped rather than guessed, which leaves 41 sites for hand work. The first attempt got `parent::__construct()` wrong: with no rule for it, resolution fell through to the CURRENT class's constructor, so `parent::__construct($message)` in `PropertySourceException` — whose own signature is `(providerId, message)` — was rewritten to `providerId: $message`. Nine tests died with "Unknown named parameter $providerId". `parent::` now resolves to the parent, and skips when that parent is a built-in, which is the case here (RuntimeException). That mistake is the argument for doing this with a test suite rather than with a linter: phpcs and `php -l` both reported the broken version clean. phpcs errors: 350 -> 41, all remaining RequireNamedParameters. Unit suite: 3413 tests, back to the single EndoflifeDateLiveSyncTest network failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The remainder all needed inheritance the first resolver did not walk. Five exception classes (`MailboxTransportException`, `IntakeChannelException`, `IntakeRoutingException`, `MessageParseException`, `CallDispatchException`) declare no constructor of their own and extend `\Exception`, so there was nothing under lib/ to parse. `callSource` is declared on `AbstractSourceSubscriptionProvider` and called through `$this->` from a subclass that does not redeclare it. And the controllers call `parent::__construct($appName, $request)` on Nextcloud's own `Controller`. So the resolver now walks `extends` for ordinary methods as well as for constructors, and carries a small explicit table for the two signatures this app does not own. Both are stable documented APIs — `Exception::__construct(message, code, previous)` and `Controller::__construct(appName, request)` — and both are reachable only through inheritance, so there is no file to read them from. Writing them down is the honest version of what was otherwise a silent skip. phpcs across lib/ now exits 0. Total for the grind: 450 errors -> 0, across five sniffs. Unit suite: 3413 tests, unchanged, with the same EndoflifeDateLiveSyncTest network failure that predates all of this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…iq-nc35-support Three conflicts, all resolved without losing either side. lib/Migration/Source/FileMigrationSource.php — development had made the SAME fix independently: narrow to File before getContent(), and rethrow the specific refusal so the Throwable arm cannot swallow it. Only the comment wording differed. Took development's version, since it is the one already on the shared branch and mine adds nothing. l10n/en.json — both sides only ADDED keys, with zero value changes on any pre-existing key, so the union is lossless. Verified that before merging rather than assuming it: base 1134, mine +1344, development +28, union 2504. Development's structure is kept and my 1342 missing keys folded in. l10n/en.js — not merged by hand. It is generated from en.json, so it was regenerated with `npm run l10n:build` after the source was resolved. This merge does not leave the branch green, and the next commit is what fixes it: development's own phpcs gate is bypassed, so 40 phpcs errors and 30 uncovered schema strings arrive with the code it brings. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The merge brought 35 commits of code that development's own bypassed
gates never judged, so phpcs went from 0 back to 40 and 30 schema strings
arrived with no catalogue key.
phpcs, 40 -> 0. Twenty-seven were named-parameter sites, taken by the
same resolver as before. The remaining thirteen were hand work, and two
are worth naming:
`EnvironmentValueSource` had `$this->environmentRead === null ? getenv($key)
: ($this->environmentRead)($key)` — flattening that the way the converter
does for assignments would have called `getenv()` even when a reader is
injected, which is the whole point of injecting one. It gets a real
if/else.
`FileMigrationSource` needed its elvis fix applied a second time. I
resolved that file's merge conflict in development's favour because both
sides had independently made the same File-narrowing fix — but only mine
had also removed the `?:` on the line below, so taking theirs wholesale
gave it back.
l10n, 30 uncovered -> 0. All thirty go into en.json as identity keys.
Twenty-six are translated in nl.json. `Id` and `URL` are deliberately
absent: a Dutch value identical to the English one is indistinguishable
from finished work and never gets revisited, whereas an absent key falls
back to English and stays visibly untranslated. The `{kind: 'webhook'|...}`
blob is left untranslated as a technical paragraph, per the decision that
this pass does titles and short descriptions.
Unit suite: 3701 tests now (the merge added 288), same single
EndoflifeDateLiveSyncTest network failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The development merge left 22 phpmd violations. Three were mine, and they are fixed rather than suppressed; nineteen came with merged code and are baselined. Mine, and why they appeared: converting ternaries to statements for `Squiz.PHP.DisallowInlineIf` trades one metric for another. `RecordOwnershipService::forObject()` ended up at an NPath complexity of 256 against a threshold of 200 — four sequential `if`s multiply, even though none of them is a decision worth reading — so the four "empty means absent" narrowings move into two small helpers. `GatewayRegistry` and `EnvironmentValueSource` each needed an if/else to keep the short-circuit the ternary had, and phpmd refuses `else`: the first assigns the already-built case up front so `fromArray()` is reached only when there is an array, and the second gets an early-return helper so `getenv()` is never called when a reader is injected. The nineteen are StaticAccess, BooleanArgumentFlag, ShortVariable, ShortMethodName and one ElseExpression in code merged from development through #2050-#2063. Same reasoning as the earlier baseline commit: development's phpmd gate is bypassed, so these surface on the first branch that runs it honestly, and baselining someone else's already-merged debt is what a baseline is for. One trap worth recording. `--generate-baseline` regenerates from the ruleset it is given, and this repo runs phpmd TWICE against one shared baseline file — the main ruleset and hydra-gates' unusedparams. Regenerating with only the first silently dropped every unusedparams entry, and the second run went from green to fifteen violations. The baseline is now the union of both, built by generating each separately and merging, and both runs exit 0. Entries also match on file AND method, so the merge invalidated nine that had simply moved. Local state on this branch: phpcs 0, phpmd 0 on both rulesets, check:schema-l10n 0 uncovered, check:l10n-js clean, 3701 tests with the one EndoflifeDateLiveSyncTest network failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Quality Report — ConductionNL/integriq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ❌ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 141/141 | |||
| npm | ✅ | ✅ 545/545 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it on any branch from the Actions tab, or locally with npx playwright test. |
||||
| Hydra gates | ❌ | ||||
| Hydra gates (axe) | ✅ |
Quality workflow — 2026-09-19 14:03 UTC
Download the full PDF report from the workflow artifacts.
phpstan is the one PHP gate I did not run locally before pushing, and it found three errors — all introduced by the style work, none pre-existing. `InboundMessage::arrayOr()` returned `array<string,mixed>|null` and fed it to constructor parameters typed `array<int,array<string,mixed>>`, non-nullable. One helper cannot answer three different shapes, so it becomes three: `listOrEmpty()` for `attachments` and `media`, `mapOrEmpty()` for `correspondent`, `rawPayload` and `fields`, and `mapOrNull()` for `location`, which keeps null because "no location" and "an empty location" are different answers. `EnvironmentValueSource::readEnvironment()` was declared `string|false`. The injected reader is a plain `callable|null`, so it may answer null where `getenv()` answers false — which is why the caller checks for both. Declaring the narrower type made `$value === null` provably dead, and phpstan said so. The declaration was wrong, not the check. Fixed by correcting the types rather than by baselining, casting or widening, per the guidance the gate prints. phpstan: 3 errors -> `[OK] No errors`. phpcs still 0, unit suite still 3701 with the one network failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gate-7 flagged seven #[NoAdminRequired] endpoints with no per-object guard. Two were real and are fixed; five accept no object id at all and now say so. FileMigrationSource read `IRootFolder::get($path)` — the SERVER root — with a path taken straight from the request, reachable through `POST /api/migration-sources/preview`, which is #[NoAdminRequired]. Any authenticated account could name any path on the instance and get the bytes back in the preview. It now resolves through the acting user's own folder and refuses without a session. This is worse than what the gate was complaining about; the gate only made me look. OwnershipController::show() answered for any object id. The ownership answer names the synchronisation, the source id and when the record was last seen, so it disclosed both existence and provenance for records the caller cannot read. It now resolves the object through the ordinary scoped read first. That guard is `local`, not 404, and the distinction matters: this method is tagged with the scenario "an unknown object answers local rather than failing", and a 404 would have contradicted its own spec. Answering `local` keeps that contract AND makes an unreadable object indistinguishable from an unknown one, which is the property worth having — the endpoint cannot be used to discover that a record exists. The other five — GatewaysController::index, MigrationSources::preview and ::validateMapping, Ownership::validatePolicy, PropertySource::suggest and ::resolve — take no object id: a static catalogue, validation of values the caller supplied, and registry lookups by registry key. They carry `@no-admin-idor-exempt` with the reason spelled out, rather than a guard invented to satisfy a checker. Three tests changed shape because the endpoints did, and one is new: an unreadable object must answer `local` and disclose nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gate-25 — `GET /api/outbound/identities/{id}/alignment` is a new public
endpoint with no contract test. It has one now, asserting the three
answers it can give and the shape of each: a panel branches on `atRisk`
to decide whether to warn that mail from this identity will be refused
downstream, so that key is the product. A real test rather than a
`@contract exclude`.
gate-106 — the register declared schema slug `message`, which `hermiq`
and `pipelinq` already own. Slugs are global on a shared OpenRegister, so
both definitions resolve to each other and the wrong one can answer. It
is now `mail_message`, which is what the schema is ("one mail message
integriq received") and what every sibling in the same file already does:
event_message, sms_message, dso_message, outbound_message,
intake_message, stuf_message. `MailIntakeService::SCHEMA_MESSAGE` and the
descriptor test follow it.
That rename cost me a detour worth recording: doing it through
json.load/json.dumps reformatted the whole file — 110 insertions for a
three-line change, because the original has inconsistent indentation and
inline arrays that a round-trip normalises. Redone as three surgical line
edits.
gate-16 — 62 methods this branch touched carry no @SPEC. Each now points
at the spec its own file already references, with the requirement
FRAGMENT stripped. That distinction is the point: `spec.md` says "this
method belongs to this feature", which is true and checkable;
`spec.md#requirement-a-send-is-a-typed-command` would have claimed a
specific requirement that `pollStatuses()` does not implement. Borrowing
a sibling's anchor is how a traceability tag becomes decoration. Verified
every referenced spec file exists.
phpcs 0, phpstan 0, 3705 tests with the one network failure.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…iq-nc35-support Development shipped #2097 "fix/hydra-gate-7-migration-preview" while this branch was doing the same gate, so four files collided. Three were the same conclusion in different words: the exemptions on GatewaysController::index, MigrationSources::validateMapping and Ownership::validatePolicy. Both sides read those as pure computation over caller-supplied arguments with no object to scope. Took development's wording, since it is already reviewed and merged there, and kept my two additions that it does not have. Those two additions are the part worth keeping, because #2097 exempted the endpoints without fixing what sits underneath them: - `FileMigrationSource` still read `IRootFolder::get($path)` — the SERVER root — with a path from the request, reachable through `preview`, which is #[NoAdminRequired]. Any authenticated account could name any path on the instance. This branch scopes it to the acting user's folder. - `OwnershipController::show()` still answered ownership for any object id, disclosing existence, synchronisation and source for records the caller cannot read. This branch resolves the object through the scoped read first, answering `local` rather than 404 so an unreadable object stays indistinguishable from an unknown one. The contract test for the alignment endpoint was an add/add: both sides wrote one. Took development's, which covers a case mine did not — a signed-in caller WITHOUT the required action being refused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scoping the delivered-file read to the acting user put readFile() at a cyclomatic complexity of 10, which is the threshold. The check moves to `actingUid()`, which also gives "whose files is this" a name a reader can find, rather than leaving it as a branch in the middle of a parser. Same trade as elsewhere in this branch: the fix for one gate adds a branch that trips another, so the honest answer is to extract rather than to suppress. phpmd back to 0 on both rulesets, phpcs 0, phpstan 0, 3708 tests with the one network failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `message` -> `mail_message` rename missed the frontend. The MailMessages widget in src/manifest.d/mail-intake.json still named `message`, and gate-53 caught it: "schema 'message' (register 'integriq') is not declared in lib/Settings/*register*.json". Worth naming the failure mode rather than just the fix. Renaming a schema slug is not one edit, it is every reference to it, and the backend ones were easy to find by grep because they are PHP constants. This one was a string in a JSON manifest fragment, which the grep I ran for code references never looked at. A widget pointed at a schema that no longer exists renders nothing — an empty list, not an error. Swept src/ and lib/Settings/ afterwards; no other reference to the old slug remains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two more references the `message` -> `mail_message` rename missed, both in the same vitest file: the widget's `config.schema`, and the register lookup `components.schemas.message.properties.status.enum` that checks the status column only colours values the schema can hold. Five places now, found one gate at a time: the register definition, the PHP constant, the descriptor test, the manifest fragment (gate-53), and this spec (Frontend Tests). Each grep I ran was correct for the shape it looked for and blind to the next one — PHP constants, then JSON strings, then JS property access. A slug is not a symbol any single search understands. Swept the whole tree afterwards with a pattern covering all four shapes; nothing else names the old slug. Verified in the container, since vitest needs a newer node than the host has: 317 frontend tests pass. The one remaining file failure, shared-instance.spec.ts, is a missing @vue/tsconfig in that container's node_modules and predates this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Quality Report — ConductionNL/integriq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 141/141 | |||
| npm | ✅ | ✅ 545/545 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ❌ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it on any branch from the Actions tab, or locally with npx playwright test. |
||||
| Hydra gates | ❌ | ||||
| Hydra gates (axe) | ✅ |
Quality workflow — 2026-09-19 14:57 UTC
Download the full PDF report from the workflow artifacts.
`schemas.message` -> `schemas.mail_message` is five characters, which put that line past prettier's print width and turned `Frontend Check (format)` red. Reflowed by prettier itself rather than by hand. `npm run format` is clean, and the spec still passes in the container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Quality Report — ConductionNL/integriq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ❌ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 141/141 | |||
| npm | ✅ | ✅ 545/545 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ❌ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it on any branch from the Actions tab, or locally with npx playwright test. |
||||
| Hydra gates | ❌ | ||||
| Hydra gates (axe) | ✅ |
Quality workflow — 2026-09-19 15:01 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/integriq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 141/141 | |||
| npm | ✅ | ✅ 545/545 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it on any branch from the Actions tab, or locally with npx playwright test. |
||||
| Hydra gates | ❌ | ||||
| Hydra gates (axe) | ✅ |
Quality workflow — 2026-09-19 15:07 UTC
Download the full PDF report from the workflow artifacts.
…mples
gate-101 wants three demo objects per schema, and eleven had none. The
gate names the tool, so this is its output:
`generate_mock_register.py .` — 201 objects across all 67 schemas.
Why it fired now, since nothing about demo data changed: the gate is
diff-scoped, and the three-line slug rename brought
`lib/Settings/integriq_register.json` into the diff, which made it look
at every schema the file declares. The eleven gaps were already there.
The diff reads alarmingly — 1240 insertions against 7597 deletions — so
it is worth saying what went: the generator no longer emits a duplicate
`components.schemas` block, because the real schemas live in
integriq_register.json. No demo data was lost. The objects it keeps are
byte-identical in shape to the ones before ("Voorbeeld Name 1"), which is
the tell that the previous file came from this same generator rather than
from anybody's hand.
Checked the one consumer before accepting it: DemoDataService reads
`components.objects` and nothing else, and its eleven tests pass against
the regenerated file. `mail_message` carries three objects and no stale
`message` remains.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Quality Report — ConductionNL/integriq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| build | ✅ | ||||
| check-specs | ❌ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 141/141 | |||
| npm | ✅ | ✅ 545/545 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it on any branch from the Actions tab, or locally with npx playwright test. |
||||
| Hydra gates | ✅ | ||||
| Hydra gates (axe) | ✅ |
Quality workflow — 2026-09-19 15:14 UTC
Download the full PDF report from the workflow artifacts.
I accepted the generator's output too readily last commit. It drops `components.schemas`, and I reasoned that was safe because the real schemas live in integriq_register.json and DemoDataService only reads `components.objects`. Both of those are true, and both missed the point: this repo's own `tests/validate-register.js` refuses a register document whose schemas block is empty, and that check runs in three CI jobs — `Frontend Check (check:specs)` and `validate` twice. So the schemas come back, from integriq_register.json, alongside the 201 generated objects. gate-101 passes and check:register passes. One asymmetry left on purpose: 49 schemas against objects for 67. The other 18 live in lib/Settings/register.d/ as OVERLAYS — lockdown and write-only patches — and several define no `slug` of their own, so merging them into a standalone document makes it invalid. I tried it: 13 errors, "missing string slug" on lti_platform, lti_tool and approval_request. Demo objects for all 67 are correct, because the gate counts against the register as assembled; the schemas block is what this one file can validly declare by itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Quality Report — ConductionNL/integriq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 141/141 | |||
| npm | ✅ | ✅ 545/545 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| lockfile sync | ✅ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it on any branch from the Actions tab, or locally with npx playwright test. |
||||
| Hydra gates | ✅ | ||||
| Hydra gates (axe) | ✅ |
Quality workflow — 2026-09-19 15:25 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
1 participant
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.
Why now
integriq could not claim NC 35 before openregister did.
appinfo/info.xmlrecords why in its own comment: openregister is a hard dependency, every integriq entity is an OpenRegister object, and on a server openregister refuses to install the app has nowhere to store anything. openregister#3777 (max-version34 → 35) is merged, so that constraint is now satisfied.Checked, not assumed
integriq's own surface was verified against the
nextcloud/ocpv34.0.4 and v35.0.0 stub trees before the ceiling was raised:OCP\Remote\*family (IInstance,IInstanceFactory,ICredentials,IUser,Api\{IApiCollection,IApiFactory,ICapabilitiesApi,IUserApi}),Collaboration\AutoComplete\AutoCompleteEvent,Files\Cache\{CacheInsertEvent,CacheUpdateEvent}— integriq references none, by fully-qualified name acrosslib/andtests/.ISchemaWrapperhand outOCP\DB\Schema\ITablewhere 32–34 hand out Doctrine'sSchema\Table, and that interface does not exist before 35 — the break that fix: openregister install and occ breakage on Nextcloud stable35 openregister#3775 exists to fix. integriq ships one migration,Version2Date20260908000000, and it type-hints no table at all.3rdpartydeclaressymfony/console ^7.4.15andsymfony/http-foundation ^7.4.15; integriq already requires^7.4and^7.4.13. Those are the two packages NC core ships that a bundled copy can collide with.The matrix is derived, not pinned
nextcloud-test-refsis removed fromcode-quality.yml, so the shared workflow reads the range fromappinfo/info.xmland the tested range can no longer disagree with the declared one. Appending"stable35"to the pinned list would have cleared gate-65 for this PR while re-arming the same trap for NC 36 — and arming it ondevelopmentrather than on the PR doing the bump, because the gate fires on the manifest/matrix disagreement itself rather than on the diff that caused it. openregister#3777 took the same decision.The ordering the old list encoded is inert now. Its first entry was load-bearing — E2E, newman and journeydoc-capture ran against
fromJSON(nextcloud-test-refs)[0], so stable34 had to lead to keep them off a server openregister cannot load (issue #1172). Those jobs now consume the shared workflow'ssingle-serveroutput, the numerically highest branch in the resolved set, and the floor is carried byinfo.xml'smin-version="32"instead of by a hand-written array.What the new stable35 leg is actually testing
additional-appsinstalls openregister fromdevelopment, and the shared workflow aborts a job whoseocc app:enablefails for an additional app. With openregister#3777 merged that enable now succeeds, so this leg is the first real measurement of integriq on NC 35 rather than a fixture failure. If openregister's declared ceiling ever lags integriq's again, this leg is where it will show.One known exception, and it resolves itself
The coverage ratchet measures the merge base by running PHPUnit there, on
single-server— which this PR makesstable35. integriq'sdevelopmenthas never run on that server, so the base cannot produce a coverage report and the step declines to compare against nothing. That is a bootstrap condition, not a coverage regression: the PR that raises the ceiling is the one that creates the leg, so its merge base is by definition the last commit predating the claim. From the next PR onward the base carries this change and the ratchet measures normally. The committed.coverage-baseline(60.65) is unchanged and still enforced on every push todevelopment. openregister#3777 hit the same condition for the same reason.AI assistance disclosure
Assisted by Claude Code: verified integriq's surface against the NC 34 and NC 35 OCP stub trees, made the
max-versionbump, and derived the test matrix from the manifest. To be reviewed and independently verified by the human contributor before merge.🤖 Generated with Claude Code