Skip to content

feat(flow): execution mode and the run-level flow token - #2161

Merged
rubenvdlinde merged 5 commits into
developmentfrom
feat/flow-executionmode-and-token
Jul 27, 2026
Merged

feat(flow): execution mode and the run-level flow token#2161
rubenvdlinde merged 5 commits into
developmentfrom
feat/flow-executionmode-and-token

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Why

Two engine gaps blocked apps from moving their bespoke chaining onto the flow engine. Both surfaced while migrating OpenConnector (openconnector#1066) — the fifth of the six "flow" systems ADR-065 converts into consumers.

1. A trigger could only ever queue. FlowTriggerService::fire() called FlowRunService::queue() and nothing else. That's the right default, but OpenConnector's object-lifecycle synchronisation runs synchronously inside the triggering save today; migrating it onto a queue-only engine would silently turn a save-time guarantee into an eventually-consistent one. An app couldn't opt out, so the migration either changed behaviour or didn't happen.

2. A node could not write run-level state. The dispatcher contract is dispatch(step, items, context): array — items come back, $context goes in by value and nothing returns. So a value couldn't survive across steps except by riding the item list, which breaks the moment a node fans out or filters (exactly what FlowItems' "context is not the data channel" rule warns about).

What changed

  • executionMode on a flowasync (default, today's behaviour) or sync. A sync flow is queued then executed inline, so one persistence path means history/retry/resume treat it identically to a drained run. A sync flow that suspends is left for the worker; the inline call never blocks on a wait.
  • FlowToken at context['token'] — a mutable run-level value bag. An object handle survives the by-value array copy, so a node writes to it with zero change to the IFlowNode signature`. The ten registered nodes and both leaf apps (hermiq, openconnector) need no change at all.
  • Propagation + return — a sub-flow is seeded with its parent's values (not the parent's instance, so a fire-and-forget child can never mutate a parent that has already moved on); a waited-on child merges its values back, child winning on conflict.
  • Pause/continue-later comes freepersistResult() already writes context back on the SUSPENDED path and execute() restores it, so the token is just serialised/rehydrated there. No new column, no migration.

Design notes

  • executionMode rides the resolver document, not the flow object, so every store (OR-native, hermiq's, any future IFlowResolver) carries it through one seam. Worth flagging: cron is not precedent hereFlowScheduleService reads it straight off the OR object (scheduleOf(ObjectEntity)), which only works for the OR-native store.
  • runInline() is deliberately best-effort: an unresolvable flow or subject leaves the run queued for FlowRunWorker, which already owns the full defensive failure handling. Duplicating those semantics would give the engine two places deciding what a broken run means, and they'd drift.

Backwards compatibility

Absent executionModeasync. Absent/malformed stored token ⇒ empty token. Existing persisted runs resume normally.

⚠️ The flow schema gains executionMode (version 1.1.0 → 1.2.0). Per openregister#2075 a shipped schema does not gain properties on re-import, so existing installs need the property added to the live schema — the same footnote that applied to cron in #2126.

Verification

  • 21 new unit tests (token semantics, by-value write-through, serialise round trip, sub-flow seed/isolation/return/conflict, all five execution-mode branches incl. a throwing sync flow not escaping the trigger)
  • 175/175 green across the full tests/Unit/Service/Flow/ suite — the change is signature-neutral
  • PHPCS 0 errors on all 6 changed lib/ files
  • openspec validate --strict passes (two spec deltas: flow-execution-mode, flow-token)

Live verification on 8080 (task 4.4) is still open.

Unblocks

openconnector-flow-migration Phases 2 and 3, and retires the app-local FlowToken helper OpenConnector carries today.

Two engine gaps blocked apps from moving bespoke chaining onto the flow engine,
both found while migrating OpenConnector (openconnector#1066).

1. A trigger could only ever queue. `FlowTriggerService::fire()` called
   `queue()` and nothing else, so an app whose work runs synchronously inside a
   save (OpenConnector's object-lifecycle sync) could not migrate without
   silently becoming eventually-consistent. Flows now declare
   `executionMode: async|sync`; `sync` runs inline within the triggering call.

2. A node could not write run-level state. `dispatch(step, items, context)`
   returns items only, so a value could not survive ACROSS steps except by
   riding the item list — which fan-out and filtering destroy. A new `FlowToken`
   sits at `context['token']`: an object handle survives the by-value array
   copy, so a node writes to it WITHOUT any change to the `IFlowNode` signature.
   Ten registered nodes and both leaf apps are untouched.

The token propagates into sub-flows (seeded as values, so a fire-and-forget
child can never mutate a parent that moved on) and a waited-on child merges its
values back, child winning on conflict. It survives pause/continue-later for
free: `persistResult()` already writes context on the SUSPENDED path and
`execute()` restores it, so the token is serialised/rehydrated there — no new
column, no migration.

`executionMode` rides the resolver document rather than the flow object, so
every store (OR-native, hermiq's, any IFlowResolver) carries it through one
seam. Note `cron` is NOT precedent: FlowScheduleService reads it straight off
the OR object, which only works for the OR-native store.

Backwards compatible: absent mode ⇒ async, absent token ⇒ empty token.

- 21 new unit tests; 175/175 full flow suite green; phpcs 0 errors on lib/
- openspec change with two spec deltas; validate --strict passes
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 8a9460c

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

Quality workflow — 2026-07-27 09:49 UTC

Download the full PDF report from the workflow artifacts.

Four api-direct specs, all passing against the live instance:

- a `sync` flow is ALREADY terminal when the triggering request returns
- an `async` flow is still `queued` at that moment (the contrast is the proof)
- a flow declaring no mode keeps the queued default
- a run persists its token through the real JSON column

The sync/async assertions are timing-free — they compare state at the instant
the create response comes back — so they cannot flake on worker cadence.

Also adds `playwright.flow.config.ts`: the main config excludes api-direct from
the chromium project (those run via Newman), so running one needs a config that
targets them. Two things it has to get right, both of which cost real debugging:
Basic auth must be sent PREEMPTIVELY (httpCredentials only answers a 401
challenge, and OpenRegister does not challenge — it serves the request as
Anonymous, whose RBAC filters everything, so the symptom is a cheerful
`200 {"results":[]}`), and `Accept: application/json` must be set or Nextcloud
answers with login HTML.

The spec resolves the flow schema THROUGH the flows register rather than by
global slug: `flow` is not unique — nine apps ship a schema with that slug — so
a global lookup lands on another app's schema and every write 400s.

Live verification also surfaced a real cross-app defect: HermiqFlowResolver
claimed every OR flow and dropped executionMode (hermiq#53).
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

✅ Live-verified — Playwright e2e 4/4 passing (task 4.4 closed)

Pushed tests/e2e/api-direct/flow-executionmode-and-token.spec.ts + playwright.flow.config.ts.

✓ a sync flow has already run when the triggering request returns   (28.8s)
✓ an async flow is still queued when the triggering request returns (24.3s)
✓ a flow that declares no mode keeps the queued default             (1.1m)
✓ a run persists a flow token through the real database             (17.8s)
  4 passed (3.4m)

The sync/async assertions are timing-free — they compare run state at the instant the create response returns, so they can't flake on worker cadence. The contrast between the first two tests is what actually proves inline execution.

🔴 Live testing caught a real cross-app defect — hermiq#53

The sync test failed at first: the flow ran async anyway, with nothing logged. Root cause was not in this PR:

BEFORE:  HermiqFlowResolver       -> ANSWERS keys=id,nodes,edges,limits      ← wins, wrong owner
         OpenRegisterFlowResolver -> ANSWERS keys=id,nodes,edges,limits,executionMode

HermiqFlowResolver was claiming every flow on the instance, including flows in OpenRegister's own store, because ObjectService::find()'s register/schema arguments do not scope the lookup — a uuid resolves cross-table and returns whatever carries it:

find(id: <OR flow uuid>, register: 'hermiq', schema: 'agentflow')
  -> ObjectEntity(register: 2487, schema: 5077)   // OpenRegister's flow store

Since FlowResolverRegistry takes the first non-null answer, hermiq's over-claim stopped OR's own resolver from ever being asked, and every field hermiq doesn't copy — executionMode, and anything added later — was silently dropped. Fixed in ConductionNL/hermiq#53 (resolver now verifies ownership). After that fix hermiq declines, OR answers, and the e2e goes green.

Worth flagging for a follow-up on this side: ObjectService::find()'s register/schema params look like scoping and behave like hints. Any other caller trusting them has the same latent bug.

Note for reviewers

Every unit test on both sides passed throughout — each app's mocks were internally consistent. Only the live run caught it, which is exactly the "green but dead" class this kind of e2e exists to find.

Drives openconnector's source-call and synchronization-run leaves through the
engine rather than through openconnector internals, because that seam is where
a leaf's contract is actually observable: a leaf can unit-test green and still
be invisible to the registry, be claimed by another app's resolver, or refuse
every config the palette can produce. All three have happened in this codebase
(hermiq#53 was exactly the second).

Asserts the failure that matters most — a step pointed at a synchronization
that does not exist fails loudly instead of returning an empty item list, which
would make a broken step indistinguishable from an empty source.

The registry assertion is skipped for a real reason, stated in the spec:
OpenRegister exposes NO node-palette endpoint. FlowNodeRegistry::palette() is
in-process only and FlowController exposes just eventCatalog(), so nothing over
HTTP can enumerate node types and a flow-authoring UI cannot discover leaves.

3 passed, 1 skipped against the live instance.
OpenConnector chains synchronizations two ways of its own (a `synchronization`
rule and a `followUps` entry), and both re-enter synchronize() with no cycle
guard — A -> B -> A recursed until the process died (fixed in openconnector#1073).

The migration's claim is that a flow expresses the same chaining better. This
spec tests that claim where it is observable — the engine — rather than
asserting it in prose:

- a multi-step chain runs in DECLARED ORDER and carries an inspectable trace
- a self-referencing chain TERMINATES instead of recursing forever (asserted on
  termination, not on a specific error: any bounded outcome is correct, hanging
  is not)
- every hop is PERSISTED as its own queryable run

Those three are exactly what recursive followUps cannot offer, which is the
whole argument for moving the chaining onto the engine.

3 passed against the live instance.
@rubenvdlinde
rubenvdlinde merged commit ae4af65 into development Jul 27, 2026
1 of 2 checks passed
@rubenvdlinde
rubenvdlinde deleted the feat/flow-executionmode-and-token branch July 27, 2026 20:59
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