fix(seed): the Hydra Triage flow was inert — the step is the edge, not the node - #91
Merged
Merged
Conversation
…t the node Every `type` and `config` in the seeded flow sat on `nodes[]`. OpenRegister's engine never reads those: `FlowEngine::stepFor()` resolves a transition to the matching entry in `edges[]`, and `RegistryStepDispatcher::dispatch()` takes `type`/`config` from that edge. A node is a Petri-net place and carries no behaviour, and a step with no `type` passes its items through untouched. So the flow imported cleanly, walked all three edges and reported `completed` having called no agent and taken no branch — no error, no warning, no log line. Measured on a live instance 2026-07-31, the same graph both ways: node form completes with the items untouched and hermiq never called; edge form runs the agent step and lands its parsed answer on the item. Every assertion in the test suite read `nodes[]`, which is exactly why they all passed. `testNoNodeCarriesExecutableConfig()` is the one that was missing, and is now the load-bearing test. `testEveryEndpointAndRouterOutputNamesAReachablePlace()` also tightens: a router's outputs must be places the ROUTING EDGE reaches, because `advanceItems()` only distributes to that transition's own `to` list — an output naming anything else drops its items silently. Second defect, same family: the agent step named the agent by its display NAME. `AgentMapper::findByUuid()` matches the `uuid` column, so it resolved to nothing. The two seeds still stay independent by name rather than by a hard-coded uuid — the name is now resolved to a uuid at seed time, and left empty when the agent is absent, because an empty agentId is refused at both validate and execute time where a name is not. 1343 unit tests green; phpcs and phpstan clean.
Contributor
Quality Report — ConductionNL/hermiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 117/117 | |||
| npm | ✅ | ✅ 657/657 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ |
Coverage: 85.3% (93/109 statements)
Quality workflow — 2026-07-31 12:38 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
to ConductionNL/openregister
that referenced
this pull request
Jul 31, 2026
…it as a no-op (#2227) * fix(flow): refuse a node that carries step config instead of running it as a no-op The step is the EDGE. `FlowEngine::stepFor()` resolves a transition to the matching entry in `edges[]`, and `RegistryStepDispatcher::dispatch()` reads `type` and `config` off that edge. A node is a Petri-net place and carries no behaviour. A `type` on a node is therefore not merely redundant — it is the whole behaviour of the flow, put where nothing looks. The engine accepted it: every transition became a pass-through (dispatch() returns items untouched when `type` is empty) and the run reported COMPLETED. No error, no warning, nothing in the trace, and an output key simply absent — indistinguishable from a flow whose steps genuinely had nothing to do. Three graphs in the fleet were authored this way and none of them failed anywhere: hydra's dispatch flow (shipped as a completed task), hydra's applier flow, and hermiq's seeded Hydra Triage flow — whose unit tests asserted on `nodes[].type` and therefore all passed. Node-shaped authoring is the natural mistake, because that is how a graph editor presents a flow, and because this class's own docblock calls itself the translation layer between what users author and what executes. `extractPlaces()` already refuses a duplicate id for the same reason ("the graph would run but not be the graph the user drew"). This is the same kind of authoring error with a larger blast radius, so it is refused in the same place, with a message naming the node and where the step belongs. Presentational keys are untouched: position, label and styling are what a canvas legitimately stores on a node, and a test pins that they still build. Fixes #2226. The three affected graphs are corrected in ConductionNL/hydra#435 and ConductionNL/hermiq#91; all three are `enabled: false`, so nothing in flight is broken by this becoming an error. 15,542 unit tests green (3 new); phpcs, phpstan and psalm clean. * style: group the `failIfExists` param tag with the rest Pre-existing phpcs failure on development, not introduced here: `SaveObject::handleObjectCreation()`'s docblock had `@param bool $failIfExists` separated from the other param tags by a blank line and misaligned, which trips "Parameter tags must be grouped together" and the type-padding sniff. Caught because CI runs phpcs over the whole tree while a local run scoped to the changed file does not.
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.
The seeded flow ran nothing
Every
typeandconfiginSeedHydraTriageFlowsat onnodes[]. OpenRegister's engine never reads those:FlowEngine::stepFor()resolves a transition to the matching entry inedges[], andRegistryStepDispatcher::dispatch()takestype/configfrom that edge. A node is a Petri-net place and carries no behaviour; a step with notypepasses its items through untouched.So the flow imported cleanly, walked all three edges, and reported
completedhaving called no agent and taken no branch. No error, no warning, no log line.Measured on a live instance, the same graph both ways:
type/configsatnodes[]status: completed, items untouched, hermiq never callededges[]Why the tests didn't catch it
Every assertion read
nodes[]. They were checking decoration.testNoNodeCarriesExecutableConfig()is new and is now the load-bearing test.testEveryEndpointAndRouterOutputNamesAReachablePlace()also tightens: a router's outputs must be places the routing edge itself reaches, becauseFlowEngine::advanceItems()only distributes items to the firing transition's owntolist. An output naming anything else drops every item routed to it, silently.Second defect, same family
The agent step named the agent by its display name.
AgentMapper::findByUuid()matches theuuidcolumn, so it resolved to nothing — swallowed into an empty answer before #89, a failed step after it.The two seeds still stay independent by name rather than by a hard-coded uuid; the name is now resolved to a uuid at seed time, and left empty when the agent is absent. An empty
agentIdis refused at both validate and execute time, where a name is not — so the flow announces its missing half instead of carrying an identifier that silently resolves to nothing.Verification
lib.