Skip to content

feat(flow): apps contribute node types — no app needs its own engine again#2074

Merged
rubenvdlinde merged 4 commits into
developmentfrom
feat/or-flow-nodes
Jul 24, 2026
Merged

feat(flow): apps contribute node types — no app needs its own engine again#2074
rubenvdlinde merged 4 commits into
developmentfrom
feat/or-flow-nodes

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Proposal: or-flow-nodes

Summary

Let any app contribute node types to the OpenRegister flow engine, so no app
ever needs a flow engine of its own again. Discovery uses Nextcloud's own
registration pattern, and the node contract is shaped after Nextcloud Flow's
IOperation everywhere it can be.

Why

hermiq shipped GraphExecutor — its own graph engine — because there was no
way to contribute a step to anyone else's. That made it the sixth thing in this
fleet called "flow". Until contributing a node is easier than writing an
engine, apps will keep writing engines.

FlowEngine already has the seam (FlowStepDispatcher), but each consumer had
to implement it, and a dispatcher with a type switch in it is half an engine.

Investigated first: can Nextcloud Flow be the node system?

No, and the reason is concrete. Core invokes an operation like this
(apps/workflowengine/lib/AppInfo/Application.php:81):

$entity->prepareRuleMatcher($ruleMatcher, $eventName, $event);
$operation->onEvent($eventName, $event, $ruleMatcher);
  1. onEvent() returns void — a node must return items; there is nowhere
    for output to go.
  2. It takes an Event and an IRuleMatcher, never data. That is a listener
    signature, not a callable one.
  3. Control is inverted: the operation asks the matcher which rules matched and
    acts alone. In a graph, the engine decides what runs.
  4. An operation is bound to an entity and its events
    (ISpecificOperation::getEntityId()) — "when X happens, do Y" is a one-step
    rule, not a step that can sit anywhere in a graph.

Wrapping an IOperation as a step would mean synthesising an Event and an
IRuleMatcher it never asked for and discarding output that does not exist.

So: our contract, Nextcloud's everything-else

  • IFlowNode mirrors IOperation's metadata methods verbatim
    (getDisplayName, getDescription, getIcon, isAvailableForScope) and
    uses Nextcloud's own IManager::SCOPE_* constants. It adds the two methods
    IOperation lacks: validateConfig() and execute(items, config, context).
  • Discovery is RegisterFlowNodesEvent, a direct copy of core's
    RegisterOperationsEvent pattern. An app writes the same listener it would
    write for Nextcloud Flow.
  • The two systems compose: RunFlowOperation registers into Nextcloud Flow
    so a core rule can start an OpenRegister flow — the direction that works.
    Nextcloud Flow keeps what it is good at; a flow adds branching, joins, loops
    and data between steps, which core cannot express.

Discovery: why the event, not the fleet's other pattern

OpenRegister already has a second discovery mechanism — MCP tool providers,
found by probing each app's info.xml, building container aliases
(OCA\OpenRegister\Mcp\IMcpToolProvider::<appId>), resolving them, and then
caching the resolution map with two invalidation mechanisms to stay affordable.

That complexity exists because it scans for something apps never announce.
Apps do announce a listener, so none of it is needed. The event is both
closer to Nextcloud and simpler than the fleet's alternative — the two goals
did not conflict here.

What Changes

  • IFlowNode, RegisterFlowNodesEvent, FlowNodeRegistry.
  • RegistryStepDispatcher — the FlowStepDispatcher every consumer gets free.
  • SetFieldsNode — the first built-in and the reference implementation.
  • RunFlowOperation + NcFlowOperationListener — the Nextcloud Flow bridge.

Out of scope (this change)


Verification

47 unit tests green (registry contribution, single dispatch, duplicate refusal, unknown-type throw, scoped palette, routing-only edge, dispatch routing, per-item application, pairing, field ordering, keepOnlySet, binary passthrough, config validation). phpcs clean on every new file.

Also fixes FlowActionServiceTest's constructor call, which had passed 7 of 9 arguments since e6925cc3b — all 12 of its tests errored before reaching an assertion, so that suite had been reporting nothing. The 9 behavioural failures underneath are #2073 and are deliberately left alone rather than papered over.

Part of the flow-parity programme: #2067 flow logic · #2068 triggers · #2069 expressions · #2070 execution tooling · #2071 MCP · #2065 integration network · #2066 code sidecar · hermiq#35.

🤖 Generated with Claude Code

…gain

hermiq wrote its own graph engine because there was no way to contribute a
step to anyone else's. That made it the sixth thing in this fleet called
"flow". Until contributing a node is easier than writing an engine, apps will
keep writing engines.

Investigated Nextcloud Flow as the node system first. It cannot serve as one,
and the reason is concrete — core invokes an operation as

    $entity->prepareRuleMatcher($ruleMatcher, $eventName, $event);
    $operation->onEvent($eventName, $event, $ruleMatcher);

`onEvent()` returns void, so a node has nowhere to put its output; it receives
an Event and an IRuleMatcher rather than data; control is inverted, with the
operation asking which rules matched instead of the engine deciding what runs;
and an operation is bound to an entity's events, which is a one-step rule
rather than a step that can sit anywhere in a graph.

So the contract is ours and everything else is Nextcloud's:

- `IFlowNode` mirrors IOperation's metadata methods verbatim and uses core's
  own SCOPE_* constants, adding the two IOperation lacks — validateConfig()
  and execute(items, config, context).
- `RegisterFlowNodesEvent` copies core's RegisterOperationsEvent pattern, so
  contributing a flow node is the same listener an app would write for
  Nextcloud Flow. Preferred over this repo's other discovery mechanism (the
  MCP provider info.xml + container-alias scan, which needs a cache with two
  invalidation paths) because apps announce a listener but do not announce a
  class to scan for.
- `RegistryStepDispatcher` means no consumer writes a dispatcher. A dispatcher
  with a type switch in it is half an engine.
- `SetFieldsNode` is the first built-in and the reference implementation.
- `RunFlowOperation` bridges the direction that works: a Nextcloud Flow rule
  can start a flow, which then does the branching, joining and data-passing
  core cannot. It does not run the flow inline — a Flow operation executes
  inside the dispatch of the event that triggered it, and an arbitrary graph
  does not belong on the critical path of a file write.

A duplicate node id is refused rather than overwritten, so a flow's behaviour
can never depend on app load order. An unknown type throws instead of being
skipped; a step with no type at all passes items through, which is the
routing-only edge, not leniency.

Also fixes FlowActionServiceTest's constructor call — it passed 7 of 9
arguments since the object-CRUD nodes landed, so all 12 of its tests errored
before reaching an assertion and the suite had been reporting nothing. The 9
behavioural failures underneath are filed as #2073 and deliberately not
papered over.

47 tests green, phpcs clean.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 3dcaa26

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 173/173
npm
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-07-24 13:08 UTC

Download the full PDF report from the workflow artifacts.

OpenRegister already bridges to Nextcloud Flow and has for some time:
`WorkflowEngine\RegisterObjectEntity` exposes object events as a Flow entity,
`WorkflowEngine\RunFlowOperation` starts a named flow when a rule matches, and
`FlowEngineRegistrationListener` registers both.

This change had added a second operation and a second registration. The
existing one is also the better of the two — it is an `ISpecificOperation`
bound to `RegisterObjectEntity`, so it is offered only on rules about
OpenRegister objects, where mine was offered on everything.

Writing a second implementation of something the app already had is exactly
the defect this programme exists to stop, committed inside the change meant to
prevent it. Removed: `lib/Flow/RunFlowOperation.php`,
`lib/Listener/NcFlowOperationListener.php`, and the duplicate listener
registration.

Repointing the existing bridge from `FlowActionService` at `FlowEngine` needs
the run queue and belongs with run persistence (#2076): a Flow operation runs
inside the dispatch of the event that triggered it, and a graph must not block
a file write.

Spec, proposal and tasks corrected to describe what exists rather than what
this change was going to add.
`phpcs` was red on `development` and had been for a while, which meant the gate
could not tell a new error from the standing ones. It is now 0 errors across
1041 files.

Real fixes:

- Two inline `?:` expressions in FlowActionService, which the standard rejects,
  rewritten as guards.
- A missing `@param` for `$objectService` (FlowActionService) and one for
  `$registerMapper` (RelationHandler) — both added when a collaborator was
  introduced and never documented.
- Named arguments on two internal calls in `WorkflowEngine/`.
- A lowercase inline comment in VocabularyImportService.
- The rest is `phpcbf` whitespace and alignment.

Three `\OC::$server->getRegisteredAppContainer()` call sites get a SCOPED
`phpcs:ignore` rather than a fix. The sniff says the accessor is removed in
Nextcloud 34; it is not — core itself calls it in
`lib/public/AppFramework/App.php:92`. Reaching another app's DI container has
no OCP equivalent (`\OCP\Server::get()` resolves the server container only), so
per-app provider discovery has to use it. The ignore names the one rule, so any
other legacy accessor still fails the gate.

Verified not to change behaviour: the two test files covering touched code
(RelationHandler, VocabularyImportService) report identically with and without
these changes — 79 tests, 234 assertions, 1 pre-existing error either way.
…never wrong

Closes the 9 behavioural failures (#2073). They were not regressions in
FlowActionService; they were caused by the constructor fix in this same branch.

`run()` matches a flow's trigger through `EventCatalogService::aliasesFor()`.
I had wired that collaborator in as a mock, and a mock returns `[]`, so every
flow was skipped before any action ran. That is why all nine failures shared
one shape: `createEvent`, `send`, `dispatchTyped` and `warning` were never
invoked at all.

`EventCatalogService` is a pure lookup over a class constant with no
collaborators of its own, so the test now uses the real one. Mocking a value
object buys nothing and, here, silently inverted the behaviour under test.

Worth recording: had I "fixed" the assertions to match observed behaviour when
they first went red, I would have deleted nine correct tests and shipped the
belief that a misconfigured agent action legitimately passes in silence.

All 59 tests in tests/Unit/Service/Flow green.
@rubenvdlinde
rubenvdlinde merged commit 0559b71 into development Jul 24, 2026
0 of 2 checks passed
@rubenvdlinde
rubenvdlinde deleted the feat/or-flow-nodes branch July 24, 2026 14:30
rubenvdlinde added a commit that referenced this pull request Jul 24, 2026
…ning (#2083)

* feat(mcp): discover tool providers by registration event, not by scanning

Two mechanisms in this app did the same job and the older one was the more
complex.

MCP discovery probes every installed app's info.xml, builds candidate container
aliases, resolves each through the container catching autoloader misses, and
caches the resolution map with two invalidation mechanisms — an app-list hash,
plus a clamped TTL because an app upgrade can add a provider without changing
the app list. Roughly 86 lines of probing plus a cache, to find nine providers.

All of it exists because it scans for something apps never announce. Apps do
announce a listener.

Nextcloud has two idioms and the split is the argument: when CORE owns a
registry it adds a `registerXProvider()` to IRegistrationContext — thirty-odd
of those, and only core can write them. When an APP owns a registry it
dispatches a typed event, which is what workflowengine does for operations,
checks and entities. OpenRegister is an app. That is already why the flow node
registry uses an event (#2074), and core ships no MCP of its own, so there is
nothing else upstream to conform to.

The announced path is collected FIRST; the alias scan still runs after it for
one release so the fleet is never broken mid-migration, and a provider whose
appId is already present is skipped so a migrated app is never collected twice.

Collection failure is logged, never fatal: an app with a broken listener should
cost its own tools, not remove MCP from the instance.

The scan and its cache come out in a follow-up, once the five apps that still
register by alias have migrated. Removing them here would break every one of
them.

* style(phpcs): restore the docblocks my splice separated from their methods

Inserting collectAnnouncedMcpProviders() ahead of collectPerAppMcpProviders()
left the alias-scan's docblock sitting above the NEW method and the old one
with none — phpcs caught both. Each method now carries its own again.

0 errors across 1052 files; 88 flow tests green.
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