Release: merge development into beta - #1711
Open
github-actions[bot] wants to merge 2985 commits into
Open
Conversation
fix(flow): attribute interactive test runs to the caller (3rd instance of the ownerless-run defect)
…gistered
Nextcloud writes oc_jobs only when an app is INSTALLED or UPGRADED. Add a <job>
to info.xml without bumping the app version and it is silently never registered
— no error, no warning, and there is no occ background-job:add to correct it.
Measured on a live instance: 24 of 31 declared jobs were absent. Not only flows —
scheduled workflows, schedule reconciliation, sync, webhook retries, notification
flushing, archival retention and DBAL introspection were all inert. 59 flow runs
sat queued and could never execute.
It hides well: the synchronous counterparts keep working. POST /api/flow-runs/test
calls FlowRunService::execute() directly, so interactive flow testing was green
while every asynchronous trigger queued into a void, staying "queued" forever
rather than failing.
This repair step parses the <job> declarations out of info.xml and adds any that
IJobList does not already have. Because it is a repair step it runs on
occ maintenance:repair, so an instance can be corrected without inventing a
version bump. Idempotent, and one unresolvable job is logged and skipped rather
than aborting the run — the point of the step is to recover an instance.
Two traps this hit while being written, both worth knowing:
1. SimpleXML cannot reach a hyphenated element as $xml->background_jobs; it
needs $xml->{'background-jobs'}. The mismatch returns nothing rather than
erroring.
2. simplexml_load_file() SILENTLY RETURNS FALSE inside the Nextcloud runtime.
NC installs a restrictive libxml external-entity loader at boot, and PHP 8
routes file access through it. The same parse works in a bare php -r process
and fails under require lib/base.php — so the step reported "no <job>
declarations found" against an info.xml holding 31 of them. Fixed by reading
the file and parsing a string. Any app code parsing XML by path is suspect
for the same reason.
Verified by controlled experiment on a live instance: deleted FlowScheduleWorker
from oc_jobs, ran the step, and it reported "Registered missing background job:
OCA\OpenRegister\Cron\FlowScheduleWorker / 1 added, 0 skipped, 31 declared" with
the row restored. Separately, a version bump plus occ upgrade took the instance
from 9 to 34 distinct registered job classes.
Refs or#2170.
fix(jobs): reconcile declared background jobs Nextcloud never registered (24 of 31 were missing)
Adds `executionMode: async|sync` so a trigger can run a flow inline, and a run-level FlowToken that propagates into sub-flows, returns to the parent, and survives pause/resume. The token is a mutable object at context['token']: an object handle survives the by-value array copy, so nodes write to it with ZERO change to the IFlowNode signature — all registered nodes and both leaf apps are untouched. Pause/resume came free: persistResult() already wrote context back on the SUSPENDED path. Verified: 21 unit tests, 236/236 flow suite after merging development, phpcs 0 errors, openspec --strict, and Playwright e2e 4/4 live on 8080. Live testing also surfaced ConductionNL/hermiq#53 — hermiq's resolver claimed every OR flow, so OR's own resolver was never asked and executionMode was silently dropped.
Fourth instance of the or#2158 defect class, after FlowMcpToolProvider::runFlow(), FlowRunService::execute() and FlowRunController::test(). FlowScheduleService::fire() queued with no user, so every natively-scheduled flow ran ownerless: context['triggeredBy'] was null and every attribution-requiring node refused. ObjectWriteNode returns 'This flow run has no owner (triggeredBy); an object write must be attributable.' A scheduled flow could therefore never write anything. A scheduled run has no session, so the owner comes from the flow object itself — the person who created and enabled it, matching the decision that a dispatch is owned by whoever made and activated the flow. This became urgent rather than theoretical: FlowRunWorker and FlowScheduleWorker were registered for the first time in #2172, so scheduled flows now actually execute. Without this fix they would execute and refuse, producing a steady stream of failed runs. Test asserts queue() receives the owner; 7/7 green in FlowScheduleServiceTest.
…tion fix(flow): scheduled runs were ownerless — 4th instance of the attribution defect
… credential providers apphost-schedule-flow-action — manifest schedules[] can only ever run ONE thing: ScheduleActionAllowList::MAP has a single entry, openconnector:synchronization. A virtual app therefore cannot schedule a flow, even though the flow engine can now both call external APIs (openconnector.source-call) and write objects (openregister.object-write). Adds a flow-run action while keeping the closed allow-list — the point of that design is that an app cannot name an arbitrary class. Attribution is mandatory and fail-closed: a scheduled run has no session, so the owner comes from the schedule declaration and the action refuses rather than running ownerless. Writing this spec found the same defect a fourth time in the adjacent scheduler (FlowScheduleService::fire queued with no user) — fixed separately in #2173. app-declared-credential-providers — the provider catalogue is runtime-immutable, so an app author cannot register the credentials their app needs. Two costs observed while building hydra-console: no codeberg/forgejo provider exists, and the github provider permitted no issue-label write (fixed in #2165 only because we could edit OpenRegister itself). The design keeps the security boundary real rather than removing it. Two lanes: a narrowing declaration (same host and auth scheme, provably-subset allow-rules) is auto-admitted because it grants strictly less than the base provider the app could already use; anything introducing a new host or path requires administrator approval. Approval is pinned to a per-entry content digest, so an app that ships benign, gets approved, then widens is returned to pending. Declarations are always app-scoped and namespaced so they cannot shadow or be borrowed from, and inject_only declarations are rejected outright — a declared inject_only entry would be unbounded secret egress authored by the app receiving the secret. NEEDS PO CONFIRMATION: whether the narrowing lane should really skip approval. Recorded as the first deferred question rather than buried in the design.
docs(openspec): schedulable flows + app-declared credential providers
) 15454 tests: 51 errors + 2 failures -> 0 and 0. 25 errors: AttributeMcpDiscoveryTest assigned \OC::$server to a service-less fake and never restored it, so every later test saw a locator whose get() returns null and Response::getHeaders() died on $request->getId(). All passed in isolation — one leak reading as 21 broken tests. Restored in tearDown so a failing test cannot leak either. 19 errors: no stubs for OCP\ContextChat\* (optional app seam, absent from a bare composer install). Added guarded stubs beside the Doriath ones, surface taken from the call sites. 4 errors: FlowRunController gained an IUserSession param; its test still passed five arguments. 2 errors: CardDavBackend stub lacked getAddressBookById(), which ContactService calls. 1 error: bootstrap STDERR output corrupted a @runInSeparateProcess worker's result channel. Emitted once now, with the existing skip switch set for inherited children. 1 failure — REAL PRODUCTION BUG: RelationsController documents that a missing/disabled app is silently skipped, but Server::get() returns NULL rather than throwing, and probing null was recorded in $errors — so a fully successful response still carried an _errors key for all 13 leaf integrations. Resolution is now separated from the call. phpcs clean; each fix verified in isolation before the full run.
…pes (#2177) FlowNodeRegistry::palette() already returned exactly what a flow builder needs and nothing exposed it over HTTP — FlowController shipped only eventCatalog(), so a client could discover what STARTS a flow but not what a flow can DO. A flow's edges[].type names a registered node, so with no palette endpoint a builder hardcodes the list, and it goes stale silently: an unknown type only surfaces when the flow runs. App contributions (openconnector's source-call/synchronization-run, hermiq's agent-step) were invisible to every HTTP client. Adds GET /api/flow/node-catalog mirroring the trigger catalog's envelope and reusing palette() unchanged. Scope-filtered — ?scope=user returns only what a non-admin may actually run. Closes #2176. 5 unit tests; phpcs clean; 15460 Unit tests, 0 errors, 0 failures.
…P surface (#2178) The registry assertion skipped because OpenRegister exposed no palette endpoint — palette() was in-process only. #2177 added /api/flow/node-catalog, so the check is now real. It immediately earned its keep: openconnector's leaves were silently failing to register on a clean install (class_exists load-order guard in register(), openconnector#1076) and no test in either repo could see it. 4/4 against an isolated NC 34 + Postgres instance on merged development.
…alled The audit-trail page's Statistics tab has fetched GET /api/audit-trails/statistics since 620d344 (2025-06-07), but the backend half was never written. The URL fell through to auditTrail#show, where the dispatcher coerces "statistics" to id 0, so every load 404'd on audit trail #0 and the four stat cards silently read 0. Add the missing route (above the {id} routes, per the rule already noted there), controller action and mapper query. Counts are lifetime rather than windowed like the dashboard's plural-keyed variant, so they agree with the unwindowed list beside them, and the keys are singular to match the contract the store and its unit spec already assume. Admin-gated at the framework level with the same requireAdmin() defence-in-depth as index/show. Optional register/schema params leave room to wire the sidebar filters through later. No frontend change needed — the response shape matches what the store has been expecting all along.
…wiring, schema dedup)
Snapshot of the local working tree taken before reconciling with
origin/development, which had moved 69 commits ahead.
Much of this tree is already upstream (27 of 36 new PHP files and 13 of 49
modified files are byte-identical to origin/development). The genuinely new
work preserved here is:
- lib/AppHost/{Controller,Service}: generic settings plane
(GenericSettingsControllerBase, GenericSettingsService, RegisterConfigResolver)
- lib/Command/DedupCollidedSchemasCommand.php
- unit tests for the above plus HandlesExceptionsTrait
- four openspec changes (apphost-settings-plane, apphost-schedule-flow-action,
app-declared-credential-providers, or-flow-object-write-node)
- lib/Settings/flow_register.json and flow e2e coverage
Committed on a stale base on purpose so the subsequent merge of
origin/development is a real three-way merge rather than a tree overwrite.
…026-07-28 # Conflicts: # lib/Capabilities/IntegrationsCapability.php # lib/Service/Flow/FlowRunService.php # lib/Service/Flow/FlowTriggerService.php # lib/Service/Flow/IFlowResolver.php # lib/Service/Flow/Nodes/SubFlowNode.php # lib/Service/Flow/OpenRegisterFlowResolver.php # lib/Settings/flow_register.json # tests/Unit/Db/SchemaAnnotationVocabularyTest.php # tests/e2e/api-direct/openconnector-flow-nodes.spec.ts
…backlog Measured on the shared dev instance: 108,151 of 227,063 audit rows (48%) carry no hash, interleaved across the whole id range (31,518..290,493 = min..max id). Three defects, all evidenced: 1. insertHashChained() seals per row under a global exclusive advisory lock (3 attempts x 50ms, fail-soft). Under any concurrency each insert pays up to 150ms and then abandons the seal anyway. Measured ~152 rows/min during the maintenance:repair that had to be killed after 74 minutes with ~12h left. The batched sealRows() path already exists but single inserts never reach it. 2. The backfill that specs/audit-hash-chain/spec.md:105 requires does not exist. harden-audit-seal-concurrency (12/12 complete) made the lock fail-soft on the explicit promise that a "later seal pass chains them" -- that pass was never built, so every contended write permanently degrades the chain. 3. verifyChain() skips ANY null hash and still returns valid: true. The comment claims "pre-migration entries" but no cutover marker exists in lib/, so the tamper-evidence check currently passes over a table that is 48% unverified. The change specifies a windowed driver around sealRows() (one lock per window, not per row), a two-phase read so sealed rows do not have their ~5.3KB payloads fetched just to contribute a chain link, a partial index for the backlog cursor (today it pkey-scans with a filter at 784ms/2000 ids), a hard-capped background job plus an occ command, and a cutover marker so unsealed rows stop hiding behind a passing verification. Design records why a naive bulk call is impossible: sealRowsLocked() SELECTs * over [min,max], which for this backlog is ~227k rows x 5,270B ~= 1.14GB in PHP.
t() gets globally imported
RegisterDetail never read its `:id` route param, relying entirely on
RegistersIndex to seed `registerStore.registerItem` before navigating.
Opening or refreshing /registers/2 therefore hit the "no register id"
branch in mounted() and redirected straight back to /registers.
- Seed the register store from the route param in mounted(); the redirect
now only guards the case where no id exists at all.
- Resolve `register` by route param as a fallback, comparing ids as
strings — route params are strings while the API returns numeric ids,
so the old strict compare could never match a route-derived id.
- Drop the redirect on fetch failure: CnDetailPage already renders
dashboardStore.error with a "Back to Registers" action.
- Add a `hydrating` flag so the first paint shows the loading state
instead of flashing "Register not found" before mounted() fetches.
loadSchemas() treated `register.schemas` as an array of ids, but the
dashboard endpoint replaces that field with fully hydrated schema
objects (DashboardService::buildRegisterEntry), so each entry
stringified into GET /api/schemas/[object Object] — one wasted request
per schema. Use the hydrated entries as-is and fetch only bare ids.
Those entries also carry the per-schema `stats` the cards render and
GET /api/schemas/{id} does not return, so the card metrics and pie
charts were rendering zeros regardless. The `properties: []` -> `{}`
fix-up moves into normalizeSchema(), which copies rather than mutates
because hydrated entries are Pinia store state.
Also fix editSchema(), which called a setSchemaItem action that only
exists on schemaStore, throwing instead of opening the modal.
+ made sure that selected wins
…feature/fixes-and-changes
POST /api/applications silently discarded every quota allocation. The
entity stores one column per allocation but the API exposes them as one
nested object, and there was no setter for that object: hydrate()'s
generic set{Key} call resolved to a non-existent setQuota(), Entity threw
BadFunctionCallException for the unknown attribute, and hydrate()'s
catch-all swallowed it. The response then reported the untouched NULL
columns, indistinguishable from "unlimited".
Add Application::setQuota() to unpack the nested structure onto the
columns. It accepts an array or a JSON string, only writes the keys the
caller actually sent (so a partial quota cannot wipe the rest), treats
non-numeric and null as unlimited, and goes through the magic setters so
dirty-field tracking marks the columns for the UPDATE. Fixing this on the
entity covers create and update alike.
`users` and `groups` had no columns at all — getQuotaData() hardcoded
them to null with a "to be set via admin configuration" note, so the
published shape had two keys that could never be saved. Version1Date-
20260728000000 adds nullable user_quota / group_quota; strictly additive
and idempotent, and existing rows keep the unlimited semantics they were
already reporting. Needs the info.xml bump to run.
Also key the quota addType() registrations by property name. Entity::
setter() looks up $_fieldTypes[$property], so 'storage_quota' never
matched storageQuota and the declared integer cast never applied — a
BIGINT column could surface as a string in the JSON response.
Tests hydrate the reported payload and assert the serialized quota
matches it exactly; without setQuota() that assertion fails with
"null is identical to 128974848".
…on drift
A single-object create currently takes 13-99s on the dev instance (six runs on
larpingapp/character, two-field payload each time: 13.6/17.8/20.4/41.0/62.8/99.1s).
This is NOT the CloudEvent storm — that was openconnector's inert recursion
guard, fixed separately, and a create now emits 1 event rather than 255. The
remainder is our own write path.
Counter deltas across one HTTP 201 create (the 41.0s run):
sequential scans of oc_openregister_schemas 5,135
sequential scans of oc_openregister_registers 6
transactions committed 12,541
Four measured costs:
1. 5,135 schema resolutions against a 1,917-row table. SchemaMapper::find()
HAS a request cache and is a shared service, so this is either a cache
key too specific (rbac/multitenancy flags multiply it 4x) or an uncached
sibling on the hot path. The query is a seq scan by construction —
SELECT * hydrates a ~2KB properties blob and LOWER(slug) defeats any
index ('Rows Removed by Filter: 1916'). ~4ms x 5,135 = ~20s, half the run.
2. Resolving an object reference whose table is unknown emits a UNION ALL
with one branch per magic table. At 2,728 tables that is 690KB of SQL:
planning 3,404.9ms, execution 546.1ms. 86% of the cost is PARSING a
statement that returns zero rows, so no index can help — and it is
usually avoidable, since character's six relation properties each
already declare their target schema.
3. 12,541 commits for one create: essentially every statement autocommits,
and the request waits on each fsync.
4. CloudEvent fan-out, audit-trail sealing (228,932 rows), notification
history and oc_activity all run before the response is returned.
Target p95 <500ms with the 2,728-table shape unchanged — fixing the write
path, not shrinking the dataset. Task 1 is deliberately 'attribute the 5,135
calls before changing anything': guessing at a hot path is how the CloudEvent
guard stayed inert for so long.
Also fixes an unrelated red gate: openspec/specs/saved-search-views/spec.md was
missing the blank line before '## Requirements', so the features-manifest
generator swallowed the heading into the feature summary and docs/features.json
drifted. 'quality / Features Check' fails on development because of it.
Conduction's standard requires named arguments on internal calls, forbids inline IFs, and has its own spacing///end conventions — the command shipped with 21 violations and turned 'quality / PHP Quality (phpcs)' red. 14 fixed by phpcbf; the rest by hand: setName/setDescription/addOption -> named arguments (four calls) splitOne/splitOneLocked/findCollisions/pickOwner call sites -> named arguments the two ternaries in execute() -> explicit if blocks All 11 PHP files changed on this branch now pass phpcs.
lib/ was 15 errors over 9 files before this branch, so 'quality / PHP Quality
(phpcs)' failed on development and on every PR opened against it.
6x curl_close($ch) deprecated since PHP 8.0 and a no-op — CurlHandle is
an object freed when it leaves scope, not a resource
needing an explicit close. Removed, with a comment so
nobody re-adds them.
3x missing @param FlowRunController::__construct($userSession),
FederatedConfigService::publish($private),
FlowScheduleService::fire($owner)
6x file header ReconcileDeclaredBackgroundJobs.php put its docblock
AFTER declare(strict_types=1), so phpcs read it as a
stray inline block rather than the file header; tag
order was @author/@license/@copyright. Moved above the
declare and reordered to @author/@copyright/@license.
phpcs now reports 0 errors across all 72 files in lib/. phpstan is unchanged —
the same 7 findings exist with and without this commit (verified by stashing),
so nothing here introduced or masked one.
…able (#2351) The flow page had no way to save, run, enable, add a step, or see run history — while its own empty state read "Add a step from the sidebar". None of it was missing. `CnFlowSidebar` implements the whole panel (step palette, Name/Description/Trigger, Enabled, Save, Run now, Recent runs), `FlowDetailSidebar` wires save/run to the store, it is registered in registry.js, and the manifest declares `sidebarComponent: FlowDetailSidebar` on the flowDetail page. CnAppRoot does resolve that key. It could still never render. CnAppRoot only falls back to the manifest's sidebarComponent as the DEFAULT content of its #sidebar slot, and this app fills that slot itself with SideBars — so consumer content wins by Vue's ordinary slot mechanic, exactly as CnAppRoot's own docblock warns. SideBars had no branch for /flows, so on a flow route it rendered nothing at all and the manifest key was live config with no effect. Adding the branch is the whole fix. Verified in the browser against a rebuilt bundle: the sidebar renders with Steps, Flow (Name, Description, Trigger, register/schema restriction, Enabled), Recent runs, Save and Run now. Built a flow from the palette, saved it — the route advanced from /flows/new to the returned uuid, so it persisted — then ran it. FlowRunWorker picked the queued run up and the history moved to `completed`, matching the row in oc_openregister_flow_runs.
…lready lists (#2352) 39 files of PHPMetrics HTML report scaffolding were committed despite .gitignore:30 already listing /phpmetrics-deps/. Nothing in the repo reads these paths — PHPMetrics writes to phpmetrics/ (composer.json) and phpqa/phpmetrics (.phpqa.yml), never phpmetrics-deps/. They also carried third-party code into an EUPL-1.2 repo, including js/clusterize.min.js (GPLv3, (c) 2015 Denis Lukov) and MIT-licensed js/sort-table.min.js and css/milligram.min.css. Untracked only; the files stay on disk and remain ignored.
* chore(license): normalise licence declarations to EUPL-1.2 OpenRegister declares EUPL-1.2 in composer.json, package.json, appinfo/info.xml and LICENSE, but 175 files still carried AGPL-3.0 licence tags. This aligns every file-level licence declaration with the licence the project actually ships under, clearing hydra gate-28 (license-triangle), which failed with 25 files under lib/. Changes are licence-identifier only: - @license AGPL-3.0-or-later <agpl-url> -> EUPL-1.2 <eupl-url> (88 tags) - @license AGPL-3.0-or-later (no URL) -> EUPL-1.2 (61 tags) - @license AGPL-3.0 / URL-first shape -> EUPL-1.2 <eupl-url> (3 tags) - SPDX-License-Identifier: AGPL-3.0-or-later -> EUPL-1.2 (64 tags) - stale AGPL URL appended after an already-correct EUPL-1.2 id removed in 6 lib/Service/File handlers (12 tags) - ConfigurationSettingsHandler reported OpenRegister's own licence as 'AGPL' when info.xml lacked one; fallback now 'EUPL-1.2' 29 files carried TWO @license tags; gate-28 only reads the first, so every occurrence was replaced rather than just the blocking one. No @copyright, @author or SPDX-FileCopyrightText line was touched. Deliberately NOT changed (reported for an explicit decision): - 12 files whose SPDX-License-Identifier is paired with 'SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors' (occ-scaffolding residue). An SPDX pair is a single statement about a named holder, so relicensing it is not ours to do mechanically. None affect gate-28. - phpmetrics-deps/ (39 vendored third-party report assets, incl. a GPLv3 file) and composer-setup.php - third-party, not ours. Unit suite before and after, identically conditioned (PHP 8.3.32): 16030 tests / 35963 assertions, 0 failures, 0 errors - unchanged. * chore(license): normalise the 12 remaining AGPL SPDX identifiers to EUPL-1.2 gate-28 reads only the first @license tag and ignores SPDX entirely, so these 12 declarations were invisible to it. Eleven of them sat directly above a Conduction '@license EUPL-1.2' PHPDoc tag in the same file — the files contradicted themselves. Also corrects openapi.json's info.license.name and the exapps/README.md licence line. Adjacent SPDX-FileCopyrightText lines are deliberately untouched. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…ng it (#2356) PR #2350 flipped 'SPDX-License-Identifier: AGPL-3.0-or-later' to EUPL-1.2 in 12 files whose adjacent line reads 'SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors'. That asserted EUPL-1.2 over a third party's copyright — a false licence claim, and worse than the AGPL contradiction it replaced. This deletes the residue block rather than relabelling it. Evidence that the block is copy-paste residue from the Nextcloud app template, not a real Nextcloud GmbH copyright: - It occurs in exactly 12 source files; the only other 'Nextcloud GmbH' strings in the repo are dependency author fields in bom-npm-test.cdx.json, which is an SBOM of real Nextcloud npm packages and is left untouched. - 11 of the 12 already carry their own '@copyright 20xx Conduction B.V.' PHPDoc tag in the same file, directly contradicting the SPDX line. - The full git history of all 12 files contains no Nextcloud GmbH author: only Conduction people (Barry Brands, Conduction Development Team, Remko, Robert Zondervan, Ruben van der Linde, Thijn) and the CI bot. - The repo's other ~270 PHP files carry Conduction copyright only, with no SPDX pair at all — this is the app-template scaffold the rest shed. tests/Unit/Controller/SettingsControllerTest.php had no other licence header, so it gains the repo's standard test-file PHPDoc rather than being left bare. No licence value is changed by this commit; the false claim is removed.
…w records its last run (#2354) * feat(flow)!: a path ends deliberately or says it is broken, and a flow records its last run A node with no outgoing edge was a silent success. Its token arrived, the step ran, the engine found no enabled transition, and the run was recorded COMPLETED — so the author saw a green run that had not done the work. Nothing failed, so nothing was logged. That is the defect this closes. Ending a path deliberately is now something a node SAYS, two ways, OR-ed: IFlowTerminalNode a marker interface on the TYPE, resolved through FlowNodeRegistry::isTerminal(), so a terminal step contributed by openconnector or hermiq needs no OpenRegister change. StopNode implements it. "exit": true on the node instance, for a sink whose step type is an ordinary action — which is what every migrated flow has, because that WAS a legitimate end of a path under the old place-and-edge reading. They are OR-ed and never AND-ed: requiring both would make every migrated flow depend on a registry the migration cannot see. A marker interface rather than a method on IFlowNode, for the reason IFlowNodeConfigKeys already documents: implementations live in other repos, and widening the interface fatals those apps on load. WARN ON SAVE, REFUSE ON RUN Saving a half-wired flow succeeds and returns the warning. A disconnected graph is the normal state of one being authored; refusing to store it would force the author to build the graph in an order that is never disconnected, which no editor can require. Running is refused. The guard sits in FlowRunService::queue(), which is the one choke point every dispatch path passes through — manual, trigger, schedule, MCP, the workflow-engine operation and a sub-flow call. Guarding FlowService::run() instead would have left cron-fired flows unguarded, and those are most of them. On refusal no FlowRun is created, and the verdict is written onto the FLOW (status/status_message naming the nodes) precisely because there is no run to read: that is what makes a refused flow distinguishable from one nobody has triggered. An accepted run clears a stale error back to ok. The schedule sweep catches the refusal PER FLOW. It iterates every due flow, so letting it propagate would abort the sweep and stop every later flow from firing — one broken definition silently disabling the rest, presenting as "cron stopped working" rather than as a fault in a named flow. A typeless node is deliberately NOT reported here. FlowDefinitionBuilder already refuses it by name, and two findings on one node for one defect is how a warning list becomes noise. LAST RUN Six nullable columns, no backfill. NULL lastRunAt means "has never run" — a value derived from run history would assert a history the column did not record. Written only when a run reaches a terminal state, so the flow list answers "how did it last go?" rather than "it hasn't finished". Also adds the canonical openspec/specs/flow-engine/spec.md, which did not exist — it lived only inside changes/ — so @SPEC can target a canonical path. Not done, and stated in tasks.md rather than quietly skipped: the schedule and trigger dispatch wiring, and the last-run write-back, are not yet pinned by tests. Both need FlowRunService built with a mocked container. The suite could not be run locally: once lib/base.php loads, NC's autoloader resolves OCA\OpenRegister\* to the INSTALLED app, not the working copy — measured with ReflectionClass::getFileName(). CI's "copy the app out" recipe does not prevent that; CI is immune only because it deploys the code under test first. Run locally against an older deployment it reports on the deployed app. CI is the authoritative gate here. BREAKING: a flow with a dead-ended node is now refused at run time instead of completing silently. Mark deliberate sinks "exit": true, or give them a terminal step type. * fix(flow): import IFlowTerminalNode, and let the dialect fixtures end deliberately StopNode gained `implements IFlowTerminalNode` without the matching `use`. StopNode lives in ...\Service\Flow\Nodes, so PHP resolved the bare name relative to THAT namespace and looked for ...\Nodes\IFlowTerminalNode. `php -l` cannot see it — the syntax is valid and the failure is at class-resolution time — so it surfaced as 16 identical PHPUnit errors plus phpstan, psalm and phpmd all reporting the same unknown interface. My local phpstan/psalm run passed because I listed the changed files by hand and StopNode.php was not among them: the check excluded the one file with the bug. The fixtures in FlowNodeConfigDialectTest, FlowNodeConfigVocabularyTest and FlowNodePreflightRegressionTest are single nodes or chains with no outgoing edge from their last node, so the new connectivity check reports them — correctly. Those suites are about a node's config DIALECT and the registry, not about connectivity, and each asserts an exact finding count; an unrelated second warning made them count two different things. Marking the last node `exit: true` makes each fixture a COMPLETE document rather than suppressing the check, and the dialect suite's positive control still asserts an exactly-empty report. * fix(flow): split the connectivity check out, and document the delegated guard Three gates, three real findings: phpmd — deadEndFindings() reached cyclomatic 13 / NPath 735, and decomposing it pushed FlowNodePreflight past the 1000-line class limit. Both are the same signal: the graph-SHAPE question does not belong in a class that answers questions about each node's TYPE and CONFIG. Moved to FlowConnectivity, which also stops the preflight becoming the place every future flow check lands. Instantiated inline rather than injected, so no constructor changes ripple into the several tests that build the preflight by hand. gate-7 no-admin-idor — FlowController::create/update were pulled into the diff by the savedBody() change and flagged as NoAdminRequired with no guard. The guard is real but DELEGATED, which is the gate's documented false-positive class: update() resolves the uuid through FlowService::find(), so an update to a flow the caller cannot see is refused exactly like one that does not exist, and create() stamps owner/organisation server-side with both outside applyEditableFields()'s allowlist. Recorded with the reason-bearing @no-admin-idor-exempt tag naming the actual guard, following the precedent in EmailsController and FileSearchController. PHPUnit — one more single-node fixture asserting an exactly-empty report, now marked exit: true for the same reason as the others: a lone node with no outgoing edge IS a dead end, and the warning would be right.
…ght this (#2353) * test(e2e): guard the flow controls, the one layer that could have caught this The flow authoring surface shipped unreachable — no save, run, enable, add a step or run history — and every layer was green while it was broken. The components existed, the routes existed, unit tests passed, the manifest validated, and the manifest key that was supposed to mount the panel (`sidebarComponent`) was silently outranked by the app's own #sidebar slot. Nothing short of opening the page and looking for the controls could have found that, so that is what this does. Asserts, in order: the sidebar renders with its palette and actions; the palette is non-empty (an empty one renders the same container, so the count matters); a step added from the palette reaches the canvas; Save persists, proven by the route advancing off `new` to the server's uuid; and Run now creates a run for that flow. It deliberately stops short of asserting the run COMPLETES. Execution is picked up by FlowRunWorker on cron, which does not run in CI — waiting for it would make the spec depend on a background job. That the run is created and attributed to the flow is the part the UI is answerable for. Hermetic per the CI floor's contract: it builds its own flow through the UI and deletes it in a `finally`, so a mid-test failure still cleans up. Verified both ways against a live instance, because a passing assertion is evidence about the assertion until it has been shown to fail: with the fix it passes, and with the fix reverted and the bundle rebuilt it fails on the sidebar assertion with the message written for exactly that case. * fix(e2e): drive the themed buttons the way this repo already learned to CI failed the new spec on the click, not on the app. Playwright resolved the "New flow" button, reported it visible, enabled and stable, scrolled it into view — and then the click action itself timed out, twice, burning the whole 45s budget with the locator perfectly matched. That is the Nextcloud themed-button behaviour `tests/e2e/global-setup.ts` already documents against the login button: "on NC's themed login the styled submit button can swallow the click". The class on the failing control says the same thing out loud — `button-vue--legacy34`. Every click in the spec now goes through one helper that asserts visibility and then dispatches the event, which drives the Vue @click handler that is the actual behaviour under test. It costs Playwright's actionability checks, so the explicit `toBeVisible()` assertions stay: those are what catch a control that is missing or covered, which is the regression this spec exists for. It passed locally three times before CI disagreed — worth recording, because the local pass was the less trustworthy of the two results.
A pinned `hydra-gates-ref` is a silent expiry date on every upstream fix: this repo cannot receive a gate-package change until this line moves. v1.4.0 is the latest tag and the first one that carries `hydra-gates/scripts/axe-run.cjs` (verified absent at v1.3.0), so it is also the first that has ConductionNL/.github#168 axe DOM scoping and ConductionNL/.github#165 gate-46 fix. `enable-axe` is deliberately NOT enabled in this commit. Ordering matters: the ref lands first, enabling axe is a separate decision.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 6, 2026
The standing 'Release: merge development into beta' PR (#1711) has head_ref 'development', so its pull_request run rendered the same concurrency group as a push to development. cancel-in-progress killed the push run, which is the only carrier of the push-only jobs (Coverage Baseline Check, SBOM, Features Extract). Those jobs report 'skipped' on the surviving PR run, which renders like a pass, so the gate never produced a verdict. Suffixes -push on the group for main/development pushes only; feature-branch dedup is unchanged. No gate weakened. Same fix as openconnector#1158.
…ne (#2361) The standing 'Release: merge development into beta' PR (#1711) has head_ref 'development', so its pull_request run rendered the same concurrency group as a push to development. cancel-in-progress killed the push run, which is the only carrier of the push-only jobs (Coverage Baseline Check, SBOM, Features Extract). Those jobs report 'skipped' on the surviving PR run, which renders like a pass, so the gate never produced a verdict. Suffixes -push on the group for main/development pushes only; feature-branch dedup is unchanged. No gate weakened. Same fix as openconnector#1158.
…kflow needs The Hydra Gates job fails with a message that says outright it is not about this repository: hydra-gates-ref <old> does not contain: scripts/lib/check_spec_anchors.py scripts/lib/check_form_labels.py scripts/lib/check_license_triangle.py The reusable workflow floats on @main and calls those scripts BY PATH inside the PINNED package, so a pin older than the scripts cannot run the gates that implement them. A pinned ref is a silent expiry date on every upstream change, and the failure reports on the pin while saying nothing about the code. v1.5.0 is the first tag containing all of them, verified by reading each path at that tag rather than assuming the newest tag has everything. Swept across the fleet: 11 of 13 repos were pinned below v1.5.0 and every one of them was failing this way.
Drops the `hydra-gates-ref:` override from the quality caller so the input falls back to the shared workflow's own default, which is already `main`. This workflow calls ConductionNL/.github/.github/workflows/quality.yml@main. Pinning the gates package to a tag while consuming the workflow at @main splits the two halves apart: the runner moves, the gate package does not. Two fleet-wide incidents came out of exactly that split. * .github#159 — 22 repos were pinned to v1.0.1, which predated the fixes that made 16 gates actually execute. Every one of those gates reported PASS. A check that did not run looks exactly like one that passed. * .github#173 — `require-full-coverage` was flipped to default-on at @main and reached the old pinned runners, which had no coverage accounting to honour it with, so they went red on gates they had no subject matter for. Unpinned, both sides move together and a gate fix lands here without a commit here. The input is still honoured: to hold this repo still for a specific reason, set it explicitly and say why. To roll it back for everyone, revert on ConductionNL/.github main. `enable-hydra-gates` is untouched. The comment block above it kept the part that explains why the tier is on and lost the part that justified the pin. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…#2364) .coverage-baseline was read as a floor by the phpunit guard and as an exact target by the push-side staleness check. Together they demand equality with a checked-in constant, which against a moving base branch is not satisfiable: closing "stale" means committing the value the tree will measure after the PR lands. Measured on openregister — committed 58.93, development advanced 16030->16038 tests, merge result measured 58.88, guard reported a 0.05% drop. coverage-guard.php gains --against=<clover.xml>, naming a report measured at the merge base. When present it is the only floor; the committed constant is reported but not enforced. Both numbers then come from one driver in one job, so the xdebug/pcov statement-counting difference cancels rather than being baked in, and the merge base cannot go stale. Ratios are compared as exact integer cross-products, not rounded percentages: at two decimals a one-statement regression read as "unchanged" and exited 0. An empty or zero-statement report is now a hard error rather than 0%, which as the merge-base side would set the floor to zero and pass every drop. Verified on real CI clover artifacts: a genuine 1.44% drop fails, an unchanged tree passes, and adding untested code fails while adding tested code passes.
…thing (#2366) * fix(e2e): the flow-controls spec was green only when it had tested nothing `flow-controls.spec.ts` failed its FIRST attempt on 6 of the 8 CI runs that executed it, always at `waitForURL` after Save, always at 23.9-24.6s — about 4s of real work plus a 20s timeout expiring in FULL. A wait that always expires in full is not a slow round-trip; it is something that was never going to happen. WHY THE SAVE NEVER LANDED `useFlowStore`'s initial state is `emptyFlow()`, whose `name` is `''`. Only `open('new')` names the flow, and `open()` runs at the TAIL of `load()`, behind `await GET /api/flows` — a flow LIST that starting a blank flow does not need. The sidebar is interactive well before that, because `nodeCatalog` was already populated by the flows INDEX page's own `load()`. So there is a window in which the editor invites a Save of a nameless flow, `FlowController::create()` answers 400 "A flow needs a name.", `store.save()` swallows it into `return null`, and `onSave()` therefore never calls `$router.replace`. Measured locally: 9 of 10 runs red, POST body `name: ""`, response 400. That window is a real defect, not a test artefact — a user who clicks Save quickly enough gets total silence, since nothing renders `store.error` and a 400 JSONResponse is not logged. It belongs to @conduction/nextcloud-vue and is reported there; this commit does not paper over it, it makes it detectable. AND THE GREEN RUNS WERE WORSE THAN THE RED ONES Instrumenting what Save actually POSTs, over 14 local runs, split three ways: POST /api/flows 400, name="" -> red (the race above) POST /api/flows 201, nodes=1 -> red at step 4: POST .../run 500 POST /api/flows 201, nodes=0 -> GREEN The spec passed ONLY when the flow it saved had no steps. A one-node `set-fields` flow cannot run at all: since #2354 a path must end deliberately, and `FlowRunService::queue()` refuses a node with no outgoing edge that is not terminal — only `StopNode` is. So every run that genuinely persisted the step failed, and every green run was green because the race had thrown the step away first. The 20s poll dressed that up as "Run now did not produce a run". WHAT CHANGES - The fixture builds the smallest flow this app calls VALID (one terminal `stop` node) instead of one it is obliged to refuse. - Save and Run assert their RESPONSE, not a route and a poll, so a rejection fails immediately quoting the server instead of after 20s naming the wrong thing. Measured: a rejected save now fails in 4.1s, a refused run in 3.3s. - The spec waits for the state a save REQUIRES (the flow has a name) rather than racing initialisation. - `clickThemed` asserts the control is ENABLED before dispatching; a dispatched event reaches a Vue handler whether or not the button is disabled. - Swallowed `cn-flow:` store errors now fail the test by name. No timeout was widened. The three 15-20s budgets are gone, not raised: with the round-trips asserted directly, what remains are 5s router and read-back assertions, against a measured worst case of 92ms. `FlowController::run()` now answers 409 with the offending node ids instead of letting `FlowDeadEnd` escape as a bare HTML 500 — a routine authoring mistake was indistinguishable from the server falling over. CI could not have diagnosed any of this: the config wrote traces to `test-results-ci/`, which the shared workflow's upload step does not glob, and `trace: 'on-first-retry'` captures the RETRY — so for a flake that passes on attempt 2, every trace on disk was of a green run. Both fixed. Reproduction: 9/10 red before, 20/20 green after, on the shipped CI config. Verified not blind by mutation: removing the sidebar branch, blanking the node catalog, rejecting the create and refusing the run each turn it red at the matching assertion. A whole-suite positive control with the app bundle truncated to 0 bytes reds 9 of 13 specs; the 4 survivors are `object-sharing.spec.ts`, which is API-level by declaration. * docs(e2e): point the flow-controls header at nextcloud-vue#607 The two store defects the spec now guards against are filed upstream; name the issue so the next reader can tell a known upstream bug from a new one. * fix(flow): declare the FlowDeadEnd contract phpstan could not see phpstan called the new `catch (FlowDeadEnd)` a dead catch, and it was right about the evidence available to it: `FlowRunService::queue()` throws it and neither it nor `FlowService::run()` said so. That is the same omission that let the refusal reach HTTP as a bare 500 in the first place — a caller who cannot see the throw in the signature does not handle it. Declared on both. phpmd's CouplingBetweenObjects then tipped to 13 on the one added dependency. Suppressed at class level with a reason, matching the TooManyPublicMethods suppression already there and the five other classes in lib/ that carry it: this class IS the flow API surface, and splitting it to lower the count would put two controllers behind one /api/flows prefix.
…InLoopExpression, untrack the phpmd result cache (#2359) * fix(quality): scope the Migration phpmd exclude to lib/, drop a foreign copyright claim, clear the coverage ratchet phpmd-unusedparams.xml carried <exclude-pattern>*/Migration/*</exclude-pattern>. PDepend compiles an exclude-pattern into an UNANCHORED regex (Input\ExcludePathFilter preg_quote()s the pattern, then turns `\*` into `.*`), so that form matches ANY path containing a `/Migration/` segment - lib/Service/Migration/, lib/Command/Migration/, any future lib/*/Migration/. Those are ordinary classes with no interface-mandated signature, so a genuine unused parameter in one would never be reported and the run would still look clean. openconnector carried exactly such a file. openregister has no lib/*/Migration/ directory today, which is precisely why this had to be fixed before one appears: the broad form fails silently and only on the day someone adds the directory. Now `*/lib/Migration/*`, matching the 19 repos that already carry the corrected shape. Twelve files carried `SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors`, scaffolding residue from the Nextcloud app skeleton. The licence sweep in #2350 relabelled the adjacent SPDX-License-Identifier from AGPL-3.0-or-later to EUPL-1.2 - which asserts that Nextcloud GmbH's copyright is EUPL-licensed. We cannot relicense a third party's copyright. Every one of the twelve is Conduction-authored: each carries `@author Conduction Development Team` and `@copyright Conduction B.V.` in its own PHPDoc, and `git log --follow` shows only Conduction committers. The stray holder is corrected to Conduction rather than deleted, so no file loses its REUSE metadata. Side effect, measured by running phpcs on both versions at the identical path: 2 pre-existing "Missing short description in doc comment" errors go away. CountInLoopExpression retired entirely - all 3 baseline entries, all 3 findings. Two are `do { … } while (count($page) === $limit)` where the page is replaced wholesale each iteration and never mutated in the body, so the count is taken once per page into a variable; one is `for ($i = 1; $i < count($rings); $i++)` over an array the body does not touch, so the count is hoisted. Behaviour is identical in all three. .phpmd.result-cache.php untracked and gitignored. It is generated output, and a correctness hazard while committed: `composer quality:phpmd-score` passes --cache, so a stale cache in the tree makes PHPMD replay a verdict for code that has since changed - a gate reporting a result it never computed. .coverage-baseline 58.87 -> 58.93. This was the ONLY red job on development: CI measured coverage that had improved past its own committed baseline. Raising it tightens the ratchet. Unit suite before and after, same container and same vendor: 16030 tests, 35963 assertions, 0 failures, 0 errors - byte-identical totals. * fix(spec): repoint 6 @SPEC anchors that gate-46 could not resolve Hydra gate-46 (spec-anchor-existence) failed on this PR with 6 unresolved targets. All six are pre-existing debt in files this PR already touches, which is what pulled them into the gate's ADR-020 diff scope; none was introduced here. 62 of the other gates passed and coverage was 60 of 60 applicable, so this was a single real failure, not a broken run. lib/Service/VocabularyImportService.php (4 tags) pointed at openspec/changes/skos-concept-registers/... - a CHANGE directory. That change was archived on 2026-07-23, so the path stopped existing the moment it moved to openspec/changes/archive/. A @SPEC tag must target the canonical openspec/specs/ home, which is where the spec lives now; two of the four also carried "#skos-002", which is not a heading, and now name the heading that actually exists. lib/ContextChat/ContentProvider.php (4 tags, 2 distinct anchors) named "#requirement-getitemurl-must-resolve-through-the-existing-deep-link-registry" and "#requirement-initial-import-must-walk-opted-in-schemas-in-batches-and-must- be-re-runnable-via-occ". Neither heading exists; both requirements were merged into one, "Requirement: getItemUrl and initial import reuse existing OpenRegister infrastructure", and the tags were never moved with it. Every target verified against gate-46's OWN two slug rules - slugify() and gh_slugify(), which differ on punctuation inside a word - by resolving each fragment back to the heading text it matches. No overlap with #2355, which repoints a different set of anchors in file-actions. * revert(quality): drop the .coverage-baseline bump — the number drifts with development I raised .coverage-baseline 58.87 -> 58.93 because that was the value CI itself recomputed on development, and Coverage Baseline Check was development's only red job. On this PR it then failed the OTHER direction: Coverage baseline: 58.93% Coverage current: 58.88% FAIL: Coverage dropped by 0.05% Not a regression from this PR. Development moved between my two CI runs — the suite went 16030 -> 16038 tests — so the merge base this PR is measured against computes 58.88, not the 58.93 that development's own HEAD computed earlier. The two jobs also check opposite things: development's runs coverage-guard.php --update-baseline and fails when the committed value is STALE, while a PR runs it plain and fails when coverage DROPS below the committed value. Pinning a number from one tree to satisfy the other is what broke this. So the bump leaves this PR. It belongs in a one-line change computed on development's own HEAD, at a moment development is not mid-merge — not carried in on a PHPMD branch whose merge base keeps moving underneath it. The job was red before this branch existed and is unaffected by it either way. Nothing is weakened: .coverage-baseline returns to development's committed 58.87, exactly as found. * fix(quality): retire CountInLoopExpression from the baseline and drop 10 entries for a deleted file phpmd.baseline.xml 519 -> 506 entries, and one rule family leaves ENTIRELY. A PHPMD baseline entry is scoped to (rule, file) - optionally a method, NEVER a line - so one entry covers every current AND future violation of that rule in that file. It is an open licence, not a record. Shrinking the count is therefore not the point; getting a family to zero is, because only then does a NEW violation of that rule fail CI. CountInLoopExpression: all 3 entries removed. The 3 findings behind them were fixed in this PR, not suppressed. Verified with a single-rule ruleset over all of lib and NO baseline in play: 3 findings before, 0 after. lib/Service/Flow/FlowActionService.php: 10 entries for a file that no longer exists (WeightedMethodCount, CouplingBetweenObjects, LongVariable, ShortVariable, MissingImport, and Cyclomatic/Npath on run/runNamedFlow/runAction). Deleted with the file; suppressing nothing; free to remove. The other 509 entries are all LIVE and were left alone. I checked, and the first answer was wrong in an instructive way: matching baseline entries against the report by rule name reported FIVE families - NPath (62), LongMethod (33), WeightedMethodCount (32), LongParameterList (14), LongClass (6) - as "entirely stale", 147 free deletions. They are not. The baseline stores the rule CLASS (PHPMD\Rule\Design\LongMethod) while the XML report writes the rule NAME (ExcessiveMethodLength), and those five differ. With the mapping applied the accounting closes exactly: 767 true findings, 767 suppressed by live entries, nothing unexplained. Uniformity across five independent families was the tell. Measured with the baseline file MOVED ASIDE, not by dropping --baseline-file: PHPMD auto-discovers phpmd.baseline.xml sitting next to the ruleset and applies it either way, so un-flagging it yields a silently baselined run that looks clean. Independently corroborates #2347's 749 + 16.
…editor state, and two wrong assertions (#2365) * fix(migration): bump the app version so the flow-status migration can run Running any flow on an existing instance 500s: SQLSTATE[42703]: Undefined column: 7 ERROR: column "status" of relation "oc_openregister_flows" does not exist Version1Date20260805100000 adds `status`, `status_message` and the four `last_run_*` columns, and it landed in #2354 — but `appinfo/info.xml` last changed its version in #2265, earlier. Nextcloud runs an app's migrations on a version CHANGE, so an instance already sitting on 0.2.17-unstable.24 never ran it and never will. A fresh install is unaffected, because it runs every migration from empty. That is exactly why nothing caught this: CI installs fresh, so CI is green while every existing instance cannot run a flow at all. Confirmed on the dev instance: the three columns were absent, `occ upgrade` after this bump created them, and a flow that had been 500ing then queued and ran. * chore(deps): @conduction/nextcloud-vue 2.2.0-vue3.5 Carries ConductionNL/nextcloud-vue#605: CnFlowDetail now reloads when the route names a different flow. Without it, opening a flow and then moving to another one or to `new` left the previous flow in the store as well as on the canvas — and `save()` picks PUT over POST from `flow.id`, so Save on a page presenting itself as a blank new flow issued a PUT against the flow just left, overwriting it. Verified against this bundle, on the same path that reproduced it: with "Hydra label transition" open, moving to /flows/new now shows an empty canvas and a Name field reading "New flow" rather than "Hydra label transition". `vue` is pinned back to ^3.5.18 by hand. `npm install` rewrote it to ^3.5.0 to match the new package's own range, which then contradicted the `overrides` entry — CI's npm 10.8.2 refuses that outright: npm error EOVERRIDE Override for vue@^3.5.0 conflicts with direct dependency Local npm 11 accepts it silently, so `npx npm@10.8.2 ci --dry-run` is what caught it: clean on development, EOVERRIDE with the rewritten range, clean again once restored. * fix(e2e): the flow spec was asserting two things that are not true Both surfaced once the spec ran against a fully migrated instance. WAITING FOR A NAVIGATION THAT NEVER HAPPENS. `page.waitForURL` defaults to `waitUntil: 'load'`, and this app uses a HASH router — a hash-only change fires no navigation event, so the wait timed out on a save that had in fact succeeded and moved the route. Polling the URL asserts the same thing without depending on an event the router does not emit. A ONE-NODE `set-fields` FLOW IS NOT RUNNABLE, AND SHOULD NOT BE. The spec built one and expected Run to produce a run. FlowRunService refuses it, with a good reason: a node with no outgoing edge that does not end the flow means "a run would stop there and still be reported as completed". The spec was asserting the absence of a guard that exists on purpose. It now places a `Stop` node — a terminal step type, so a single one is a complete flow — which saves AND runs. Confirmed by hand first: the same sequence with `Edit fields` is refused and with `Stop` returns one run. * ci: move the gate pin to v1.5.2 so gate-28 stops failing empty-scope PRs Hydra Gates reported 59 gates green and failed anyway, because gate-28 counted itself APPLICABLE and did not run: [gate-28] license-triangle: SKIPPED (structural) — lib/ exists and composer.json declares license=EUPL-1.2, but 0 in-scope lib/**/*.php file carried an @license ... so NOTHING was compared Which is true, and not a gap. This PR's diff is an info.xml, a package.json, a lockfile and one spec — no PHP at all. Nothing was in scope, which is ADR-020's diff-scoping working exactly as intended, and NOT APPLICABLE is the classification that exists to say so. With require-full-coverage now on by default, the misclassification fails every PR that touches no lib PHP. .github#182 fixed it, and the fix had been sitting on main untagged since. Every consumer pins a tag, so an untagged fix reaches nobody — v1.5.2 was cut at that commit for this bump. * ci: bring coverage-guard.php up to the version the shared workflow requires The PHPUnit job failed after all 16038 tests passed: scripts/coverage-guard.php predates merge-base comparison. Update it from ConductionNL/.github before enabling the ratchet. The shared workflow probes `coverage-guard.php --capabilities` for `against` before trusting the ratchet on a pull request, and refuses rather than silently skipping — a check that did not run must not look like one that passed. This repo's copy is the 1.6KB version that has no such flag. Its own comment says "every repository that sets enable-coverage-guard was updated before this workflow changed, so this should never fire; if it does, the repository is the thing to fix." openregister was missed by that sweep, which is why it fires here. Taken verbatim from nldesign, which carries the current version — and whose docblock cites THIS repo as the measured case for why the merge-base floor had to replace the committed constant. The fix was written for openregister's problem and openregister never received it. `.coverage-baseline` (58.87) is untouched; under the new script it is reported for information on a PR and the merge-base measurement is the floor. procest carries the same stale 1.6KB copy and will hit this the moment it enables the ratchet. * chore: re-run CI The coverage-guard fix pushed at 07:37 did not produce a Code Quality run — GitHub emitted no `synchronize` for it, so the PR's checks stayed pinned to the previous commit's failure. A workflow_dispatch on the same head passed (23 jobs, 0 failures), but a dispatch run is not what the PR reports, and merging over a check describing older code is the habit this repo has already been bitten by (#2227, #2228). This empty commit exists only to make the PR's own checks describe the code that will actually merge. * fix(e2e): wait for the flow to load, and say why a save was refused CI failed on "Save did not move the route off `new`", which is a symptom three different faults share: the request was refused, the request was never sent, or it succeeded and the router did not follow. The spec could not tell them apart, so the first job was to make it say. It now records every /api/flow* call with its status and reads the sidebar's error text, and re-throws with both. That turned an opaque timeout into: flow API calls: ... | POST api/flows -> 400 | ... sidebar error : (the sidebar showed no error) A 400 on create is "A flow needs a name." — and the Name field shows "New flow", so the name was missing at the moment of the POST rather than missing from the form. `load()` resolves the catalogues and the flow independently: the palette can be populated and clickable while `open('new')` has not yet stamped the default name, and a node added in that window is saved against a nameless flow. Driving the UI by dispatched events, with no human pause anywhere, lands in that window almost every time — which is why CI reproduced it and a hand-driven browser did not. The spec now waits for the Name field to carry a value before adding a step. That is the observable proof the flow has loaded, not merely that the panel has rendered. Three consecutive local runs pass. Worth noting separately: the store logs a failed save to the console and renders nothing, so a user gets a Save button that silently does nothing. That is a real gap in CnFlowSidebar, not something this spec should paper over.
…ommand injection) (#2368) quality / Security (composer) is red on every PR here as of today: Advisory ID: PKSA-rdkp-vv9z-mjkg CVE: CVE-2026-67434 — OS Command injection Affected versions: <3.13.6|>=4.0.0,<4.0.2 Reported at: 2026-08-05T23:53:11+00:00 The advisory was published YESTERDAY and roave/security-advisories installs as dev-latest each run, so the same lockfile was clean on 2026-08-05 and is vulnerable on 2026-08-06 with no commit in between. The last green run is evidence of when it ran, not that the lockfile is safe. composer.json's existing constraint already permits the fixed version, so this is a lockfile move only: 1 update, 0 installs, 0 removals. Verified the diff touches exactly two lines, both the version string, and no other file. Part of a fleet sweep — 13 of 16 repos checked were on the affected 3.13.5.
…2370) The Security (composer) job started failing on every branch today: Advisory ID: PKSA-rdkp-vv9z-mjkg CVE: CVE-2026-67434 Title: OS Command injection Affected versions: <3.13.6|>=4.0.0,<4.0.2 Reported at: 2026-08-05T23:53:11+00:00 The lock held 3.13.5. Nothing here caused it — the advisory was published last night — and it affects development just as much as any branch, which is why this is its own commit rather than a rider on a dependency bump. `composer.json` already allowed the fix (`^3.9`), so only the lock moves: 3.13.5 -> 3.13.6. No constraint change, no other package touched. Verified with `composer audit --locked`, which is the set CI installs from: the CVE is absent. (A plain `composer audit` still reports it here, because this checkout's vendor/ is container-owned and still holds 3.13.5 — the lock was updated with --no-install.) The audit also reports dompdf, guzzle and phpspreadsheet advisories that CI does not fail on. Those predate today and are a separate decision; this commit deliberately does not widen its scope to them.
Carries ConductionNL/nextcloud-vue#608, which closes #607: the flow editor no longer lets a user press Save before the store has a flow to save. The window was real and wide. `emptyFlow()` has `name: ''`, only `open('new')` supplies the default, and `open()` ran behind `await GET /api/flows` — a list a blank flow does not need — while the sidebar was already rendered and Save already enabled. Saving in that window posted `name: ""` and the API answered 400 "A flow needs a name." A refused save rendered nothing at all, so the user saw the button flicker and no more. The same late `open('new')` also reset the flow, wiping a step already placed on the canvas. Verified against this bundle, in the window itself: clicking Save 120ms after opening a blank flow — where a 400 was previously reproducible 9 times in 10 — returns 201 with `name: "New flow"` and the route advances to the new uuid. `vue` is pinned back to ^3.5.18 by hand again. `npm install` rewrites it to ^3.5.0 to match the new package's own range, which contradicts the `overrides` entry, and CI's npm 10.8.2 refuses that with EOVERRIDE while local npm 11 accepts it silently. `npx npm@10.8.2 ci --dry-run` is clean with the range restored. Worth automating; for now it is a hand check on every bump of this package.
…hpcs errors (#2371) Security squizlabs/php_codesniffer 3.13.5 -> 3.13.6. OS command injection, GHSA-hmqg-cxww-wqhq / CVE-2026-67434, reported 2026-08-05. Affected: <3.13.6 | >=4.0.0,<4.0.2. All 16 fleet repos checked were on 3.13.5. Pre-existing phpcs errors, fixed here per the always-fix-pre-existing rule lib/Db/Webhook.php and lib/Migration/Version002003000Date20251013000000.php both had a file docblock with no short description. These are NOT caused by the phpcs bump — proven by an explicit A/B on the same tree: arm A phpcs 3.13.5 -> rc=1, 4 ERROR lines arm B phpcs 3.13.6 -> rc=1, 4 ERROR lines, byte-identical after stripping ANSI codes Each arm's version string was asserted before running, so the two arms really did differ; the first comparison I wrote matched nothing in EITHER arm and was discarded as worthless. 3.13.6 introduces no new findings — development was simply already red. Verified - composer audit --locked: 'No security vulnerability advisories found' (was 1). - vendor/bin/phpcs --version -> 3.13.6. - composer phpcs: rc=1 before -> rc=0 after. - php -l clean on both edited files. - Positive control: a deliberately non-conforming file under lib/ made phpcs exit 2 with 13 findings. Probe removed. - config.platform.php is 8.3 here; the run above was on a host PHP where the vendor tree parses, so the rc=0 is a real pass and not a parse-error 255.
… npm high) (#2373) GHSA shell injection via an unsanitised --workspace argument; affected 2.1.0 - 4.2.1. This was the only remaining high-severity npm advisory on development after the php_codesniffer CVE fix. npm audit --package-lock-only on development: before: 0 critical, 1 high after: 0 critical, 0 high Reachability: this repo passes no --workspace, so the advisory was not exploitable here. Bumped anyway because it is cheap and the SBOM step runs this binary directly. Verified (CI is node 20 / npm 10.8.2; local npm 11 prunes entries CI needs, so the lockfile was regenerated with npx npm@10.8.2) - npx npm@10.8.2 ci: rc=0, lockfile md5 identical afterwards. - The bump IS exercised: CI runs 'npx @cyclonedx/cyclonedx-npm', which resolves the local devDependency. That binary now reports 6.0.0, and the EXACT CI command succeeded (rc=0) producing a valid CycloneDX specVersion 1.5 SBOM with 652 components — identical to the 4.2.1 baseline, so the major bump changed the tool without changing the artifact. - npm run build rc=0; npm run lint rc=0. Both baselined on untouched development first. - check:specs rc=0, test:l10n rc=0, jest (npm test) rc=0 - Licence read from the LOCKFILE: cyclonedx-npm 6.0.0 is Apache-2.0. vue3-apexcharts is 1.8.0 MIT, still the pre-proprietary line, untouched.
…es (#2375) axe-core sat in `dependencies`, declaring an accessibility *testing* library as an application runtime dependency. Measured: nothing under src/ imports axe-core, and the built production bundle does not contain it — 0 hits for axe own signature rule id `aria-allowed-attr` across the built js/, while that string is present in node_modules/axe-core/axe.min.js (positive control proving the grep can match). What put it in every app manifest is @conduction/nextcloud-vue, which declares axe-core as an OPTIONAL peerDependency. nc-vue does use it, but only in `src/testing/a11y.js` — a testing helper never imported from `src/index.js`, so it never reaches an app bundle. nc-vue own file header states axe-core "is a devDependency" and that consumers wanting the a11y assertion "add axe-core to their OWN devDependencies". This change follows that instruction. Not a bundle-size fix; the bundle is byte-identical. It stops a test dependency being declared as production surface (SBOM, `npm ci --omit=dev`, advisory triage). An optional peer is satisfied by a devDependency, so nothing breaks. Verified: npm ci + production build exit 0; full CI green on this PR.
… 28+ (#2372) * fix(contextchat): OCP\ContextChat is NC 33+, but this app supports NC 28+ Two classes named an `OCP\ContextChat` type in their CONSTRUCTOR, which is fatal on every Nextcloud below 33 — `lib/public/ContextChat/` does not exist on stable31 or stable32, and appinfo/info.xml declares min-version="28". The load happens in the container, not at class declaration: `SimpleContainer::resolve()` calls `new ReflectionClass()` on each parameter type. That call is the load. ContextChatReindexCommand listed in appinfo/info.xml, so Console\Application::loadCommandsFromInfoXml() reflects it on EVERY occ invocation. Observed in CI on NC 31.0.14.1 during `occ app:enable`: Interface "OCP\ContextChat\IContentProvider" not found. It is logged at level 3 and the enable still reports success — loud in the log, invisible in the exit code, which is how it survived. ContextChatSubmissionListener registered for ObjectCreatedEvent, ObjectUpdatedEvent and ObjectDeletedEvent, so it is resolved on EVERY object write. On NC 28-32 that made every create, update and delete throw ReflectionException, for a feature the instance is not even using. Its docblock claimed IContentManager was "always resolvable via DI"; it is not. Both now resolve through the container inside the method body, behind an interface_exists() guard, and both call sites in the listener are guarded separately — submitContentItem() and removeContentItem() each read ContentProvider:: constants, and loading THAT class is fatal too (its header implements the missing interface). ContentProviderRegistrationListener is deliberately unchanged: it is only ever resolved by dispatching ContentProviderRegisterEvent, which cannot fire on a server without Context Chat. Registering it is safe because `::class` on an FQCN is compile-time and never autoloads. VERIFIED with a probe that maps OCP\ContextChat\* to nothing, exactly as those servers do, then mimics SimpleContainer::resolve(): ContextChatReindexCommand before FATAL Interface "OCP\ContextChat\IContentProvider" not found after OK ContextChatSubmissionListener before FATAL Class "OCP\ContextChat\IContentManager" does not exist after OK ContentProvider (control) still FATAL — proves the probe really does simulate NC < 33 rather than passing everything The first probe I wrote proved nothing: it read parameter types via getType()->getName(), which returns a string WITHOUT loading the class, so the unfixed command "passed". The fatal only appears once each parameter type is itself reflected. TESTS. tests/Unit/ContextChat/ContextChatVersionGuardTest.php — 4 tests pinning the shape, since a machine where OCP\ContextChat resolves cannot reproduce the fatal. Its first version had the same class of hole as the probe: it checked only whether the parameter TYPE NAME contained OCP\ContextChat, so re-adding `ContentProvider $contentProvider` to the command — the original defect, verbatim — still passed, because ContentProvider is an OCA class. It now walks class_implements()/class_parents() as well. Mutation-verified after the fix: re-add ContentProvider to the command's constructor 1 failure drop one of the listener's two guards 1 failure restored OK (4 tests) The existing ContextChatSubmissionListenerTest passes unchanged in substance — only the constructor wiring moved to a container mock returning the same IContentManager mock, so all 26 assertions still observe what they did before. * fix(phpstan): ignore the RETURN-type variant for OCP\ContextChat too phpstan failed with: Method ContextChatSubmissionListener::contentManager() has invalid return type OCP\ContextChat\IContentManager. which is itself confirmation of the defect this branch fixes: openregister's own nextcloud/ocp stubs ship NO ContextChat at all, exactly as NC 28-32 does not. phpstan.neon already ignores the OCP\ContextChat class as a parameter or property — but it words a parameter/property as 'has invalid type' and a return as 'has invalid RETURN type', and only the former was listed. The new private accessor returns ?IContentManager, so it needed the second wording. Not a new kind of suppression: the OCA\Bookmarks block directly above is the same shape — an optional peer app resolved lazily via \OCP\Server::get() behind a class_exists guard — and already carries its own 'has invalid return type OCA\Bookmarks\' entry for this exact reason. The block's existing comment explains why ignoring beats baselining here: a baseline entry goes stale the moment the optional component IS present. Verified narrow rather than blanket: replacing the return type with an unrelated non-existent class still produces 'Found 1 error', and phpstan is [OK] No errors once restored. * fix(spec): point the ContextChat @SPEC anchors at headings that exist gate-46 spec-anchor-existence failed with 10 unresolved @SPEC targets, all in the two files this branch touches. They are pre-existing — development does not fail gate-46 because the gate is diff-scoped and these files were not in any recent diff — but they became mine to fix the moment I edited the files. Every anchor named a requirement heading that was never written. The spec has three requirements; the code cited four inventions of them: #requirement-only-opted-in-schemas-must-have-their-objects-submitted-… #requirement-only-published-objects-must-be-submitted-to-context-chat #requirement-object-deletion-must-remove-submitted-content-from-… all three describe ONE requirement, "Only opted-in, published objects are submitted to Context Chat", whose body also covers deletion ("Deleted objects … SHALL have their content removed … rather than left stale"). Repointed to its real slug. #requirement-initial-import-must-walk-opted-in-schemas-in-batches-and-… is "getItemUrl and initial import reuse existing OpenRegister infrastructure", whose body carries the batching and occ-scoping language. Repointed likewise. Nothing in the spec was changed: the requirements already say what the code claims, the citations were simply written from memory rather than from the headings. Verified: gate-46 PASS after the rewrite, and FAIL — 1 unresolved when a deliberately bogus anchor is injected, so the pass is a verdict rather than the gate having gone quiet.
) remark-cli, remark-lint-list-item-indent, remark-preset-lint-consistent and remark-preset-lint-recommended are declared but never used. Verified dead on four counts, not a manifest grep alone: npm scripts referencing remark NONE .remarkrc* config files NONE "remarkConfig" key in package.json absent workflows referencing remark NONE The absence checks are positive-controlled: the same repository listing that returns nothing for remark does return .babelrc, eslint.config.js and webpack.config.js, so "no config" is a real absence rather than a broken lookup. Removing them also drops their transitive tree (unified/mdast/micromark), which is most of the lockfile delta. That delta was controlled for npm-version skew: regenerating the lockfile against the UNMODIFIED base with the same npm 10.8.2 produces zero change, so the deletions are genuinely the remark tree and not a reformat. Lockfile regenerated with npx npm@10.8.2, never --legacy-peer-deps (that strips every `peer: true` entry — 529 of them in a sibling repo). Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…s exist (#2378) * chore: require Nextcloud 32, which is where the ContextChat interfaces exist * chore: correct the stale NC range in the CI compose comment
…e/compat (#2377) None of these are imported anywhere in src/. Measured with a local grep over the checked-out tree, positive-controlled so an empty result means a real absence and not a broken search: the same grep finds 66 files importing @nextcloud/axios in openregister. bare "axios" 0 imports the apps use @nextcloud/axios @fortawesome/* 0 imports @vue/compat 0 imports @vue/compat is worth a note. A naive grep DOES hit it in webpack.config.js — but only inside comments recording that it was REMOVED: "build runs on the REAL Vue 3 runtime — NOT @vue/compat" "PURE VUE 3 (ADR-066): the @vue/compat MODE-2 compiler shim is gone" A checker that greps a bare string matches every comment about a thing as readily as a use of it. Confirmed dead by reading the hits, not counting them. NOT removed from opencatalogi, which genuinely uses both: 2 files import bare axios and 3 import @FortAwesome. Same manifest line, opposite verdict — so this was decided per repo, not swept. Lockfile regenerated with npx npm@10.8.2, never --legacy-peer-deps. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
Coverage Baseline Check fails on a non-empty git diff after --update-baseline.
Measured 58.88% on two independent development push runs 8h apart
(31050093224 and 31074199287); .coverage-baseline said 58.87.
This RAISES the baseline to the measured value, which is what the job asks for
('The Coverage Baseline Protection job accepts any value that does not LOWER
the baseline'). No threshold weakened, no waiver added.
…autoload prelude (#2358) * docs(apphost): correct the laziness claim and document the autoload prelude Bootstrap.php claimed 'Lazy by construction — a disabled OpenRegister never fatals NC bootstrap', and Routes.php claimed that referencing no OR symbol makes requiring it 'safe even when OpenRegister is disabled'. Both hold for the closure BODIES, not for resolving Bootstrap/Routes themselves: calling either is an ordinary autoload of an OCA\OpenRegister\ class, and it throws when that prefix is unavailable. That is the normal case for leaves sorting before openregister, because getEnabledApps() sort()s the app list and Coordinator::registerApps() calls registerAutoloading() then register() one app at a time. Documents the required autoload prelude as part of AppHost adoption, records why loadApp() and a relative vendor/autoload include are both wrong, and adds the matching requirement + scenarios to the canonical apphost-boilerplate spec. Documentation only — no behaviour change. * chore(gates): declare the gate-47 opt-out for this documentation-only diff [hydra-gate-security-change-has-tests exclude] Documentation-only diff: two docblocks under lib/AppHost/ and one openspec spec file. gate-47 classifies Bootstrap.php as security-touching because the FILE CONTAINS matching tokens, not because this diff changes any of them — no executable statement is added, removed or altered, so there is nothing for a test to exercise. Recorded as an empty commit because the gate reads the head commit message, and a re-run replays the original event payload rather than an edited PR body. * chore(ci): re-trigger quality after moving the gate-47 opt-out into the PR body The previous opt-out lived only in the commit message. On a pull_request event actions/checkout checks out the refs/pull/N/merge ref, so the gate's `git log -1 --pretty=%B` reads GitHub's merge commit, not ours, and never sees it. HYDRA_GATE_PR_BODY is the path that works — and a re-run replays the original event payload, so a fresh push is needed for the edited body to be picked up.
…eadings (gate-46) (#2355) * docs(spec): split file-actions into behaviour-named requirements Replaces the two catch-all headings ("Object register folder management", "File CRUD operations on objects", "Object tagging via Nextcloud system tags") with requirements named for the behaviour they actually contract, so the @SPEC anchors in FileService can point at something that exists and stays meaningful. * fix(spec): repoint 10 dangling @SPEC anchors onto the new file-actions headings (gate-46) * fix(spec): repoint the tag-preserving-redaction anchor at the canonical spec (gate-46) The old anchor pointed into openspec/changes/, which is a change dir, not the canonical openspec/specs/ home.
…r repos (#2380) #2378 raised min-version 28 -> 32. Its premise was correct WHEN WRITTEN and is no longer: "the registration listener cannot prevent it, because the failure happens when PHP reads the class header, not when the provider is registered" That was true, and #2372 is what changed it. #2372 removed every EAGER reference to ContentProvider — the command's constructor typehint and the listener's — so the class is now only loaded from inside interface_exists('OCP\ContextChat\IContentManager') guards. On a server without OCP\ContextChat the class header is never read: the feature is inert, and nothing logs. WHAT THE FLOOR ACTUALLY COSTS. min-version is enforced at INSTALL time, so 32 makes `occ app:enable openregister` refuse on 31. Eight fleet repos install openregister as an additional-app while testing stable31 — procest, shillinq, portaliq, openbuild, decidesk, doriath, hermiq, larpingapp. Each now fails its seed with {"success":false,"message":"OpenRegister is not installed or enabled"} and then a 404 from /api/configurations/import, because the fallback importer belongs to the app that did not install. Observed on procest#758, where it looked for a while like a manifest bug in that PR. EVIDENCE THAT 31 WORKS. procest's e2e installs openregister at `development` and runs stable31. Its run at 2026-08-06T14:04Z — six minutes AFTER #2372 merged at 13:58Z — passed. The app demonstrably boots, enables and serves on NC 31 with that fix in place. The second reason the CI compose gave for a 32 floor does not hold either: OCP\DB\Types::DATETIME_IMMUTABLE is present in stable31 as well as stable32. Checked directly against both branches. ALSO CORRECTS MY OWN ERROR. #2372 and its test say "NC 33" throughout. That came from a local lookup against refs this checkout does not have, which answers ABSENT for everything — the failure mode a positive control exists to catch, and I did not run one at the time. Verified properly against raw.githubusercontent, with IUserManager.php as the control: stable31 IContentProvider 404 control 200 stable32 IContentProvider 200 control 200 So the interfaces arrive in 32, not 33. Every "NC 33" claim in the listener, the command and the guard test is corrected to 32. 28-31 remain untested by this repo's own CI, exactly as #2378 observed. That is a real coverage gap and worth closing, but it is a different thing from declaring those versions unsupported — the eight repos above ARE the coverage, and they were green on 31 until the floor moved. 26 ContextChat tests pass; phpcs clean.
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
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated PR to sync development changes to beta for beta release.
Merging this PR will trigger the beta release workflow.