Skip to content

feat(flow): a user task arms its own deadline - #3350

Merged
rubenvdlinde merged 2 commits into
developmentfrom
feat/user-task-arms-its-deadline
Sep 3, 2026
Merged

feat(flow): a user task arms its own deadline#3350
rubenvdlinde merged 2 commits into
developmentfrom
feat/user-task-arms-its-deadline

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

The business-timer capability had no production caller. FlowTimerService::arm() was referenced only from its own unit test, and nothing resolved it from the container at runtime. The escalation ladders, the working calendar and the SLA calculator were built, tested and unreachable: a flow author could describe a deadline and nothing would ever measure it.

What changed

UserTaskNode arms a timer when it creates its task, bound to subjectType: task and the task uuid. That is the same pair FlowTimerSubjectTerminalListener already cancels on, so completing or cancelling the task disarms its deadline rather than leaving a timer firing against a subject that is finished. The node also declares sla, calendar, ladder, escalationRules, purpose, legalEffect and onExpiry as config keys, so an author can express the deadline where the task is defined.

Three properties, because they are what makes this safe in a shared engine

A node with no sla arms nothing. A flow that never mentioned a deadline behaves exactly as it did before. All 36 pre-existing UserTaskNode tests pass untouched.

An SLA that cannot be armed fails the node, rather than logging and continuing. A task carrying a declared deadline that nothing is measuring is worse than a task with no deadline at all, because the flow then reports a term it is not keeping. For a wettelijk deadline that is a legal defect, not a logging concern. No existing flow can reach this branch, since none of them set an SLA.

Only keys the author actually set are forwarded. arm() has its own defaults for purpose, legalEffect and onExpiry; passing nulls through would silently replace them.

Verification

  • Full suite: 19045 tests, 46386 assertions, exit 0. The 3 risky tests are pre-existing in TextExtractionServiceTest and untouched here.
  • Negative control: removing the arm call fails testADeclaredSlaArmsATimerBoundToTheTask. The test can distinguish the fix from its absence.
  • Circular DI ruled out by reading the graph, not by the tests passing: FlowTimerService -> TaskService never reaches a flow node or the node registry. The node is container-resolved, so a cycle would surface at runtime rather than in a unit test that constructs it by hand.
  • Three new tests cover the armed case, the no-SLA case, and the unarmable case.

Spec: a twelfth requirement in openspec/changes/flow-business-timers/specs/flow-business-timers/spec.md.

Why it matters downstream

dossiq retired its own workflowTemplate engine onto OpenRegister flows. Step-level SLAs were the one thing the projection could not carry, because the target had no way to arm them. With this in place the projection can express deadlines, and workflowTemplate can eventually retire rather than living on as the only place a term can be written.

🤖 Generated with Claude Code

FlowTimerService::arm() had no production caller anywhere in OpenRegister.
Every reference to it lived in its own unit test, and it was never resolved
from the container by anything that runs. The escalation ladders, the working
calendar and the SLA calculator were all built, all tested and all
unreachable: a flow author could describe a deadline and nothing would ever
measure it.

UserTaskNode now arms a timer when it creates its task, bound to
subjectType 'task' and the task's uuid. That is the same pair
FlowTimerSubjectTerminalListener already cancels on, so completing or
cancelling the task disarms its deadline instead of leaving a timer firing
against a subject that is finished.

Three properties worth stating, because they are what makes this safe to add
to an engine every app shares:

- A node with no `sla` arms nothing and behaves exactly as before. All 36
  pre-existing tests pass untouched.
- An SLA that cannot be armed fails the node rather than logging and
  continuing. A task carrying a declared deadline that nothing is measuring
  is worse than a task with no deadline, because the flow reports a term it
  is not keeping; for a wettelijk deadline that is a legal defect, not a
  logging concern. No existing flow can reach this branch.
- Only keys the author actually set are forwarded, so arm()'s own defaults
  for purpose, legalEffect and onExpiry still apply instead of being
  overridden with nulls.

Verified: full suite 19045 tests green; negative control (remove the arm
call) fails testADeclaredSlaArmsATimerBoundToTheTask; no circular DI, since
FlowTimerService -> TaskService never reaches a flow node or the registry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 51f5d80

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-03 07:26 UTC

Download the full PDF report from the workflow artifacts.

The merge-base coverage guard failed the branch by 0.13%: 33 added
statements with exactly one never executed. It was `$title = null`, the
branch that turns an empty title into null before arming.

Nothing reached it because every test in the class builds its config through
a helper that always supplies a title, so the branch was dead on the coverage
report and nothing would have noticed if it stopped working.

It is a real case, not filler. `title` is optional on a user task, and the
timer renders it in the escalation notice: '' produces a blank subject line
where null lets the timer describe its subject instead. The new test passes
whitespace rather than omitting the key, because a title of spaces is still
no title and trim() is what makes the two the same.

Negative control: removing the branch so an empty title reaches the timer as
'' fails testATitlelessNodeArmsItsTimerWithNoTitle. 40 tests, 155 assertions.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 57bd935

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-03 07:38 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 4309cec into development Sep 3, 2026
46 checks passed
@rubenvdlinde
rubenvdlinde deleted the feat/user-task-arms-its-deadline branch September 3, 2026 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant