feat(flow): openregister.map, own the Twig engine, and the sync-decomposition design (#2313)

* feat(flow): add openregister.map so a flow can transform data mid-walk

The engine had no node that transformed data, so any flow needing to reshape a
payload had to route it out to an endpoint rule and back. That is why
integrations ended up expressed as endpoint chains — not because endpoints
fitted better, but because the flow engine could not map.

The node maps PER ITEM, like FilterNode, so one authored mapping reshapes a
collection without the author drawing a loop. It resolves a mapping by numeric
id, uuid, slug or reference: a flow definition is portable between instances
where the numeric id differs, so an exported flow that resolved only by id would
break on import while looking correct.

An unresolvable mapping FAILS the step. Returning the items unchanged would
record a completed step that transformed nothing, and the error would then
surface at some later step reading the un-mapped shape, far from its cause.

Verified: live in /api/flow/node-catalog (18 nodes), and 8 unit tests. The
fail-closed test was confirmed by mutation — patching the node to return items
on an unresolved mapping makes exactly that test fail, and nothing else.

Also corrects the proposal against the RUNNING catalogue rather than a reading
of the code. Two claims in it were wrong: OpenConnector does contribute nodes
(source-call, synchronization-run), so `synchronization` already had one; and
the rule dispatcher has 23 types, not 18. The remaining gap is still thirteen,
but its membership is different from what was written.

* feat(mapping): own the Twig engine; let apps contribute what only they can

Moves mapping's Twig surface into OpenRegister and gives other apps a way to
add the functions OpenRegister cannot provide for itself.

Ported from OpenConnector's copy, so mappings authored there keep evaluating:
createSlug, json_decode, b64enc/b64dec. createSlug is copied byte-for-byte on
purpose — mappings persist slugs as object identifiers, so changing the
transformation would silently orphan previously-written objects.

`json_decode` AND `jsonDecode` are both registered. OpenConnector's templates
call the first, OpenRegister's runtime spelled it the second. A mapping is
authored data, so renaming the function it calls breaks it at evaluation time
with nothing at author time to warn you.

RegisterMappingFunctionsEvent is the contribution point, same shape as
RegisterFlowNodesEvent. registerFunction() adds the function AND allowlists it
in one call, because the environment is sandboxed: a contributed function that
is not allowlisted fails as "unknown function" deep inside a mapping, nowhere
near where it was registered.

Collection is best-effort — a missing app or a throwing listener must not stop
mappings evaluating, since the engine has to work on an instance where nothing
else is installed.

Verified live through the sandboxed engine:
  createSlug("Hello World App") -> "hello-world-app"
  json_decode('{"a":1}')["a"]   -> 1
  b64enc("hi")                  -> "aGk="
and the negative control still holds — system() and an unlisted filter are both
refused, so the sandbox was widened deliberately, not disabled.

* docs(flow): design for decomposing the sync monolith, and first-class iteration

openconnector.synchronization-run is the last monolith in the catalogue: one
node that runs an entire synchronisation. The run history records it as a single
step, so which page failed and which record was skipped are not queryable — the
exact ambiguity the step table was built to remove.

Four capabilities are trapped inside it with no node equivalent: pagination,
hash change-detection, synchronisation-contract resolution, and the contracted
write. The contract one is load-bearing: without it a flow-built sync is not
idempotent, so a second run duplicates rather than updates. That is why the
monolith still has to be used whole, and why it is deprecated rather than
deleted by this change — deleting it first would strand every existing sync.

The design decision is how to model iteration. Three options, written up in
design.md:

A. Cycle in the graph — what the engine does today. Executes correctly, rejected
   as the AUTHORING model: a back-edge looks identical to a forward edge, so the
   most important fact about the graph is carried by edge direction; loop
   membership is inferred rather than declared; the bound is a whole-run ceiling
   shared by every loop; and non-convergence is diagnosed after the side effects.
B. Sub-flow per iteration — rejected. Forces a one-loop-per-flow split unrelated
   to how the author thinks about the work, and fragments run history across
   runs, reintroducing the ambiguity we just removed.
C. A declared loop REGION — chosen. The loop is a node that owns its body, so
   membership is data. The builder can then draw it as a container rather than
   an edge that happens to point backwards, bounds are per-loop and authored,
   non-convergence becomes a save-time validation error, and each body step
   records its iteration index so "page 7 failed" is a query.

Also records why publiccode is the right first example rather than a toy: it
needs a PAT for rate limits, and the fleet's credential shape means the flow
must never hold one. doriath stores the secret zero-knowledge; OpenRegister's
`github` credential is a HOST-LOCKED PROXY whose resolveInjectable() returns
null — a routing signal meaning "use request()", not a denial. So the harvest
needs openregister.broker-call, which asks the broker to make the call
server-side. Integrating correctly with doriath means never fetching the token.

openregister.loop is renamed in the palette to "Batch items" (it batches, it
does not loop) but KEEPS its id — stored flow definitions reference it, and
renaming an id breaks authored data. Same reasoning that kept both json_decode
and jsonDecode when mapping consolidated.

* docs(flow): correct two design calls — rename the id, and keep credentials on the Source

Both corrections came from review and both reverse what I had written.

**openregister.loop gets a NEW ID, not just a new label.** I had kept the id on
the grounds that stored flow definitions reference it. Wrong trade: a node whose
id says `loop` and whose behaviour is `batch`, sitting next to a real `iterate`,
is a trap that re-arms every time someone new reads the catalogue. Unlike a Twig
function name — which a person typed into a mapping template we cannot safely
rewrite — a node id is a reference the system writes and can rewrite. So it
becomes `openregister.batch`, stored flows are MIGRATED, and the old id stays a
resolvable alias for one release so a flow exported before and imported after
still resolves. The alias logs when used, so the tail of un-migrated definitions
is visible rather than assumed empty.

**No openregister.broker-call.** I had proposed a second node for calls using a
brokered credential. That exposes an implementation detail as a modelling
choice: from the author's chair both nodes call a configured source, and picking
correctly requires knowing which credential SHAPE the source carries — which is
exactly what the broker exists to hide. Choosing wrong yields "resolveInjectable
returned null", which reads as a permission problem and is not one.

One node instead. A Source may reference a doriath-held credential and resolves
it by shape: injectable is attached to the request as now, host-locked is handed
to OpenRegister's broker to perform server-side. The author configures a
credential and calls the source. The brokering stays under the waterline, and
the property that matters is unchanged — the token is never handed to the flow.

* test(mapping): assert Twig functions by NAME, not by count

testGetFunctionsReturnsArray asserted exactly 2 functions and broke the moment
mapping consolidated (7 now). The count told us a number had changed — not
whether anything was MISSING, which is the only question that matters here: a
stored mapping calls these by name, so losing one breaks authored data at
evaluation time with nothing at author time to warn you.

Now asserts each expected name is present, and adds a case pinning json_decode
as reachable BOTH ways: OpenConnector's templates call `json_decode(x)`,
OpenRegister's use `x|json_decode`. Consolidation kept both forms rather than
picking one, and nothing was testing that.

12 tests, 67 assertions.

* style: clear phpcs/phpmd on the mapping-consolidation surface

All introduced by this branch; none pre-existing.

- phpcs spacing across the new event, MapNode and MappingService docblocks.
- MappingRuntime: TooManyPublicMethods suppressed with the reason — a Twig
  runtime's public methods ARE the vocabulary templates may call, so the count
  is the size of that vocabulary. Splitting it would mean two runtimes and a
  rule about which functions live where, a distinction template authors would
  have to know and could not see.
- json_decode: CamelCaseMethodName suppressed. The snake_case name is not a slip
  and cannot be corrected — it is the identifier stored templates already
  contain.
- MapNode: StaticAccess (FlowItems::item is the item constructor every node
  uses) and UnusedFormalParameter ($context is part of the IFlowNode contract;
  a mapping transforms the item, not the run), matching ExplodeNode's reasoning.

Left alone: GenericStoreService's inline-IF, which this branch does not touch.

84 tests / 169 assertions green across Twig and MapNode.

* feat(flow): openregister.iterate — a declared loop region

The engine could already loop: it is a Petri net, so an edge drawn backwards is
a cycle. That is an execution capability, not an authoring one. A back-edge
looks identical to a forward edge, so the most important fact about a graph —
that a region repeats — was carried by edge DIRECTION. Loop membership was
inferred by tracing rather than declared, the bound was a whole-run ceiling
shared between every loop, and non-convergence was diagnosed only after the
side effects had happened.

Here the loop OWNS its body, so membership is data: a builder can draw the
region as a container, the bound belongs to the loop that overran so the error
names it, and each body step runs with its iteration index in scope.

Termination is deliberately ONE rule: stop when the source returns no items.
Pagination falls out of that without a second concept — a page past the end is
empty — and `context['iteration']` carries the index so the source can ask for
the right page.

Two behaviours worth stating because getting either wrong is silent:
- Items ACCUMULATE across passes. Returning only the final batch would discard
  every earlier page while still reporting success.
- A non-converging loop FAILS by default. `onLimit: stop` is available but must
  be chosen; the default cannot be "quietly keep going and then quietly stop".

Validation refuses a sourceless loop, an empty body, a typeless body step and a
non-positive limit — at SAVE time, because a loop that cannot terminate is the
one authoring mistake whose cost is paid in side effects.

The dispatcher is resolved from the container at execute time, not injected:
this node lives in the registry the dispatcher reads from, so constructor
injection would close a cycle mid-population.

11 tests, and both mutations caught: returning only the last batch fails 2
tests, never failing on overrun fails 1.

* feat(flow): rename loop->batch with migration; split iterate validation

**openregister.loop becomes openregister.batch.** It never looped — it splits
items into fixed-size batches — and next to the new openregister.iterate the old
name was a trap that re-armed for every new reader.

The id changes, not just the label. A node id is a reference the SYSTEM writes
into a flow definition, unlike an identifier a person typed into a template, so
it can be corrected and the data rewritten. Version1Date20260804000000 rewrites
the quoted id in the nodes and edges of every stored flow and reports the count.
The registry keeps a LOGGED alias for one release, covering the one case the
migration cannot reach — a flow exported before the rename, imported after — so
the size of that tail is observable rather than assumed to be zero.

The migration matches the quoted id as text rather than walking the structure:
an exact quoted match cannot hit a prefix, and decode/re-encode would risk
reordering keys in definitions it has no business touching.

Also: IterateNode::validateConfig split into assertSource/assertBody/
assertBounds (was CC 11 / NPath 216), and the flow-iteration spec written — the
@SPEC anchors on the new code pointed at a file that did not exist yet, which
PHPCS would have accepted and gate-46 would not.

Merged development in, which brought AppHost. Fixed the inline-IF phpcs failure
in GenericStoreService while there.

NOT fixed, and pre-existing: tests/Unit/AppHost has 3 errors + 7 failures on
development. Verified by reverting my GenericStoreService change and re-running
— identical counts, so none of it is mine. Left for whoever owns that work
rather than absorbed silently into this PR.

Author: rubenvdlinde

appinfo/info.xml

@@ -40,7 +40,7 @@ Open Register drijft apps zoals OpenCatalogi, Procest, Pipelinq en Software Cata
 
 Vrij en open source onder de EUPL-licentie.
     ]]></description>
-    <version>0.2.17-unstable.22</version>
+    <version>0.2.17-unstable.23</version>
     <licence>EUPL-1.2</licence>
     <author mail="info@conduction.nl" homepage="https://www.conduction.nl/">Conduction</author>
     <namespace>OpenRegister</namespace>

lib/AppHost/Service/GenericStoreService.php

@@ -292,9 +292,17 @@ private function decodeBody(StoreDescriptor $descriptor, string $body): array
         }
 
         // Some OpenRegister responses are a bare list; accept that too.
+        // A non-list decode is treated as no results rather than passed through:
+        // callers iterate this, and handing them an associative array would
+        // iterate its VALUES as if they were records.
+        $results = [];
+        if (array_is_list($decoded) === true) {
+            $results = $decoded;
+        }
+
         return [
             'outcome' => self::OUTCOME_OK,
-            'results' => (array_is_list($decoded) === true ? $decoded : []),
+            'results' => $results,
         ];
 
     }//end decodeBody()

lib/Listener/FlowNodeRegistrationListener.php

@@ -30,6 +30,8 @@
 
 use OCA\OpenRegister\Service\Flow\Nodes\ExplodeNode;
 use OCA\OpenRegister\Service\Flow\Nodes\FilterNode;
+use OCA\OpenRegister\Service\Flow\Nodes\IterateNode;
+use OCA\OpenRegister\Service\Flow\Nodes\MapNode;
 use OCA\OpenRegister\Service\Flow\Nodes\FlowStateNode;
 use OCA\OpenRegister\Service\Flow\Nodes\LoopNode;
 use OCA\OpenRegister\Service\Flow\Nodes\MergeNode;
@@ -68,6 +70,8 @@ class FlowNodeRegistrationListener implements IEventListener
      * @param ObjectWriteNode $objectWrite The built-in "Write an object" node.
      * @param ObjectReadNode  $objectRead  The built-in "Read objects" node.
      * @param FlowStateNode   $flowState   The built-in "Flow state" node.
+     * @param MapNode         $map         The built-in "Map" node.
+     * @param IterateNode     $iterate     The built-in "Repeat until done" node.
      */
     public function __construct(
         private readonly SetFieldsNode $setFields,
@@ -82,7 +86,9 @@ public function __construct(
         private readonly RouterNode $router,
         private readonly ObjectWriteNode $objectWrite,
         private readonly ObjectReadNode $objectRead,
-        private readonly FlowStateNode $flowState
+        private readonly FlowStateNode $flowState,
+        private readonly MapNode $map,
+        private readonly IterateNode $iterate
     ) {
 
     }//end __construct()
@@ -115,6 +121,8 @@ public function handle(Event $event): void
         $event->registerNode(node: $this->objectWrite);
         $event->registerNode(node: $this->objectRead);
         $event->registerNode(node: $this->flowState);
+        $event->registerNode(node: $this->map);
+        $event->registerNode(node: $this->iterate);
 
     }//end handle()
 }//end class

lib/Migration/Version1Date20260804000000.php

@@ -0,0 +1,146 @@
+<?php
+
+/**
+ * Rewrites the renamed `openregister.loop` node id in stored flow definitions.
+ *
+ * The node never looped — it splits items into fixed-size batches — and sitting
+ * next to the real `openregister.iterate` the old name was a trap that re-armed
+ * for every new reader. So it became `openregister.batch`.
+ *
+ * A node id is a reference the SYSTEM writes into a flow definition, which is
+ * what makes correcting it different from correcting a Twig function name: a
+ * template is typed by a person and cannot be safely rewritten, a flow's node
+ * list is a JSON structure we own end to end. So the id is fixed and the data
+ * migrated, rather than the wrong name being kept forever.
+ *
+ * The registry keeps a logged alias for one release, covering the one case this
+ * migration cannot reach: a flow exported before the rename and imported after.
+ *
+ * SPDX-FileCopyrightText: 2026 Conduction B.V. <info@conduction.nl>
+ * SPDX-License-Identifier: EUPL-1.2
+ *
+ * @category Migration
+ * @package  OCA\OpenRegister\Migration
+ *
+ * @author    Conduction Development Team <dev@conduction.nl>
+ * @copyright 2026 Conduction B.V.
+ * @license   EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12
+ *
+ * @link https://OpenRegister.app
+ *
+ * @spec openspec/changes/flow-sync-decomposition/specs/flow-iteration/spec.md
+ */
+
+declare(strict_types=1);
+
+namespace OCA\OpenRegister\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\IDBConnection;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+/**
+ * Migrates `openregister.loop` to `openregister.batch` in stored flows.
+ */
+class Version1Date20260804000000 extends SimpleMigrationStep
+{
+
+    /**
+     * The old node id.
+     *
+     * @var string
+     */
+    private const OLD_ID = 'openregister.loop';
+
+    /**
+     * The corrected node id.
+     *
+     * @var string
+     */
+    private const NEW_ID = 'openregister.batch';
+
+    /**
+     * The database connection.
+     *
+     * @var IDBConnection
+     */
+    private IDBConnection $db;
+
+    /**
+     * Constructor.
+     *
+     * @param IDBConnection $db The database connection.
+     */
+    public function __construct(IDBConnection $db)
+    {
+        $this->db = $db;
+
+    }//end __construct()
+
+    /**
+     * Rewrite the node id in every stored flow definition.
+     *
+     * Operates on the `nodes` and `edges` JSON as TEXT, matching the quoted id
+     * exactly. A structural walk would be more elegant and is not worth it here:
+     * the id appears only as a JSON string value, an exact quoted match cannot
+     * hit a prefix (`openregister.loopback` would not match `"openregister.loop"`),
+     * and decoding/re-encoding every definition risks reordering keys or losing
+     * numeric precision in flows this migration has no business touching.
+     *
+     * @param IOutput $output        Migration output.
+     * @param Closure $schemaClosure Schema closure returning an ISchemaWrapper.
+     * @param array   $options       Migration options.
+     *
+     * @return void
+     *
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     *
+     * @spec openspec/changes/flow-sync-decomposition/specs/flow-iteration/spec.md
+     */
+    public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void
+    {
+        $schema = $schemaClosure();
+        if ($schema->hasTable('openregister_flows') === false) {
+            $output->info('No flow table yet; nothing to rename.');
+            return;
+        }
+
+        $needle      = '"'.self::OLD_ID.'"';
+        $replacement = '"'.self::NEW_ID.'"';
+        $touched     = 0;
+
+        $qb = $this->db->getQueryBuilder();
+        $qb->select('id', 'name', 'nodes', 'edges')
+            ->from('openregister_flows');
+
+        $result = $qb->executeQuery();
+        $rows   = $result->fetchAll();
+        $result->closeCursor();
+
+        foreach ($rows as $row) {
+            $nodes = (string) ($row['nodes'] ?? '');
+            $edges = (string) ($row['edges'] ?? '');
+
+            if (str_contains($nodes, $needle) === false && str_contains($edges, $needle) === false) {
+                continue;
+            }
+
+            $update = $this->db->getQueryBuilder();
+            $update->update('openregister_flows')
+                ->set('nodes', $update->createNamedParameter(str_replace($needle, $replacement, $nodes)))
+                ->set('edges', $update->createNamedParameter(str_replace($needle, $replacement, $edges)))
+                ->where($update->expr()->eq('id', $update->createNamedParameter((int) $row['id'])));
+            $update->executeStatement();
+
+            $touched++;
+            $output->info(
+                sprintf('Flow "%s": %s -> %s', (string) ($row['name'] ?? $row['id']), self::OLD_ID, self::NEW_ID)
+            );
+        }//end foreach
+
+        $output->info(sprintf('Node rename: %d flow definition(s) updated.', $touched));
+
+    }//end postSchemaChange()
+}//end class

lib/Service/Flow/FlowNodeRegistry.php

@@ -49,6 +49,28 @@ class FlowNodeRegistry
      */
     private array $nodes = [];
 
+    /**
+     * Node ids that were corrected, mapped old => new.
+     *
+     * A node id is a reference the SYSTEM writes into a flow definition, unlike
+     * a Twig function name which a person types into a template — so unlike
+     * those, an id can be corrected and the stored data migrated. A migration
+     * rewrites existing rows; this alias covers the tail the migration cannot
+     * reach: a flow exported before the rename and imported after it.
+     *
+     * Resolving through here is LOGGED, so the size of that tail is observable
+     * rather than assumed to be zero. The alias is removed one release after the
+     * rename.
+     *
+     * @var array<string, string>
+     */
+    private const RENAMED = [
+        // Renamed because it never looped: it splits items into fixed-size
+        // batches. Sitting next to the real `openregister.iterate`, the old name
+        // was a trap that re-armed for every new reader.
+        'openregister.loop' => 'openregister.batch',
+    ];
+
     /**
      * Whether contribution has already been collected this request.
      *
@@ -213,6 +235,21 @@ public function palette(int $scope=IManager::SCOPE_ADMIN): array
     public function get(string $type): IFlowNode
     {
         $this->load();
+
+        if (isset($this->nodes[$type]) === false && isset(self::RENAMED[$type]) === true) {
+            $this->logger->info(
+                message: sprintf(
+                    '[FlowNodeRegistry] Flow node "%s" was renamed to "%s"; resolving via the '
+                    .'compatibility alias. A flow definition still references the old id.',
+                    $type,
+                    self::RENAMED[$type]
+                ),
+                context: ['file' => __FILE__, 'line' => __LINE__]
+            );
+
+            $type = self::RENAMED[$type];
+        }
+
         if (isset($this->nodes[$type]) === false) {
             throw new UnexpectedValueException(
                 sprintf('No app provides the flow node type "%s". Is the app that owns it installed and enabled?', $type)