refactor(flow)!: delete hermiq's second flow engine; contribute nodes only - #148
Merged
Merged
Conversation
… only hermiq walked its own `agentflow` graphs through a 727-line `GraphExecutor` while OpenRegister's engine walked the same documents through a node registry. Two engines over one document type, and they had drifted apart in the way that matters most: `GraphSidebar` builds its palette from OpenRegister's node catalogue "and nothing else", so every node dragged onto the canvas carries a NAMESPACED id — `openregister.set-fields`, `hermiq.agent-step`. `GraphExecutor::runNode()` switched on BARE ids — `condition`, `router`, `agent-step`, `object-write` — and stripped no namespace. Every palette node therefore fell to `default:`, was logged at info as "unknown node type; skipped", and the walk carried on. A graph authored in the builder executed ZERO steps and reported success, on both paths that reached it: the manual run endpoint AND `GraphRunRequestedListener`, which was wired to object create/update/delete. That is why the hydra pipeline still worked: it is schedule-driven, and the schedule path went through OpenRegister's engine, whose registry THROWS on an unknown type rather than skipping it. The same flow document ran correctly on one trigger and silently did nothing on another. Removed: `Service/Graph/GraphExecutor`, `Controller/GraphController`, `Listener/GraphRunRequestedListener` and its three event registrations, the `/api/graph/run` route, and the resolver stubs and tests that outlived the resolver. Kept, because node contribution IS the supported integration surface: `HermiqAgentNode`, `HermiqWorkloadNode`, `HermiqFlowNodeListener`. `SeedHydraTriageFlow` now seeds into OpenRegister's native flow store. It stays disabled and ownerless: a repair step runs with no session, so there is no identity to attribute the flow to, and seeding it enabled would mean picking an owner for a graph that runs agents — a privilege decision nobody made. Enabling it is a human act that supplies one. Refs: openregister/openspec/changes/flow-engine-unification
One conflict, in Application.php: development added the Talk-bot lifecycle listener next to the graph ingress this branch deletes. Resolved by keeping development's AgentBotLifecycleListener registrations in full and dropping only the GraphRunRequestedListener ones. The two happened to sit in the same hunk because both hang off the object lifecycle; they are unrelated features, and the Talk-bot work is not this branch's to revert. The merge left ObjectCreatedEvent/UpdatedEvent/DeletedEvent used but unimported — the graph hunk had carried those imports. Restored; without them Application would have fataled on boot and taken every hermiq route with it. Also corrected prose that still described the deleted engine as live: the seed step's docblock, HermiqAgentNode's, and three schema descriptions naming HermiqFlowResolver and CnFlowCanvas. Those read as instructions to the next person and none of them were true any more.
Three failures, all introduced by this branch: - phpstan: SeedHydraTriageFlow used `Application::APP_ID` with no import, so it resolved to OCA\Hermiq\Repair\Application — a class that does not exist. That was a runtime fatal in the seed path, not just an analysis complaint. - psalm: deleting the flow-resolver stubs removed hermiq's only declarations of OpenRegister's flow contracts, while the rewritten seed step started using Db\Flow and Db\FlowMapper. Replaced with stubs for what is actually used. - phpcs: whitespace left by the development merge. The Flow stub keeps its accessors MAGIC — @method tags plus __call — because the real class is an OCP Entity that serves them through Entity::__call. A stub whose methods are more concrete than the original is worse than no stub: it passes locally and then fails against the live class, which is exactly how the ObjectEntity::getUuid() stub burned six CI legs. FlowMapper declares only findAllFlows() and insert(). Every method in a stub is a promise that hermiq may call it; the seed step is the only place hermiq touches OR's flow store directly, so the promise stays that small. Verified in a container with NC core: phpstan OK, psalm no errors, phpcs clean across lib/ and both new stubs.
rubenvdlinde
requested review from
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
August 4, 2026 10:27
Resolves the two conflicts this branch carried. Both are files that development changed AFTER this branch was cut, and in both the branch side of the conflict was empty or a strict subset of development's: - tests/Stubs/Db/Flow.php (add/add) — took development. Its stub round-trips through __call; the branch's returned null for every getter, which is the shape that makes "the seed sets the organisation" untestable (hermiq#140). - lib/Repair/SeedHydraTriageFlow.php — took development. Its side adds the PHPMD coupling suppression and the hermiq#140 organisation guard that refuses to write an org-less orphan row. The branch side of both hunks was empty. Nothing was reverted: the four deleted engine files (GraphExecutor, HermiqFlowResolver, GraphRunRequestedListener, GraphController) are absent from both sides and remain absent from the merge result.
Contributor
Quality Report — ConductionNL/hermiq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 117/117 | |||
| npm | ✅ | ✅ 744/744 | |||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ✅ |
Quality workflow — 2026-08-04 10:44 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
added a commit
that referenced
this pull request
Aug 5, 2026
…failing CI (#157) v1.0.1 is `f4d9756` (2026-08-03) and predates three gate fixes, so every Hydra Gates run this repo has ever made executed a script in which 16 gates reported PASS when their helper never ran (ConductionNL/.github#147), gate-33 had no axe report to read and never said so (#148), and gates 6 and 7 reported PASS on an empty scope (#149). The tick was identical either way, which is why nothing in this repo's history shows it. That pin is now also RED, and the mechanism is worth writing down. quality.yml is referenced `@main` while this package is PINNED, so the two can desync. #164 flipped `hydra-gates-require-full-coverage` to default true in the shared workflow, and that flag requires a gate to DECLARE itself not-applicable. v1.0.1 contains ZERO `_skip` calls; v1.3.0 has 36. v1.0.1 has no vocabulary to declare, so every absent prerequisite became "DID NOT RUN" and failed the job — for gates the repo has no subject matter for. Measured on this branch, diff-scoped against origin/development exactly as CI scopes it, in a private mount namespace with a private tmpfs (the runner's ~50 /tmp/hydra-gate-*.log paths are shared state and two concurrent runs corrupt each other's counts, .github#158 item 6): v1.0.1 exit 98 FAIL — "GATES THAT DID NOT RUN: 24 33" v1.3.0 exit 0 PASS — those gates named NOT APPLICABLE, with reasons Independently confirmed end-to-end: doriath#160 changed this one line and nothing else, and its Hydra Gates job went failure -> success. v1.3.0 is `f7eaf2a` = .github@main at the time it was cut. Refs ConductionNL/.github#159
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.
Why
Hermiq carried a second flow engine alongside OpenRegister's. This deletes it, leaving hermiq to contribute nodes to the single OR flow engine (ADR-065). Net -2036 / +232.
Removed:
lib/Service/Graph/GraphExecutor.phplib/Flow/HermiqFlowResolver.phplib/Listener/GraphRunRequestedListener.phplib/Controller/GraphController.phpHermiq keeps
HermiqAgentNode/HermiqWorkloadNode— the node contributions OR's engine dispatches.Why it needs a verdict rather than a merge
This branch is what the hydra flows have been exercised against, but it has never had a CI run — no PR existed. Opening one so the deletion is measured rather than assumed.
Two things a reviewer should look at specifically:
tests/Stubs/Db/Flow.phpandFlowMapper.phpare new stubs. Stubs that declare an API the real dependency does not have have already produced a whole passing suite against a fiction in this fleet. These should be checked against OpenRegister's actualFlow/FlowMappersignatures.lib/Repair/SeedHydraTriageFlow.phpchanged alongside the deletion; seeding runs on a fresh install, where there is no session.Verification status
Not yet verified beyond the branch building locally. CI on this PR is the first real measurement.