feat(approval): consolidate the approval engine onto ordered task sequences (flow-approval-consolidation) - #3302
Merged
Conversation
The flow-task-entity storage layer (openspec/changes/flow-task-entity): openregister_tasks with the resolved 23-shape union (no overdue column, no app-named column, performer_type as an open string vocabulary so the coming external type is an append, definition_version shipped now), openregister_task_candidates as the pooled-inbox index half of the candidate pool, openregister_task_relations as the typed anchor table, and openregister_task_audit with an append-only mapper whose update and delete refuse. claim() is a conditional update so the database decides the race, and the inbox page and total share one predicate builder. Ticks tasks 1.1 and 1.2.
Normalisation at the boundary: TaskState publishes the one legacy-status mapping onto the six CMMN states (collapsed distinctions survive on outcome, an unmapped value is refused naming itself, and a migration may declare its source vocabulary so procest's status 'open' fails loudly); TaskPriority lands all four fleet scales on low|normal|high|urgent and refuses pipelinq's 'normaal'; TaskTemporalProjection is the ONE overdue derivation, computed and never stored. TaskAuthorizationService decides every verb fail-closed BEFORE mutation: no group backend, an unresolvable role or an unknown performer type each DENY rather than skip, and no nullable service-unavailable return exists. TaskPerformerResolver carries the five routing strategies; a strategy that finds nobody assigns nobody. Ticks tasks 2.1, 2.2, 3.1 and 3.2.
…inbox TaskService: create/offer/claim/unclaim/assign/reassign/delegate/ resolve/complete/cancel, each authorized fail-closed before any mutation, each audited in the SAME transaction (an audit-write failure unwinds the completion), denials audited too. One write path maintains the candidate JSON and the candidate index rows together; state and is_terminal move in one statement; claim is the mapper's conditional update so the race's loser gets a conflict; a rejecting outcome without a comment is refused before anything moves; the template snapshot is frozen at creation. Cancellation propagation (design D-8): FlowRunMapper::update() - the one choke point every terminal run write passes - dispatches FlowRunTerminalEvent, and TaskRunTerminalListener terminates that run's open tasks with the reason recorded. Idempotent, and a task with run_uuid null is structurally out of reach. TaskInboxService answers assigned/pooled/watched/by-object in the datastore over one shared predicate set, with the total from the same predicates, subject context batched per page, a display title synthesized on read for titleless tasks (never persisted), and the derived temporal projection attached to rows only. Ticks tasks 4.1-4.5, 5.1, 6.1 and 6.2.
TaskController + /api/flow-tasks routes for the inbox, the detail and audit reads (both visibility-checked) and every lifecycle verb. The family is named for the flow-tasks CAPABILITY, not a flow requirement - /api/tasks itself already belongs to the CalDAV VTODO leaf. Every method declares its auth posture attribute AND delegates its actual authorization to TaskAuthorizationService inside the service; refusals translate uniformly (400 validation, 403 denial, 409 conflict naming the current state, 404 absence). SeedTaskFixtures installs the five seed groups from design.md (pooled municipal check with no run, delegated approval with enforcing expiry on a run, agent task with a typed checklist, one completed 'approved' and one terminated by propagation, plus audit fixtures including one DENIED entry), idempotent on uuid - and is REGISTERED in appinfo/info.xml in this same commit, because an unregistered repair step is a known fleet defect. Ticks tasks 7.1 and 8.1.
Table-driven legacy-status and priority tests including the live fleet defects: procest's status 'open' against a source vocabulary that does not define it is refused naming the value, and pipelinq's 'normaal' is refused on every scale. Authorization: a stranger who knows the uuid is denied on every verb, an unresolvable role denies naming the role, an absent group backend denies, an unknown performer type denies, watchers read and never act. Concurrency and transactionality: the two-claim race yields one assignee and one conflict, a rejection without a comment moves nothing, and an injected audit-write failure rolls the completion back. Derivation: overdue flips with an injected clock while the stored row stays byte-identical. Inbox: the page and the total run over the SAME criteria object (25 of 120 reported honestly), a titleless task presents a synthesized display title while its stored title stays null, and a runless task completes identically to a runful one. Mutation-checked: disabling assertMay fails 7 tests; removing the transaction rollback fails 2; both restored byte-identical (sha256-verified). Ticks tasks 9.1, 9.2 and 9.3.
'a stranger is refused on the task detail route': a provisioned second account gets 403 on the detail read AND on complete, and the task provably did not move - the positive control for the hole where knowing a uuid was the whole check. 'the inbox route returns tasks with subject context': one request lists the assignee's work with a display title, the subject slot, the derived overdue flag and a datastore total, and when the dev seed offers a live object the anchored subject context rides along. Both specs clean up after themselves and skip loudly when the instance cannot provision a stranger. Ticks task 9.4.
The repo's PHPCS standard requires named arguments on calls into OCA code and forbids ternaries. Exception constructors now name message:, the transactional/stringOrNull/arrayOrNull/respondWith calls name their parameter, the nine isset-ternaries collapse into one intOrNull helper, and the two remaining conditionals become explicit if blocks. No behaviour change: the 93 task tests are unchanged and green.
…ed down TaskBuilder now owns intake: boundary data in, a validated unsaved Task out, with every published vocabulary applied there once. That returns TaskService to lifecycle only and under the complexity budget. The authorization switch becomes a verb-to-rule table with static dispatch; the inbox predicate builder splits into scope, visibility and filters; the migration's indexes and the seed fixtures get methods of their own; sort and direction travel as one parameter (-dueAt) so the inbox route keeps a single-digit parameter list. The few remaining suppressions each carry their justification inline. Behaviour unchanged: the 93 task tests pass untouched except for injecting the builder.
…tant The mapper overrides now guard their entity type with a throwing instanceof check, which is what lets the generic QBMapper<T> contract type-check (and turns a wrong entity into a named error instead of a silent write). candidateMembershipPredicate declares the IQueryFunction it returns. The migration uses Types::DATETIME_MUTABLE, the constant the current DBAL stubs actually define.
PHPMD MissingImport on the type guards added in the PHPStan pass. No behaviour change.
…eature/flow-user-task-node
…eature/flow-business-timers # Conflicts: # lib/Db/FlowRunMapper.php
The node creates ONE task through TaskService on its first firing with items, stores the uuid in its own resume slot, and suspends with a heartbeat that is never null (findAbandonedSignals reaps null and would fail slow approvals at 14 days). Later firings read the TASK for terminality, never context.signal, so two user-task nodes in one flow keep independent answers. The outcome bag lands on every item's json under outcomeKey (default task), with decided/rejected separating a person's decision from a task that merely ended; failOnReject is opt-in. The advance budget (ADR-098 D9) is 0 | N | "all", null refused by name (FlowAdvanceBudget). TaskService announces every committed terminal transition (TaskTerminalEvent, after the transaction); the listener wakes the run with an empty signal and, per the node's stored budget, continues it through FlowRunAdvancer with a per-walk ceiling the engine consumes (CONTEXT_ADVANCE_BUDGET). A spent budget parks the run as due. Branch mootness: the engine reports pruned exits to FlowTaskMootness, which terminates the task of a user-task node standing on a cleared place. Run terminality stays with TaskRunTerminalListener from #3258. Ticks tasks 1.1-1.4, 2.1-2.3, 3.1-3.2, 4.1-4.3, 5.1-5.2.
… works on PostgreSQL Two blocking defects from the independent review of #3258: - offer bypassed per-task authorization and rewrote the assignee. Any authenticated user could POST offer {"routingFallback": "mallory"} on an assigned active task, become its assignee, and then complete it. offer now needs the requester (or an administrator), refuses a task that already has an assignee (unclaim or reassign instead), and is in the stranger-denied test's verb list, whose omission is how the suite stayed green. - The watchers LIKE ran against a json column, which PostgreSQL refuses (operator does not exist: json ~~ unknown): every non-admin inbox request 500ed there. The column is now cast per platform (AS TEXT on PostgreSQL, AS CHAR elsewhere), every identifier in the mapper's raw SQL goes through the platform's own quoter (no backticks), and TaskMapperTest pins the cast on a PostgreSQL platform mock. The should-fix items: authorization runs before the terminality check and a task the caller may not see answers 404 on every route, verbs included (no existence or state oracle); over HTTP the requester is pinned to the actor and a terminal creation state is refused, with import() as the trusted path for migrations and the user-task node; every state-changing write is a conditional update (WHERE is_terminal = false) so a second completion conflicts instead of overwriting; delegate refuses an empty delegate and keeps naming the original performer on re-delegation; an unexpected failure is a generic 500 whose detail goes to the log; propagation continues past a failing task; role pools are matched by the inbox EXISTS; the seed fixtures run only when app config openregister/seed_demo_tasks is true; the CSRF posture is recorded in the controller docblock; the e2e stranger scenario fails loudly instead of skipping and now covers the non-admin inbox and a watcher. Class-level @SPEC tags on every new class. TaskControllerTest is the contract test for all fourteen /api/flow-tasks endpoints (gate-25 PASS). Checks on this tree: PHPCS 0, PHPMD 0 (touched files), PHPStan 0, Psalm 0, 120 task tests green.
…ier config CI's Frontend Check (format) named tests/e2e/task-inbox.spec.ts. Formatted with @nextcloud/prettier-config's options (tabs, no semicolons, single quotes, trailing commas, print width 85, operators leading).
…ask-entity # Conflicts: # lib/Db/FlowRunMapper.php
…builder CI's pgsql cell did not fail in PHPUnit (17860 tests, 0 failures, and app:enable succeeded); it failed the coverage guard: the changed files measured 3.85% (69/1791 statements) against a 7.32% base, because the suite runs with strict coverage metadata and collaborators exercised through TaskServiceTest were never credited. This adds the tests that were genuinely missing: the four entities' typing, hydration and serialisation (no overdue anywhere in the stored row); the terminal-run listener and event; the seed step off by default, seeding the five groups when on, idempotent on uuid, and continuing past a failing fixture; and TaskBuilder credited where TaskServiceTest exercises it. 133 task tests green.
…istory Three additive tables: openregister_flow_timers holds a budget and a suspension ledger rather than a target instant (design D-2), with fire_at and next_rung_at as indexed derivations for the two range scans (D-8) and no overdue column of any kind; openregister_flow_timer_fires is unique on (timer_uuid, rung_key) so the INSERT is the rung claim (D-7); and openregister_flow_timer_events is append-only evidence with no update or delete path. The fire and event mappers refuse update() and delete(). TaskMapper::update() now announces terminality through TaskTerminalEvent from the one choke point every terminal task write passes, inside the verb's transaction, so timers can be cancelled in the same operation (D-9). Tasks 1.1 and 1.2.
…eature/flow-business-timers # Conflicts: # lib/Db/FlowRunMapper.php
…ation Node: one task per node per run across a heartbeat wake, empty firing creates nothing and does not suspend, a claim is not a completion, the signal slot cannot answer for a performer, askedAt is not restamped, two nodes need two answers, outcome placement under json.<outcomeKey>, rejection as a branch with failOnReject opt-in, a terminated task is not a rejection, the config-validation table (advance 0/3/all accepted; null, '', -1, 'unlimited' refused naming the value; no performer refused). Budget: FlowAdvanceBudget shapes; the engine parks at a spent ceiling and the worker walk runs the remainder; an oversight veto still applies in-request; pruned exits reach the mootness collaborator. Bridge: the provenance stamp, offer-on-strategy, 0/N/all continuation, and a failed continuation leaving the run due. Propagation: a run stopped with two tasks terminates both once across two observations; a losing branch takes its task; a run-less task is never touched. TaskService announces terminality once, after commit, and a listener failure cannot undo it. Mutation-checked: dropping the run-less guard, the performer guard or the signal-ignore each turns the suite red. Splits the node's config intake into UserTaskConfig and groups the form so the node stays under the PHPMD complexity and method-length ceilings. Ticks tasks 6.1-6.4.
…w-user-task-node The task entity's review fixes change the surface this node consumes: create() is now the HTTP intake (requester pinned to the actor, terminal creation states refused), so the bridge uses the trusted import() path and stamps the run's owner as requester itself; offer is the requester's verb and refuses an assigned task, both of which the bridge satisfies by construction (requester is the actor; an assigned task is created active and never offered); state changes go through updateIfOpen(), which the hand-built test services now answer true. Terminality announcement moves from a per-verb call into transactional() itself, after the commit: every mutation passes there, so no verb can forget it, and it keeps TaskService under the PHPMD method ceiling the merged import()/persistOpen() pushed it against. Slot assembly moves into UserTaskConfig::slotValues() for the same reason on the node's coupling. Ticks tasks 6.1-6.4.
Authored through POST /api/flows, run through the synchronous test endpoint, driven through the flow-tasks verbs. Two scenarios need the worker (the default budget parks for it; a run has no stop verb, so the operator's kill switch is the stop) and drive it through occ in the dev container, skipping loudly where occ is not reachable. Each test carries its scenario slug for gate-19. Ticks task 6.5 (19/19).
gate-16 named assignee(), outcomeKey() and renderedTitle().
The pgsql cell's coverage guard credits only statements in classes a test names with @Covers (strict coverage metadata), and the changed files measured 20.16% against a 23.46% base. Two moves: every task test class now names each class it actually exercises (TaskBuilder, TaskState and TaskPriority behind TaskServiceTest, TaskInboxCriteria behind the inbox and controller tests, the entities and exceptions everywhere they are built), and the largest untested code is now walked: TaskMapper's whole query vocabulary, the candidate, relation and audit mappers, and FlowRunMapper's terminality announcement, over a fluent query-builder double that records which predicate and column each query sets, so a conditional update without its openness guard or a pooled scope without its EXISTS would fail here. Plus the untested TaskService verbs (unclaim, assign, reassign, cancel, resolve, terminateAsMoot, relations, intake refusals), TaskInboxService's batched subject context, and the controller's filter, flag and failing-backend paths. 168 task tests.
An absolute path into another checkout has no business in the tree; the symlink only served a local Playwright --list and stays untracked.
…eature/flow-user-task-node
…eature/flow-business-timers
…box e2e spec Vue Quality (eslint) on #3258 named four errors in tests/e2e/task-inbox.spec.ts: perfectionist/sort-named-imports (the specifiers now read apiRequest, expect, test) and import-extensions/ban-inline-type-imports (APIRequestContext is a top-level import type). eslint --fix plus a plain run: 0 errors; the sibling flow-engine.spec.ts as control also 0; prettier clean.
…tests that can fail One working calendar (computed rules, Easter computus, Koningsdag observed shift; an enumerated-only calendar is refused because it expires), one calculator (hours, businessDays, calendarDays as DATES so a term lands at the same wall-clock time across DST), and one lifecycle: arm with a stored anchor, suspend into a consumed-value ledger, resume by re-projection, extend once (the override is a separate operation recorded as such), supersede on a moved anchor with copy-forward of the rungs still in the past, cancel on subject terminality inside the write that made it terminal. The ladder is seeded data (14/7/2/0). A rung is claimed by the unique INSERT before its transition is raised; an expiry by a conditional UPDATE before its outcome is applied as a named task action (skip, error, dead_letter, transition:<action> leave four distinct task states). The sweep is two index range scans on the existing 300s cadence; counts report work performed and a hit limit is logged as truncated. Overdue is never written: describe() derives it, and a suspended timer has no fire_at to be overdue by. Tests run against in-memory fakes with real semantics (range scans filter, claims are conditional, the ledger is unique), including the restart, the overlapping pass, the downtime gap and the fire_at identity after every operation. Tasks 1.3, 2.1, 2.2, 3.1-3.4, 4.1, 4.2, 5.1-5.3, 6.1, 6.2, 7.1, 7.2.
…eature/flow-user-task-node
…scope A top-level type import, sorted specifiers, the .ts extension on the relative import, and the repo's prettier config. Also carries #3258's matching fix for task-inbox.spec.ts through the merge.
…eature/flow-business-timers
…der schemas Gate-51 (schema-property-titles) wants a human-friendly title beside each description; the twelve properties of working-calendar and escalation-ladder now carry one.
…iness-timers branches One event class, two dispatch points told apart by a committed flag: the mapper announces terminality inside the verb's transaction for timer cancellation (D-9), the task service announces it after commit for the run continuation (D-5). UserTaskTerminalListener skips uncommitted dispatches.
…pproval-consolidation # Conflicts: # appinfo/info.xml # lib/AppInfo/Application.php # lib/Db/TaskMapper.php # lib/Event/TaskTerminalEvent.php # lib/Listener/UserTaskTerminalListener.php # lib/Service/Flow/FlowEngine.php # lib/Service/Flow/FlowTaskBridge.php # lib/Service/Task/TaskService.php # openspec/changes/flow-user-task-node/tasks.md # tests/Unit/Service/Flow/FlowTaskBridgeTest.php
…uences
The chain and step runtime retires onto the task service (ADR-098 D1, full
migration, no facade): ApprovalChain/ApprovalStep, their mappers, the
approval service and controller, the four step events, the nine routes and
the two Vue panels are removed. The declarative surface stays byte-identical
and re-points: the installer compiles x-openregister-approval-chains into a
derived task template, the gate listener refuses through the same two error
codes against the SEQUENCE, and a rejected cycle is closed and kept instead
of deleted. Separation of duties moves into a pre-decision guard, evaluated
against the actor AND on_behalf_of, before the performer check.
New: openregister_task_sequences + TaskSequence(Service|Mapper), a trusted
enable verb, TaskSequenceCompletedEvent, sequence columns on tasks, the
correlation key on await-signal suspensions with a fail-closed
/api/flow-run-signals/{key} route, the in-flight data migration repair step
(registered in info.xml, verifying loudly, idempotent) and the operator-run
rollback command.
Tasks 1.1, 1.2, 2.1, 2.2, 2.3, 3.1, 3.2, 3.3, 4.1, 4.2, 5.1, 6.1, 6.2, 6.3.
…pproval-consolidation # Conflicts: # appinfo/info.xml # lib/AppInfo/Application.php # lib/Migration/Version1Date20260901150000.php # lib/Service/Task/TaskService.php
…and the data migration Gate and advance listeners re-tested against the sequence store; sequence provisioning, in-request advance, rejection propagation and termination; separation of duties refused before the performer check, delegated self-decisions included; enable and consume verbs (consume refuses a second authorization); correlation key resolved from the item and fail-closed delivery (404 not buffered, 409 wakes nothing); the data migration over a seeded in-memory database, idempotent on a second run, failing loudly and naming chain, object and step on an unreconcilable row. The HITL retirement inventory and retired-surface fixtures land beside them. Tasks 7.1 (fixtures), 8.1, 8.2 (unit half).
…and contract coverage The no-shim assertions walk lib/ for retired class bindings and legacy-table readers; the HITL inventory test fails on any approval_request property with no named home; the rollback command writes decisions back and reports the facts the legacy schema cannot hold; the sequence mapper's access paths are exercised; the retired approval requests leave the Newman collection and the signal-by-key contract joins it (gate-25); the event replacement mapping is published as migration documentation for filinq. Tasks 4.3, 6.3 (test half), 7.1, 7.2 done; all 19 tasks ticked.
phpcs named-argument and line-length fixes; PHPMD complexity splits in the migration step, the rollback command and the compiler; suppressions moved onto the class docblocks where PHPMD reads them; the duplicated TooManyMethods tag from the timers merge dropped; the pre-existing else in the pg_trgm re-migration simplified while passing by.
…pproval-consolidation # Conflicts: # lib/AppInfo/Application.php # lib/Listener/FlowTimerSubjectTerminalListener.php # lib/Migration/Version1Date20260901000000.php # lib/Service/Flow/Timer/FlowTimerDefinitionStore.php # lib/Service/Task/TaskService.php # lib/Settings/flow_timer_register.json # tests/Unit/Listener/FlowTimerSubjectTerminalListenerTest.php # tests/Unit/Repair/FlowTimerRepairStepsTest.php # tests/Unit/Service/Flow/Timer/EscalationLadderServiceTest.php # tests/Unit/Service/Flow/Timer/FlowTimerServiceTest.php # tests/Unit/Service/Flow/Timer/FlowTimerSweepTest.php # tests/Unit/Service/Flow/Timer/SlaCalculatorTest.php # tests/Unit/Service/Task/TaskServiceTimerOutcomeTest.php
…quence panel Every new suite lists the collaborators it executes, so the coverage cell keeps what the tests earn; prettier settles the panel.
…e left on TaskMapper
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ❌ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 543/543 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-01 19:30 UTC
Download the full PDF report from the workflow artifacts.
… 36 locales
test:l10n named the ten TaskSequencePanel strings missing from en.js; they
land in en plus every required locale, inserted in each file's own style so
nothing is re-serialised. Czech 'Role {role}' is recorded as a justified
cognate (role IS the Czech word). All six frontend-check legs pass locally:
test:l10n, test:l10n:parity, check:l10n-js, check:schema-l10n, check:specs
and format.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 543/543 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-01 19:49 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Sep 3, 2026
…chor (#3360) The mapper could answer "what happened to THIS object" three ways and "has this approval ever run" not at all. Every existing finder constrains anchor_object_uuid, so an aggregate over a template had no anchor to pass. buildiq is the caller that needed it. Its automation dry-run panel used to read the newest ApprovalStep on a chain; when #3302 retired that surface there was no replacement for the aggregate, so the panel degraded to reporting nothing at all (buildiq#651). This restores it. One row off the existing template_id index, ordered like its siblings and limited in the QUERY rather than trimmed in PHP. Tested with a negative control, not just a green run: adding an anchor predicate and dropping the limit makes the new test fail with "Failed asserting that an array does not contain 'anchor_object_uuid'". A finder that quietly constrained the anchor would answer a different question than the caller asked, which is the one thing this method must not do. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the merged openspec change
flow-approval-consolidation(19 of 19 tasks). The fleet's three approval systems converge on the task entity: the chain and step runtime retires onto an ordered task sequence, the declarative gate keeps refusing through the same contract, and the openconnector HITL semantics get named homes. ADR-098 decision 1: full migration, no facade.Built on
flow-task-entity(#3258),flow-user-task-node(#3269) andflow-business-timers(#3272), all now on development. This is the last open change of the chain.What lands
The sequence.
openregister_task_sequencesplusTaskSequence(Service|Mapper): provision creates every position and enables only the first; an approving decision enables the next in the same request; a rejection closes the sequence and terminates every remaining task, with the record kept, never deleted (design D-5). The template snapshot and the amount tier freeze at provisioning, so a schema or amount edit cannot re-shape a running approval (D-3).The declarative surface, re-pointed.
x-openregister-approval-chainsis byte-identical; no schema in any app changes. The installer compiles it into a derived task template (deterministic template id, so gate lookups and migrated sequences meet on one id).ApprovalChainGateListenerkeeps its two error codes and its fail-closed policy, now against the sequence.ApprovalChainAdvanceListenersubscribes toTaskSequenceCompletedEvent.Separation of duties, one notch stricter. Enforced before the performer check by
TaskSequenceDecisionGuard, read from the frozen snapshot, default ON. A delegated decision whoseon_behalf_ofis the requester is refused on the same grounds. The refusal is its own exception type, so the reason stays honest.Retirement.
ApprovalChain,ApprovalStep, both mappers,ApprovalService,ApprovalController, the fourApprovalStep*Eventclasses, the nine routes and the two Vue panels are gone. No shim. A repository-walking test (ApprovalRetirementSurfaceTest) proves the only remaining mentions of the legacy tables are the migration and rollback code.The data migration.
Repair\MigrateApprovalChainsToTasks, registered inappinfo/info.xmlin the same change: chains become templates, each(chain, object)set becomes one sequence, each step becomes a task at its own ordinal with its role as candidate group and its original creation time. Decided steps migrate their decision into the task audit, attributed to the original decider (a departed decider keeps the recorded string). Reconciliation columns land on both sides, every stage is guarded on them, and the in-migration verification fails loudly naming chain, object and step. The cutover timestamp lands in app config underapproval_consolidation_cutover. The legacy tables are not dropped.Rollback.
occ openregister:approval:rollback-to-steps(deliberately a command, never an automatic repair step) writes task-side decisions back onto step rows for what the legacy schema can hold and reports what it cannot: performer type, on-behalf-of, mandate, per-entry audit. Before the first post-cutover decision, plain redeploy suffices.Correlation.
openregister.await-signalgainscorrelationKey, resolved from the item at suspension and copied onto an indexed run column.POST /api/flow-run-signals/{key}delivers by business key with resume's authority: zero matches is 404 and not buffered, more than one is 409 and wakes nothing. A flow without a key behaves exactly as before.HITL contract.
TaskService::consume()is the home ofconsumedAt: an approval authorizes exactly once. The checked-in retirement inventory (tests/fixtures/approval-consolidation/) maps all 22approval_requestproperties to a home or a recorded decision, with a test that fails on a homeless one.retired-approval-surface.jsonhands the hydra-gates anti-pattern gate its route, event and shape lists.Consumers: what filinq must change
filinq/docudesk is the one registered subscriber to the four removed events, and its listener also drove
ApprovalServiceback. It breaks at app load on deploy, by design, and migrates infilinq: migrate-signing-to-or-tasksagainst the published mapping indocs/development/approval-events-migration.md: subscribe toTaskTerminalEventandTaskSequenceCompletedEvent, reply throughTaskService::complete(). This change must land in the same release train as the filinq migration. opencatalogi and softwarecatalog declare no chain (verified by grep across the fleet'slib/Settingsdescriptors: zerox-openregister-approval-chainsoutside openregister).Merge-conflict resolutions worth reviewing
TaskTerminalEventis unified with acommittedflag:TaskMapperannounces terminality inside the verb's transaction (timer cancellation, D-9),TaskServiceafter commit (run continuation, sequence progression). Listeners filter on the flag.Version1Date20260901170000; the duplicate copy this branch carried was dropped.Mutation-check evidence for the fail-closed gate
Three mutations were applied one at a time; each was caught by a named test, and the restored code runs green (18 tests, 38 assertions):
false(fail open)testAnUncompilableChainFailsClosedtestARunningSequenceBlocksWithoutProvisioningAgaincompleteInternaltestTheRequesterIsRefusedBeforeThePerformerCheckChecks
--threads=1 --no-cache): 0 errors--memory-limit=2G): 0 errorstests/Unit: 18,916 tests, 46,034 assertions, 0 failuressrc tests scripts: 0 errors; prettier clean on the changed Vue files--scope-to-diff --base origin/development: all 57 applicable gates green, 0 FAIL@coversand@usesrosters