Release: merge development into beta - #1711
Open
github-actions[bot] wants to merge 2965 commits into
Open
Conversation
…#2149) discover() returns repo cards and install() takes a bundle, but nothing read a published bundle FILE out of a repo — so a user who found a repo couldn't install it through the engine. fetchBundle(repo, path, credentialId?) reads the repo's contents file (anonymously for public repos, via the broker when a credential is set), base64-decodes and JSON-decodes it to the bundle. Exposed as GET /api/federated-config/fetch?repo=&path=. This is the capability hermiq's cutover needs to retire its own GitHubTemplateCatalogService fetch path: discover (topic search) → fetch (this) → install, all on the shared engine. 2 unit (decodes a contents file; rejects missing content). Live-verified on 8080: fetched a real openbuild-app.json from a public GitHub repo and decoded it; a missing path is a 404. @SPEC openspec/changes/federated-config-sharing/specs/federated-config-sharing/spec.md
…t global/per-app Two apps (or two registers) that each want a schema under the same generic slug (e.g. 'automation') could end up sharing one row: ImportHandler's existing-schema resolution matched by application (or globally, without an app id) rather than by the target register's own schema set, so an importing register could silently reuse a same-slug schema it did not own. This is the root cause of the OpenBuild 'automation' collision (CRM app's schema #71 reused instead of creating OpenBuild's own). - ImportHandler::importFromJson() precomputes, per schema slug, the union of the pre-import schema ids already attached to the register(s) this import declares that slug for (computeRegisterScopedSchemaIds()). - importSchema() resolves the existing schema via SchemaMapper::findBySlugInIds() scoped to that set (resolveSchemaWithinRegisterScope()), ahead of the app-scoped/global fallback (resolveSchemaByApplication()/resolveSchemaGlobally()), which now only apply to schemas with no register context in the import. - Migration Version1Date20260726000000 drops the schemas_org_app_slug_unique DB index; no replacement index is added since schemas are many-to-many with registers (769 schemas shared across >1 register today) and "unique within a register's set" cannot be expressed as a single-table constraint. The invariant is enforced at the service layer instead. openspec/changes/per-register-schema-slug-uniqueness has the full design and spec delta. Tests prove: two registers importing the same slug get two distinct schemas; re-importing the same register/slug updates in place; a schema shared across multiple registers is untouched by an unrelated import.
…ter-resolver integration collections (#2142) All three tests/integration/*.postman_collection.json collections were red against NC stable32, live-verified on a disposable NC32 instance: - apphost-observability: real bug. GET /api/health and /api/metrics both 503'd with "App controller is not enabled" — Application.php's registerAppHostObservability() wired every supporting AppHost service (ManifestLoader, HealthCheckExecutor, MetricsEngine, metric sources) but never registered the GenericHealthController/GenericMetricsController aliases routes.php's 'AppHost\Controller\GenericHealth#index' / 'AppHost\Controller\GenericMetrics#index' route names resolve to, so self-dogfooding of OpenRegister's own ADR-006/ADR-040 observability contract was dead on arrival. Fixed by registering both controllers under the exact DI key NC's RouteParser derives. Also fixed the collection's health_path/metrics_path, which pointed at a never-existed /api/apphost/health path and the bare (no /index.php/) URL form that 404s on the CI PHP built-in server. - magic-mapper-import: env/setup gap + stale test. The collection referenced dev-only fixtures never committed to the repo (softwarecatalogus_register_magic.json, magic-mapper-data/module.csv) and posted to a route that no longer exists (/configurations?force=true; the real route is /api/configurations/import). Rewritten to self-provision its own register + schema and exercise RegistersController::import's actual status codes (400 missing schema, 404 unknown register, 200 success) against a committed CSV fixture, then verifies the imported rows via GET /api/objects (object storage is unconditionally backed by OpenRegister's per-schema MagicMapper tables per SettingsService::getSearchBackendConfig()). Added a .gitignore exception so the CSV fixture (blocked by the blanket `*.csv` rule) ships with the app. - register-resolver: real bug + URL fix. GET /api/registers/{unknown} threw an uncaught DoesNotExistException, 500ing with an HTML error page instead of a clean 404 (the assertion already tolerated 500, masking it). RegistersController::show() had no try/catch, unlike the identical DoesNotExistException guard SchemasController::show() already has. Added the same catch(DoesNotExistException -> 404) / catch(Exception -> 500) pattern, tightened the collection's assertion to require the clean 404, and updated the unit test (testShowThrowsWhenNotFound -> testShowReturns404WhenNotFound) that had documented the broken behaviour as expected. Also fixed every request URL: base_url is overridden by CI to a bare host (http://localhost:8080), so paths must hardcode /index.php/apps/openregister/api/... the way every other collection in this directory does — this collection's base_url default baked in the app path instead, which the CI override silently stripped. All three collections pass 0 failures on a disposable nextcloud:32-apache instance, run exactly as the Code Quality "Integration Tests (Newman)" job invokes them. tests/Unit/Controller/RegistersControllerTest.php and tests/Unit/AppHost/* (152 + 121 tests) pass; phpcs clean on both touched lib/ files.
Sync the 3 new requirements into the canonical data-import-export spec and archive the change directory per repo convention.
…ug-uniqueness fix(import): scope schema slug uniqueness to a register's own set, not global/per-app
…andler (#2151) The Newman "openregister-crud" and "rbac" integration collections passed in isolation but failed in the CI job's combined sequential run (all ~14 collections against one shared NC32 instance, no DB reset). Root cause was a register-resolution gap in the authorization path, exposed by cross-collection state pollution. PermissionHandler::getRegisterForSchema() and getRegisterAuthorization() loaded the register via RegisterMapper::find($id) with the default org-scoped multitenancy filter. Their sibling id lookup (getAllRegisterIdsWithSchema) is deliberately global/unscoped, so once the id is known the entity fetch must be too — it is an authorization-policy read, not a tenant-scoped data read. When an earlier collection (openregister-crud) left multitenancy enabled and switched the admin's active organisation (never restoring it, and deleting its test org), a legitimately-linked register whose organisation no longer matched the caller's active-org pointer became unresolvable: find() threw DoesNotExistException, which was re-thrown as AuthorizationUnresolvableException and FAIL-CLOSED — denying even a non-admin creating/reading their OWN object in an OPEN schema (404s, IDOR fail-closed firing on a legit register). Fix: load the register with _rbac:false, _multitenancy:false in both resolvers. This does NOT weaken tenant isolation — object-row multitenancy is enforced separately on the data reads (organisation column filters + MagicRbacHandler's active-organisation condition). Verified: rbac section 3 (cross-org isolation, "admin org-Z object filtered out", "cross-org GET denied 404 no existence leak") still passes. The fail-closed remains intact for genuinely-unresolvable schemas (no register link → null → open baseline is unchanged; real DB errors still throw). Verified: full 14-collection sequential run on a fresh NC32 = 0 failures (baseline reproduced rbac failing). Unit tests: 114 PermissionHandler + RegisterMapper tests green on nextcloud:34.
…uses) (#2152) Both the openregister-crud and rbac Newman collections failed in the Code Quality "Integration Tests (Newman)" job. Reproduced exactly as CI (newman on the host runner, CWD = tests/integration, one fresh NC32+Postgres container, no reset). Root causes and fixes: 1. sabre/xml vendor-shadowing (the real cause of BOTH failures) OpenRegister's composer.lock pinned sabre/xml 4.1.0 (pulled transitively by the sabre/vobject dev-dep). 4.1.0's XmlSerializable declares `xmlSerialize(): void`; core Nextcloud ships sabre/xml 2.2.11 (no return type). OR's autoloader registers 4.1.0 first, so core's CalDAV classes fail the signature check with a PHP fatal the moment any calendar is provisioned. - rbac: POST /ocs/v1.php/cloud/users (create e2euser) -> calendar home provisioning -> CalDAV -> fatal -> HTTP 500 (S0). - crud: "File 1: Create File on Object" is the first file op on a fresh install, so OR creates its "openregister" system user on the fly -> same calendar fatal -> 500, and File 3-6 then 404. Fix: pin sabre/xml to ^2.2 (matches the platform) in composer.json and regenerate the lock (sabre/xml 2.2.11, sabre/uri 2.3.4). sabre/vobject 4.5.8 is unaffected (it allows sabre/xml ^2.1). 2. rbac 1a: core first-login skeleton-copy lock race e2euser's first authenticated request triggers Nextcloud core's one-time skeleton copy (completeLogin -> copySkeleton), which under the DB locking provider (CI has no Redis) raises a LockedException on "/<uid>/files/Documents" -> 500 on whatever endpoint is first. Added an "S0b" warm-up request in the rbac Setup folder that logs e2euser in once (tolerant of any status) so the RBAC assertions see a settled home. No RBAC behaviour is weakened; 1a still asserts 403/404. 3. crud file-upload fixture path (hygiene) The Import requests referenced an absolute container path (/var/www/html/custom_apps/...) that never resolves on the CI host runner, so the file silently failed to load and Import tested nothing. Committed a small tests/integration/test-import.csv fixture (with a .gitignore exception) and switched the form-data src to a collection-relative path; Import now returns 200. 4. UserService quota query (real bug, fired in CI on every /user/me) getUsedSpaceMemorySafe queried a non-existent oc_storages.size column and joined oc_storages.id (varchar) = oc_mounts.storage_id (bigint), which PostgreSQL rejects (SQLSTATE[42883] operator does not exist). Rewrote it to read oc_filecache.size for the user's home-mount storage root (bigint join, no cast needed, works on Postgres/MySQL/SQLite) and hardened the catch to \Throwable so a stray Error can never turn a quota lookup into a 500. Fixes 36 pre-existing UserServiceTest errors. Verified from a CI-faithful host-CWD reproduction: crud 194/194 and rbac 41/41 pass, full 14-collection sequence 0 failures, UserService unit suites green.
…tandard (#2154) The change captured only the initial design (5 requirements, most tasks open). It now reflects everything shipped and merged across the fleet: - 6 new requirements — schema-marker mechanism (REQ-FCS-006), Ed25519 signing + verification (007), per-org group RBAC (008), config sets (009), repo-creation + topic-tagging + fetch bridge (010), and installable config-set repositories / OpenBuild convergence (011). Each with scenarios; `openspec validate --strict` passes. - tasks.md updated to the shipped reality with PR references across openregister (#2134/#2135/#2140/#2147/#2148/#2149), nldesign #192, the 8 app markers, hermiq #126/#127, openbuild #181, nc-vue #242 — plus the real-GitHub end-to-end verification and the two external follow-ups.
…t config (#2155) Two gaps in the store's governance/usability: - **Publish now honors visibility.** `publish(..., private: bool)` and the controller's `visibility: private` param let a freshly created store repo be private (previously every store repo was forced public). Defaults to public (backward-compatible); the token needs rights to create private repos. - **The trust controls are API-manageable, not occ-only.** New admin-gated `GET/PUT /api/federated-config/trust` read and write the org's source allowlist, trusted publisher keys, and publish/install group lists — plus a `trustKey` convenience that appends a public key to the trusted-keys list (idempotent). Non admins get 403; an unknown field is a 400. This is the backend a governance settings UI needs (the config keys previously required `occ`). Unit: trust read/write round-trip (set fields, append keys idempotently, unknown field throws). Live-verified on 8080: admin GET/PUT + trustKey append work, non-admin GET is 403, unknown field is 400.
End-user + admin guide for the federated configuration store: concepts (types, bundles, topics, provenance), choosing the store credential, publish, discover/fetch/install, the admin trust/governance API (allowlist, trusted keys, publish/install groups), and configuration sets / installable app repos. Auto-listed via the Docusaurus autogenerated sidebar.
…ects (create/update/upsert/guarded delete) The flow engine's nine built-in nodes transform items but none persists anything; 'no code, just flows' cannot materialise an object. Adds openregister.object-write: per-item create/update/upsert/delete with composite match keys, a per-step write cap (default 1000, fail-on-exceed), RBAC + attribution through the normal ObjectService path (fail-closed on ownerless runs — see #2158), and explicit item-level error semantics. Hardens the existing ObjectService::patchObject() (uuid-safe, RBAC forwarded, acting user, merge semantics) and widens deleteObject() for sessionless attribution. First consumers: hydra-cache maintenance + triage results (hydra-console chain); sibling change: openconnector-flow-nodes.
…elete objects Adds ObjectWriteNode, the built-in flow node that lets a graph flow write back to OpenRegister rather than only read and branch. Supports create, update, upsert and a guarded delete, resolving its target by uuid/slug/id scoped to a register+schema pair. Threads an explicit acting user through the write path so a sessionless caller (flow run, cron, import pipeline) is attributable rather than anonymous: - ObjectService::deleteObject() takes an optional $currentUser and evaluates the RBAC `delete` check against it instead of passing userId: null. - ObjectService::patchObject() is rewritten from a thin array_merge facade into the fleet's supported PATCH-semantic write path: RFC 7386-shaped recursive merge (absent key preserves, explicit null clears, lists replace wholesale), register/schema scoping, and no int cast on the identifier (the #1638 defect class). The merged result still goes through saveObject(), so validation, audit trail and events all apply. Registers the node on FlowNodeRegistrationListener alongside the other built-ins. Covered by ObjectWriteNodeTest and ObjectServicePatchObjectTest. Change: or-flow-object-write-node
fixed webpack config so building does not crash
…Y (or#2164) The per-schema agent-context allowlist was absent from ANNOTATION_VOCABULARY, so validateConfigurationEntry() silently dropped it on save. Because Hermiq's AgentContextBuilder (and its JS twin src/utils/agentContext.js) read exactly that key, EVERY agent leaf on EVERY schema fleet-wide resolved an EMPTY context. Fail-closed, so never a data leak — an absent allowlist means 'expose nothing' — but the capability was wholly inert, and invisible by construction: the schema saved with HTTP 200, no validation error reached the caller, and the only signal was a log line. This is the FOURTH recurrence of the same defect class in this list, after x-openregister-processing, x-openregister-contextchat and x-openregister-shareable, each of which carries its own comment saying so. Adds two regression tests (flat allowlist and the per-property refinement form) asserting both that the key survives the round-trip and that it is not reported as dropped. Verified live: PUT of an allowlist onto the hydra-cache 'finding' schema now reads back intact where it previously returned ABSENT.
fix(schema): x-openregister-agent-context was dropped at save — every agent leaf resolved an empty context
…node docs(openspec): or-flow-object-write-node — flows can write objects (create/update/upsert/guarded delete)
…forge label writes Three linked fixes that together unblock a governed app commanding a forge. or#2158 — FlowMcpToolProvider::runFlow() called FlowRunService::queue() without the acting user, so triggeredBy was null on every agent-dispatched run. Resolves the user from IUserSession and passes it through; null stays null rather than a fabricated uid. or#2158 (deeper cause) — FlowRunService::execute() built the node context from runUuid + resuming and NEVER copied the run's owner into it. Nodes read context['triggeredBy'] to attribute what they do: ObjectWriteNode refuses to write without it, SubFlowNode propagates it to child runs, Hermiq's agent node runs the turn as that user. Nothing in lib/ ever wrote that key, so EVERY trigger reached its nodes ownerless and only hand-injected contexts (tests, harnesses) worked — which is why object-write appeared verified while the natural path would have refused. An explicit context value still wins, so a caller can attribute a run to someone other than whoever queued it. or#2159 — runFlow and flowRunStatus now declare ADR-063 readOnlyHint / destructiveHint / idempotentHint / scope, so consumers classify them by declaration rather than by the fail-closed fallback. or#2165 — the github provider's allowRules permitted no issue-label write, so the broker refused label-driven forge automation even with a valid PAT. Adds POST/DELETE on issue labels plus PATCH on the issue, and the GitLab equivalent (one PUT, since GitLab sets labels as issue fields rather than a sub-resource). SECURITY-INVARIANT CHANGE, needs an explicit reviewer look: the catalogue previously enforced 'no provider grants DELETE' as a hard test. That invariant is retained but narrowed to an allowlist of exactly one sanctioned rule (github issue-label removal); any other DELETE still fails the test. Tests: new guards proven to fail without each fix. Full suite delta 0 errors / 0 failures against a CI-red baseline (117/17 before and after).
fix(flow,credentials): attribute flow runs to their owner + permit forge label writes
FlowRunController::test() queued its run without the acting user — the same defect as or#2158 in FlowMcpToolProvider::runFlow(), in the one dispatch path that has a session by definition. Consequence: the run was ownerless, so context['triggeredBy'] was null and every attribution-requiring node refused. ObjectWriteNode returned 'This flow run has no owner (triggeredBy); an object write must be attributable.' The interactive 'run this flow now' button could therefore never exercise a write node — the only runs that appeared to work were harness runs with a hand-injected context. Live-verified end to end after this fix, on a naturally triggered run with no injected context: status=completed, triggeredBy=admin, log shows write1 completed itemsIn=1 itemsOut=1, and the object landed in hydra-cache (findings 7 -> 8, owner=admin) read back through the API.
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.
…rence-provider-convergence
…07-23 spec: market-gap wave 2026-07-23 (4 OpenRegister changes)
…ape-hatch spec(leaf): mount(el,props) escape hatch — cross-Vue-major leaf render (fixes #44 systemically)
…er-convergence spec(leaf): resolve IReferenceProvider convergence question from ADR-066
…union of concrete JSONResponse types
PHPStan reported:
Method AuditTrailController::statistics() should return
JSONResponse<200|401|403, array{error?: string, total?: int, create?: int,
update?: int, delete?: int, read?: int}, array> but returns
JSONResponse<200, array{total: int, create: int, update: int, delete: int,
read: int}, array{}>.
The admin gate is present and correct — requireAdmin() returns the 401/403
response and statistics() returns it unchanged. The defect is purely in the
@psalm-return docblock: JSONResponse's template parameters are INVARIANT, so a
single JSONResponse whose template arguments are themselves unions (200|401|403
plus an all-optional data shape) is satisfied by no return statement at all —
not even the ones it was written to describe.
Rewritten as a union of concrete JSONResponse types, one member per branch,
each the exact type that branch produces (including array{} for the default
headers, which is what the constructor actually yields — the old
array<never, never> was the second half of the mismatch). No baseline entry, no
widening; the annotation now documents all three status codes and is checkable.
Verified locally on PHP 8.3: phpstan clean (was 1 error), phpcs clean, psalm
clean on the changed file.
…listener `\OCP\Util::addScript()` is the canonical Nextcloud asset API and is exposed as a static method only — there is no injectable DI equivalent reachable from an event listener, which is handed only the event. Wrapping the call in a seam class would relocate the identical static call rather than remove it. Suppression sits on the `handle()` METHOD docblock — the narrowest scope PHPMD actually honours, verified empirically (a file-docblock annotation would not apply). Matches the in-repo precedent for the same API in `ScriptManifestLoader::addEntryScripts()`. No baseline entry added.
various bug fixes
feat(flow): RunFlowOperation frontend settings component (flow-name input)
chore(deps): @conduction/nextcloud-vue 3.0.0-vue3.6
enable-hydra-gates defaults to false, so quality / Hydra Gates had been SKIPPED on every run this repo ever had. Pinned to v1.0.1, the pin openbuild already runs. enable-axe deliberately left off.
…pping (#2337) A skipped job and a passing job are indistinguishable in the Quality Report. Every gate turned on here reported 'skipped' in every run. Each newly-enabled leg was measured against this tree BEFORE being enabled; the results are in the PR description. Legs that were measured failing are enabled anyway - the defects are pre-existing, and the only thing that changed is that CI can now see them. Journeydoc Capture and enable-axe are deliberately NOT enabled.
* chore(deps): move to @conduction/nextcloud-vue 2.2.0-vue3.1 The 3.0.0-vue3.* line is being withdrawn from npm. The major was cut from a real BREAKING CHANGE footer (the retired action-list flow editors), but it applied to a prerelease channel only our own apps consume, so the line is resumed at 2.x rather than carried forward. 2.2.0-vue3.1 is the first release on the resumed line and is a superset of 3.0.0-vue3.6 — it additionally carries the recovered Vue 2 -> Vue 3 component conversion and the CnGraphCanvas port/loop work. Despite the lower version number this is not a downgrade in content. Verified rather than assumed: `npm install` resolves node_modules/@conduction/nextcloud-vue -> 2.2.0-vue3.1, and `npm run build` exits 0. * fix(deps): regenerate the lockfile with npm 10 to match CI CI runs node 20 / npm 10.8.2. Regenerating with local npm 11 omits the `"dev": true` markers npm 10 writes, which can put package.json and the lockfile out of sync for `npm ci`. No dependency actually changed: 0 packages added, 0 removed, 0 version changes — this is 52 restored `dev` markers. Verified with `npx npm@10.8.2 ci --dry-run` (exit 0).
…meter only (#2343) * fix(quality): scope the Migration phpmd exclusion to UnusedFormalParameter only phpmd.xml carried a TOP-LEVEL <exclude-pattern>*/Migration/*</exclude-pattern>. A top-level exclude-pattern is applied by PDepend at file-collection time, so it drops lib/Migration from EVERY rule in the ruleset, not from one rule. Measured on phpmd 2.15.0 / PHP 8.4.22, that line was silently swallowing 11 real findings: 3 NPathComplexity, 3 ElseExpression, 2 StaticAccess, 2 ExcessiveMethodLength, 1 CyclomaticComplexity. The same file also had a second, NESTED <exclude-pattern>*Migration*</exclude-pattern> inside the UnusedFormalParameter rule. PHPMD 2.15 honours exclude-patterns only as direct children of <ruleset>; nested ones are parsed and discarded, so that one was inert. UnusedFormalParameter now lives alone in phpmd-unusedparams.xml with its own top-level exclude-pattern, so the exclusion applies to that rule and nothing else. `composer phpmd` runs both rulesets as separate legs, worst exit code winning, and neither leg can short-circuit the other. OCP\Migration\IMigrationStep mandates the changeSchema / preSchemaChange / postSchemaChange signatures, so that one rule genuinely cannot apply to migrations. With the exclusion scoped, 246 now-redundant @SuppressWarnings(PHPMD.UnusedFormalParameter) annotations were deleted from 174 files in lib/Migration (236 exact + 10 with a stray space before the paren, which were never valid annotations). No suppression was added anywhere. All 11 surfaced findings are fixed with behaviour-preserving refactors: column-addition ifs replaced by a spec list plus a loop, platform branches extracted into named private methods, one else inverted to an early return, and two \OCP\Server::get() static calls replaced by an injected Psr\Container\ContainerInterface (keeping the lazy resolution the original comment asks for). Column order, SQL text and IOutput messages are unchanged. Verified: both legs exit 0 over all 1400 files in lib/, and both were positive-controlled (a planted UnusedFormalParameter outside lib/Migration and a planted ElseExpression inside it are both reported, exit 2). phpcs clean over lib/. Unit suite unchanged at 16007 tests / 35916 assertions before and after. Refs ConductionNL/.github#155 Refs #2338 * fix(quality): clear the two Hydra gates this PR's diff scope surfaced Both are pre-existing debt in files this PR already touches; the gates are diff-scoped, so they only became visible now. gate-28 license-triangle: five migrations carried @license AGPL-3.0-or-later while composer.json declares EUPL-1.2. Corrected to EUPL-1.2 — this is a docblock correction to match the declared licence, not a relicensing. gate-46 spec-anchor-existence: Version1Date20260511100000 pointed its @SPEC at openspec/changes/scholiq-deps/tenant-key-api/tasks.md, a change directory that was never archived under that name, so the target does not resolve. Retargeted at the canonical openspec/specs/saas-multi-tenant/spec.md, which is where the openregister_tenant_keys requirements live. (The same stale pointer also sits in lib/Service/TenantKeyService.php; that file is outside this PR's diff and is left for a follow-up rather than widening the scope.) --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
* chore(deps): move to @conduction/nextcloud-vue 2.2.0-vue3.3
Picks up the two releases that landed after 2.2.0-vue3.1:
2.2.0-vue3.2 four dashboard defects — date-range chip shows its dates and
calendar-aligned presets, a dangling labelResolve no longer
renders a raw UUID, and the table's "View all" pins to the
bottom instead of scrolling away
2.2.0-vue3.3 gridstack's stylesheet now ships with the library that
requires it; CnFormDialog splits over-long schema descriptions
behind an info popover; CnContextMenu closes again on outside
press and stops hijacking every popper with a cursor transform
Lockfile regenerated with npm 10.8.2 to match CI's node 20 toolchain — local
npm 11 prunes optional entries that do not apply to the current platform, which
makes CI's `npm ci` fail with "Missing: ... from lock file". Running `npm ci`
locally does not reproduce it, because npm 11 accepts its own lockfile.
Verified: `npx npm@10.8.2 ci --dry-run` exits 0, and `USE_LOCAL_LIB=false
npm run build` exits 0 with no unresolved modules and no reference to a sibling
nextcloud-vue checkout. USE_LOCAL_LIB=false is load-bearing: webpack aliases
@conduction/nextcloud-vue to ../nextcloud-vue/src when that sibling exists, so
a plain build can silently compile the sibling instead of the package under test.
* fix(deps): restore the optional lockfile entries npm 11 pruned
The verification build ran `npm install` under local npm 11 AFTER the lockfile
had been regenerated with npm 10.8.2, which silently re-pruned the optional
entries that do not apply to this platform — per-arch esbuild/rolldown/
lightningcss binaries and @nextcloud/vue's optional pinia and vite.
CI runs npm 10.8.2, whose `npm ci` requires those entries, so it failed at
Install dependencies and took every dependent job with it.
Regenerated with `npx npm@10.8.2 install --package-lock-only` and this time
nothing runs npm 11 against it afterwards. Verified with
`npx npm@10.8.2 ci --dry-run` (exit 0).
…h fan-out, and make bsn/user real formats (#2336) * feat(audit): add the seal sweeper the fail-soft path always promised sealRow() and sealRows() log "a later seal pass will chain it" whenever the seal lock is contended. There was no later pass. Nothing swept unsealed rows, so every fail-soft skip was permanent. Measured on the dev instance: 49,123 of 308,937 audit rows — 15.9% — had no hash and never would have. A row with no hash is a row the chain cannot vouch for, and the chain exists so an auditor can say "this history has not been rewritten" from evidence rather than assertion. AuditSealJob runs every 5 minutes, up to 10 passes of 500 rows. sealUnsealed() takes the OLDEST unsealed rows in id order, so it is resumable by construction: a tick that stops early is simply resumed by the next. It delegates to sealRows(), which derives the predecessor once per batch and chains forward — measured at 1.14 ms/row against 14.85 ms/row for the inline per-row seal. Verified: 49,123 -> 48,623 in one pass (exactly 500), and verifyChain() over the swept range returns valid=true with zero duplicate predecessors. FOUND WHILE VERIFYING, and NOT caused by this change: the chain is already broken at id 153230. 5,314 rows share a previous_hash with a sibling across 2,413 distinct predecessors, one of them used by 442 rows. That is the signature of concurrent seal passes each reading the same predecessor and then writing — exactly the race SEAL_LOCK_KEY was later introduced to prevent. The damage predates the lock. My swept range (31518-53387) contains zero duplicates and verifies clean, so the sweeper does not add to it; but it cannot repair history either, and a re-chain of the corrupted region is its own piece of work. Complexity suppressed with the reason: splitting an audit-integrity class to satisfy a threshold risks the property it guarantees. * feat(audit): repair the broken chain the sweeper cannot touch, and surface it The sweeper (previous commit) seals rows with NO hash. It cannot repair rows with a WRONG one, and the dev instance has 5,314 of those: rows chained onto a shared predecessor across 2,413 distinct predecessors, one used by 442 rows. That is a fan-out, not a chain, and it predates SEAL_LOCK_KEY. This is the re-chain that commit named as its own piece of work. - AuditHashService::rechainAll() walks every row in id order under the seal lock, deriving each previousHash from the row actually before it, so the result is one chain by construction. It REWRITES stored hashes, so it is an occ command and never a scheduled job — "something rewrote the audit hashes" is exactly the event the chain exists to make suspicious — and both ends of the run are logged at warning level so the rewrite is itself in the record. - openregister:rechain-audit-trail verifies before and after, with --dry-run and a confirmation prompt. The verification either side is the point: a repair that cannot show the chain was broken before and whole after is indistinguishable from one that quietly rewrote a healthy chain. It exits FAILURE if the chain still reports invalid. - verifyChain() now walks in windows. The DB was never the constraint — Postgres returns the whole trail by index scan in ~350 ms — the client was: libpq buffers an entire result set before PHP sees a row, so `select *` over 309,090 rows at ~5.8 KB wide pulls ~1.8 GB into the driver. That memory is held in C, so memory_get_peak_usage() cannot see it and the failure is not a PHP fatal but a SIGKILL. Measured: an occ run died with the OS killer while PHP still reported a 57 MB peak. Windowing bounds the driver buffer and cuts a partial walk from ~129 s to under a second. - getIntegrityStatus() + GET /api/audit-trails/integrity + a LogIntegrity admin card. Three COUNT/MAX queries, deliberately NOT a verification: binding a settings page to verifyChain() would make opening settings expensive enough that an admin stops opening it. The card keeps the two distinct — coverage is free and continuous, verification is explicit and operator-initiated. info.xml registers the command only; no version bump. * test(audit): serve the windowed verifyChain walk in the tombstone harness The merge broke these three tests and CI would have been the first to say so. wireRows() mocked the old unbounded query: one fetch() cursor, no expr(), no setMaxResults. verifyChain() now pages by id, so expr()->gt() was called on a null expression builder. The mock serves one populated window and then an empty one, which is how the walk terminates — serving rows forever would hang the suite rather than fail it. Nothing about what the tests ASSERT changed: testChainStaysValidAcrossA- Tombstone still expects valid=true and testTamperedRowStillBreaksTheChain still expects valid=false on the same harness, so the two remain each other's positive control. * fix(audit): page verifyChain so it stops being OOM-killed, and document sealing verifyChain() issued one unbounded `select *` over the whole audit trail. The database was never the constraint — Postgres returns all 309,090 rows by index scan in ~350ms — but the client is: libpq buffers an entire result set before PHP sees the first row, so at ~5.8KB per row that pulled ~1.8GB into the driver. That memory is held in C, so memory_get_peak_usage() reported a serene 57MB while the OS SIGKILLed the process. The failure mode was therefore the worst available one: verification did not fail, it VANISHED — no PHP fatal, nothing in the log, and an operator left with no signal that the chain had gone unchecked. Now walks in 500-row windows keyed on id. Same verdict on the live trail (brokenAt 153230, 500 verified, 6381 skipped), 128.7s -> 0.1s. Paging adds exactly one new way to be wrong — losing previousHash across a window boundary — so AuditHashVerifyPagingTest pins it down: a chain split across windows verifies clean, a row tampered AT the boundary is caught at the boundary, an entirely-unsealed window does not turn a gap into a false tamper alarm, and an empty trail terminates. Mutating the carry-over to reset per window turns two of them red. Also drops rechainAll()'s `skipped` counter, which nothing could ever increment — a field structurally pinned at 0 reads as "nothing was skipped" when it means "never measured". Docs: versioning-and-audit.md explained hash chaining but never said when sealing runs, that it is fail-soft, or that a gap is not a tampered entry. Adds that, the sweeper's schedule, and the repair procedure — and corrects the documented verify endpoint, which had the wrong path, wrong params, and described from/to as timestamps when they are entry IDs. * fix(audit): the backlog alarm was dead code, and psalm was the one that said so Four quality findings on this branch, all real: 🔴 AuditSealJob's "the hash chain has gaps that are not closing" warning could NEVER fire. An `if ($sealed === 0) { return; }` sat above it, so the only state that reaches the warning — sealed nothing, backlog non-empty — had already left the method. Psalm called it a ParadoxicalCondition; operationally it means the sweeper could stop working and the alarm written to say so would stay silent, which is the exact failure the sweeper exists to end. Reading the backlog before the early exit makes it reachable, and the early exit now covers only the true steady state (sealed nothing, nothing outstanding). Added tests/Unit/BackgroundJob/AuditSealJobTest.php — the job had NO test at all, which is how the dead branch survived. Verified as a positive control: with the early return restored, testWarnsWhenNothingSealedAndABacklogRemains FAILS. It also carries its own negative control (the steady state must stay silent) so the alarm cannot be satisfied by simply warning always. 🔴 getIntegrityStatus() called $qb->func()->max('id', 'last_sealed'). The max() function builder takes only the field — unlike count(), it has no alias parameter — so the second argument was swallowed and aliased nothing. Same family as the named-arg-on-a-variadic trap. phpmd: rechainAll() and verifyChain() were both over the 100-line method threshold. Extracted rechainWindow() and readChainWindow() rather than widening anything, so the outer methods read as the repair's and the verification's shape and the per-row rule lives in one place. ExcessiveClassLength is suppressed with a reason and a named next step (move the operator-initiated re-chain to its own service) rather than a threshold change. phpcs: named parameter on getHelper(), and the two duplicated before/after report blocks in the command folded into summarise() — which removes both inline ternaries and makes the two ends of the run print the same fields, so a reader comparing them is comparing like with like. Local: phpmd clean over lib, psalm clean on the changed files, 321 audit and retention tests green. The 10 tests/Unit/AppHost failures in the full local run are byte-identical to development and untouched by this branch — development's own CI has PHPUnit green — so they are this instance, not this change. * test(audit): pin the repair to producing a chain, not a fan-out rechainAll() rewrites hashes that already exist — the one operation here the audit trail is designed to make suspicious — and had no test. It also had the easiest possible way to be silently useless: derive previousHash once and reuse it, and every row gets a hash, every row looks sealed, and the chain is exactly as broken as before. That IS the bug it was written to repair (5,314 rows over 2,413 predecessors on the live trail, one shared by 442 rows), so reproducing it in the fix would be invisible. So the assertion is not "rows got hashes" but "row N's previousHash is row N-1's hash". The fixture seeds all three rows pointing at one shared predecessor, and asserts those stale values are gone. Dropping the `$previousHash = $hash` carry-forward in rechainWindow() turns it red. Also covers the refusal path — the repair must decline when the seal lock is held, since competing with a concurrent pass is how the fan-out arose — and getIntegrityStatus(), including the empty-trail case that would otherwise divide by zero on a fresh install's settings page. Earns back the 0.02% the coverage guard flagged, with tests worth having rather than by moving the baseline. * fix(audit): satisfy phpstan on the re-chain command, and refuse to guess consent Two findings, the second one substantive: - `?? 0` on $result['tombstonesPreserved'] was redundant against rechainAll()'s declared array shape, and phpstan said so. - getHelper() returns HelperInterface, which has no ask(). The call only worked by luck of what Symfony happens to return. Rather than casting the complaint away, the command now checks for a QuestionHelper and FAILS if it does not have one. Treating a missing helper as consent would rewrite every stored audit hash on the strength of an environment quirk — for a destructive repair behind a confirmation prompt, "could not ask" must never mean "yes". --force remains the supported way to say yes without a prompt. * perf(audit,logging): stop sealing on the write path, and stop re-reading our own writes Three problems found by running the thing rather than reading it. SEALING ON THE WRITE PATH CORRUPTED THE CHAIN. Sealing takes an exclusive lock, so under concurrency some rows sealed and some fell through the fail-soft path unsealed. A row sealed AFTER a gap chained onto the newest SEALED row, skipping the gap — so when the sweep later filled that gap, the gap and the row after it shared one predecessor. That is a fan-out, which verifyChain() cannot tell from tampering. Caught live: rows 455956 and 455957 both chained onto 455955, and verification went from valid=true to valid=false BECAUSE the sweeper ran. Sealing now happens only in AuditSealJob. With one sealer, unsealed rows are always a contiguous TAIL rather than holes punched mid-chain, so no later row can chain across a gap. The fan-out is not handled, it is unreachable. The write-path tests assert never() on sealing, as the invariant it now is. The sweep also re-chains from the oldest gap FORWARD rather than filling in place, bounded at MAX_SWEEP_RECHAIN, so legacy interleaving self-heals without a five-minute cron ever attempting a 300k-row rewrite. AN ABANDONED SEAL LOCK SILENTLY DISABLED SEALING. ILockingProvider has no owner and no liveness check, and DBLockingProvider only reaps expired rows from a separate job, so a process killed inside its critical section held the lock for the rest of its TTL — measured at 46 minutes. Every sweep in that window returned 0, which is ALSO the value meaning "nothing to seal": a dead sweeper and an idle one were indistinguishable while the backlog grew. acquireSealLock() now stamps appconfig, and breakStaleSealLock() takes over a lock held longer than any real pass can run, logging a warning because a process dying inside a critical section is worth seeing even when recovered from. WE RE-READ EVERY ROW WE WROTE. The magic tables have exactly ONE database-generated column, `_id` (nextval); `_created`/`_updated` carry no column default (59,292 of 59,292 rows have both set, so PHP writes them). The UPDATE re-read therefore fetched back the values it had just sent — and its own catch already returned the input entity when the read failed, so that was settled. Removed: one less query per update. The INSERT re-read stays, because the insert helper returns void and that read is genuinely how the id arrives; dropping it would hand callers null ids. It is now wrapped in a transaction instead, which is what Nextcloud's check actually asks for — isTransactionActive() is the first branch of the dirty-read test, since a transacted read goes to the primary. The commit sits in a finally so the lost-write throw cannot leak an open transaction. Both mattered because a "dirty table read" attaches a synthetic exception whose serialised backtrace measured 5.9MB, on every insert and every update. LOGGING. info was being used for "something happened": 700 info against 514 debug. MagicMapper, on every save, went 39 info -> 3, keeping only table creation, DDL and bulk deletion — rare and structural. All 13 entry-traces are gone ("Starting createFromArray", "About to update", "...called"); getOrganisationForNewEntity emitted four info lines per save to answer one question and now emits one debug recording the outcome, and createFromArray went from seven narrating lines to one saying what it created. NOTIFIER. Nextcloud deprecated InvalidArgumentException for declining a notification, and every notifier is offered every notification — so the routine decline logged a warning each time, dozens per dashboard load. UnknownNotificationException says the same thing silently, matching AnnotationNotifier which already did it correctly. * perf(objects): read the generated key from the INSERT, and stop waking eight apps to seed baseline data TWO WRITES, NOT THREE. insertObjectInRegisterSchemaTable() returned void, so the generated `_id` was thrown away and had to be recovered by SELECTing the row back — against a table written milliseconds earlier, which is exactly Nextcloud's "dirty table read" condition and cost ~5.9MB of serialised backtrace per object. It now returns lastInsertId(). Two facts make that exact rather than merely convenient, and both must keep holding: these tables carry exactly ONE sequence (`_id`; verified against information_schema — no other column has a default), and the call is the very next statement on the same connection. A second serial column in the magic-table shape would break it, and the comment says so. `_id` was never the identity anyway. saveObjectToRegisterSchemaTable() returns the UUID, which PHP generates; the id is an internal key catching up. Verified live: insert and update each emit ZERO dirty reads, and the id returned matches the row (min=max=1 on a single-row table, so it could not have been a coincidence). The rare path where an INSERT loses a uuid race and lands as an update still reads, since there is no generated key to report — but it uses the raw row fetch, not the hydrating one, and keeps the lost-write check that #2212 needed. EIGHT APPS WOKE FOR EVERY SEEDED OBJECT. docudesk, softwarecatalog, opencatalogi, openbuild, hermiq, zaakafhandelapp and hrmq all subscribe to object lifecycle events. Measured mid-repair: 155 "DocuDesk: Processing event", 116 compliance-subscriber calls, 116 queued text-extraction jobs — running document extraction and compliance scoring over content that shipped WITH the app, before anyone had configured anything. Seeding is not a user action, so there is no intent for a listener to react to. importSeedData() now runs inside SystemOperationContext, and MagicMapper withholds lifecycle events while it is active. Gating the DISPATCH rather than each listener is the point: one change here instead of eight across apps we do not all own, and it cannot be half-adopted — a listener that never learns of an event cannot forget to check. Proven with both controls, because "no events fired" is otherwise exactly the result a broken test gives for free: a normal save outside the context still dispatches (1), the same save inside it does not (0). Deferral was considered and is not the answer here. defer_object_events is unset, so nothing defers today; DeferredObjectEventJob hardcodes ObjectCreatedEvent and ignores the `action` it is passed; and an update cannot be deferred at all, since ObjectUpdatedEvent needs oldObject, which a later job cannot recover. Recorded so the next person does not rediscover it. * fix(schemas,objects): make bsn and user real formats, and stop a debug log killing a save BSN WAS BUILT, WIRED, AND UNREACHABLE. BsnFormat implements the 11-proef and is already registered with the value validator, so OpenRegister could checksum a burgerservicenummer all along — it just refused to accept a SCHEMA that declared `format: bsn`, because PropertyValidatorHandler's allowlist never got the entry. The two lists disagreed, and the cost was not cosmetic: procest declares `format: bsn` on a burgerservicenummer, so its schema import failed, which failed schema creation, which failed its "Load default ZGW API mapping configurations" repair step. An app went unconfigured over a missing word in an array. `user` now exists as a format too, and means what it says: UserFormat asks IUserManager whether the account exists. A user id is syntactically just a string, so a pattern could assert nothing — the backend is the only authority, and without it a schema could carry a deleted account's id forever while every consumer resolved it to nothing. Both verified in both directions. BSN: 111222333 accepted, 111222334 (one digit off) rejected, the all-zero sentinel rejected, a short value rejected. user: "admin" accepted, a non-existent uid rejected, empty and whitespace rejected. And an invented format is still rejected at schema level — the allowlist did not become permissive, it became correct. A DEBUG LOG WAS CRASHING THE SAVE PATH. convertRowToObjectEntity() is declared `?ObjectEntity` and does return null for a row it cannot hydrate. Every other call site checks. findAcrossAllMagicTables() did not, and the first thing it did with the result was dereference it — inside a logger->debug() whose only job was to report what had been found. So an unconvertible row did not degrade the search, it killed the request with "Call to a member function getUuid() on null". Live effect: 11 Shillinq RetentionRule objects failed to rematerialise on EVERY repair, because a DocuDesk enrichment listener reached this lookup and one row would not hydrate. The row is now skipped with a warning and the search continues. Re-running the exact repro that failed: ok=3 fail=0. Also completes the system-operation event gate. The first attempt covered MagicMapper::insert()/update() and missed the BULK dispatchers in SaveObjects and the batched-update path, so a configuration import kept fanning out to eight apps while the gate looked applied. That is the second time this session a gate read as correct and was not, which is why both are now asserted rather than assumed. * fix(schemas): allow a nested property to omit 'type', as JSON Schema does A schema with no `type` means "any type" in JSON Schema. OpenRegister rejected it outright, and that was not a lenience worth defending — it forced authors to declare a type they do not have. procest's CMMN sentry is the case that exposed it. `ifPart: {field, operator, value}` compares `value` with LOOSE equality against bool/string/int by explicit design ("a sentry author should not have to match PHP's strict type rules"), requires an ARRAY for the in/notIn operators, and numeric for gt/lt. No single type is honest there. Requiring one would have meant writing a lie into the schema; refusing the omission instead failed the entire import. Type stays REQUIRED at the top level, because those properties become columns: mapColumnTypeToSQL() takes a `string $type` and receives $column['type'] directly, so a typeless top-level property is a TypeError during table creation, not a permissive read. Nested properties are stored inside a JSON column and derive nothing, so the omission costs nothing there. Depth is the discriminator — validateProperties() builds '/name' for a top-level property and appends per level. Controlled in both directions: a top-level typeless property is still rejected, a nested one is accepted, and procest's real caseModel schema — the one that has been failing every repair — now validates. * refactor(logging): reserve info for events, not for narration OpenRegister emitted 700 info calls against 514 debug — inverted, because info was being used for "something happened". A repair run was consequently a wall of lines reporting that nothing had changed, and the one line that mattered was indistinguishable from the 300 that did not. Now 460 info / 745 debug. What moved, and the rule applied: FilePublishingHandler, UpdateFileHandler — step-by-step narration of a single method ("Original file parameter", "After cleaning", "Object folder path", "Attempting to get file"). All debug. ImportHandler — 50 -> 4. The per-app decisions ("Skipping {app}: config content unchanged") describe the MOST COMMON outcome of a repair; reporting the non-event at info is what made the log unreadable. Kept: a register was created, an update applied against the version ordering, the seed-data summary with its counts, and an app's version actually changing. SaveObjects — including one line literally labelled "DEBUG - ..." emitted at info. Kept the Wave-12 safeguard REJECTION: a refusal to write is what someone comes to the log to find. TextExtractionService, ConfigurationController, FolderManagementHandler, ConfigurationCheckJob, CrudHandler, OrganisationService — same treatment, with a state change (risk level), an outcome (notifications sent), and two reached-but-unimplemented paths promoted back. One demotion was reverted rather than pushed through. TextExtractionServiceDeepTest asserts that "Object no longer exists, skipping extraction" logs at info, with a comment saying so. That is a deliberate contract — the line explains why queued work did NOT happen, and silence there looks like the job never ran. The level is now justified in the code instead of only in a test. * test(objects): pin the bulk save path's system-operation gate The first attempt at this gate did not cover this path. Gating MagicMapper::insert()/update() looked complete — a live probe showed one event outside SystemOperationContext and zero inside — while a configuration import carried on fanning out to eight apps through emitChunkSideEffects(), which the probe never touched. The gate read as applied and was not. A live re-probe could not settle it either, and the way it failed is the point: the bulk path defaults to `_events: false`, so the negative control dispatched nothing and the "zero inside the context" result proved exactly nothing. A second attempt with `_events: true` was then rejected by the bulk safeguard (BulkSafeguardException), and a third with an admin session ran past ten minutes — because a real session wakes the very fan-out being measured. Hence a unit test, with the control in the file rather than in a separate run: - outside a system operation the emitter dispatches (without this, an emitter that never dispatched anything would look identical to a working gate) - inside one it dispatches nothing - and events RESUME afterwards, since a suppression that outlived its context would silence every later save in the request — the same outage as the fan-out, reached from the other side and far harder to notice Replacing the gate with `if (false)` turns the second and third red while the control stays green. * chore(docs): regenerate features.json The Features Check gate runs the shared extractor with --check and failed on this branch. Regenerated with the same script the gate uses (.conduction-shared/scripts/extract-features.py), so the committed file and the gate's expectation agree. * fix(quality): satisfy the gates, and cover what the coverage gate was right about phpcs, phpstan and psalm each caught something real rather than cosmetic. psalm's AssignmentToVoid was the sharpest: insertObjectInRegisterSchemaTable() now returns the generated id, but its docblock still said `@return void`, so static analysis was reading the OLD contract while the code returned an int. A docblock that disagrees with its signature is worse than none — it is the version tooling believes. phpstan then found the loose end from a reverted decision: PropertyValidatorHandler kept the logger it was given for the permissive-format behaviour that no longer exists, so the dependency was written and never read. Removed rather than suppressed; an injected collaborator nothing uses is a claim about the class that is not true. The coverage gate was also right, and its baseline may never be lowered, so this earns it back with the two things that genuinely had no tests: UserFormat — the negative case IS the format. A user id is syntactically just a string, so only the backend can say it names nobody; the tests assert the unknown-user rejection, that blank and non-string values never reach the backend at all (coercing 42 to "42" would turn a type error into a lookup miss that reads as "no such user"), and that whitespace is trimmed. The abandoned-lock recovery — conservative in one direction and decisive in the other, and both are asserted. A lock held moments ago is left alone, because stealing one a live pass still holds puts two writers in the chain and reintroduces the fan-out the lock exists to prevent. A lock with no recorded timestamp is also left alone, failing safe rather than guessing — which is why the two left by an interrupted upgrade had to be cleared by hand. A lock held longer than any bounded pass could run is broken and taken, and a break that itself fails reports failure rather than handing the sweep a lock it does not hold. * feat(flow): a node is the action, an edge is sequence, and a token has one exit The engine's authoring format was its own intermediate representation: a node was a Petri-net PLACE carrying no behaviour, and the EDGE carried `type` and `config`. `FlowDefinitionBuilder` threw on a node carrying a step, and its own comment said why the mistake kept happening — "node-shaped authoring is the natural mistake, BECAUSE THAT IS HOW A GRAPH EDITOR PRESENTS A FLOW". It diagnosed the defect and declined to treat it. Three fleet graphs were authored that way, ran, reported COMPLETED, and did nothing. So the model inverts. A node carries the step; an edge says what runs next. The Petri net survives as the lowering: node N -> transition T_N carrying N's type/config, plus place in(N) edge A -> B -> in(B) added to T_A's targets no outgoing -> terminal place end(N) no incoming -> in(N) is initial join: true -> one input place per incoming edge Places are named after their node, which is load-bearing twice: per-item routing matches an item's tag against the output PLACE name, so a prefix would silently drop every routed item into an empty branch; and the marking is the user-visible answer to "where is this run?". CONVERGING EDGES ARE A MERGE, NOT A JOIN. The Hydra sequencer reaches its exit from several mutually exclusive paths — lowering those to a join would require all of them and deadlock every run, while still producing a valid definition. So `in(N)` is shared; synchronising is opt-in via `join: true`. CONDITIONS LIVE ON THE NODE, AS NAMED EXITS. A node declares its branches (`exits: [{id, condition?}]`) and an edge says which it leaves (`fromExit`). That is what lets a node have several exit points, and what lets an editor draw one port per branch — the branches exist before any line does, which an edge condition could never manage. A TOKEN IS UNIQUE AND EXCLUSIVE. Exactly one exit is taken per firing, chosen in declaration order with the unconditioned exit as the else. symfony/workflow marks every output place, so the unclaimed ones are withdrawn after apply(); without that the losing branch simply ran an iteration later, with no error. AND THE ELSE IS MANDATORY. A node that conditions its exits must declare one, refused at build time by name. A token with nowhere to go does not fail — the run stops, reporting nothing, which is indistinguishable from a flow that finished. A test asserted exactly that as correct behaviour ("ends the run cleanly"); it now asserts the refusal, with a positive control. The old shape is REFUSED, not reinterpreted: any edge carrying a `type` names itself and points at the migration. Accepting both would let a half-migrated flow run, skip the step nobody claimed, and report success — the original defect wearing a migration as a disguise. `FlowNodePreflight` walks nodes. Left on edges it would inspect a list where nothing carries a type, find nothing, and call every document valid without having looked — a validator that cannot fail is worse than none. 404 flow tests green (399 at baseline). The 10 AppHost failures in the full run are pre-existing and unrelated — verified by stashing this change. * fix(flow): validate must refuse a pre-inversion document, not report it valid Moving the preflight onto nodes closed one hole and opened another. An un-migrated flow carries every step on an EDGE, so the node walk found nothing to inspect, produced no findings, and the report said "valid" — about the one document shape the engine will certainly refuse. Measured live: `POST /api/flow/validate` on the real Hydra sequencer returned `valid: true, blocking: 0` while `FlowDefinitionBuilder` would refuse it outright. The editor's "Check this flow" button would have told an author their un-migrated flow was fine. The pre-inversion check now runs in `inspect()` as well as in the builder, and returns early: every later finding would be about a document in a shape nothing reads, and burying the one actionable message under sixteen others helps nobody. The sequencer now reports all 16 steps by name, each pointing at the migration. Covered by a test AND its positive control — the same flow validating once the step moves onto the node — because a refusal test is otherwise satisfied by a preflight that refuses everything it is shown. * feat(bulk): give the streaming write path a caller, and fix the gates it surfaced STREAMING BULK UPSERT WAS UNREACHABLE. Hydra's orphaned-write-capability gate found SaveObject::saveObjectsStreaming() and clearReferenceValidationCache() with ONLY test callers — implemented, unit-tested by calling the class directly, and reachable from no production code. Checked rather than assumed: the gate is right, not a false positive. They are a matched pair built as the prerequisite for a streaming import that was never built. routes.php still carries the epitaph: "The objects import route was also removed — use the registers import endpoint instead." Wired into BulkController behind an opt-in `stream` flag, defaulting to today's behaviour. Opt-in because the two paths have OPPOSITE trade-offs, and defaulting either way would be wrong for half the payloads: default ultraFastBulkSave — fastest writes, but never consults the reference-validation cache, so rows that reference each other cost N×M round-trips resolving them. stream each row through saveObject(), which engages that cache; repeated targets resolve from memory, the payload is consumed lazily, and a failed row is recorded rather than failing the call. Choosing automatically would need a size/reference threshold nobody has measured, so the caller decides. ObjectService clears the reference cache at the batch boundary, which is exactly what clearReferenceValidationCache() was written for. SPEC for the UI half. widget-record-import covers dropping a spreadsheet of RECORDS on a register. It is deliberately NOT the file widget's streaming upload: saveObjectsStreaming() streams rows shaped like saveObject() input, and file bytes never pass through it. The two compose — dropping 200 PDFs is a FileService concern, the 200 resulting objects are what this streams — and keeping them apart stops the file widget growing a record-parsing responsibility it has no business owning. The spec makes column mapping explicit (a silently dropped column is the failure that makes imports untrustworthy), requires a dry run, and requires failed rows to export in the input's shape so a user can fix and re-drop only those. GATES this diff surfaced, all pre-existing and all now fixed: gate-46 Dead @SPEC anchors pointing at archived change dirs, in seven files this branch happened to touch. Repointed at the canonical specs they should have named — @SPEC targets openspec/specs/, never a change dir. gate-28 Two files carried `@license AGPL` while their own SPDX header and composer.json both said EUPL-1.2. Not a licence change: a stale tag contradicting the file's own identifier. Also adds the re-chain command's missing tests, which cover the part that matters about a destructive repair — that it refuses without consent, writes nothing under --dry-run, and exits FAILURE when the chain is still broken afterwards rather than reporting success on a repair that did not take. * refactor(bulk): extract writeBatch() from save() save() had grown past PHPMD's length limit once the streaming path landed. Move both write paths into a private writeBatch(), which also gives the stream/ultraFastBulkSave trade-off one place to be explained instead of a comment block wedged inside the request handler. The psalm-return annotation on save() described only the non-streaming shape, so it no longer matched what the method can return; the extracted method carries a plain JSONResponse return type instead. Also adds the class-level @SPEC tag phpcs was warning about (the file-level docblock had it, the class docblock did not). * i18n: translate the audit/flow settings strings into all 36 required locales The l10n parity gate requires every European locale to carry every English source key. The log-integrity settings section and the flow defaults panel added 46 new strings, and two older keys ("Add Application", "Loading organisations...") had never been translated, leaving 48 missing per locale. Also fixes how plurals are stored. @nextcloud/l10n's translatePlural() looks up "_<singular>_::_<plural>_" and indexes the resulting array with the locale's own plural function. Two things were wrong: - nl and tr kept their plural arrays under the *singular* key, where translatePlural never finds them. translate() falls back to element [0], so the plural form of all six n() call sites was unreachable — "Verwijder 5 object" rather than "objecten". Re-keyed to the identifier translatePlural actually reads. - The new "%n entry has no hash yet" pair existed only as two flat strings. That resolves via the fallback path but collapses every locale to a two-way split, which is wrong for the thirteen locales with more than two plural categories. Each now carries a full form array: three for the Slavic locales, four for Maltese and Slovenian (which has a dual), five for Irish. Verified per locale that no existing key was removed and no existing value changed, that each file keeps its own nplurals rule, and by resolving the new plural through each locale's rule for n = 1, 2, 3, 5, 11, 21. * refactor(flow): split the engine and builder along the seams they already had The action-node work grew FlowEngine by 312 lines and FlowDefinitionBuilder by 336, putting seven PHPMD violations on the branch that development does not have — class length, class complexity, and three on one method. Rather than raise the thresholds or baseline the findings, the classes are split where the code was already separable: FlowTokenRouter which exit a token takes, and which places it reaches FlowItemPlacement which items sit on which place, and which travel FlowGraph what a node is called as a place; which edges touch it The seams are not arithmetic. Exit selection reads the DOCUMENT — nodes, exits, edge conditions — while placement reads the MARKING, and the two were only ever coupled through the taken-exit list they hand each other. The graph helpers are pure functions over ids that decide nothing. The moved groups called nothing but each other, so no service dependency moved with them. FlowNodePreflight::inspect() was over on three counts at once (length, cyclomatic, NPath); extracting its two loops fixed all three. The two collaborators are constructor parameters, added LAST and defaulted. Three unit tests construct the engine positionally and one passes the oversight registry third, so a parameter inserted ahead of it would have silently rebound that argument — leaving a test that passes while checking nothing. phpmd.baseline.xml gains StaticAccess entries for the two new files. That is not new debt: FlowEngine already carried a file-level entry for the same calls to FlowExpression and FlowItems, and moving code should not re-open a decision already taken. Verified: phpmd over all of lib reports nothing; phpcs, psalm and phpstan clean on every file touched; 406 flow tests pass. Confirmed the tests can actually fail — inverting the exit condition in the extracted takenExits() breaks four of them. * docs(spec): write the register-folder requirement two services already cite gate-46 flagged `#object-register-folder-management` in RegisterService and FileService as unresolved, and it was right: five `@spec` tags pointed at a requirement nobody had written. The behaviour is real and load-bearing — registers, schemas and objects each own a backing Nextcloud folder, provisioned on demand — so the fix is the missing requirement, not a retag to something that does not describe it. Written from what the code does: provisioning on create, healing of legacy null/string folder values on update, nesting of object folders under their register's, idempotent creation, and a folder failure that is logged without aborting the write. * docs(gate-57): mark clearCurrents live, with the cross-repo callers named gate-57 reported ObjectService::clearCurrents() as an orphaned write capability. It is not: openconnector's EndpointService calls it from six sites before each fresh lookup. The gate builds its caller index from the repo under scan, and CI checks out openregister alone, so a method whose only callers live in a sibling app reads as dead. This is the exact case the gate's own docblock records — hydra#106, against this very method — noting that acting on the verdict would have broken endpoint rendering. The documented `@orphaned-write-capability exclude` annotation is the mechanism for it, so it is used here with the call sites named rather than a bare assertion, so the next reader can check the claim instead of trusting it. The gate stayed quiet locally because sibling apps are checked out next to this one and its index found them. * ci: state the full-coverage opt-out instead of inheriting the new default The shared workflow's `hydra-gates-require-full-coverage` default flipped to `true` on .github main this afternoon, so this repo picked it up mid-PR: every gate PASSES, and the job fails anyway because two gates did not report. Neither is a defect this branch introduced. gate-33 consumes the axe report, and `enable-axe` is deliberately off here for the reason recorded three lines below — vanilla Nextcloud already carries serious/critical axe violations, so switching it on alongside the gates would confuse "this repo has an accessibility defect" with "core does". gate-4 likewise produced nothing under the pinned v1.0.1. Recorded as an explicit `false` with the condition for removing it, rather than left to inherit, so the next reader sees a deferral with a reason attached and not a control that quietly went missing.
…failing CI (#2349) v1.0.1 is `f4d9756` (2026-08-03) and predates three gate fixes, so every Hydra Gates run this repo has ever made executed a script in which 16 gates reported PASS when their helper never ran (ConductionNL/.github#147), gate-33 had no axe report to read and never said so (#148), and gates 6 and 7 reported PASS on an empty scope (#149). The tick was identical either way, which is why nothing in this repo's history shows it. That pin is now also RED, and the mechanism is worth writing down. quality.yml is referenced `@main` while this package is PINNED, so the two can desync. #164 flipped `hydra-gates-require-full-coverage` to default true in the shared workflow, and that flag requires a gate to DECLARE itself not-applicable. v1.0.1 contains ZERO `_skip` calls; v1.3.0 has 36. v1.0.1 has no vocabulary to declare, so every absent prerequisite became "DID NOT RUN" and failed the job — for gates the repo has no subject matter for. Measured on this branch, diff-scoped against origin/development exactly as CI scopes it, in a private mount namespace with a private tmpfs (the runner's ~50 /tmp/hydra-gate-*.log paths are shared state and two concurrent runs corrupt each other's counts, .github#158 item 6): v1.0.1 exit 98 FAIL — "GATES THAT DID NOT RUN: 24 33" v1.3.0 exit 0 PASS — those gates named NOT APPLICABLE, with reasons Independently confirmed end-to-end: doriath#160 changed this one line and nothing else, and its Hydra Gates job went failure -> success. v1.3.0 is `f7eaf2a` = .github@main at the time it was cut. Refs ConductionNL/.github#159
…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.
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.