Release: merge development into beta - #3477
Open
github-actions[bot] wants to merge 137 commits into
Open
Conversation
The docstring states that CnPageRenderer "forwards [pages[].title] to the
page component as a raw prop without translating it", and excludes the
field on that basis. That is no longer true.
Measured 2026-09-06 on integriq, which wires
`<CnAppRoot :translate="translateForApp">` exactly as src/App.vue does
here, against a Nextcloud 34 instance with the user set to nl and the
app's l10n/ deployed: the page heading rendered "StUF-berichten", not
"StUF messages". Report card labels and descriptions translate too;
CnReportsPage builds resolvedCards with this.tr(card.label).
On integriq the stale exclusion had a cost: four live keys were reported
UNUSED and clean:l10n offered to delete them. Deleting a translated page
title removes the localised string and leaves the English source
rendering correctly, so nothing fails and nobody notices.
THIS CHANGE IS PREVENTIVE HERE, NOT A FIX. Measured on this repository:
check:l10n reports 769 unused before and 769 after, and clean-l10n
proposes an identical set. Every page title openregister declares already
has a literal t() call in src/, so none were at risk. The guard matters
the moment that stops being true, which a reports page would do at once
(this repo declares none today).
Adding a field to this set can only make the cleaner more conservative,
never less, which is why the no-op is the expected outcome rather than a
reason to doubt the change.
The docstring now records how to verify a field: against the DOM, not the
source. The template renders {{ card.label }} and only resolvedCards
shows the translation, so reading the component alone reproduces the
wrong conclusion, and the measurement needs the app's l10n/ deployed,
which the built bundle does not carry.
Verified: eslint, prettier and test:l10n all exit 0.
…0260906094202 chore(sync): carry beta back into development
…fest docs(l10n): correct what collectDynamicKeys says about page titles
…0260906124808 chore(sync): carry beta back into development
Carries the flow-editor work the trigger config removal and the guarded Run button belong to: the settings modal no longer offers a trigger, because what starts a flow is a step on the canvas, and Run explains itself instead of letting the engine refuse several seconds later in words about nodes. The pin was already ahead of what was installed — package.json said ^2.37.0 while node_modules held 2.36.1, so npm install had not been re-run since the pin last moved. This bump and a rebuild close both gaps.
#3490) First half of flow-semantic-versions: the comparison a semantic version is derived from, with its spec. THE RULE IS ABOUT REMOVAL. A consumer of a flow can depend on three things — that a step exists, that a path connects, and that a step still reads the key it read — and every one is broken by taking something away, none by adding. So a removed node, a removed edge, or a removed config key on a SURVIVING node is MAJOR, and everything else is MINOR. Changing a VALUE can break a consumer too and is not detectable as such from the graph. Guessing would produce majors nobody believes, and a version people ignore carries no information at all. That gap is what the author's explicit override is for, and why the override only goes UP: the diff is evidence, and evidence can be added to but not argued down. 🔴 A KEY THAT WENT WITH ITS NODE COUNTS ONCE. It is one change — the node went — and counting it twice inflates the list the author reads before publishing, which is the part that makes the verdict credible. TWO DIALECTS, ONE EDGE. The editor writes `from: 'a'` and the dossiq projections write `from: ['a']`. Both normalise, because reading them as different would report a removal and an addition on a graph nobody touched. That is not hypothetical: both shapes are live in one flow on the demo instance. WHY THE ORDINAL SURVIVES, in the design: `version` is an integer in two tables with a UNIQUE (flow_uuid, version), `FlowRun.flowVersion` is what a RUN PINS for its whole life including a suspension of weeks, and 28 call sites read it. Replacing all that to buy a label would put the run pin at the mercy of a derivation — a bug would then not mislabel a version, it would repoint a run. Semver is an additional fact; the ordinal stays identity and ordering. Verified: 10 tests, 5 of which break under mutation of the removal detection and the surviving-node guard. 1145 flow unit tests green; PHPCS, PHPStan and Psalm clean.
…is (#3492) * feat(flow): store a semantic version, and work out what the next one is Second half of flow-semantic-versions: the columns, the migration, and the arithmetic. The comparison itself landed in #3490. 🔴 THE ORDINAL IS UNTOUCHED. `version` stays an integer, stays unique with `flow_uuid`, and stays what a RUN PINS for its whole life — including runs suspended for weeks on a human task. Semver is an additional, author-facing fact in a nullable column. Replacing the ordinal would migrate two unique indexes, every run row's pin and 28 call sites to buy a label, and would put the run pin at the mercy of a derivation: a bug would then not mislabel a version, it would repoint a run. A draft has NULL rather than 0.0.0. It has not been compared with anything yet, and a null that means "not derived" is honest where a number is a claim. `semver_source` records `derived` or `backfill` beside the value, because the back-fill cannot know whether a historical publish was breaking — the graphs it would compare are the ones it is being run to describe. A version that says where it came from can be distrusted correctly; one that silently claims to be derived cannot. THE ARITHMETIC IS ITS OWN CLASS, apart from the diff, because they fail differently: the diff reads two graphs and says what was taken away, this reads a verdict and says what to call the next version. Together, a change to the numbering could break the comparison, and the comparison is the half with evidence behind it. FIRST PUBLISH IS ALWAYS 1.0.0, whatever the diff said. Calling it 2.0.0 because it "removed" everything from nothing would be arithmetic rather than meaning. PATCH IS ALWAYS 0: nothing in a graph distinguishes a fix from a feature, and three digits look more precise than two. AN AUTHOR MAY RAISE AND NEVER LOWER. They know which values a consumer reads and the diff does not, so they can add a major it did not find. They cannot publish a removal as minor: the diff is evidence, and the refusal names what went. Verified: 10 tests, 4 of which break under mutation of the major branch and the refusal. PHPCS, PHPStan, Psalm and PHPMD clean. `<version>` bumped in the same commit so the migration actually runs — checked with scripts/check-migration-version-bump.php, which is gate-110's local half.⚠️ phpcs caught me inserting both properties BETWEEN their neighbour's docblock and the property it described, orphaning two docblocks. Moved below; the diff is now pure addition with no removals. * feat(flow): publishing derives the semantic version, proven on a live instance Wires the diff and the arithmetic into `FlowVersionService::publish()`, and proves the result over the HTTP API rather than through mocks. DERIVED AGAINST THE PUBLISHED GRAPH, not the previous ordinal. They are usually the same and are not always: a deprecated version, or a draft opened and abandoned, leaves a gap — and what a consumer is running is the published one. 🔴 AN UNREADABLE PREVIOUS GRAPH IS NOT A FAILURE TO PUBLISH. The definition row may have been pruned, and refusing would let a LABELLING feature block a release. It falls back to a minor and says so in the log, rather than a confident major nobody can check. The controller passes `bump` through and answers a refusal with 409 and `kind: version-bump-refused` — an UnexpectedValueException rather than a lifecycle refusal, because the flow's state is fine and the request is not. Both entities now serialise `semver`, and FlowVersion also `semverSource`, so a back-filled version can be distrusted correctly. PROVEN ON :8080, six Playwright specs over the live API: first publish is 1.0.0 with the ORDINAL STILL 1 — the point of the whole design, since the ordinal is what a run pins; adding a step is 1.1.0; removing a step is 2.0.0; removing a config key from a surviving step is 2.0.0; an author raises a value-only change to 2.0.0; and publishing a removal as minor is refused with the removed step NAMED in the message. Two of my own fixtures were wrong and the suite said so. `/draft` answers 201 because opening a draft CREATES a version row, not 200 — that cost five red tests about versions they were not testing. And the added step in the minor case dangled, so the dead-end guard refused the publish before any version was derived: the guard working, my fixture not. 4358 unit tests green across the flow and controller suites; PHPCS clean on all of lib/; PHPStan and Psalm clean on every changed file. * feat(flow): number the versions that predate semantic versions The last piece of flow-semantic-versions: existing published versions get a number, and the number says where it came from. 🔴 IT DOES NOT INVENT HISTORY. The repair cannot know whether the third publish of a flow was breaking — the graphs it would compare are precisely the ones it is being run to describe, and older definition rows may have been pruned. So it numbers a flow's published versions in ordinal order (1.0.0, 1.1.0, 1.2.0) and stamps `semverSource = backfill` beside each. That marker is the point: a version that says where it came from can be distrusted correctly, and the UI says so on hover. One that silently claims to be derived cannot. A DERIVED VERSION IS NEVER OVERWRITTEN. It is a stronger fact than anything this step can produce, so only rows with no semver are touched. Drafts are skipped entirely — a draft has not been compared with anything. 🔴 IT MUST NOT FAIL AN UPGRADE. A flow whose history is incomplete is already in that state; a repair that turns a reporting gap into a failed `occ upgrade` makes it everybody's problem. Each flow is stamped in its own try, failures are counted, and the step reports them.⚠️ I WROTE `findAll()` AND THE STEP SILENTLY DID NOTHING. FlowMapper has `findAllFlows()`. The call threw, the step reported itself skipped, the upgrade went green, and only counting the rows afterwards found it. It now uses the paged `everyFlow()` the sibling repair uses — paged because the mapper's default limit is 100, and a repair that stamps the first hundred flows and reports success is worse than one that fails. Verified ON :8080 by running `occ upgrade` and counting: 28 rows back-filled, 25 derived, and the 8 without a version are exactly the drafts. One flow's three versions read 1.0.0, 1.1.0, 1.2.0 in ordinal order. 1155 flow unit tests green; PHPCS, PHPStan and Psalm clean; `<version>` bumped so the step actually runs, checked with scripts/check-migration-version-bump.php. * fix(flow): the three gates this branch tripped, and one that predates it PRETTIER on the new e2e spec. Formatted; the SPDX header survived the write, checked rather than assumed. GATE-16 spec-coverage wanted a @SPEC on every changed method. Added to the six I wrote — they do implement the spec, so the tag is a statement rather than a formality. PHPMD CouplingBetweenObjects, which is the interesting one. FlowVersionService was ALREADY at 13 against a limit of "under 13", and `development`'s Code Quality job was already failing because of it. My two new collaborators took it to 16. So the comparison and the numbering moved INTO FlowSemanticVersion, which now answers the whole question through one `forPublish()` — a caller asking what the next version is should not also have to know that answering it means reading two graphs. That is a real reduction, 16 to 15, and it makes the service's helper do one thing: decide WHICH graph is being replaced and survive not finding it. 15 is still over, so the class carries a documented suppression. A version transition is where a flow's separate concerns have to meet — the version rows, the head that mirrors them, the definition store, the lifecycle guard, the trigger index that must move in the SAME transaction, the numbering, the connection, the log. Splitting the class would move the coupling into a caller rather than remove it, and would spread the transaction boundary — the thing that makes a publish safe — across two objects. The suppression says that, rather than raising the limit for every class in the app. Verified: phpmd clean across all of lib/Service/Flow, phpcs and PHPStan clean on every changed file, prettier clean, 1155 flow unit tests green. * feat(flow): tell the author what a publish will be called, before they publish The derivation was already right and entirely invisible: an author found out a change was major by publishing it, and the first surprising major is the one that teaches them to ignore the number. `GET /api/flows/{id}/version-preview` answers what the next version would be and, when it is major, names what the publish takes away. It never refuses: asking a question is not asserting an answer, so the refusal stays at the publish, where the author has actually claimed something. The preview and the publish read the SAME two graphs, through the same `publishedGraphOf()` and the same `FlowSemanticVersion`. A preview computed a second way is a second opinion, and the first time the two disagree the author learns to believe neither. Two gaps this also closes: - `Flow` mirrored `semver` but not `semverSource`, so the UI's "this number was back-filled" branch could never fire — it was reading a field the backend had no column for. The column, the entity field and the publish now carry it. - The back-fill stamped version ROWS only. Every historic flow would have kept showing its ordinal in the pill for ever: the rows right and the screen wrong. It now mirrors the live version onto the flow, marked `backfill`, and still never overwrites a derived value. The repair had no test at all, which the coverage guard caught and was right about. It has one now, including the history-with-a-gap case: ordinals 1, 2 and 5 number as 1.0.0, 1.1.0, 1.2.0, because numbering by the ordinal would invent a 1.4.0 whose 1.2.0 and 1.3.0 nothing ever published. Every new test was seen RED by mutating the implementation, not by being written after it: numbering by ordinal, dropping the flow mirror, and reading a null published graph each kill their own assertion. Local: phpcs, psalm, phpstan and phpmd clean on every changed lib file (phpmd per directory, never over all of lib); 1,314 unit tests green; the six live-API Playwright specs green against :8080.
…nd it (#3496) * feat(flow): every step says what kind of thing it is, and where to find it Two independent facts about a step type, neither of which the engine could answer. The KIND is semantic and is what an interchange needs: a BPMN export has to emit `userTask`, `gateway`, `sendTask`. The CATEGORY is a grouping and is what a palette of 65 entries needs so an author can find anything at all. They are deliberately not derived from one another. `await-signal` is a `receiveTask` and by kind belongs with the integration steps, but it is the machine half of a pair whose other half is "Ask a person", and an author choosing between the two looks in ONE place. So it is `receiveTask` and `human`. THE VOCABULARY IS BPMN'S, NOT OURS. The kinds are exactly BPMN's element types and the list is closed, because its whole value is being the vocabulary an interchange already speaks; a local synonym would have to be mapped back on export and the mapping is the part that rots. A value outside either list is dropped and logged rather than served: `userTsak` would otherwise reach a BPMN export as an element type nothing recognises, and grow a palette group of one. BOTH ARE OPTIONAL, AND THAT IS LOAD-BEARING. On the live instance 38 of the 65 step types come from other repositories on their own release cycles. PHP has no interface defaults, so requiring the methods would fatal those apps on the next release, and guessing on their behalf would write a wrong element type that nobody revisits because it looks answered. They are defaulted in `FlowNodeTaxonomyResolver` instead, exactly as `IFlowNodeConfigKeys` already does it, so an undeclared node still registers, still appears, and says `serviceTask` / `other` where its owner can see it. All 27 nodes this app ships declare both. Verified against the running catalogue, not the source: 65 entries, every one carrying both fields, zero of ours in `other`, and 38 contributed nodes present under the defaults. Two quality findings this change caused, both fixed rather than suppressed: - `FlowNodeRegistry` went to coupling 13. Rather than add a suppression, the "what does this node say it is" question moved out whole — kind, category and role — leaving the registry with FEWER dependencies than it started with. It now decides which nodes exist; the resolver reads their declarations. - `DecisionTableNode` tipped to complexity 51. Its `assign()` turned out to be byte-identical to `SetFieldsNode`'s, and the latter's docblock said so out loud. One copy now, in `FlowFieldPath`, carrying the richer of the two explanations: two copies of a rule about how authored structure is built is the shape that drifts, and the drift would be invisible. Every new test was seen RED by mutating the implementation: serving a declared value without checking the vocabulary, and defaulting the category to something other than `other`, each kill their own assertion. Local: phpcs, psalm, phpstan and phpmd clean across lib/Service/Flow and lib/Service/Flow/Nodes (phpmd per directory, never over all of lib); 1,290 flow and repair unit tests green; 3,205 controller tests green; 5 live-API Playwright specs green against :8080. * test(flow): pin what every shipped node declares, and tag the resolver's constructor Two CI findings on the taxonomy change, both real. gate-16 named one changed public method with no `@spec`: the resolver's constructor. Tagged. The coverage guard was the interesting one. 54 new methods across 27 nodes, each a one-line `return IFlowNodeTaxonomy::…`, and nothing executed any of them: the registry tests drive doubles, so the real nodes' declarations were shipped untested. The guard was right that the change added statements without covering them, and the honest fix is not to cover the lines but to assert the FACT they carry. So the table of what each node is — the review this change asked for — is now mechanical. A new node with no entry fails until somebody writes down what kind of thing it is; a changed assignment fails until somebody changes it here too, deliberately. That is the failure worth catching: a `switch` that stops being a `gateway` changes what a BPMN export says about the process, and nothing else in the suite would notice. Proven by making exactly that change: it fails, naming the node and the field.⚠️ The nodes are built without their constructors. `getKind()` and `getCategory()` return a constant and read no state, so an instance with no dependencies answers exactly as a real one does; constructing 27 classes for real would mean mocking l10n, url generators and mappers to ask each of them one question that cannot depend on any of it. 163 assertions over the 54 methods. phpcs, phpmd and 1,174 flow unit tests green. * fix(flow): attribute the taxonomy test's coverage, and tag a method the gate found Two CI findings, both mine to fix. `@coversNothing` was wrong on the declared-taxonomy test. The coverage guard refused the change again with the SAME numbers as before, which was the tell: a test that attributes no coverage leaves the 54 new methods reading as untested, which is indistinguishable from not having written it. It now names the 27 node classes it covers. gate-16 named `FilterNode::validateConfig` as a changed method with no `@spec`. It is pre-existing debt — the method predates this change and the class only became "changed" because its declaration line gained the interface — but it is debt this change surfaced, so it is tagged rather than left. 🔑 MY LOCAL GATE RUN WAS THE INSTRUMENT THAT LIED. `check_spec_coverage.py` takes its base through the `HYDRA_GATE_BASE_REF` environment variable and a positional path; I invoked it with `--base`, which it ignores. It inspected nothing, printed `# count=0`, and exited 0 — a clean pass that had checked nothing at all. Invoked the way the runner does it, it reproduced CI's finding exactly. * test(flow): attribute the resolver's coverage to the test that drives it The registry test exercises `FlowNodeTaxonomyResolver` on every assertion and claimed only the registry, so the resolver's own statements counted as untested — the same gap as the node declarations, one file over.
flow-controls has failed every development push since b532c9f (08:07Z 2026-09-07) and still fails on 853db46. 1 failed, 74 passed, both attempts. THE FAILURE MESSAGE POINTED AT THE WRONG THING. It read "Needs @conduction/nextcloud-vue >= 2.24.0" while d13b0e5 had already put the app on 2.39.0, so anyone reading it goes looking for a release that shipped long ago. `flow-publish` exists in 2.39.0 and it is a MENU ITEM. CnFlowSidebar pushes Publish into `flowActions` and renders it as an NcActionButton inside an NcActions — the `#secondary-actions` slot here, since this app mounts `<CnFlowSidebar />` with no `embedded` prop and NcAppSidebar wraps that slot in its own menu. CnFlowLifecycleControls states it outright: WHAT IS DELIBERATELY NOT HERE — THE VERBS. Publish, Create draft version and Deprecate live in the header's action menu. So the item cannot be visible until the menu is opened, and asserting on it directly can only ever time out. Two things it is NOT, both checked before writing this: - not `isDraft`, which is `(lifecycleStatus || 'draft') === 'draft'`, so a newly created flow defaults to draft and the action IS pushed; - not `disabled`, which would render a disabled item that is still visible. The item is absent, not disabled.⚠️ THE HELPER TRIES SEVERAL TRIGGERS ON PURPOSE, and that is a statement about what I could verify rather than a hedge. The two hosts label their trigger differently, and this workstation cannot run openregister's e2e — its instance carries thirty-odd apps and the shared global setup times out logging in at 30s, twice, before any test starts. So rather than guess one selector, it tries each and stops at the first that reveals the item; if none does it throws naming every trigger it tried AND every button actually on the page. The previous assertion pointed at a library version and cost an afternoon. eslint and prettier exit 0 and playwright lists the test. The proof is the development push run. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…#3498) * feat(flow): a performer reference carries its type, and resolves late "jdoe" on an assignee field is ambiguous on any real instance: a Nextcloud deployment may hold a user AND a group of that name, and the guard this prepares to replace accepted both — uid equality first, then group membership. So "may this person answer" had two answers and took whichever came first. A `PrincipalReference` says which one it means, and an app that owns a concept the engine has never heard of — a position on a body, a function, a case role — says so in the same shape, through the same event idiom flow nodes already use. 🔴 RESOLVED LATE AND RE-RESOLVED EVERY TIME, NEVER FROZEN AT CREATION. A municipal approval outlives the roster it was raised against: a task assigned to the bezwaarcommissie in March must be answerable by whoever sits on it in June, and one assigned to the afdelingshoofd must follow the post rather than the person who held it. Freezing is simpler and faster and gets the domain backwards in the direction that hurts — it keeps authorising someone who has left and stops authorising the person now responsible. The cost is one membership lookup on a verb a human is performing by hand. Three readings that preserve behaviour rather than quietly widening it: - A BARE STRING STILL MEANS `user`. Reading it as "try every type" would be friendlier and would widen who may answer on every flow already stored. - A LEGACY FIELD'S NAME TYPES ITS ENTRIES. `candidateGroups` holds bare group names; reading them as users would move who may answer on every stored flow. - A SINGLE `{type, id}` MAP IS ONE REFERENCE. A map is itself an array, so a list reader that does not check for sequential keys reads its two VALUES as two references, silently assigning the task to a user called "group". An unknown type resolves to nobody and says so in the log rather than throwing: this runs while authorising an answer, and a type whose app was disabled since the flow was authored must refuse the answer, not return a 500 where "you may not answer this" belongs. A resolver that throws is caught for the same reason. 17 unit tests, each seen RED by mutating the implementation. The three mutants were the design's own central mistakes: caching the resolution (2 fail), reading a bare string as a group (3 fail), and reading a single map as a list (1 fail). This is section 1 of the change. The guard, the node and the picker follow. * feat(flow): the answer guard resolves a typed reference, and keeps the old reading for bare strings `mayAnswer()` compared the acting uid to a stored STRING: uid equality first, then group membership. So `"bezwaar"` authorised the user AND the group's members, and which of the two the author meant was never recorded. The guard now reads the recorded value's SHAPE, because the shape is the information: - A BARE STRING keeps its old, wider meaning, unchanged. Every stored flow on every instance was authored against exactly those semantics, and narrowing them would silently stop authorising the groups the fleet's flows name by bare string. Which of the two an old string meant is a question for a person; the repair will report the ambiguous ones and rewrite only what resolves one way. - A TYPED reference means exactly what it says, resolved fresh on every answer through the resolver registry. Whoever holds the post today may answer; whoever held it in March may not, however the task records who was asked. `recordedFor()` still answers with one string for callers that want one, but the guard no longer goes through it: flattening a reference to a string before the guard sees it is precisely what loses the distinction. The task service's pool guard takes the SAME path for typed candidates, after its three legacy branches so nothing they already admit can be narrowed. Two guards over one question is how they come to disagree, and a disagreement there means a task somebody can complete and cannot see, or the reverse. Fail-closed throughout, in the same direction as the existing missing-group- manager branch: no registry, an unknown type, or a resolver that throws all REFUSE. Asserted rather than inferred from a passing suite. The hot path is untouched: the inbox never calls this guard — it predicates in the datastore — and nothing here resolves row by row. 8 new tests, each seen RED by mutating the guard: routing a bare string through the registry fails 4 (the regression that would break every stored flow), and admitting instead of refusing without a registry fails 1. 1,491 flow and task tests green; phpcs, phpmd, psalm and phpstan clean. The two `PrincipalReference::` call sites carry a documented StaticAccess suppression: they are named constructors on a value object, which is the canonical idiom and indistinguishable to that rule from a static service call. * feat(flow): the node reads typed performers, and is called "Ask a person or group" The step has always been able to ask a group and never said so: the guard resolved a bare name as a uid OR a group, so a good part of the fleet's approvals are addressed to a committee under a label that named one person. The name now says what it does. `UserTaskConfig` reads every performer field as typed references, including one new `candidates` field that expresses what three fields did and may mix types. 🔑 THE THREE LEGACY FIELDS ARE READ BY THEIR NAMES, and that is what makes this safe. `candidateUsers`, `candidateGroups` and `candidateRole` differ only in the kind of thing you type into them — a type system implemented as field names, which existed because the field was a text box. Each is read as candidates of ITS OWN type, so no stored flow changes meaning and none needs migrating. `candidateRole` reads as a GROUP because the pre-typed guard resolved it through `isInGroup()`; any other reading would move who may answer on every stored flow that uses it. 🔴 AN UNKNOWN TYPE IS REFUSED AT SAVE. AN EMPTY RESOLUTION IS NOT. These are two kinds of wrongness discovered by two different people. An unknown type is a defect in the document: the author is at the keyboard, the field is on screen, and the fix is to pick another type — so the refusal names both the type and the id. An empty resolution is a fact about the instance and it changes; a committee with no members today has members next week, and refusing the SAVE would make the flow unauthorable for a reason that has nothing to do with the flow. `namesAPerformer()` now reads references rather than strings, because a `{type, id}` map casts to the string "Array" — non-empty, so the old test called a typed assignee "named" for entirely the wrong reason. Without a registry nothing is refused: an instance that cannot say which types exist must not decide that none of them do. 8 tests, seen RED by mutating the implementation: reading `candidateGroups` as users fails 1 (the stored-flow regression), and refusing an empty resolution at save fails 2. 1,499 flow and task tests green; phpcs, phpmd, psalm, phpstan and gate-16 clean. * feat(flow): a step asking nobody fails loudly instead of raising a task for nobody 🔴 THE MEASURED DEFECT. A step naming a group with no members — or one that does not exist — created a task addressed to nobody. The run suspended, a heartbeat re-read it every few minutes, and NOTHING said anything: the task existed, the run looked healthy, and the approval simply never happened. Silence was the defect. The step now fails, naming what it asked and that nobody holds it. That is strictly better even when the author chooses to continue past it, because a failure is subject to the flow's own `onError` policy — which is a decision the author gets to make, and silence is not. 🔑 REFUSED AT TASK CREATION, NOT AT SAVE. An empty resolution is a fact about the instance and it changes: a committee with no members today has members next week. Refusing the save would make the flow unauthorable for a reason that has nothing to do with the flow. This is the moment somebody actually needs to be found. An unassigned step is untouched — that is deliberately open — and an instance with no resolver refuses nothing. Five Playwright specs over the live API, all green against :8080, including the positive control: a group that DOES have members still raises its task. The refusal is asserted on the run's own `status: failed` and the named reason, not on "the response mentions an error somewhere" — a looser check passes for the suspended run this test exists to refuse.⚠️ `POST /flows/{id}/run` answers 201, not 200: running a flow CREATES a run. Asserting 200 cost two red tests about behaviour they were not testing. * test(flow): cover the built-in resolvers, their registration, and the typed pool guard The coverage guard refused the change and was right: 170 statements added and three whole classes with no test at all — both built-in resolvers, the listener that contributes them, and the task guard's typed branch. What the new tests actually assert, beyond executing the lines: - A DELETED ACCOUNT RESOLVES TO NOBODY, not to its own uid. Returning the uid would authorise an identity nobody can log in as, and would hide the commonest way a performer goes away behind a resolution that looks successful. - A missing group and an empty group are the same fact, deliberately: from the caller's side both mean there is nobody to ask. - The built-ins register through the SAME event every consuming app uses, so the contribution path is exercised by its owner and cannot rot unnoticed. - The task guard's typed branch admits whoever holds the reference, and a bare candidate keeps working unchanged. Removing the branch fails one; that is the test biting.⚠️ A task with no `performerType` is denied before the pool is ever consulted, so a fixture without one tests the determinability guard and says nothing about candidates. Cost two red assertions until the denial message was read rather than guessed at. 1,510 flow and task tests green; phpcs clean.
Brings the flow editor halves that read the engine features already on
development: the publish preflight dialog (which fetches
`/flows/{id}/version-preview`), the step picker grouped by the per-node
category, the health dot, the run sidebar and the version pill.
Verified on the dev instance after `occ upgrade`: the catalogue serves 65
entries with a kind and category on every one, all 27 nodes this app ships
declaring; `openregister.user-task` reads "Ask a person or group"; and the
preflight answers `{"verdict":"minor","next":"1.1.0","current":"1.0.0"}`.
hydra-gates v1.15.1 -> v1.16.0 nc-vue - -> 2.37.0 Lock-only: both packages are already declared with caret ranges that permit these versions, so nothing about what this app ACCEPTS changes - only what it currently resolves to. Opened by the weekly fleet shared-dependency bump, because a lock nobody re-resolves is a pin nobody chose. Merging is gated by this repository's own suite, deliberately: taking hydra-gates v1.8.1 added patchObject() to a published interface, which is a load-time fatal for any concrete double that implements it without the method. CI is the only thing that can tell a safe bump from that. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…t the rest (#3505) A stored `"bezwaar"` means a user OR a group, because the guard it was written against tried both. This repair resolves each string once and rewrites it as `{type, id}` where exactly one kind of thing answers to that name. 🔴 THREE OUTCOMES, AND ONLY ONE OF THEM MAY BE ACTED ON. resolves under exactly one type → rewrite; the string meant that resolves under more than one → LEAVE IT, and report, naming both resolves under no type → LEAVE IT, and report, naming where The ambiguous case is not hypothetical, and the dev instance proved it on the first run: `"admin"` is BOTH a user and a group there, on five steps across four flows. Today both may answer; choosing one would silently narrow who can, and that is a decision for a person, not for a step running unattended during an upgrade. 🔴 IT MUST NOT FAIL AN UPGRADE. A flow naming a performer nobody can be is already broken, and a repair that turns pre-existing breakage into a failed `occ upgrade` makes it everybody's problem instead of its owner's. Every unmigrated string is reported BY NAME with the flow, the step, the field and which of the two reasons applies — "3 could not be migrated" tells nobody which flow to open. 🔑 A FIELD WHOSE NAME SAYS THE TYPE NEEDS NO GUESS. `candidateGroups` holds group names and always did, so its entries are typed without resolving anything: the field name is the evidence, not the roster.⚠️ A TEMPLATE IS NOT A NAME, and the first draft got this wrong. The dev instance carries `{{ case.assignee }}`, which is resolved per item when the step runs — nothing to look up and nothing broken about it. The first run reported it as "already broken", a false alarm that would have sent somebody looking for a flow that works correctly. Templates are now left alone silently. Verified against the live instance: unambiguous group names typed (`behandelaars`, `bezwaarcommissie`, `Beleidsadviseur`), `admin` left bare and reported, the template left alone, upgrade successful. 11 unit tests, each seen RED by mutating the implementation: taking the first type when several answer fails 1, and dropping an unresolvable entry fails 1 — the latter only after the test was strengthened, because "nothing was stored" cannot tell "left alone" from "deleted".
…ker (#3508) openregister's E2E job has been red all day, on two specs, both waiting 15s for `.cn-flow-sidebar__palette` and reporting "the step palette did not render". It rendered. It moved. nextcloud-vue 2.40.0 says so in the component's own header:⚠️ THE PALETTE IS NOT HERE ANY MORE. A live instance serves SIXTY-FIVE step types, and a one-per-row list that long in a 300px column is a scroll rather than a chooser. It is `CnFlowStepPickerModal`, opened from the toolbar, where the same entries render as a grid. With it went the Steps tab, and with that the tab strip. Only the CSS rules stayed behind in the sidebar, which is exactly why this read as a rendering failure: the class still existed, so nothing about the message pointed at a retirement. The page snapshot in the trace is unambiguous — the sidebar carries a heading and a Runs region and nothing else, and the toolbar carries "Add a step". Both specs now open the picker from the toolbar and assert inside it: · the ITEM count, not the container, is what proves the catalogue loaded. An empty picker renders the same dialog as a full one, so without this a missing lock step reads as a lock bug rather than as a load failure. · picking a step closes the dialog (`add()` emits `close`), so flow-lock-nodes reopens it per node, and both specs assert the dialog is hidden before touching the canvas. A dialog left open is a focus trap over every interaction after it. flow-controls opens the picker where the step is ADDED rather than in the "the controls exist" block: that block asserts the button, which is the control that exists there now. `tests/e2e/flow-engine.spec.ts` carried the same stale locator. It sits outside `tests/e2e/ci`, which is the only path CI runs, so it would have waited out its timeout for whoever ran it next rather than failing a job. Fixed with the others. prettier and eslint clean. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bumps [phpstan/phpstan](https://github.com/phpstan/phpstan-phar-composer-source) from 2.2.9 to 2.2.13. - [Commits](https://github.com/phpstan/phpstan-phar-composer-source/commits) --- updated-dependencies: - dependency-name: phpstan/phpstan dependency-version: 2.2.10 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…3511) #3508 moved these specs onto the step picker and got the hard part right: the picker opens, the catalogue loads, the item is clicked, the dialog closes. Then flow-controls failed at the step did not reach the canvas locator('.cn-flow-detail__node').filter({ hasText: 'End' }) The step HAD reached the canvas. It was the wrong step. I replaced an exact match with a loose one. The old locator was `palette.getByText('End', { exact: true })`; the new one was `.filter({ hasText: 'End' })` over the whole picker item — and each card carries the step's NAME, its role word, its DESCRIPTION and its catalogue id, with `hasText` matching case-insensitively as a substring across all of it. Sixty-five steps go through that filter, and 'End' appears in more than one of them. flow-lock-nodes had the same defect and it is sharper there: the file exists to tell "Lock an object" and "Unlock an object" apart, and the first is a SUBSTRING of the second, so the loose filter matched both and `.first()` decided which one the test was really about. Both now filter on the name element with an anchored regex, so the card's description and id cannot satisfy them. prettier and eslint clean. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
#3507) * feat(flow): a run keeps the set of objects it is working with, by role "The run should keep an array of objects that it is interacting with" — and it now does, as a DECLARED set rather than a derived one. 🔴 THE AUDIT CANNOT SERVE THIS PURPOSE, and that is the whole design decision. It already answers "what did this run write", but a case flow READS a case it never writes, WRITES a task and a document it does not consider subjects, and may write one object at three steps for three different reasons. The derived list is too much and too little at once — and decisively it carries no ROLE. `attachTo: case` needs a name, and a list of things that happened has none to give. So the two coexist: derived answers the auditor, declared answers the author. A ROLE, NOT AN INDEX. `attachTo: 2` would work, would be unreadable, and would break the moment a step is inserted. Roles are per-run and unconstrained on purpose — `case`, `besluit`, `aanvraag` are all fine — and a typo is caught by the addressing rule failing loudly rather than by a registry somebody has to maintain. The refusal NAMES the roles the run holds, which is what makes that trade work. 🔑 IDEMPOTENCY IS NOT OPTIONAL. A user-task node is re-entered on a heartbeat, by design, with its task still open, so any node that records a subject is re-entered too. Forty wake-ups leave one subject and log nothing: only a DIFFERENT object under a held role is a replacement. REPLACEMENT IS RECORDED, NOT REFUSED. Superseding a draft decision with a final one genuinely means `decision` to be the new object, and refusing would force authors into `decision2`. But it is also exactly how a later step silently attaches to the wrong record, so the log entry names both objects and the role — which makes "why is this task on the old decision" answerable in under a minute. ON THE RUN ROW, NOT IN A RESUME SLOT: slots are per node, and a three-week approval must wake up still knowing what its case is. The repair seeds ONE thing: the `trigger` entry, from `subject_uuid`, which is a fact the run already carried. 🔴 NOTHING is back-filled from the audit — those declarations would then be ADDRESSABLE by `attachTo`, and a task attached to an object no author ever declared is worse than one that fails loudly the first time somebody uses it. 9 unit tests, each seen RED by mutating the implementation: making a re-fire append fails the heartbeat test, and returning nothing for an unheld role instead of failing fails two.⚠️ `findAllRuns()`, not `findAll()` — writing the latter once already produced a repair that threw, reported itself skipped, and let an upgrade go green having done nothing. * test(flow): cover the trigger-subject repair and the run's serialisation The repair had no test at all, which is the worst place in this codebase to have none: a repair step runs at occ upgrade and nowhere else, and it warns without failing, so one that throws reports itself skipped and lets the upgrade go green having done nothing. The tests pin the four things it must get right: it seeds only from the column the run already carried, it leaves a run that already declared something alone, it reads every page rather than the first, and an unresolvable mapper or one bad row skips instead of failing the upgrade. The paging test found a trap in its own double rather than in the step. The real findAllRuns takes (flowId, status, limit, offset), so a callback reading the first two arguments as limit and offset sees null for both, pins the page index at zero and loops forever. That is a hang, not a failure, and only the double can get it wrong: the step itself calls with named arguments. FlowRun's three date branches in jsonSerialize were uncovered. All three are separately nullable, so one being formatted is no evidence about the other two, and they are asserted separately.
…ompt (#3503) * feat(flow): an agent is a performer of the same step, asked with a prompt You can now address the ask-a-person step to an agent. It is the SAME step: the agent completes its task through the ordinary verbs, past the same guard, into the same audit, against the same outcome vocabulary a person's answer takes. The only difference is what it is handed — a person gets fields to fill in, an agent gets a prompt. 🔴 AN AGENT RESOLVES TO EXACTLY ONE IDENTITY AND NEVER THROUGH A GROUP. That is the whole safety property. The answer guard compares the acting uid against what a reference resolves to, so if an agent reference ever expanded through group membership, the agent's step would become answerable by every human in that group — and, the other way round, a human's step by the agent. The resolver has no notion of groups at all, which is asserted rather than assumed. An agent with no real identity resolves to nobody, and the step then fails loudly rather than raising a task addressed to something that cannot log in. That reuses the refusal built for an empty group, deliberately: they are the same fact. 🔑 THE NODE DISPATCHES; IT NEVER INVOKES A RUNTIME. The engine has no business knowing how an agent runs, and naming the app that does would put a consuming app's name in OpenRegister. The task is created FIRST and the event second, so if nothing is listening the task simply sits there — reassignable to a person like any other, which is the escape hatch that makes an agent performer safe to use at all. Asserted: an unanswered agent task reassigned to a person is answerable by that person and no longer by the agent.⚠️ THIS IS `AgentRunRequestedEvent`'s FIRST DISPATCHER. It was declared and fired by nothing; an event nobody sends is a contract nobody can rely on. `UserTaskNode` tipped to complexity 53 doing this, so the two performer questions moved out whole into `UserTaskPerformers` rather than taking a suppression: the node orchestrates a task's lifecycle, and this decides who the task is for. Both of its questions are asked at creation time for the same reason — that is the moment somebody actually has to be found. 7 tests, each seen RED by mutating the implementation: dispatching for every performer rather than only agents fails 1 (the widening that would make every stored flow start calling an agent), and sending an empty prompt fails 2. 1,528 flow and task tests green; phpcs, phpmd, psalm, phpstan and gate-16 clean. * test(flow): attribute the extracted performer class's coverage `UserTaskPerformers` came out of `UserTaskNode` to keep the node under the complexity ceiling, and the tests that drive it on every assertion still claimed only the node — so 53 extracted statements read as untested. The coverage guard refused the change and was right about what it saw. No behaviour changes; the two suites that exercise the class now say so. * fix(flow): a typed assignee reached the task row as the string "Array" Casting a {type, id} map to a string yields the literal "Array". That string was written into the task row AND into the run's resume slot, where the answer guard reads it as a legacy bare string, matches it against no uid and no group, and leaves the task answerable by nobody. That is the exact silence typed principals exist to remove, put back by a cast. PrincipalReference::storedString() now owns the conversion, because it is knowledge about references rather than about a step. A bare string passes through trimmed, since every flow ever authored names people that way. A single reference becomes type:id. Several references are not one stored value, so the answer is empty and the task pools, which is what the candidate fields are for. type:id is what an inbox can predicate on in the datastore, which is the rule the design sets: the resolver decides authorisation, not listing, and an inbox resolving a reference per row would resolve it a hundred times on one page. TaskInboxCriteria::assigneeNames() expands the caller into the few strings that mean them: the bare uid, user:<uid>, and group:<id> for each group they are in. agent: is deliberately absent, so an agent's task stays out of a person's inbox. The resume slot keeps the reference's shape, because the shape is the meaning: the guard reads a bare string as the old wider union and a typed reference as exactly what it says, resolved fresh on every answer. UserTaskPerformers had no direct test. Its two methods were only ever reached through a node fixture that passes a null registry, so the half that matters never executed. It is tested directly now, and the four remaining uncovered lines in UserTaskNode are covered too.
…r says which types exist (#3506) * feat(flow): an agent is a performer of the same step, asked with a prompt You can now address the ask-a-person step to an agent. It is the SAME step: the agent completes its task through the ordinary verbs, past the same guard, into the same audit, against the same outcome vocabulary a person's answer takes. The only difference is what it is handed — a person gets fields to fill in, an agent gets a prompt. 🔴 AN AGENT RESOLVES TO EXACTLY ONE IDENTITY AND NEVER THROUGH A GROUP. That is the whole safety property. The answer guard compares the acting uid against what a reference resolves to, so if an agent reference ever expanded through group membership, the agent's step would become answerable by every human in that group — and, the other way round, a human's step by the agent. The resolver has no notion of groups at all, which is asserted rather than assumed. An agent with no real identity resolves to nobody, and the step then fails loudly rather than raising a task addressed to something that cannot log in. That reuses the refusal built for an empty group, deliberately: they are the same fact. 🔑 THE NODE DISPATCHES; IT NEVER INVOKES A RUNTIME. The engine has no business knowing how an agent runs, and naming the app that does would put a consuming app's name in OpenRegister. The task is created FIRST and the event second, so if nothing is listening the task simply sits there — reassignable to a person like any other, which is the escape hatch that makes an agent performer safe to use at all. Asserted: an unanswered agent task reassigned to a person is answerable by that person and no longer by the agent.⚠️ THIS IS `AgentRunRequestedEvent`'s FIRST DISPATCHER. It was declared and fired by nothing; an event nobody sends is a contract nobody can rely on. `UserTaskNode` tipped to complexity 53 doing this, so the two performer questions moved out whole into `UserTaskPerformers` rather than taking a suppression: the node orchestrates a task's lifecycle, and this decides who the task is for. Both of its questions are asked at creation time for the same reason — that is the moment somebody actually has to be found. 7 tests, each seen RED by mutating the implementation: dispatching for every performer rather than only agents fails 1 (the widening that would make every stored flow start calling an agent), and sending an empty prompt fails 2. 1,528 flow and task tests green; phpcs, phpmd, psalm, phpstan and gate-16 clean. * feat(flow): the performer fields are principal pickers, and the server says which types exist Three things the typed reference made possible and nothing had yet used. THE FIELDS SAY WHAT THEY ACCEPT. Every performer field declares `principal` rather than `text`. The field always accepted a person OR a group and could only ever say one of them, so authors typed a committee's name into a box labelled for a user and nothing recorded which they meant. `Candidates` and `Prompt` are declared too, so the editor can offer the one field that replaces three and the instruction an agent is given in place of a form. `performerType` is gone from the FORM. With every reference carrying its own type it was a second, weaker copy of the same fact, and two sources of one truth is how they drift. The COLUMN stays, because other things read it, and it is now derived from the reference — an explicitly authored value still wins, so no stored flow changes meaning. THE SERVER SAYS WHICH TYPES EXIST. `GET /api/flow/principal-types` answers with what this instance can resolve. An editor that offered only what it can SEARCH would silently refuse a position, a function or a case role — types contributed by apps whose search it does not know — and the author would have no way to say what they meant. Verified live: it answers `agent`, `group`, `user` on the dev instance. THE RECORDED RESOLUTION IS EVIDENCE, AND NO GUARD READS IT. `flow-tasks` requires the task to record who was asked, and the danger is that such a field quietly becomes an authorisation list: it is a snapshot of a roster that has since moved. Asserted: somebody recorded in March who has since left the committee still appears in the record and may no longer answer. Two quality findings, both fixed by moving code rather than suppressing it: - `FlowController` would have reached ten constructor parameters. The endpoint has its own small controller instead — it belongs beside the node catalogue by subject, and it depends on none of the other nine. - `UserTaskConfig` reached the complexity ceiling and coupling 13. The type refusal and the performer-kind derivation moved into `UserTaskPerformers`, which now owns EVERY question about who a step asks: whether their type is known, whether anybody holds them, what kind they are, and how an agent is asked. The reader's job is to read fields. 4,737 unit tests green across flow, task and controller; phpcs, phpmd and gate-16 clean. * test(flow): attribute the extracted performer class's coverage `UserTaskPerformers` came out of `UserTaskNode` to keep the node under the complexity ceiling, and the tests that drive it on every assertion still claimed only the node — so 53 extracted statements read as untested. The coverage guard refused the change and was right about what it saw. No behaviour changes; the two suites that exercise the class now say so. * fix(flow): a typed assignee reached the task row as the string "Array" Casting a {type, id} map to a string yields the literal "Array". That string was written into the task row AND into the run's resume slot, where the answer guard reads it as a legacy bare string, matches it against no uid and no group, and leaves the task answerable by nobody. That is the exact silence typed principals exist to remove, put back by a cast. PrincipalReference::storedString() now owns the conversion, because it is knowledge about references rather than about a step. A bare string passes through trimmed, since every flow ever authored names people that way. A single reference becomes type:id. Several references are not one stored value, so the answer is empty and the task pools, which is what the candidate fields are for. type:id is what an inbox can predicate on in the datastore, which is the rule the design sets: the resolver decides authorisation, not listing, and an inbox resolving a reference per row would resolve it a hundred times on one page. TaskInboxCriteria::assigneeNames() expands the caller into the few strings that mean them: the bare uid, user:<uid>, and group:<id> for each group they are in. agent: is deliberately absent, so an agent's task stays out of a person's inbox. The resume slot keeps the reference's shape, because the shape is the meaning: the guard reads a bare string as the old wider union and a typed reference as exactly what it says, resolved fresh on every answer. UserTaskPerformers had no direct test. Its two methods were only ever reached through a node fixture that passes a null registry, so the half that matters never executed. It is tested directly now, and the four remaining uncovered lines in UserTaskNode are covered too. * test(flow): cover the step's own readings and the unknown-type refusal The config reader's list, template and null handling had no direct test, and these are the readings a flow author trips over rather than the ones a developer does: a list written as "approved, rejected" in a text field, a trailing comma producing an empty candidate nobody can be, a title carrying a template, a field left blank that must stay null rather than become an empty string in a date column. UserTaskPerformers::refuseUnknownTypes and kindFor are covered too. The refusal names both the type and the id, because the author is at the keyboard with the field on screen, and an instance with no registry refuses nothing: one that cannot say which types exist must not decide that none of them do. * test(flow): adapt the performers test to this branch's constructor UserTaskConfig does not take the resolver registry here: the unknown-type refusal moved onto UserTaskPerformers, which is where the registry now lives.
With the picker and the exact-match fixes in, this spec now builds the
flow correctly — the trace shows the End step on the canvas, an edge from
the start node, and Run enabled. It then times out at 45s, and the error
it reports is the CLEANUP that ran afterwards:
page.evaluate: Target page, context or browser has been closed
at deleteFlow (...:221)
which names neither the control it was waiting for nor the menu it was
waiting in. The last locator in the trace is `[data-testid="flow-publish"]`.
THE PAGE HAS THREE BUTTONS NAMED "Actions": the canvas has one and
NcAppSidebar renders one in its header. `openFlowActionsMenu` took
`.first()` of each candidate group, so it opened a menu that does not
carry the flow actions, found no Publish there, and moved on having
tried ONE of the three. Its own descriptive error never fired, because
the test budget ran out first.
Two changes:
· try EVERY button each selector matches, not just the first, and
press Escape between attempts so a menu that did open cannot cover
the next candidate;
· bound each click at 5s. An unbounded click on a control that never
becomes actionable spends the budget the remaining candidates need,
which is how a helper written to name its failure managed to produce
a timeout somewhere else instead.
The sidebar header menu is tried first now, because that is where these
actions actually live.
prettier and eslint clean.
Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…3519) With the actions-menu fix in (#3517), the menu opens and Publish is pressed. The flow then stays a Draft and the spec reports Publish was pressed but the flow never became published — the store still shows the draft it started as, so the POST was rejected or swallowed There was no POST to reject. The trace's entire request log for this test is the flow's own `POST /api/flows` (201) and the cleanup DELETE. Nothing was ever sent to publish. The menu item's handler is `run: () => { this.publishOpen = true }`. It mounts `CnFlowPublishDialog` — the confirmation carrying the next version, what the bump removes, and any refusal — and PUBLISHING is what that dialog's own primary button does. The spec pressed the opener and waited for the outcome of a button it never reached. Now it asserts the dialog opens, then confirms through `[data-testid="flow-publish-confirm"]`. A REFUSAL IS READ OUT RATHER THAN WAITED ON. `flow-publish-refusal` renders in the same dialog when the version bump is declined, which is a legitimate answer; without this the confirm would sit out its budget and report the lifecycle pill instead of the reason printed on screen. prettier and eslint clean, SPDX header intact. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
#3519 made the failure precise — "pressing Publish did not open the publish confirmation" — and the page snapshot shows why it could not: - button "Actions" [expanded] - menu "Actions": - menuitem "Edit flow" - menuitem "Enable" - menuitem "Publish" ← right there, unclicked The menu opened. The item was present. Nothing happened. `data-testid` is a fallthrough attribute on `NcActionButton`, and that component's root is the `<li>`; the handler is bound to the `<button>` inside it. `clickThemed` dispatches the event straight at the element it is handed, so aiming it at the `<li>` fires an event nothing listens for. Playwright's own `.click()` would have walked down to a clickable descendant and hidden this, which is the trade this file already documents for using a dispatched event. Now driven through `getByRole('menuitem', { name: 'Publish' })` — the accessibility tree names it, so this is also how a screen-reader user reaches it — with the testid's inner button as a fallback if the role is ever renamed. Same shape as the `flow-settings-name` fix in integriq #1889: a testid on a wrapper component is not a testid on the control. prettier and eslint clean, SPDX header intact. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…h the global server (#3523) A unit-test run took 19 GB of RAM and 6 GB of swap on 2026-09-08. The bootstrap loaded the workspace's uninstalled lib/base.php and kept running with a half-built OC::$server; four classes then asked that container for app services, and without the app's factories it autowired the MagicMapper -> SettingsService -> ValidationOperationsHandler -> ValidateObject cycle until memory ran out. Twelve unit test files hit a 1 GB cap within seconds in that state. - tests/bootstrap.php only boots a root whose config.php says installed; an explicit root that cannot boot stops the run instead of pretending. - phpunit.xml caps memory at 2G (the suite peaks at 454 MB). - SaveObject, ObjectService, ObjectsController and EntityRecognitionHandler resolve services through their injected app container. - SaveObjectRetentionResolutionTest pins the seam; two ObjectsController tests register services on the container mock instead of OC::$server. - Two stale UnusedParam psalm baseline entries removed. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
… a task to it (#3513) * feat(flow): a run keeps the set of objects it is working with, by role "The run should keep an array of objects that it is interacting with" — and it now does, as a DECLARED set rather than a derived one. 🔴 THE AUDIT CANNOT SERVE THIS PURPOSE, and that is the whole design decision. It already answers "what did this run write", but a case flow READS a case it never writes, WRITES a task and a document it does not consider subjects, and may write one object at three steps for three different reasons. The derived list is too much and too little at once — and decisively it carries no ROLE. `attachTo: case` needs a name, and a list of things that happened has none to give. So the two coexist: derived answers the auditor, declared answers the author. A ROLE, NOT AN INDEX. `attachTo: 2` would work, would be unreadable, and would break the moment a step is inserted. Roles are per-run and unconstrained on purpose — `case`, `besluit`, `aanvraag` are all fine — and a typo is caught by the addressing rule failing loudly rather than by a registry somebody has to maintain. The refusal NAMES the roles the run holds, which is what makes that trade work. 🔑 IDEMPOTENCY IS NOT OPTIONAL. A user-task node is re-entered on a heartbeat, by design, with its task still open, so any node that records a subject is re-entered too. Forty wake-ups leave one subject and log nothing: only a DIFFERENT object under a held role is a replacement. REPLACEMENT IS RECORDED, NOT REFUSED. Superseding a draft decision with a final one genuinely means `decision` to be the new object, and refusing would force authors into `decision2`. But it is also exactly how a later step silently attaches to the wrong record, so the log entry names both objects and the role — which makes "why is this task on the old decision" answerable in under a minute. ON THE RUN ROW, NOT IN A RESUME SLOT: slots are per node, and a three-week approval must wake up still knowing what its case is. The repair seeds ONE thing: the `trigger` entry, from `subject_uuid`, which is a fact the run already carried. 🔴 NOTHING is back-filled from the audit — those declarations would then be ADDRESSABLE by `attachTo`, and a task attached to an object no author ever declared is worse than one that fails loudly the first time somebody uses it. 9 unit tests, each seen RED by mutating the implementation: making a re-fire append fails the heartbeat test, and returning nothing for an unheld role instead of failing fails two.⚠️ `findAllRuns()`, not `findAll()` — writing the latter once already produced a repair that threw, reported itself skipped, and let an upgrade go green having done nothing. * feat(flow): record a run's declared subjects and attach a task to one A node knows its run only as a uuid in the context array, and the subject set lives on the run row, because it must survive a three-week suspension that a per-node resume slot does not. This is the one place that turns the uuid into the row, records, and stores. Two deliberately opposite failure modes: Recording never fails the step that did the work. The object has already been written or locked by the time this runs, so failing here would report a step as failed after its effect landed. An unreadable run is logged and the step carries on. Attaching does fail, before the task exists. A task attached to nothing looks fine in every list and is wrong in the one place it matters, so an unheld role throws and no task is created. The refusal names the roles the run does hold, which is what makes an unregistered role vocabulary workable. Attachment goes through the task's existing objectUuid, registerId and schemaId. Three things already read those fields, so the attachment works everywhere immediately; a new column would have needed all three taught about it, and the two that were not taught would have been found by a user. Refs openspec/changes/flow-run-subjects-and-answers * feat(flow): let object-write and lock-object declare what the run is working on Both nodes accept an optional subjectRole. Naming one records the object the step acted on under that role, so a later step can attach its task to it by name. Naming none records nothing: the subject set is declared, never derived. A set that filled itself would be the audit-derived object list again, which already exists and answers a different question, and every entry in it would then be addressable by attachTo, inventing declarations no author made. One object, or none. A role is a singular name, so a step that wrote forty objects has no single one to mean by it. Recording the fortieth would be arbitrary and recording all forty in turn would fire thirty-nine replacement warnings for a set that ends up holding one entry anyway. So a multi-object step records nothing and says so once, and the author finds out at the point of use, where the later attachTo fails naming the roles the run does hold. The count decision lives in the recorder because it is the same decision for both nodes. The write node skips a delete: the object is gone, and a role pointing at it would attach a task to a record that no longer exists. The lock node records only after every target is held, so a run never claims a subject it failed to lock, and records no register or schema because it locks by uuid and never resolves them. The RBAC rationale moved from inside execute() to the class docblock, where a reader looks for it, which also brings the method back under its length budget. Refs openspec/changes/flow-run-subjects-and-answers tasks 3.1, 3.2 * test(flow): the run read serves its declared subjects, and the two lists disagree GET /api/flow-runs/{uuid} already carries the declared set, because the entity serialises it. What had no test is the thing the whole change rests on: that the declared set and the audit-derived object list answer different questions and must keep disagreeing. The run in the test READS a case it never writes and WRITES a task it does not consider a subject, so the case appears only in the declared set and the task only in the derived list. The audit answers the auditor; only a declaration answers the author, and only a declaration carries a role. A run that declared nothing answers an empty set rather than a missing key, and a run nobody may see is a 404 here too: the declared set names the records a flow is working on, so it is as sensitive as the run itself. * docs(flow): mark the delivered subject-recording tasks Sections 1, 2, 3 and 5 are done. Section 4 (attachTo on the user-task node) waits on the agent-performer and principal-picker branches, which rewrite the same node. * fix(flow): keep the lock node under its complexity budget The node guarded the empty role itself and the recorder guards it too, which is one rule in two places. That second copy took LockObjectNode to a complexity of exactly 50, which is the configured threshold, so CI's phpmd refused it while my per-file local runs passed: CI runs the whole lib tree with a baseline file and a second unused-parameter ruleset, and I had run neither. The guard now lives only in the recorder, where the decision belongs and where it is tested. The node's own test asserts what is left to assert: that a step naming no role passes no role on, rather than substituting one. * feat(flow): a task can hang on an object the run declared earlier The user-task step accepts an optional attachTo naming a subject role of its run. The created task is anchored to that entry through the task row's existing objectUuid, registerId and schemaId, so the subject-anchored inbox read, the case sidebar and the portal visibility rule all keep working with no change. A new column would have needed all three taught about it, and the two that were not taught would have been found by a user. The anchor overrides the item's own, which is the point of the field. Without attachTo the task hangs on whatever record the step was raised from; with it, the task hangs on the object the author named, which may be one no item in this stream carries. Naming a role the run never recorded fails the step and creates nothing. The refusal lands before the task exists, because a task attached to nothing looks fine in every list and is exactly the task an author believed was attached to the case. It names both the role asked for and the roles the run does hold, which is what makes an unregistered role vocabulary workable. Refs openspec/changes/flow-run-subjects-and-answers tasks 4.1, 4.2 * test(flow): cover the run's declared subjects in the Newman suite The whole scene over the live HTTP API: a case is created and recorded under the role case, the case is locked, a person is asked with attachTo: case, they answer with a form value, and the answer routes. Newman rather than Playwright, because these are HTTP contract assertions and playwright.config.ts excludes api-direct from every project. A spec there runs only when a developer invokes an ad-hoc config by hand, so CI executes none of it, and a green spec with zero CI coverage looks identical from the outside. That is the same reason the delegation and register-descriptors collections were moved here. Not a unit test, because every part of this already has one and they all passed while the parts did not meet. The recorder writes onto the run row, which only exists once a run is persisted; attachTo reads that row back in a later step of the same run; and the anchor has to survive the task bridge, the task row and the inbox read. A unit test can assert each hand-off and cannot assert that the chain holds. Three scenarios: the scene itself, a write naming no role declaring nothing while the audit still shows the write, and attachTo naming an unheld role failing the step and leaving no task. The last is the assertion the whole capability exists for. Registered in run-all.sh, which CI runs through api-test-coverage.yml on every PR to development. A collection that is not registered there runs nowhere. * ci(newman): run the flow-subjects collection Registering a collection in run-all.sh is not enough to make it run: the workflow passes an explicit COLLECTIONS subset, so anything outside that list executes nowhere however carefully it is registered. flow-subjects belongs in the default set. It seeds its own register and schema and removes them again, so it hardcodes none of the dev-container state that keeps platform-annotations and files out, and it drives every run through the synchronous test endpoint rather than the background worker, which is what keeps flow-engine out. * docs(flow): record what shipped and where the proof lives Sections 4, 5 and 6.1/6.2 are done. The proof is a Newman collection rather than a Playwright spec, and the entry says why: playwright.config.ts excludes api-direct from every project, so a spec there would be green and cover nothing. 4.3 asserts the attached task through the subject-anchored inbox read. The case sidebar is a dossiq surface reading the same three task fields, so it is covered by the choice of fields rather than by a second assertion here. * fix(flow): serialise a run's subjects as an object, never an empty array Found by the new Newman collection on its first CI run, which is what it is for. json_encode turns an empty PHP array into [], so a run that declared nothing served a JSON ARRAY where every populated run serves a MAP. A typed client cannot read both, and nothing in PHP notices: assertSame([], ...) passes on the array the encoder then flattens. The cast makes the wire shape one thing, and the tests now assert the encoding rather than the PHP value. The collection also learned to explain itself. Every assertion in the scene folder reads something the run was supposed to produce, so when the run fails they all fail for a reason none of them names. The suspend assertion now carries the run body in its message. * fix(test): the scene's edges are sequence, not actions The engine refused the whole run: an edge carrying type and config is the pre-inversion shape, and it says so plainly — an edge is sequence and a node is the action. The route edge came from tests/e2e/api-direct/flow-engine.spec.ts, which still carries that shape because nothing runs it. That is the same reason this file is a Newman collection rather than a Playwright spec, and it is a neat demonstration of the cost: a spec CI never executes goes stale without anyone finding out, and then gets copied. The branch was never asserted here anyway, so the graph is now a straight line and the prose no longer implies otherwise. * fix(tasks): a form answer writes as the run that raised the task A flow that locked its case and then asked a person with a form on that case could never be answered. The completion came back 500 with LockedException: "Object is locked by flow run …, running as admin" — naming the very run that was trying to write. The object-lock guard already intends to allow this. It reads the acting run from an ambient context precisely so a flow that locks its case at one step is not turned away by its own lock at the next. But that context is set while the ENGINE walks the graph, and a form answer arrives on a PERSON's HTTP request, where nothing had set it — so the run's own write read as a stranger's and was refused. TaskFormCompletion now pushes the task's runUuid around the subject write and pops it in a finally, so a refused write does not leave the rest of the request pretending to be a run. Found by the flow-subjects Newman collection on its second CI run, driving the scene from the brief: create a case, lock it, ask a person with a form on it. Every earlier layer passed while this one did not, which is exactly what a collection that walks the whole scene is for. * test(flow): a register holding objects declines deletion, and that is correct The teardown asserted the fixture register and schema were gone and got 409. That is not a failed teardown: the runs created case objects in the register, and a register holding objects is not deletable, which is the behaviour wanted everywhere else. Accepting 409 is safe here because every fixture is namespaced with the run stamp, so nothing a later run does can collide with what is left, and CI destroys the container when the job ends. A 5xx still fails, which is the distinction worth keeping: a register declining to disappear under its own contents is not the same as the register read breaking. --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
Every gate measured by exit code rather than by its summary line, which is the distinction that matters here: phpunit has printed OK and exited 1, and phpmd over the whole lib tree is OOM-killed and reports that as a pass, so it is swept per directory with the baseline instead. lint clean, phpcs 0, phpmd E=0, psalm 0, phpstan 0, phpunit 0 over 19,434 tests, and both l10n gates 0 across all 36 locales. Also notes that CLAUDE.md's "Known state" claim that test:l10n is red at HEAD is stale. It passes, and the 17 keys it names are present — a stale note about a red gate is how a real red one later gets waved through. Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
…ks (#3528) On an instance with hundreds of schemas the provider took one lock per schema in a single transaction and failed with SQLSTATE[53200] out of shared memory, swallowing the error into an empty result. Search now runs in bounded chunks, keeps ordering and links, and logs a failed chunk as an error while returning the rest. Work in progress checkpoint: checks and live verification follow in the next commit.
…#3530) * feat(openspec): contacts-leaf-cases-panel * feat(openspec): content-search-index * feat(openspec): files-leaf-save-to-object * feat(openspec): favourites-and-recent * feat(openspec): rbac-department-role-matrix * feat(openspec): audit-log-page * feat(openspec): query-related-schema-rows * feat(openspec): notes-leaf-rich-text-lock-export * feat(openspec): registry-subscriptions * feat(openspec): activity-leaf * feat(openspec): generated-identifier * feat(openspec): lifecycle-over-reference-field --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl>
… global server (#3532) * refactor(jobs): background jobs resolve collaborators through an injected container The eight jobs that reached for \OC::$server at run time now take the app container in their constructor. Their tests register mocks on a per-test container mock (new RegistersContainerServices trait) instead of on the process-wide fake server, which never reset and leaked mocks between tests. BlobMigrationJobTest no longer needs its capture-and-restore dance or its skip; its twelve cases run. Also: psalm and phpmd get explicit 2G memory limits in composer.json, and the NoLegacyServerAccessors sniff test points at the vendored hydra-gates sniff and runs again (it was skipped for a PHPCS 3.9 bug and aimed at a file this repo no longer carries). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * refactor: search trail, magic mapper and organisation controller use their injected collaborators SearchTrailController takes the SearchTrailMapper in its constructor and the mapper gains the clearAllLogs() the admin endpoint always called (the method never existed, so clearAll threw an \Error for an undefined method). MagicMapper's cross-table lookups use the register and schema mappers it already has; OrganisationController reads the session it already has. The tests stop registering mocks on the global server. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * refactor: services take the collaborator they used to fetch from the global server ApiTokenSettingsController takes IClientService, EndpointService the AgentMapper, ActivityFilterService and the seven MarkerLookupTrait providers a LoggerInterface, TenantLifecycleService the IUserManager, DeleteObject the OrganisationMapper and ConfigurationSettingsHandler the IAppManager. ImportHandler takes an optional IAppManager (twelve test files construct it positionally); without one the seed-data app dependency check is skipped with a debug log instead of reaching for the server. The provider factories in Application.php pass the logger. Tests that registered mocks on the global server now inject them, and the seams that were unreachable before (token probes, provisioning the org admin, the active organisation on delete, version info) get one test each. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * refactor: link services, the relations controller and the grant resolver resolve through an injected container RelationsController, AnalyticsLinkService, DeckLinkService, BookmarkLinkService, DeckCardService and FileVersioningHandler take a ContainerInterface and resolve the other apps' classes (Deck, Bookmarks, Files_Versions, Analytics) and the pluggable leaf link services through it, wrapped exactly as before (class_exists guard, try/catch, null on absence). DeckCardService also takes the IUserManager it used to fetch by string. CaseTokenService and ObjectGrantResolver already accepted an optional container with a global fallback; the container is required now and the fallbacks (an anonymous PSR-11 shim and a Server::get of the container) are gone. The tests inject a container mock; the relations controller gets two tests for the leaf path that used to be unreachable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * docs(schema): say why the entity keeps its guarded global lookup Schema is an entity, not DI-built, and resolveIntegrationRegistryIds() runs from setConfiguration(), which every mapper calls while hydrating a row. Threading the registry ids through those thirty call sites is a change of its own, so the lookup stays behind its isset() guard, with a targeted phpcs:ignore and a docblock that says so. It is the one remaining site in lib/ outside AppInfo, AppHost and Migration. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * refactor: mark the deliberate container lookups, and wire the last services Closes the openregister half of the 2026-09-08 memory incident. Every remaining \OC::$server->get() / \OCP\Server::get() in lib now carries a phpcs:ignore naming why it stays, so the new fleet sniff can land as an error: - two PSR-11 adapter shims that ARE the container being handed to a collaborator, so there is nothing to inject into them - three probes for optional sibling apps (keepiq credential services, AppAPI PublicFunctions), each guarded by class_exists and try/catch - one provider fallback, tried only after the injected container - Db\Schema, an entity rather than a DI-built class, which already documented its guarded lookup Also wires the container and logger arguments the earlier commits' constructors now require, and drops two dead branches static analysis had flagged: AgentMapper findByUuid returns a non-nullable Agent and throws, so its null check could never run, and IResponse::getHeader returns an array, so its null coalesce could never fire. Neither changes behaviour. Verified locally: 19,454 unit tests exit 0, peak 454 MB; phpcs exit 0 across all 47 touched lib files; the fleet sniff reports 0 unannotated lookups in lib. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Conduction Release Bot <release-bot@conduction.nl> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Implements OpenRegister's registry-subscriptions capability, the design Ruben chose on 2026-09-11 for Tier B item B22 over integriq's brp-kvk-store-and-subscriptions (rejected the same morning, retired in ConductionNL/integriq#1970, merged as c1d34690). - x-openregister-registry schema annotation, validated (blocking) at schema save. - openregister_registry_subs state table, keyed by object uuid. - @self.registry render mirror. - POST/DELETE .../registry-subscription to request/end a subscription, guarded by the caller's update permission. - POST /api/registry/{registry}/updates, the connector's inbound update: applies only owned properties, refuses the rest, writes an explicit registry-actor audit row. Query lenses (_registry[state]/[updatedBefore]) are deliberately deferred; see design.md's implementation addendum. CI: run 34688988262, a pull_request-triggered run that actually attached to this PR, conclusion success, 46 checks, all pass or correctly skipping. development advanced by one commit since (an unrelated fix, only appinfo/info.xml's version string conflicted); resolved and verified locally by exit code (php -l, phpcs, phpstan, gate-16 spec-coverage, migration-version-bump check) rather than waiting through another full CI cycle against a fast-moving base. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…#3661) * feat(archival): a schema can declare the three archival facts MDTO asks for GAP A3. aggregatieniveau, beperkingGebruik and dekkingInTijd appeared in zero PHP files. The export learned to emit them in #3618, but nothing wrote them, so it could only ever omit them, and an omission reads exactly like a record that genuinely has none. x-openregister-archival now takes three keys beside retention: aggregationLevel: Dossier useRestriction: { type: "Geen beperking", description: "..." } temporalCoverage: { type: "Looptijd", startProperty: startdatum, endProperty: einddatum } temporalCoverage names date PROPERTIES rather than dates, the way sourceDateProperty does. MDTO defines dekkingInTijd as the period the record's CONTENT pertains to, which differs per row, so a literal date on a schema would be the same wrong answer for every one of them. Unknown keys are refused at the top level and inside each block, the way unknown retention keys already were, and a term outside MDTO's own begrippenlijst is refused too. Both lists are formally OPEN, so that check is stricter than the standard, and deliberately: the exported element cites the list it took the term from, so a term that is not on the named list would make the document claim a provenance the term does not have. MdtoTerms is the one home for those lists, so the check that refuses a term and the document that cites it cannot disagree. RetentionEvaluator resolves the facts for the row, ArchivalDecisionResolver emits them into _retention in the abstract English vocabulary with the object's own retention block overriding the schema, and MdtoValueReader gains the annotation as its third source so a schema-declared fact reaches the document. Each is present only when established, absent otherwise, the same rule UnestablishedValues applies to the rest of the block. * test(archival): the declared facts, from schema to document Four cases each, as the change asks for: declared on the schema, overridden on the object, absent (emitting nothing, no placeholder), and invalid (refused). The validator refuses an unknown key at the top level and inside each block, a term outside MDTO's begrippenlijst, and a literal date where the annotation takes a property name. The evaluator resolves temporalCoverage against the ROW, truncates a timestamp to the date union MDTO allows, and drops an end date the row cannot supply while keeping the start. The resolver emits each fact into _retention, lets the object's own block win over the schema's, reads the TMLO spelling, and omits a fact nobody declared. The generator turns a schema-declared fact into elements, and the XSD suite gains a case whose facts come only from the annotation, so the document that carries them is still validated against the vendored schema. * docs(archival): A3 is resolved, and D1 records what the export now emits The proposal still said these three facts have no writer, which was true when it was written and is the reason the export could only omit them. A3 now records the annotation that declares them, the per-object override, the property-naming rule for temporal coverage, and the two checks that refuse a mistake instead of exporting it. It also records what openbaarheid turned out to be: MDTO has no such element, because publicity is carried by beperkingGebruik, whose "Geen beperking" term is how a record says it is open. So that part of the finding is a vocabulary question for WOO rather than a missing writer. D1 separates what was absent at audit time from what is absent now. Still missing: betrokkene and the relation elements that carry aggregation structure, which openregister has no source for until a schema can say which relation expresses containment. * docs(archival): the spec for the three declared facts Names the annotation keys, the resolution order, the property-naming rule for temporal coverage, and the two refusals: unknown keys, and terms outside the begrippenlijst the exported element cites. Records why that second check is stricter than MDTO, whose lists are open, and what it would take to support a local term instead. * refactor(archival): split the facts validator out, and fix the two style findings phpmd put ArchivalAnnotationValidator over the complexity threshold and resolve() over the method-length one, both because of this change. The three fact validators are now ArchivalFactsValidator. That is a real seam, not a shuffle: the original validates the disposal DECISION, a duration and the rules that override it, while these validate FACTS about the record against MDTO's begrippenlijsten. Moving methods within a class would have raised complexity rather than lowered it, which is what happened the last time I tried it. phpcs also wanted a named argument in the shortened resolver call, and an inline if replaced by a statement.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
… the schema (WOO-563) (#3638) * fix(import): keep the register-schema link when a version check skips the schema An import that re-ran with an unchanged schema version took the skip path before the register-schema link was written, so the schema stayed unattached to the register it was imported into and every later register-scoped lookup missed it (WOO-556 finding B24). The link is now established on the skip path too, so a no-op re-import converges on the same state as a first import instead of leaving the register half-wired. UNFINISHED — committed to preserve the work across a WSL crash. Not yet verified: the new tests/Unit/Service/Configuration/ImportHandlerRegisterSchemaLinkOnVersionSkipTest.php has not been run, no quality gates have been run, and there is no PR. Refs: WOO-563 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(import): make the register/schema mapper doubles match the real signatures The doubles declared find(string $id) while RegisterMapper::find takes ($id, $_rbac, $_multitenancy) and SchemaMapper::find takes ($id, $_extend, $_rbac, $_multitenancy). Production calls both with NAMED arguments, and PHPUnit forwards an invocation's named arguments to the callback as named arguments, so the closure would die on "Unknown named parameter $_rbac" — an Error, which the DoesNotExistException catch below does not see, so the per-register Throwable guard swallowed the whole register import. The suite would then read as "the fix does not work" when the fixed code was never reached. Verified against the branch: 4 passed with the fix, and the two behavioural tests fail without it (restored HEAD~1's ImportHandler, re-ran, got exactly the twin-fork and missing-link failures back). Refs: WOO-563 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(import): one implementation of the schema-link union rule Review follow-up on #3638. The full register update and the version-gated skip path each carried their own array_unique(array_merge(...)), with different flags. Harmless for the ids actually stored, but the rule "union, never replace" was stated at length beside each copy — the exact shape that rots, and #2935 was this rule holding in one place and not the other. Both now call unionSchemaIds(), which states it once. Also tightens the skip path's no-op guard: it compares against the DEDUPED current list instead of the raw one, so a register whose stored list already held a duplicate no longer gets a silently de-duplicated row written from a path whose premise is that it writes nothing. Configuration suite still 409 passing; phpcs, phpmd and php -l clean. Refs: WOO-563 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(import): return the mutated register, not update()'s Entity-typed value CI on #3638 failed one test in 19844: ImportServiceRegisterAutoCreateTest never configures registerMapper->update(), so PHPUnit returns a bare Entity stub for it, and linkImportedSchemas()'s Register return type rejected that with a TypeError. The path never wrote before WOO-563, so it never met the stub. QBMapper::update() returns the same instance it was given, so persisting and then returning the entity we mutated is identical at runtime and only differs in the declared type. Verified in the container with the worktree bootstrap: the four WOO-563 tests plus ImportServiceRegisterAutoCreateTest, 7 passed, 34 assertions. Refs: WOO-563 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…WOO-567) (#3639) * fix(objects): keep the instant of a date-time with a non-UTC offset A format: date-time value written with a non-UTC offset came back with the offset replaced by +00:00 and the clock time untouched, so the instant silently moved. PATCH {"term":"2026-10-20T00:00:00+02:00"} echoed +02:00, and the following GET returned 2026-10-20T00:00:00+00:00 — two hours later than what was sent. For a WMEBV objection term (art. 2:10) that is a legally relevant shift, not a display quirk (WOO-556 finding B23). Normalises through the instant instead of the clock face, and carries the same rule into the magic-mapper search and statistics handlers so filters and sorting agree with what is stored. The spec gains the corresponding requirement. UNFINISHED — committed to preserve the work across a WSL crash. Not yet verified: the 138 new lines of DateTimeNormalizerTest have not been run, no quality gates have been run, the PATCH/GET cycle has not been replayed on the dev instance, the negative-offset and DST-boundary cases are unconfirmed, and there is no PR. Existing stored values may already be wrong (clock-time-as-UTC); whether a migration is needed is still open. Refs: WOO-567 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(objects): do not convert a calendar date through a timezone Self-review catch on #3639. The WOO-567 fix routed BOTH `date` and `date-time` properties through formatForDatabase(), which converts to UTC before storing. That is right for a date-time, which names an instant, and wrong for a `date`, which names a calendar DAY and has no instant. Converting moved it in both directions — measured on this branch: 2026-10-20T00:00:00+02:00 -> stored 2026-10-19 (a day early) 2026-10-20T23:30:00-05:00 -> stored 2026-10-21 (a day late) So a due date submitted from a client that sends an offset silently landed on the wrong day — the same class of bug WOO-567 is about, introduced one format over while fixing it. The write path now splits on format: a date-time is converted, a date keeps the wall-clock reading it arrived with, which is what this code did before WOO-567 touched it. Five new tests pin both halves, including the contrast case where the same input is preserved as a date and converted as a date-time. Refs: WOO-567 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * style(objects): import DateTimeImmutable and DateTimeZone instead of qualifying them inline phpmd's MissingImport rule failed CI on #3639 at the two write sites that did `new \DateTimeZone(...)` inline. Imported both classes next to the existing `use DateTime;` and dropped the leading backslashes at all four usages. No behaviour change; php -l, phpcs and phpmd clean on the file. Refs: WOO-567 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dy calls (#3662) countBySchemas() returns 0 and findBySchemas() returns [], unconditionally. Neither has a caller anywhere in lib/, src/, tests/ or appinfo/: measured zero references outside their own definitions. They are the shape this repository has been removing all week: code that looks like a working query and silently answers 'nothing'. A future caller would get 'no objects in these schemas' from a method that never looked, and would have no reason to doubt it. Whoever needs to count or find across schemas should write it against the magic tables deliberately, the way the retention sweep does, rather than inherit a lie. getSizeDistributionChartData() is left in place on purpose: it answers with an empty chart, but it HAS a caller (DashboardService::getObjectsBySizeChartData), so deleting it would break the dashboard. Implementing it for real is a separate change. Verified: phpcs exit 0, phpunit tests/Unit/Db 1609 tests exit 0.
…0260912131909 chore(sync): carry beta back into development
…app-owned state machine (#3666) * feat(lifecycle): add the public provider contract for app-supplied actions An app whose transitions live in data rather than in the schema has no way to answer available-actions today. This is the contract it implements, a sibling of the guard and action interfaces: read-only, called on a GET, and throwing rather than returning an empty list when it cannot answer. * feat(lifecycle): resolve provider tags through a fail-closed registry Models LifecycleGuardRegistry: OpenRegister container first, IServerContainer as fallback for an FQCN another app owns, a per-request instance cache, and no static container lookup. An unresolved tag throws rather than answering with an empty list, because an empty list is a legitimate answer here and a silent failure would be indistinguishable from one. LifecycleProviderException extends RuntimeException so existing catch sites keep behaving, while the controller can still tell a provider failure from a missing object. * feat(lifecycle): answer available-actions from a provider when one is declared The provider branch sits between the static map and the graph block, so a schema that declares transitions keeps them whatever else it says, and TransitionEngineGraphTest's precedence case stays green. Whatever the provider returns is republished through publishedInputs(), the same normalisation the write path uses, so the inputs contract cannot drift per mode. A provider that throws is wrapped rather than swallowed: an empty list is a legitimate answer, so a failure that looked like one would render as a dead timeline the client believes is correct. The two new PHPMD suppressions carry their reasons in the file, following the house idiom rather than growing the baseline. * feat(lifecycle): validate provider-mode annotations at schema-save time Provider mode is shape-checked like graph mode: field required, enum relaxed because the app owns the state vocabulary, initial accepting either form. The provider tag itself must be a non-empty string, since a mistyped one resolves to nothing on a GET, in front of a user. Two modes on one field are refused rather than settled by precedence. The engine does pick one, but the one it drops would read as declared and never run, which is the failure the graph-condition refusal already guards against. * feat(lifecycle): refuse to store a broken provider declaration Most lifecycle findings are advisory and stored as written, because refusing the whole import over a partial annotation breaks register imports. A provider declaration is the wrong shape for that policy: it fails on a GET rather than at save time, so an author sees a save succeed and a user sees a dead timeline. * feat(lifecycle): answer 502 when a provider cannot be read The widget reads 404 as no lifecycle and anything else as could not read, but an empty action list is a successful answer meaning no moves from here. So a provider failure that collapsed into that list would render as a dead timeline nobody knows is broken, which is the bug this change exists to fix. Caught before the RuntimeException branch it extends, so the existing 403 and 404 paths are untouched. * test(lifecycle): pin the provider mode, its precedence and both fail-closed routes Covers what is published, that static transitions still win, that inputs run through the same normalisation the write path uses, that an unresolvable tag and a throwing provider both refuse rather than answering with an empty list, that a blocked entry keeps its reason, and that a provider with genuinely no moves still answers an empty list as a success. Plus the validator's provider cases, the SchemaMapper save refusal, and the controller's 502 beside the untouched 403 and 404. * docs(lifecycle): specify provider mode, its precedence and its fail-closed rule The new methods carry @SPEC tags pointing at this capability, so a reader following one should find the mode described. Documents the three published requirements: what provider mode is and where it sits in the precedence order, that a provider which cannot answer must fail closed rather than return an empty list, and what the validator accepts and refuses.
…ncode (#3664) The Twig SecurityPolicy in MappingService allowlisted json_decode and not its counterpart. A stored mapping that encodes a value to JSON failed at render with a SecurityError naming a filter, which reads as a broken mapping rather than a policy gap. json_encode is a Twig core filter and a pure value transform. The policy already denies every method and property, so this widens nothing an attacker can reach. Two tests: json_encode renders, and a filter nobody allowlisted (striptags) is still refused. Without the second, the first would also pass on a sandbox that had been switched off. Closes #3663
…es not refuse the schema (#3674) openregister#3661 added a top-level allow-list to `x-openregister-archival` and refused any key outside it. The annotation is optional and an extra key declares nothing, but refusing it refuses the SCHEMA, and `ImportHandler` drops a refused schema and every object that needed it. Two apps went red within the hour on payloads they had carried for weeks: filinq 9 of 22 schemas, on `category` / `action` / `responsibleParty` pipelinq the `ticket` supertype, on a `_note` Neither failure named this annotation. Each surfaced as an HTTP 412 out of the app's own demo-data seeding, four layers away, because the rejected schema left `ticket_schema` / `correspondence` unresolved in app config. Every app clones openregister@development at CI run time, so the check reached the whole fleet the minute it merged. An unknown key at the TOP level is now reported as a warning and ignored, which is the rule R07 already applies one level up: an unknown `x-openregister-*` key is dropped and logged, never fatal. A key inside the annotation should not be stricter than the key that contains it. Everything that validates a fact the schema actually DECLARED stays fatal: a malformed ISO-8601 retention, an unknown key inside `retention` / `useRestriction` / `temporalCoverage`, and a term outside MDTO's begrippenlijst. Those sit beside a real declaration, where a typo changes a meaning rather than adding an inert key. Verified: the two new tests reproduce both apps' payloads verbatim and fail without the fix (mutation: dropped the `severity` line, 2 failures, exit 1; restored, 20 passed). Archival suite 122 passed. phpcs on the changed files reports 0 findings on lines this change touched. Inherited, not fixed here: phpcs reports 71 pre-existing `@spec`-tag warnings on untouched lines of SchemaMapper.php. Refs: openregister#3661
* feat(e2e): refuse the shared instance unless the run names it The dev box runs one Nextcloud on localhost:8080 that bind-mounts everyone's checkouts. A suite that seeds and deletes objects there is editing a colleague's environment, and both playwright configs could land on it by accident: the flow config carried its own 'http://localhost:8080' fallback. tests/e2e/shared-instance.ts holds the guard. resolveBaseUrl() now ends with assertInstancePermitted(), and playwright.flow.config.ts goes through resolveBaseUrl() instead of its own fallback, so there is one entrance. Aiming at the shared instance still works, it just has to be named: OPENREGISTER_E2E_ALLOW_SHARED_INSTANCE=http://localhost:8080. CI is exempt, where localhost:8080 is the runner's own throwaway instance. The unit test sits in tests/unit/ because jest is this repo's unit runner and its testPathIgnorePatterns skips tests/e2e/, so a copy next to the guard would never have run. * fix(e2e): stop the guard's unit test failing on CI, and satisfy prettier Two things the first commit got wrong. The guard exempts CI on purpose, and the unit runner runs on a runner, so the three refusal cases passed on a laptop and failed everywhere it mattered. Measured on this branch before the fix: CI=true npx jest gave 3 failed, 7 passed. The test now clears CI and GITHUB_ACTIONS in a beforeEach and restores them after. jest has no vi.stubEnv, so that is written by hand. Both files also failed prettier --check, which is its own CI leg and is not implied by eslint passing. Re-rendered from the template and reformatted. Verified: 10 of 10 pass with CI=true and without. The whole jest suite goes from 37 suites / 324 tests to 38 suites / 334 tests, so the file is collected rather than dark, and playwright still collects 259 tests in 53 files, the same as development.
…y delete (#3683) * fix(calendar): refuse a VEVENT with no start, and make delete actually delete Two faults in the object-anchored calendar endpoints, found while writing e2e coverage for the Appointments section of dossiq's case Work tab. DTSTART is now required (#3680). createEvent() emitted the property only when the caller supplied one, and both callers, CalendarEventsController and TaskEventsController, validated the summary alone. A VEVENT without DTSTART is not merely incomplete: its firstoccurence is NULL so it matches no time-range query and no calendar view can show it, and Sabre's ITip plugin then answers 500 to both DELETE and PUT on it. No client can remove such an event or add the missing property afterwards; only a SQL delete clears one. The guard lives in CalendarEventService::createEvent(), the single place a VEVENT is serialised, so both controllers are covered by one check, and both already turn a thrown Exception into a 400. VTODOs are unaffected. The task projector writes DUE and no DTSTART, which is correct for a task, and it does not go through this builder. DELETE now deletes (#3681). CalendarEventsController::destroy(), the endpoint behind the leaf's "Delete meeting" row action, called unlinkEvent(), which ends in updateCalendarObject: it strips the X-OPENREGISTER-* properties and the link row and never touches the meeting. It then answered {"success": true}, so cancelling a hearing left the hearing on everyone's calendar while the case confirmed the deletion, and the two row actions were the same operation. TaskEventsController::destroy() carried the identical fault. Both now call a new CalendarEventService::deleteEvent(). The controller test asserted unlinkEvent, so it had locked the defect in place. It now asserts deleteEvent and that unlinkEvent is never called. Four new service tests cover the start guard and the delete path, and each new test was confirmed to fail against the old code: reverting destroy() reddens only the controller test, and removing the start guard reddens only testCreateEventRefusesAPayloadWithNoStart. Fixes #3680 Fixes #3681 * test(calendar): the task-side destroy test asserted the defect too CI caught this: I ran the three calendar test files and treated that as the suite, so TaskEventsControllerTest went unrun. Its destroy test expected unlinkEvent, and was green precisely because the endpoint did the wrong thing. Its name said so outright: testDestroyStripsTheVeventAndRemovesTheLink. It now asserts deleteEvent is called and unlinkEvent is not, matching the twin test on CalendarEventsControllerTest, and is renamed to the promise rather than the behaviour. Full suite after the change: 20108 tests, 49282 assertions, 0 failures.
* feat(lifecycle): provider mode can take the move it offers * test(lifecycle): pin the provider write path and its three error codes * test(lifecycle): pin the no-mode annotation, not the static one, on the provider guard * docs(openspec): specify the provider write half and its three error codes * style(lifecycle): name the exception message parameter * refactor(lifecycle): split the delegating-mode dispatch out of applyTransition
appDisplayName was unset, so every CnAppRoot surface that reads it (NcContent's appName, CnRequestFeatureDialog, and — once nextcloud-vue#1145 ships — the setup wizard's title) fell back to the bare appId "openregister" instead of a proper name. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…enregister owns (#3688) * spec(openspec): working-calendar-admin, the administered working calendar (rows 8.12, Q8.20) * spec(openspec): end-date-roll-on-the-calendar, the Atw roll on a timer (row 8.11) * spec(openspec): calendar-change-recomputes-timers (row Q8.17) * spec(openspec): calendar-time-zone (row Q8.19) * spec(openspec): term-engine-diagnostic (row Q8.18) * spec(openspec): object-watchers, follow an object you do not own (row 13.18) * spec(openspec): timeline-entry-visibility, internal or public per entry (row 6.15) * spec(openspec): object-presence, who has an object open (row Q2.31) * spec(openspec): note-edit-history (row Q6.17) * spec(openspec): relation-types-with-inverses (row 2.26) * spec(openspec): identity-survives-a-move (row Q2.30) * spec(openspec): saved-view-count-alert (row 9.13) * spec(openspec): view-group-share, the openregister half of row 9.4 * spec(openspec): send-at-on-the-messaging-leaf (row 6.9) * spec(openspec): reply-threading-by-headers (row Q6.18) * spec(openspec): feature-toggle-surface (row 11.15) * spec(openspec): settings-change-audit (row Q10.13) * spec(openspec): scoped-api-tokens (row Q13.20) * spec(openspec): field-rules-by-state (row 11.25) * spec(openspec): scoped-api-tokens cites the existing OAuth2 scope rule * spec(openspec): sensitive-field-reveal-audit, the openregister half of row 5.6 * spec(openspec): property-code-list-from-concept-scheme, the openregister half of row 11.10 * spec(openspec): dedup-check-before-create, the openregister half of row 2.24 * spec(openspec): macro-flows-with-next-item (row 3.20) * spec(openspec): migrate-run-between-versions (row 3.16) * spec(openspec): external-register-view-leaf (row 5.13) * spec(openspec): competitor-parity-2026-09 umbrella, coverage confirmation, index and build order
…, for a period (#3690) * feat(people): a person on an object is a user or a contact, in a role, for a period The contact link table takes a Nextcloud user beside a CardDAV contact, a validity window and a note, and one person may hold several roles on one object. A schema declares its roles under configuration.linkRoles, and a role outside that vocabulary is refused. PersonLinkService is the write surface for both kinds, UserLinkWriter the account side, and every write is announced so an app can project it. OpenSpec change people-on-objects. * fix(people): the role vocabulary reader carries its spec tag
…ill lists (Q2.33, Q13.23) (#3692) * spec(openspec): an object archives, hides, stays read-only and restores, gap register row Q2.33 * spec(openspec): a grant on a parent reaches its descendants, gap register row Q13.23 * docs(openspec): index the two openregister gaps the regenerated register still lists
…stops 500ing (#3693) * fix(contacts): the link mapper answers find(), so unlinking a person stops 500ing ContactService::unlinkContact(), updateRole() and the controller's legacy id path all call ContactLinkMapper::find(); QBMapper has none and the class did not declare one, so every unlink of a contact answered 500 with "Call to undefined method". Measured on a running instance while verifying dossiq's people surface. The service's own tests could not see it: the mapper double declared the method with addMethods(['find']), which adds a method the real class lacks. The double now doubles only what exists, and a mapper test pins every method its callers use. * fix(deck): the deck link mapper answers find() too, and the baseline stops hiding both psalm's baseline carried UndefinedMethod:find for ContactService twice and for DeckCardService once: the same missing mapper method, recorded rather than fixed. DeckCardService::unlinkCard() has been answering 500 the same way. Both mappers now declare find(), and the entries are gone, which findUnusedBaselineEntry would have failed the build over anyway. * test(deck): the mapper double stops adding a method the class now has PHPUnit refuses addMethods() for a method the class declares, which is the whole point: the double can no longer claim a find() the real mapper lacks.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…3696) * fix(mocks): stop doubles inventing methods the real classes cannot answer A double built with addMethods() CREATES the named method on the mock, so the real class is never consulted: a suite full of them passes while the production path answers 500. Six such doubles were hiding live bugs. - ViewUpdatedEvent had no accessors at all, and ConfigurationUpdatedEvent had only getNewConfiguration()/getOldConfiguration(), while the webhook listener calls getView()/getConfiguration() like every other event. Both dispatches were a fatal error; both tests doubled the event to invent the accessor. The events now carry the accessor their siblings do and the tests build real events. - WebhookLog has a `webhook` property, not `webhookId`, so the retry job and the retry endpoint both threw out of Entity::__call. The job's test had built a TestWebhookLog subclass with an invented webhookId property to keep itself green. Callers now use getWebhook() and the crutch is gone. - SchemaCacheHandler serialized `tags` and `register` off Schema, which declares neither, so every cache write threw; two tests even asserted the reconstruct path ALWAYS fails, pinning the bug as the behaviour. Both fields are gone and the reconstruct tests assert a rebuild. - TextExtraction ObjectHandler fell back to getName() on Schema and Register, neither of which has a `name` property, for any entity without a title. - Source has no `configuration` property, column or serialized field, only a leftover @method docblock, so all three Twig token functions and the REST fetcher's identifier lookup threw. The token functions now read authConfig, the flat credential map every other reader in the app uses. tests/Unit/MockedCallSurfaceTest.php makes it mechanical: every addMethods() name in the suite must be backed by a real property on a class with __call, and a class without __call may not be given methods at all. It fails with file, line, class and method, and carries a second test so an empty scan cannot read as a pass. * chore(mocks): inline the dead identifier lookup and satisfy the comment style gate
…here a right cannot be taken away (Q13.25) (#3698) Gap register v3 (ConductionNL/market-intelligence #128) ends on two gaps with no change. This is the openregister one. Row 13.25 asks where a role's permissions come from: is the grantable set published, and can a right be removed as well as given. Batch 12 of round 4 proposed it and left dossiq unrated, saying the permission resolver needed reading end to end rather than a grep. That reading was done for register v3 against dossiq development at 9c478d810, and the row rates no. What exists here does not close it. rbac-scopes publishes six permission types and supports named roles whose actions array names CRUD verbs, but a custom verb exists only as a vote at evaluation time, so nothing can enumerate what may be granted. GET /api/scopes answers what a user may do and never where the grant came from. And every rule adds: the only way to remove a right is to stop granting it somewhere else. rbac-inherits-to-children put a deny out of scope in one sentence: no competitor in the register has one. Huly ships nine Forbid permissions beside its fifty-two, driven in batch 12. This change depends on that one and completes the rule from the other side. Four things: a published catalogue of grantable verbs, a declaration for custom verbs beside the vote that evaluates them, a deny that wins over any broader grant including an inherited one, and provenance on every answer, including the deny that removed one. Administration cannot be denied away. dossiq's half is named: the mandate matrix names its grantable set instead of untyped decisionTypes and caseTypes strings, and the role editor gains the permission half a role has never had. The catalogue unblocks that halfway through, which is why it is built first. No implementation. openspec validate permission-provenance-and-deny --strict passes.
…dark suites wave 4) (#3700) * test(controllers): resolve the six controllers from the container ControllersIntegrationTest built every controller by hand, positionally, so RegistersController gaining five dependencies errored all 101 tests before their first assertion. They now come from the app container with only IRequest and IUserSession overridden, and the override is restored in tearDown so no later file inherits the doubles. Measured in a real Nextcloud: 101 tests / 101 errors becomes 98 tests / 98 pass. Three tests were removed rather than skipped: they drove SettingsController methods that ea99a50 deleted with the SOLR index. Also fixes the PHP warning that surfaced once the tests ran: clearing the object cache read a 'entries' key getStats() never had, so it reported 0 cleared and raised two warnings per call. * test(objects): resolve ObjectsController from the container too Same rot, same fix: 59 tests errored on argument 5 of a hand-built constructor. The controller now comes from the container with IRequest, IUserSession and IGroupManager overridden, and the group-manager double answers isAdmin() from the same flag as getUserGroupIds(), because the container hands it to SaveObject as well, whose isAdmin() returns bool. Measured: 59 tests / 59 errors becomes 55 tests / 55 pass. Four tests were removed rather than skipped: they drive publish()/depublish(), which the deprecate-published-metadata spec removed in 68091c3. * test(suites): a canary that fails when a container double is left behind Two Service files override IRequest, IUserSession and IGroupManager on the app container. An override is process-global, so a missing restore would hand this file's doubles to every later file, and the failures would surface as somebody else's problem. The canary sorts after both and asks the container whether it is still answering with real services. * style(tests): pass the container helpers and the canary assertion by name The named-parameters sniff flagged the fourteen calls this branch added.
* fix(edepot): a schema-declared archival fact now reaches the SIP Proven on a live instance before changing anything. A schema declaring aggregationLevel, useRestriction and temporalCoverage, an object with no per-object override, and the real transfer path building a SIP: the object's GET carried all three facts, and the mdto.xml inside the package carried none of them. beperkingGebruik fell back to "Nader te bepalen", the term that means nobody recorded one, while the schema had recorded one. The cause is that the annotation block is derived, not stored. RenderObject computes it while rendering a response and nothing persists it, so a path that loads an object straight from the mapper, which is what the transfer does, saw no annotation at all. Everything #3661 shipped was a silent no-op on the one path it exists for. ObjectArchivalAnnotation derives it from the schema, reusing the same RetentionEvaluator the render path uses, and MdtoSourceReader passes it down as the third layer of its source chain. A stored block is honoured when the caller already has one, so a rendered object costs no schema lookup, and resolving several facts for one object costs one lookup. The two alternatives were worse. Persisting the evaluated block adds a copy that goes stale the moment a schema's annotation changes, which is the drift this area has spent the week removing. Calling the render path from the transfer path couples the two for a value that is a schema fact rather than a presentation concern, and RenderObject reaches every object response. Same instance, after the fix: aggregatieniveau Dossier, dekkingInTijd 2021-01-01 to 2021-12-31 resolved from the row's own properties, beperkingGebruik "Geen beperking" with its description, and the document still validates against MDTO-XML1.0.1. * test(edepot): guard the SIP against losing a schema-declared fact The guard is at the packaging level, not the reader's: SipPackageBuilder through MdtoXmlGenerator, MdtoSourceReader and ObjectArchivalAnnotation, with only the schema source and the temp manager stubbed. It opens the built package and reads the MDTO document out of it. That level is the point. Every unit test of the reader passed while the SIP carried none of the facts a schema declared, because the reader was handed a rendered object and the transfer path is handed a mapper-loaded one. The mutation that reproduces the shipped defect, never deriving the annotation from the schema, reddens this test and nothing else. It also asserts the packaged document still validates, and that a recorded restriction does not come out as "Nader te bepalen", the term that means nobody recorded one. Drops a dead branch in ObjectArchivalAnnotation: getSchema() returns ?string, so the comparison against 0 could never fire. * style(archival): give the memo uuid property its @var tag * style(tests): pass arguments by name on the lines the SIP fix added The named-parameters sniff flagged thirty calls this branch added or changed, plus a file docblock after declare() and an undocumented property.
…Service suites (wave 5) (#3704) * test(mappers): drop the retired published argument from the Schema and Register mapper tests SchemaMapperIntegrationTest and RegisterMapperIntegrationTest still passed the ?bool $published parameter that 12927d3 (2026-03-13, "Remove published/depublished metadata system in favor of RBAC \$now rules") deleted from find(), findAll(), findBySlug(), findMultiple() and getSchemasByRegisterId(). Every call landed a null in the bool $_rbac slot, which is 35 TypeErrors. The calls now name the flags they mean. RegisterMapper::findAll() also lost its _extend parameter, so its tests drop that argument too. Three tests changed meaning rather than shape: - testFindBySlugWithPublishedParam and testFindWithPublishedBypass tested the removed parameter itself. Deleted. - testCreateSchemasWithSameTitleThrowsUniqueConstraint expected a unique-index violation. Version1Date20260726000000 (2026-07-26) dropped schemas_org_app_slug_unique deliberately, because schemas are many-to-many with registers and "unique within a register set" cannot be a single-table index; the invariant moved to ImportHandler. Rewritten to pin what is true now: two schemas may share a slug and both stay retrievable. - testCreateFromArrayGeneratesFacetForDateFormatProperty expected terms for a date-time property. Schema::determineFacetType() has answered date_histogram for a date format since 1367ae3 (2025-09-02), and the date-named-property test three lines above asserts exactly that. The test disagreed with its own neighbour. SchemaMapper: 77 of 108 passing, 28 errors, 3 failures. Now 106 tests green. RegisterMapper: 18 of 25 passing, 7 errors. Now 25 tests green. * fix(audit): spell the retention interval per platform, as the search-trail twin already does AuditTrailMapper::setExpiryDate() built DATE_ADD(created, INTERVAL n SECOND), which is MySQL and MariaDB only. SearchTrailMapper::setExpiryDate() hit exactly this when the hourly LogCleanUpTask started calling it and was taught to spell the interval per platform; the audit-trail copy was left behind, so it would throw on every PostgreSQL install the moment anything called it. The integration test had been skipping on PostgreSQL with "DATE_ADD is not supported", which reads like a platform limit and was really a defect hiding behind a skip. The skip is gone and the test runs on both backends. Reverting the platform branch reddens it with the PostgreSQL syntax error. AuditTrailMapperIntegrationTest also called MagicMapper::insertEntity(), which went with the blob objects table; insert() is the successor. 30 of 35 passing with 4 errors and 1 skip before, 35 green after. * test(rbac): assert the fail-closed rule on the emitter a cli process can reach MagicRbacHandlerIntegrationTest carried three tests for the #1953 rule: a match rule whose dynamic variable resolves to null MUST emit the impossible predicate rather than dropping the condition. They drove searchObjectsInRegisterSchemaTable and asserted on rows, and they could not pass under PHPUnit at all. That path emits through applyRbacFilters(), which returns early for an anonymous caller in a cli process (4496c7f, 2026-06-10), so the branch under test was unreachable and every row came back. They now assert the same rule on buildRbacConditionsSql(), the UNION path emitter. It is production code, it carries no cli carve-out, and an anonymous caller reaches it from a cli process exactly as from the web. The emitted SQL is asserted directly: the impossible predicate appears for the null-resolved half, the static half is still ANDed in rather than lost, and a fully resolvable rule picks up no denial at all. Removing the fail-closed branch reddens the two denial tests and leaves the over-denial guard green. What stays uncovered from a cli process is the single-table QueryBuilder emitter. Narrowing that cli trust is a product decision, not a test repair: SystemOperationContext exists for the scoping, but cron.php does not define OC_CONSOLE, so a naive narrowing would blind every background job. The file says so where the tests are, and the PR raises it. 39 of 42 passing with 3 failures before. 42 green after. * test(organisations): seed the offset test instead of skipping on an empty instance testFindAllRespectsOffset skipped when the instance held fewer than two organisations, so it only ever ran on a database someone else had dirtied. It creates the two rows it needs now, the way the limit test beside it already does. The Database suite has no skips left. * test(services): repair five fixtures that fell behind their production signatures None of these five changed behaviour, they changed shape, and each test was constructing or seeding against a version of the code that no longer exists. - AggregationRunner (15 errors): the fixture created a register and a schema and never linked them. Naming a register is a boundary now, so RegisterScopedSchemaResolver refuses to resolve a slug against a register that carries no schemas rather than guessing at a same-slug schema elsewhere. The fixture links them, which is what its own error message asked for. - RelationHandlerCircuitBreaker (7 errors): RelationHandler gained a registerMapper dependency. - RegisterI18nPhase2 (2 errors): LanguageMiddleware gained a translationMapper and a logger. Its request stub also pinned getParam to _translations, and the middleware asks for _lang first, so the stub answers "not given" to any name. - MailSmartPicker (1 error): ObjectReferenceProvider takes an ObjectPreviewFormatter now, where the test still passed the five collaborators the formatter was extracted from. AggregationRunner 0 of 15, now 15 green. RelationHandler 0 of 7, now 7 green. RegisterI18nPhase2 10 of 12, now 12 green. MailSmartPicker 5 of 6, now 6 green. * test(object-service): retire the publish tests, scope the two that name a register Three tests drove ObjectService::publishObjects(), depublishObjects() and publishObjectsBySchema(). Object-level published metadata was retired in 12927d3 (2026-03-13) in favour of RBAC $now rules and those methods went with the routes, so the tests are deleted and the file says where visibility over time is covered instead (RbacOperatorMatchingIntegrationTest). Two more were casualties of 5e7471b (2026-08-22, "make {register} a hard boundary on every path that names one"). ObjectService is a singleton, so testSetSchemaWithSlug resolved its slug against whichever register the previous test had left set, and testSetSchemaWithNonexistentIdThrows expected the old unscoped ValidationException where the boundary now raises the more specific SchemaNotInRegisterException. Both name their register first and follow the contract. 59 of 64 passing with 4 errors and 1 failure before. 61 tests green after. * test(text-extraction): follow the Word reader into its own handler, and the unknown-method contract TextExtractionWordExtractionTest reflected into resolveWordReader() and walkWordElements() on TextExtractionService. Both moved to TextExtraction\WordExtractor with the extract-god-class-services split, and extractWord() is a one-line delegation to it now. The tests reflect into the handler that owns the code; the source-reading guard on the failure-path log reads WordExtractor.php for the same reason. TextExtractionServiceIntegrationTest expected detectEntities() to throw "Unknown detection method". 35d3d46 (2026-06-18) put resolveMethod() in front of the match: an unrecognised name resolves to the effective backend method and falls back to regex with a warning, so the throw arm is unreachable for a caller-supplied name. That is deliberate, a typo in a request parameter must not 500, and the test pins the resolution instead. Word extraction 4 of 8 with 3 errors and 1 failure, now 8 green. TextExtractionService 96 of 97, now 97 green. * fix(dashboard): make recalculate sizes actually walk the magic tables DashboardService::recalculateSizes() called MagicMapper::findAll() with ["register" => id, "schema" => id] as FILTERS. A magic table is one table per register and schema pair, so findAll() requires the register and schema ENTITIES and answers an empty array with a warning when it does not get them. The method therefore processed zero objects on every install and reported success while doing nothing, which is what the restored test caught. It now walks the register and schema pairs, narrowed by whichever id the caller named, and re-saves each object through the pair its table belongs to. An unresolvable pair counts as one failure and is logged instead of taking the whole run down. Walking no pairs reddens the test. Three more tests in ServicesIntegrationTest were following moved or changed contracts rather than broken behaviour: - authorizeJwt, authorizeOAuth and authorizeApiKey became protected in 3fd738a (2026-05-28), the orphan-auth gate pass, and no public route drives them. The three tests reach them by reflection, because "a malformed credential MUST raise AuthenticationException" is worth keeping pinned. - McpToolsService::callTool() throws InvalidArgumentException for a tool nobody owns and McpServerController maps that to a JSON-RPC error; the test expected an isError envelope that predates the mapping. - listTools() pinned a hard count of three. Tools are contributed by providers and there are seven now, so the test asserts the three canonical tools and the shape of every entry instead of a number someone has to bump. 101 of 108 passing with 1 error and 6 failures before. 108 green after. * test(permissions): follow the fail-closed rule and the eight-argument signature Three failures in ObjectHandlersIntegrationTest, none of them a broken behaviour. - testHasGroupPermissionActionNotInAuth asserted that an action a non-empty authorization block does not list is GRANTED. PermissionHandler denies it: the empty-block default is the only one that still grants, and admin and owner have returned before that point. Asserting the old default-open answer would be asserting a hole, so the test follows the rule. - testHasGroupPermissionComplexEntry passed nine arguments where hasGroupPermission() takes eight. The active organisation stopped being an argument: ConditionMatcher resolves $organisation through OrganisationService. The test logs in, reads the organisation the matcher will see, and passes the object organisation it should match, restoring the half that proves a dynamic match can grant. - testBuildSearchQueryPublishedFilter expected _published=true to be normalised to a boolean. Version1Date20260313130000 (2026-03-13) dropped the _published column from every magic table with the published metadata system and no code reads the filter, so the test is deleted and the file says where visibility over time lives now. 72 of 75 passing before. 74 green after. * fix(settings): count the object-cache entries that exist, and stop pinning a fixed bug Two tests in SettingsHandlersIntegrationTest asserted that clearCache() throws a TypeError, and said so in their names: the distributed cache reports "all" rather than a number for cleared, and summing that into an int used to be fatal. The sum guards with is_int() now, so the tests were failing because the bug they pinned had been fixed. They asserted the repair instead: the call returns its envelope, totalCleared is an int, and no error is collected. Removing the guard reddens both. Repairing them surfaced the next layer. clearObjectCache() computed its cleared count as $beforeStats["entries"] - $afterStats["entries"], and CacheHandler::getStats() has no entries key: it reports cache_size, query_cache_size and name_cache_size. Every call raised two "Undefined array key" warnings and reported 0 entries cleared, so the admin action always claimed it had cleared nothing. It counts the three in-memory caches clearCache() actually empties. 75 of 77 passing with 2 failures before. 77 green after, and the run is warning free. * test(mcp,schemas): follow the objects guard to its provider, and build the controller by signature McpToolScopingIntegrationTest reflected into a private McpToolsService::executeObjects(). The objects tool is contributed by Mcp\BuiltIn\ObjectsToolProvider now and its invokeTool() raises the same "Both register and schema IDs are required" guard, one level below callTool()s try/catch exactly as the spec scenario asks. The provider is public, so the test calls it directly instead of reflecting. RuntimeSchemaReloadTest hand-listed twelve constructor arguments for SchemasController, which takes seventeen now, so all four tests errored. It builds the controller from the container BY SIGNATURE and splices in its own stubbed request: resolving the controller straight from the container would inject the real IRequest, and these tests drive it through a stub. A dependency added tomorrow is picked up without editing the test. Three more things that file was hiding: - POST and PUT on /api/schemas are gated on manage permission, so with no session the controller answered 403 and the cache-invalidation contract under test was never reached. The fixture logs in as admin and tearDown restores the session. - Its request stub answered null to every getParam(), including the ones the controller passes a default for. A real IRequest returns the default, so show() got null where it expected an array and died in in_array(). The stub honours the default now. - The delete-guard test passed a Register positionally into saveObject()s ?array $extend and reported the resulting TypeError as "object persistence not available", skipping itself. Named arguments, and the test runs. McpToolScoping 6 of 7, now 7 green. RuntimeSchemaReload 0 of 4 with 4 errors, now 4 green with no skips. * test(graphql): turn the admin bypass off so the reference-existence tests can see it The two rejection tests wrote as admin, and reference-existence validation has an operator escape hatch: an admin skips it entirely unless reference_validation_admin_bypass is off (SaveObject, default true). So a dangling reference was accepted and the tests reported exactly that, "Update silently accepted". The file turns the flag off for its own run and restores the previous value in tearDown, which is the same thing an operator does to enforce validation for everybody. Skipping the validation call reddens both tests. 1 of 3 passing before. 3 green after. * test(fixtures): stamp the real organisation, and put the one a test pins back Three tests failed only inside the full Service run, all on the same tenant scoping. ServicesIntegrationTest::createTestObject() stamped the literal string "default" in the organisation column, which holds an organisation UUID and so matched nothing. Every read that applies the tenant filter skipped those rows, and two tests counted zero objects they had just created. The production SaveObject path stamps the callers organisation; the fixture does the same now. It only showed up once the account had an active organisation, and that is the second half: AggregationRunnerIntegrationTest calls setActiveOrganisation(), which writes a PERSISTENT user setting, not session state. It changed the admin account for every later test, every later file and every later run. The file captures the previous value and restores it, deleting the setting when there was none. Note for whoever meets this next: OrganisationService::getActiveOrganisation() also assigns and persists one when the account has none ("Auto-set active organisation to oldest"), by design, so an instance that has ever read it has one. That is the state a fixture has to be correct under, which is why the stamp matters more than the restore. Run together, AggregationRunner then RbacScopeDiscovery then Services: 2 failures before, 128 tests green after. * test(scopes): build the fixture as the caller the test acts as testAdminBypassReturnsAllFiveActions failed inside the full Service run with an empty scope list. SchemaMapper::insert() stamps the active sessions organisation on the row, and ScopesController::index() resolves schemas with multitenancy ON, deliberately. The fixture was built under whatever user the previous file had left in the process-global session, so the two schemas landed in another tenant: the tuples existed and the admin could not see them. setUp establishes the admin session before creating the fixture; tearDown already restored the previous user. * test(unit): follow recalculate sizes to the pair walk, and count the cache sizes that exist The dashboard fix on this branch walks the register and schema pairs instead of passing ids to MagicMapper::findAll() as filters, and the cache fix counts cache_size, query_cache_size and name_cache_size instead of an entries key getStats() never had. Twelve unit tests still pinned the old shapes. The recalculate tests now assert which pair tables were read for each filter, that an unresolvable pair counts as one failure, and that a pair listing that throws is reported. Removing the register filter reddens the register filter test on its assertion.
… changes (#3707) * feat(watchers): the watchers table and the subscription service Task 1.1 and 1.2 of object-watchers. A watcher is a per-user, per-object subscription stored outside the object, so following writes no audit entry and no version on the object it follows. The unique index on (user_id, object_uuid) makes subscribing idempotent. WatcherService is the one place the three permission postures live: read to watch (already proven by resolving the object), update to see the list, and manage (owner or administrator, per ADR-010 Rule 4) to change somebody else's subscription. A watcher may always remove themselves. * feat(watchers): the watch routes, the @self markers and the watching lens Task 2.1 and 2.2. Five routes under the object, in their own controller rather than in the 5,000-line ObjectsController: every one resolves the object through ObjectService first, so an object the caller cannot read is a 404 and no watcher row can be written without read. @self.watching is set for any signed-in reader and omitted for an anonymous one; @self.watcherCount only for a reader with update. Both come from WatcherService's per-request memo, so a page of objects costs one query for the marker and one for the counts rather than one per row. _watching=true narrows _ids rather than joining the filter grammar, and an empty subscription set falls back to a literal no object can carry, so the lens can never widen to the whole register. * feat(watchers): the watchers recipient block, resolved with a read check Task 3.1. A rule declares {"watchers": true} on the schema, per ADR-031, and the dispatcher resolves it: no app dispatches a watcher notification itself. The validator refuses anything but boolean true, so a rule that quietly addresses nobody is caught when the schema is saved rather than at midnight. The read check runs at dispatch, not at subscribe time, because group membership changes after people subscribe. A watcher who has lost read gets nothing and the subscription is removed, so the audience stops carrying people who are not in it. An empty readable-user list means the schema's read rule is open or could not be resolved, which is not evidence anyone lost access, so nothing is dropped there. Deleting an object prunes its watchers, or a re-created uuid would inherit followers who never chose to follow it. * test(watchers): unit, Newman and Playwright coverage for the subscription Task 4.1 and 4.2. Four unit suites (the service, the lens, the resolver, the validator), a self-cleaning Newman collection for the routes, and a Playwright spec admitted to the CI floor by tests/e2e/ci/playwright.config.ts. The e2e proves the capability is reachable: routes registered, auth attributes admitting a non-admin, markers on an ordinary read, the lens narrowing a list, and an unreadable object answering 404 rather than 403. It deliberately does not assert notification DELIVERY: that runs through the queue and a background job, so the assertion would be a timing race rather than coverage. The resolver's read check and list healing are asserted in NotificationRecipientResolverWatchersTest, which the spec's exclusions now name by test rather than saying coverage lives somewhere unnamed. * style(watchers): named parameters on every new call site phpcs CustomSniffs.Functions.NamedParameters on the lines this change added: 88 findings, all of them an argument passed positionally. Applied with a token-aware rewriter rather than a regex so a comma inside a nested call or an array literal could not be read as an argument separator, then verified by re-running php -l, the four suites (32 tests, 53 assertions, green) and phpcs on the same files (zero findings). * style(watchers): wrap the watcher route entries under the line-length rule * fix(watchers): bump the app version, and clear the phpmd findings this change added Three things check:strict caught that the diff check could not. The migration would never have run. Nextcloud executes an app's migrations only when <version> is greater than the installed_version it recorded, so a new migration with an unmoved version makes `occ upgrade` answer 'No upgrade required', exit 0, and create no table. Nothing would be logged and nothing would fail; the first symptom would have been every watcher endpoint answering 500 on an instance that already had the app. The entity's two new getters had no caller, and pushed ObjectEntity one over PHPMD's public-member ceiling. The render layer reads the properties directly, so they are gone rather than suppressed. The prune listener now removes subscriptions through WatcherService rather than reaching past it to the mapper, so 'what happens to a subscription when its object goes' has one definition. The lens's lookup moved into its own method, which is what its complexity was actually measuring. The two remaining PHPMD findings on WatcherService carry reasons rather than a baseline entry. Verified: phpmd both rulesets exit 0, the four new suites 32 tests / 53 assertions green, diff check GREEN (php -l, phpcs, phpstan, phpunit, eslint all 0 new).
…me (#3709) * spec(discovery): the rules engine gets an operator surface Cluster 19 of the round 4 discovery build plan, 17 candidates, decision D3 option 2 with the JSON AST for computed values. The three rule changes in flight carry the vocabulary; this carries the inventory, the run log, the ceiling, the dry run and the replay. * spec(discovery): object dates publish a calendar feed Cluster 9 of the round 4 discovery build plan, candidates C-deadlines-22 and C-communication-52, decisions D11 (publish the feed first) and D5. The strongest single capability in the sweep: thirteen passers, twelve driven. * spec(discovery): a bulk action is a job with a preview, progress and skips Cluster 52 of the round 4 discovery build plan, eight candidates, five of them must, no decision blocking. Kept in wave 1 under D6: the promotion bar is relevance-led, so three driven passers is not a reason to drop it. * spec(discovery): an object carries a read state per user Cluster 62 of the round 4 discovery build plan, six candidates, seven driven passers, dossiq no on all six. The bell empties because the work was done, which is dimpact-zac's idea and nobody else's in the corpus. * spec(discovery): code lists get a hierarchy, a validity window and fields of their own Cluster 6 of the round 4 discovery build plan, 20 candidates, three of them must, ledger rows 11.34, 11.40, 11.44 and 11.45. Builds on property-code-list-from-concept-scheme, which stays as written. * spec(discovery): the delete window is stated and the destruction is recorded Cluster 39 of the round 4 discovery build plan, five candidates, four of them must. Decision D10 as taken: no new recycle state, the existing soft delete carries this, beside object-archive-state, and dossiq's delete guard stays as it is. * spec(discovery): archiving is a process with sign-off, and it lives here Cluster 43 of the round 4 discovery build plan, 17 candidates, twelve of them must. Decision D7 as taken: option 1, and the owner moved from filinq to openregister, because the evidence has to sit where the objects are. filinq keeps the formats, dossiq keeps the resultaattype. * spec(discovery): publish the property vocabulary the validator already accepts Cluster CT-1 of the round 4 case-type depth study, rows A1, A2, A3, A5, A9, A11, A12, A13, B7 and B10. The openregister half: twenty types and six constraint keys are validated and nothing publishes them, so every leaf app writes a shorter list. Also records that x-openregister-extends-form uses the platform namespace and the platform does not define it. * spec(discovery): the JSON AST becomes the administered computed-value engine Cluster CT-3 of the round 4 case-type depth study, row B2, ledger row 3.17. Decision D3's second half: the AST is auditable, diffable and safe by construction, and Twig stays for schemas authored in code. * spec(discovery): roles, provenance, deny and access inside the query Clusters 11 and 54 of the round 4 discovery build plan, 24 candidates, fifteen of them must, six matrix holes. Decision D22 option 1 names this change, so it is extended rather than duplicated: the filter compiles into the query, the record says what you may do, provenance reads both ways, and a grant may be derived, scoped and given an end. * spec(discovery): field rules gain a condition, states gain entry and exit, a choice needs a source Clusters CT-2 and CT-4 of the round 4 case-type depth study. Both existing changes are named by the build plan as the vehicle, so they are extended: a rule may read any declared property and may make a field required (the two gaps D3 names), a state carries entry and exit conditions, and a choice property with no source of values is refused at schema save. * spec(discovery): index wave 1 on the competitor parity umbrella Nine new changes and three extensions, with cluster, candidates, size, decision and dossiq consumer per row, the four decisions that differ from the recommendations, the build order, and the extends-form finding.
…t is on (#3711) * feat(timeline): every timeline entry says which side of the counter it is on A note now carries a visibility of internal or public, stored in the comment's own metadata, internal by default and internal when absent, so a note written before the flag existed can never reach a citizen. Setting or changing it takes update on the object, and a change is written to the object's audit trail naming the note and both values. Every merged feed row carries the flag too. NC Activity rows are always internal. A caller without update on the object is served the public view whatever it asks for, on the notes endpoint, the activity feed and the generic dispatch of the two timeline leaves, so dropping the query parameter is not a way around it. * test(timeline): the default, the guard, the audit entry and the enforced filter * test(timeline): the e2e spec for the four visibility scenarios, and tick the tasks * style(tests): the coding standard on the lines this change adds * refactor(notes): update() hands the decision to a helper, so it stays inside the complexity the standard allows
…3714) * feat(calendar): validate every working-calendar write, and refuse a delete open timers depend on WorkingCalendar::fromArray() was already the validating constructor, but it only ran at ARM time. A calendar written through the objects API, the admin page or a configuration import was accepted on save and refused days later on an unrelated timer. Two listeners on the object lifecycle close that. The validation listener runs the same validator on ObjectCreatingEvent and ObjectUpdatingEvent for a working-calendar object, so all three doors refuse identically with the validator's own message (422). The delete guard refuses a calendar an armed or suspended timer names, with the count and up to ten uuids; the delete handler now honours a status a hook names, so that refusal reads 409 rather than a 422 telling the caller to fix a body that has nothing wrong with it. The descriptor grows an authorization block on the register and both schemas (read authenticated, create/update/delete administrator) and the repair step's version moves to 1.1.0, without which the block would land on fresh installs only. * feat(calendar): the working calendar becomes an administered thing The engine half has existed since flow-business-timers: a calendar is an object in the flow-timers register and WorkingCalendarService resolves it. Nothing administered it. The gap register's note on row 8.12 counted five private implementations of a working day in the fleet, four different definitions of one, and a grep for a holiday settings screen that returned nothing. This adds the surface, without adding a second source of truth. A working calendars section on the OpenRegister admin settings page lists every calendar and edits the working week, the hours in a day, the rules, the closure days and the owning organisation. It writes through the objects API like any other client and keeps no store of its own, so the validation the form meets is the validation a script meets. One endpoint of its own, and it is a read: POST /api/flow-timers/calendars/ preview runs the rules of an UNSAVED definition over a year and returns the dates, so a rule can be read before it is committed to. It stores nothing. The form carries the notice that armed deadlines keep the dates they were given. Recomputing them belongs to calendar-change-recomputes-timers. Tests. The arithmetic is pinned across the 2026/2027 year boundary, which is the case a tabulated calendar gets wrong in the most expensive way: a term that starts in December has to consult two years' holidays in one walk, and a table that ran out reports a January full of working days with no error anywhere. Every 2027 moving feast is pinned by a term that has to step over it, because the competitor corpus found five fleet engines disagreeing on exactly Goede Vrijdag, Tweede Paasdag, Hemelvaartsdag and Tweede Pinksterdag and agreeing on the fixed dates. Koningsdag 2026 asserts the Sunday rule does NOT fire on a Monday, which a 2025 fixture cannot catch. * feat(calendar): validate every working-calendar write, and refuse a delete open timers depend on WorkingCalendar::fromArray() was already the validating constructor, but it only ran at ARM time. A calendar written through the objects API, the admin page or a configuration import was accepted on save and refused days later on an unrelated timer. Two listeners on the object lifecycle close that. The validation listener runs the same validator on ObjectCreatingEvent and ObjectUpdatingEvent for a working-calendar object, so all three doors refuse identically with the validator's own message (422). The delete guard refuses a calendar an armed or suspended timer names, with the count and up to ten uuids; the delete handler now honours a status a hook names, so that refusal reads 409 rather than a 422 telling the caller to fix a body that has nothing wrong with it. The descriptor grows an authorization block on the register and both schemas (read authenticated, create/update/delete administrator) and the repair step's version moves to 1.1.0, without which the block would land on fresh installs only. * feat(calendar): the working calendar becomes an administered thing The engine half has existed since flow-business-timers: a calendar is an object in the flow-timers register and WorkingCalendarService resolves it. Nothing administered it. The gap register's note on row 8.12 counted five private implementations of a working day in the fleet, four different definitions of one, and a grep for a holiday settings screen that returned nothing. This adds the surface, without adding a second source of truth. A working calendars section on the OpenRegister admin settings page lists every calendar and edits the working week, the hours in a day, the rules, the closure days and the owning organisation. It writes through the objects API like any other client and keeps no store of its own, so the validation the form meets is the validation a script meets. One endpoint of its own, and it is a read: POST /api/flow-timers/calendars/ preview runs the rules of an UNSAVED definition over a year and returns the dates, so a rule can be read before it is committed to. It stores nothing. The form carries the notice that armed deadlines keep the dates they were given. Recomputing them belongs to calendar-change-recomputes-timers. Tests. The arithmetic is pinned across the 2026/2027 year boundary, which is the case a tabulated calendar gets wrong in the most expensive way: a term that starts in December has to consult two years' holidays in one walk, and a table that ran out reports a January full of working days with no error anywhere. Every 2027 moving feast is pinned by a term that has to step over it, because the competitor corpus found five fleet engines disagreeing on exactly Goede Vrijdag, Tweede Paasdag, Hemelvaartsdag and Tweede Pinksterdag and agreeing on the fixed dates. Koningsdag 2026 asserts the Sunday rule does NOT fire on a Monday, which a 2025 fixture cannot catch. * chore(release): 2.1.20-unstable.20260914110204 The seed repair step's REGISTER_VERSION moved to 1.1.0, so the app version has to move too. Without it occ upgrade answers "No upgrade required." and the new working-calendar descriptor never reaches an instance that already ran the step. * fix(calendar): the two findings check:strict caught on this branch phpmd read an else expression in the validation listener. The chain becomes three guard clauses, which is the shape the delete listener already uses and reads the same way. FlowTimerRepairStepsTest pinned the descriptor version at 1.0.0. The seed step now passes 1.1.0, so the expectation moves with it. * fix(calendar): clear the findings the gates and phpcs read on this diff psalm lost the event type once the else went, because setErrors() is declared on each write event rather than on a shared parent. The body moves into validate(), which takes the union explicitly, and handle() stays two guard clauses. The watcher body carries its own @SPEC exclude: gate 16 reads handler() as a method of its own, not as part of the watcher it sits in. The importFromApp expectation is spread over several lines and names the callback parameter, so the line this change had to touch no longer breaks the line-length and named-parameter sniffs. * chore(release): 2.1.21-unstable.20260914121746 The third merge of development brought its own stamp, which left this branch level with development and moved nothing. The seed step's REGISTER_VERSION is at 1.1.0, so the app version has to move or occ upgrade never reads the step. This stamp sits above every open release branch, so whichever merges first the version still goes forward.
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.