The flow projection emitted UUIDs into a node that resolves names - #1707
Merged
Conversation
…s one blocker
Task 4 of workflow-definitions-to-flow was deferred because the definition
carries per-step SLAs, checklists and roles the projection does not. That
reason was about the MIGRATOR, not the engine, and it is now partly out of
date: graphOf() reads transitions and never touches steps, while OpenRegister
has since gained homes for most of what a step holds.
This records the mapping field by field against what was actually read in
OpenRegister's source. Two corrections to the obvious guess, both of which
would have shipped a projection that looked complete:
- the config key is formRequireChecklist, not requireChecklist; the short
name would validate as unknown and the checklist would simply not appear
- dueAt is ADVISORY by its own help text, so projecting a step SLA onto it
would show a date and escalate nothing
And one genuine blocker, recorded as OPEN rather than assumed away. A business
timer is a runtime record armed against a subject, not a field in a flow
document, and UserTaskNode's config keys name no escalation ladder. So how a
projection DECLARES an SLA and its ladder is unanswered, and writing the
projection before it is answered would drop the escalation while reporting
success — the exact failure this change exists to avoid.
…definitions-to-flows
…thing arms it"
`FlowTimerService::arm()` takes exactly what a dossiq step holds — sla
{value,unit}, calendar, ladder, escalationRules, purpose, legalEffect,
onExpiry, extensionMax — bound to a subject with optional runUuid/nodeId.
Field for field, the model fits.
But it has NO production caller. Measured across OpenRegister's whole tree:
every `->arm(` lives in FlowTimerServiceTest. Not UserTaskNode, not the flow
engine, not a listener, and no DI registration in AppInfo. The capability is
built, tested, and unwired.
So a per-step SLA cannot be carried by writing flow JSON: nothing reads an SLA
off a node and arms a timer. Either OpenRegister wires UserTaskNode to arm on
open and cancel on completion (the right home, ADR-065), or dossiq arms it from
a listener — which puts timer orchestration back in a leaf app, the thing this
change exists to stop.
Consequence recorded in the sequencing: the projection can carry every other
field, and retiring workflowTemplate CANNOT complete until the timer is wired
upstream. Retiring the only home of a per-step SLA while nothing else can hold
it is the exact forgetting this document opens by warning about.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…olves names Found while starting phase 2, by reading what a LIVE template actually stores rather than what the seed files do. `DossiqTxSetStatusNode` states its contract in its own description: it moves a case to a status of its case type "named rather than referenced by id", because a statusType uuid is minted per installation and a flow carrying one is portable nowhere. The migrator's docblock says the same. The data says otherwise. The SEED files store statuses as names, which is what this migrator was written against. The STORED objects do not — the seeder resolves those names to statusType uuids on import — so on any live instance `fromStatus`, `toStatus` and `step.status` are all uuids. Measured on a running instance: 9 of 9 step statuses and every transition endpoint. So every flow the projection has produced carries uuids into a node that resolves names. Those flows could not have moved a case. WHY NOTHING CAUGHT IT: the projections arrive DISABLED, deliberately, so the old runtime keeps driving cases. A disabled flow never runs, and the e2e asserts only that the projected flows exist and are switched off. The defect was unobservable by construction — which is exactly why phase 2 had to look at the data before adding to it. `statusNames()` indexes statusType uuid => name once per pass; `statusName()` resolves one reference and PASSES AN UNRESOLVABLE VALUE THROUGH, so a seed-shaped template whose endpoints are already names projects exactly as before. Both cases are tested, and the uuid test fails against the old code. Verified live: re-projected all 10 flows and read them back — every `dossiq.setStatus` node now carries a name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rubenvdlinde
requested review from
Rem-Dam,
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
September 3, 2026 05:55
The coverage ratchet was right: `statusNames()` added branches nothing
exercised — an empty index, and a statusType row with no name.
Both are behaviours worth pinning rather than coverage padding:
- an empty index must PASS REFERENCES THROUGH, because that is the
documented degradation and it is what keeps a seed-shaped template
projecting when statusTypes cannot be read;
- a row with no name must be SKIPPED rather than indexed as empty. Mapping a
uuid to '' is worse than leaving it unresolved: the node would be handed
nothing to resolve instead of something it can refuse.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Quality Report — ConductionNL/dossiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 540/540 | |||
| 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 06:01 UTC
Download the full PDF report from the workflow artifacts.
Contributor
Quality Report — ConductionNL/dossiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 540/540 | |||
| 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 06:06 UTC
Download the full PDF report from the workflow artifacts.
The ratchet's numbers, read rather than guessed at: 28 statements added, 21 covered, 7 not. The seven were the two paths where the status resolution gives up — an unconfigured statusType schema, and a read that throws. Both are worth pinning for the same reason: losing the resolution costs readability in the projected flow, while losing the PROJECTION costs the migration. Each test asserts the flow is still written, not merely that nothing exploded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Quality Report — ConductionNL/dossiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-manifest | ✅ | ||||
| check-vue3-compile | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 106/106 | |||
| npm | ✅ | ✅ 540/540 | |||
| 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 06:59 UTC
Download the full PDF report from the workflow artifacts.
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.
Found while starting phase 2 of
workflow-definitions-to-flow, by reading what a live template stores rather than what the seed files do.The contract, stated twice
DossiqTxSetStatusNode's own description:…because a statusType uuid is minted per installation, so a shipped flow carrying one is portable nowhere. The migrator's docblock says the same thing.
The data says otherwise
The seed files store statuses as names — which is what the migrator was written against. The stored objects do not: the seeder resolves those names to statusType uuids on import. So on any live instance,
fromStatus,toStatusandstep.statusare all uuids.Measured on a running instance: 9 of 9 step statuses and every transition endpoint were uuids, and the two sets overlapped completely.
So every flow this projection has produced carries uuids into a node that resolves names. Those flows could not have moved a case.
Why nothing caught it
The projections arrive disabled, deliberately, so the old runtime keeps driving cases. A disabled flow never runs, and
changed-surfaces.spec.tsasserts only that the projected flows exist and are switched off.The defect was unobservable by construction. That is precisely why phase 2 had to look at the stored data before adding anything to the projection.
The fix
statusNames()indexes statusType uuid → name, once per pass rather than per template.statusName()resolves one reference and passes an unresolvable value through, so a seed-shaped template whose endpoints are already names projects exactly as before.Both cases are tested. The uuid test fails against the old code; the seed-shape test guards the passthrough so the fix cannot break the case it was built to preserve.
Verified live
Re-projected all 10 flows on a running instance and read them back:
2796 PHPUnit tests, phpcs clean.
Also in this branch
design.mdrecords the answer to the phase-2 blocker:FlowTimerService::arm()fits a dossiq step field for field, but has no production caller anywhere in OpenRegister — every->arm(is in its own unit test. So a per-step SLA cannot yet be carried by flow JSON, and retiringworkflowTemplatemust wait for the engine to wire its own timers.🤖 Generated with Claude Code