Procházet zdrojové kódy

docs: design single-node re-execution for the workflow editor

fszontagh před 1 měsícem
rodič
revize
2f2aeb0dd0

+ 411 - 0
docs/superpowers/specs/2026-08-12-single-node-re-execution.md

@@ -0,0 +1,411 @@
+# Single-Node Re-Execution - Design
+
+Date: 2026-08-12
+
+## Goal
+
+Let somebody re-run one node of a finished execution - in place, in the
+editor, with the current draft config - without re-running the whole
+workflow. The motivating incident: a refactor broke a pipeline in three
+places, each hidden behind the last, and each fix was verified by re-running
+the entire workflow (minutes of image generation and a published blog post)
+to check what amounted to a one-line expression change. n8n calls this
+"execute node" or "pin data + re-run"; the ask here is the same shape.
+
+Six questions have no obvious answer and are treated in order: where the
+input comes from, which iteration, where the result goes, what to do about
+side effects, what "the fixed node" means, and what ships first.
+
+## What already exists
+
+Checked against the code before this design was written.
+
+### The stored execution record has no input
+
+`NodeExecutionResult` (`src/runner/workflow_engine.hpp:73-107`) does carry an
+`input` field, and `executeNode` populates it at
+`src/runner/workflow_engine.cpp:1412` (`result.input = input;`). But
+`ExecutionResult::toJson()` - the function that turns the in-memory result
+into the document written to the `executions` collection - drops it on the
+floor. `workflow_engine.cpp:271-298`:
+
+```cpp
+nr["nodeId"] = result.node_id;
+nr["status"] = nodeStatusToString(result.status);
+nr["startedAt"] = result.started_at;
+nr["finishedAt"] = result.finished_at;
+// Note: input is intentionally not stored to avoid data duplication
+// Each node's input can be reconstructed from upstream node outputs + connections
+nr["output"] = keep_full_outputs ? result.output : truncateLargeValues(result.output);
+nr["error"] = result.error;
+nr["retryCount"] = result.retry_count;
+if (result.loop_node_id) nr["loopNodeId"] = *result.loop_node_id;
+if (result.loop_iteration) nr["loopIteration"] = *result.loop_iteration;
+```
+
+`webui/src/api/workflows.ts:462-473` mirrors this exactly - the TypeScript
+`NodeExecution` type declares `input?: any` but the backend never sends it, so
+the field is always `undefined` in the editor today. This is a deliberate,
+documented decision in both languages, not an oversight: storing input was
+judged to duplicate the upstream node's output.
+
+That decision is precisely what blocks replay. The comment's premise -
+"reconstructed from upstream node outputs + connections" - is available for
+the *first* node the user wants to re-run, since the immediate upstream
+node's `output` **is** stored. It stops being available the moment
+reconstruction requires re-running expression evaluation, merging multiple
+upstream branches, or replaying loop-item slicing, none of which is
+persisted anywhere. Section 1 below covers what to do about this.
+
+### `loopNodeId` / `loopIteration` already identify one iteration
+
+Confirmed at `workflow_engine.cpp:292-297`. Absence of both fields means "not
+in a loop" - it is not a sentinel value
+(`workflow_engine.hpp:84-93`). Inside a loop body, node ids carry an
+`_iter_<n>` suffix in the in-memory `node_results` map
+(`workflow_engine.cpp:242-244`), and a plain-id "mirror" copy is filtered out
+of the persisted `nodeExecutions` array via `is_loop_mirror`
+(`workflow_engine.cpp:258-260`) so it never double-counts as an execution.
+The array is sorted by `(started_at, seq)` (`workflow_engine.cpp:246-269`),
+so multiple iterations of the same node appear as distinct, ordered entries,
+each carrying its own `loopIteration`. This is exactly what the ask needs:
+"this node, as it ran in iteration 4" is already an addressable record via
+`(nodeId, loopNodeId, loopIteration)`.
+
+### There is a working pattern for running one node in isolation - but not with real input
+
+`POST /api/v1/nodes/:type/options` (`src/webserver/api/node_controller.cpp:26-127`)
+already executes a single node outside of any saved workflow. It builds an
+inline, unpersisted one-node workflow document in-process
+(`node_controller.cpp:63-74`):
+
+```cpp
+nlohmann::json node;
+node["id"] = "options";
+node["type"] = node_type;
+node["name"] = "Options";
+node["config"] = config;
+node["position"] = {{"x", 0}, {"y", 0}};
+
+nlohmann::json inline_workflow;
+inline_workflow["name"] = "options:" + node_type;
+inline_workflow["nodes"] = nlohmann::json::array({node});
+inline_workflow["connections"] = nlohmann::json::array();
+inline_workflow["settings"] = nlohmann::json::object();
+```
+
+sends it via gRPC as `ExecuteWorkflowRequest.inline_workflow` with
+`wait_for_completion = true`, and the runner
+(`src/runner/runner_service.cpp:45-88`) parses it, stamps the real
+`workflow_id` onto it so credential/storage scoping still resolves, and runs
+it through the ordinary `engine_.execute(...)` - the same path a real
+workflow run takes. The inline workflow is never persisted
+(`runner_service.cpp:50-52`). The webserver then pulls the single
+`nodeExecutions` entry with `nodeId == "options"` back out of the returned
+`ExecutionResult` and hands its `output` to the caller
+(`node_controller.cpp:106-118`).
+
+The gap: this endpoint hardcodes `input = {}`
+(`node_controller.cpp:61-62`, "It runs with empty input, which is all a
+listing needs") and there is no request field for caller-supplied input. It
+solves "run this node type with this config" but not "run this node with
+this config against this input."
+
+One level down, `executeNode`
+(`src/runner/workflow_engine.hpp:319-323`, body from
+`workflow_engine.cpp:1405`) is the function both the normal graph walk
+(`workflow_engine.cpp:873`) and the loop-body walk
+(`workflow_engine.cpp:3089`, `executeLoopBody`) call per node. It takes an
+already-resolved `input` verbatim (`ctx.input = input;` at
+`workflow_engine.cpp:1509`) and does not touch `ExecutionResult::node_results`
+or any prior-node-output map - all input resolution happens in the caller,
+before `executeNode` is invoked. It needs a `Workflow` object only for `id`
+(storage/credential namespacing) and `settings` (storage permissions,
+retention) - not the real persisted workflow, as the inline-options path
+already proves by handing it a synthetic one-node `Workflow`. In short:
+**`executeNode` has no structural obstacle to being called with an arbitrary
+saved input outside of a full graph walk.** The inline-workflow-over-gRPC
+mechanism is the right scaffold to extend, not a new one to invent.
+
+`GET /api/v1/executions/:id/retry` exists as a route
+(`execution_controller.cpp:445-458`) but is an unfinished stub - it reads the
+old execution, does nothing with it, and always reports fake success. It is
+not a foothold for this feature; treat it as dead code to eventually
+implement or remove, out of scope here.
+
+### The editor holds one draft config, scoped to whichever modal is open
+
+There is no zustand store or persistent per-node draft abstraction. In
+`webui/src/pages/WorkflowEditorPage.tsx`, `editingConfig`
+(line 377, `useState<Record<string, any>>({})`) is seeded from the in-canvas
+ReactFlow node's current data when its config modal opens
+(`openNodeConfig`, lines 1901-1923, specifically `setEditingConfig(node.data.config || {})`
+at line 1918), lives only while that modal is open, and is written back into
+the canvas's `nodes` array on Save (`saveNodeConfig`, lines 2337-2362) - not
+yet to the backend; that is a separate, later "Save workflow" action gated by
+`hasChanges`. There is no ready-made "get current draft config for node X"
+call outside the modal's own lifecycle.
+
+### No node declares side effects
+
+Grepping the repo for `sideEffects` / `SideEffect` returns nothing. The
+parsed node metadata, `NodeDefinition`
+(`src/runner/node_registry.hpp:29-48`), has `category` (a free-text UI
+grouping tag, not a safety signal) and `is_trigger` (means "starts a
+workflow," not "safe to re-run") and nothing else resembling idempotency or
+danger. The module interface documented in `docs/nodes.md:7-12,59-70` is
+exactly `configSchema`, `inputSchema`, `outputSchema`, `execute` - no
+optional fifth field for this today.
+
+### Execution records are replaced whole, never patched
+
+`workflow_engine.cpp:2438-2494`: on persist, the engine does
+`storage_.get("executions", id)` to check existence, then either
+`storage_.upsert(...)` (refresh TTL on a re-paused Waiting execution) or
+`storage_.update("executions", result.execution_id, result.toJson())` -
+always the full document, rebuilt from the complete in-memory `node_results`
+map. There is no code path that patches one entry inside a stored
+`nodeExecutions` array. The one existing precedent for "the record changes
+after its first write" is a paused (`EXECUTION_STATUS_WAITING`) execution
+resuming and running more nodes - and even then the whole document is
+regenerated and replaced, not edited in place. There is no precedent at all
+for rewriting a record to claim something different happened than what was
+originally observed. That absence of precedent is itself an argument (see
+Question 3): grafting a re-run's result into the original record would be a
+new, unprecedented kind of mutation, not an extension of an existing one.
+
+## 1. Where does the input come from
+
+The stored record has no input (see above), so replay cannot simply read a
+field. Three options:
+
+**(a) Start persisting input.** Add `input` back to `toJson()`, symmetric
+with `output`, subject to the same `truncateLargeValues` treatment for large
+payloads. This makes replay exact - literally the JSON the node received,
+including anything upstream expression evaluation, branch merging, or loop
+slicing produced - and it costs nothing at execution time (the value already
+exists in `NodeExecutionResult::input`, per `workflow_engine.cpp:1412`; the
+change is at serialization). The "avoid duplication" comment undersells the
+cost, too: for most nodes, input is a reference to the immediately upstream
+node's output, so the duplication is a second copy of already-small JSON.
+Only nodes with unusually large input (e.g. a node fed a big prior output
+directly, or a loop item that assembled a large object) would see the
+truncation path trigger, exactly as output already does.
+
+**(b) Reconstruct at re-run time from upstream outputs + connections.** This
+is what the existing comment gestures at. It requires re-running the
+workflow's expression evaluator against the *stored* upstream outputs and
+the *saved* connection graph, entirely in the webserver or a new runner
+path, duplicating logic that today only exists inside the graph walk in
+`workflow_engine.cpp`. It breaks down for anything the walk resolved that
+isn't a pure function of upstream `output` values - loop-item slicing for a
+node inside a loop body is the clearest case, since a loop iteration's input
+is a slice of the loop's collection at a particular index, not something
+visible in any single upstream node's stored output as a self-contained
+value.
+
+**(c) Require the caller (the UI) to supply input explicitly**, sourced from
+whatever the `ExecutionResultsPanel` already has loaded (the upstream node's
+stored `output`, or the target node's own last `output` if the user is
+happy replaying "whatever ran last time" without re-deriving it). This is
+what n8n effectively does with "pinned data."
+
+Recommendation: **(a)**. It is the smallest change, it is exact rather than
+approximate, and it turns "which iteration" (Question 2) and "the fixed
+node" (Question 5) into simple record lookups instead of a second expression
+engine. (b) is real work for a weaker guarantee - it re-derives something
+that was already computed once and thrown away. (c) is a fallback worth
+keeping in the design as what the UI falls back to for *executions recorded
+before this change ships* (their records will never have `input`, since
+nothing rewrites history - see Question 3), but should not be the primary
+mechanism for new executions once (a) lands.
+
+Concretely: add `input` to `ExecutionResult::toJson()` at
+`workflow_engine.cpp:271-298` alongside `output`, add the corresponding
+field to `webui/src/api/workflows.ts`'s `NodeExecution` (already declared,
+just unused), and it flows through `GET /api/v1/executions/:id`
+(`execution_controller.cpp:327-343`) unchanged, since that endpoint already
+returns the whole stored document verbatim.
+
+## 2. Which iteration
+
+`(nodeId, loopNodeId, loopIteration)` already uniquely addresses one
+execution record, per the confirmation above. The re-run request should
+carry these three fields (loopNodeId/loopIteration omitted for a node not in
+a loop), the webserver looks up the matching entry in the stored
+`nodeExecutions` array of the given execution, and reads its `input` (once
+Question 1's change ships). The `ExecutionResultsPanel`, which the task
+states is already the surface where a user browses iterations, is the
+natural place to put a "Re-run this iteration" action per visible node
+entry - it already has the exact triple in hand for whatever row is being
+displayed.
+
+## 3. Where does the result go
+
+Not into the stored execution record. Two independent reasons converge here:
+the task's own framing (amending history makes the record untrustworthy,
+and this project had an incident today from exactly that kind of record
+asserting something unobserved), and the codebase's own precedent
+(Section "Execution records are replaced whole, never patched" above) - the
+system has never mutated a record to claim something different happened,
+and the one case that looks like mutation (a resumed Waiting execution)
+actually appends real new observations, it doesn't retcon old ones.
+
+The result of a re-run is a **scratch result**, not a history record. Its
+home and lifetime:
+
+- **Home**: a new, unpersisted response returned directly by the re-run
+  endpoint - not written to the `executions` collection at all. This
+  mirrors the existing `nodes/:type/options` pattern exactly (inline
+  workflow, `wait_for_completion`, read the one `nodeExecutions` entry back
+  out of the gRPC response, discard the rest) - nothing there touches
+  storage today, and this feature shouldn't start.
+- **Where it's shown**: the editor keeps it as local component state
+  attached to the node being re-run (comparable to how `editingConfig` is
+  scoped to whichever modal is open) - e.g. a "last re-run result" panel
+  next to or replacing the historical output view for that node, visibly
+  labeled as a re-run rather than folded into the historical output so a
+  user can never mistake it for what actually happened during the real
+  execution.
+- **Lifetime**: it lives exactly as long as the editor tab/session does.
+  Navigating away, refreshing, or closing the node's panel discards it.
+  This is honest about what it is - a verification aid, not a record - and
+  it avoids inventing a second persistence tier (a "scratch executions"
+  collection, TTL policy, cleanup job) for a feature that exists to answer
+  "did my one-line fix work" in the time it takes to click a button. If
+  usage shows people want to keep or compare re-run results across
+  sessions, that is a deliberate follow-up with its own retention design,
+  not a default.
+
+## 4. Side effects
+
+Three options were named in the task; take them in order.
+
+**Do nothing beyond a warning.** Cheapest, ships immediately, matches n8n's
+own choice. Cost: the warning is generic ("this node may have side
+effects") because nothing today lets a node say whether it has any, so
+either every re-run gets the same boilerplate warning (which people learn to
+click through, defeating the point) or the warning is skipped entirely,
+in which case it isn't really a warning.
+
+**Let a node declare `sideEffects: true`.** Requires: a new optional field
+on the module interface documented in `docs/nodes.md`, parsing it into
+`NodeDefinition` (`node_registry.hpp:29-48`) alongside `is_trigger`, and
+exposing it through `NodeDefinition::toJson()` to the editor. Cost is real
+but small - one field, one parse site, one serialization site - and it's the
+only option that lets the warning be specific ("Publish To Blog will publish
+again") instead of generic, and lets the UI visually distinguish
+side-effect-free nodes (most `set-fields`, `condition`, `code` nodes) from
+ones that aren't, which is exactly the distinction that would have let the
+user in the motivating incident re-run the condition node and the broken
+call node freely while getting a pointed warning only when they got near
+the loop's actual side-effecting step.
+
+**Dry-run.** Would require every node author to implement a second code
+path (`dryRun` alongside `execute`), or the runner to intercept and no-op
+specific known-dangerous operations (HTTP calls, storage writes) generically
+- neither is realistic given nodes are arbitrary JS running in QuickJS with
+direct access to `fetch`/storage/credential APIs (per `docs/nodes.md`).
+This is real work with no existing scaffold, and it silently produces wrong
+verification results for any node whose "dry" path diverges from its real
+path (e.g. a node whose real behavior depends on a live external response).
+Rule out for this feature.
+
+**Recommendation: declare-and-warn** (the second option), not "do nothing."
+The cost is one boolean field end to end - genuinely small next to the other
+two - and it directly serves the stated goal: letting someone re-run a
+condition or a call node with confidence while being stopped short of
+blindly re-running `Publish To Blog`. Node authors who don't set the field
+default to "unknown," which the UI should render as a mild, generic caution
+(not silence, not a blocking confirmation) - so the field is additive and
+nothing regresses for the 80-plus nodes that exist today until each is
+reviewed and tagged. Tagging existing nodes is a follow-up, not part of this
+design; it does not block ship.
+
+## 5. What counts as "the fixed node"
+
+Explicit design decision: **re-run uses the current editor draft config
+(`editingConfig`, whatever is in the modal right now, unsaved or not) with
+the recorded input from Question 1/2.** Not the config the workflow last
+saved, not the config from the original execution - the live draft, because
+the entire point is verifying an edit before committing it.
+
+Consequence worth stating plainly, because it's easy to get wrong: **the
+re-run does not touch the node's own input.** If the edit being tested would
+itself change what upstream nodes produce - because the node under test is
+also, say, a condition whose branch choice determines what an earlier node
+in the graph would have received - a single-node re-run cannot see that.
+Input is fixed at "what this node received last time"; only the node's own
+processing of that fixed input is being re-verified. This is exactly right
+for the motivating incident's second and third bugs (a stale-path
+expression, a reference to a deleted node) - both are properties of one
+node's own config against its recorded input - and it is exactly wrong for
+any question of the shape "would earlier nodes behave differently." That
+second question needs "re-run this node and everything downstream" (or
+upstream), which is out of scope - see Question 6.
+
+## 6. Scope
+
+**Ship:**
+- Persist `input` in `NodeExecutionResult`'s serialized form
+  (`workflow_engine.cpp:271-298`), threaded through to the TS type already
+  declaring it unused.
+- A new endpoint, e.g. `POST /api/v1/executions/:id/nodes/:nodeId/rerun`,
+  accepting `{ config, loopNodeId?, loopIteration? }`, looking up the
+  matching stored `nodeExecutions` entry for its `input`, and reusing the
+  inline-workflow-over-gRPC mechanism from `node_controller.cpp:26-127` -
+  same pattern, but with the looked-up `input` instead of hardcoded `{}`,
+  and the caller-supplied `config` instead of whatever the saved workflow
+  document has. Never writes to the `executions` collection.
+- Editor: a "Re-run this node" action wherever a node's result is already
+  shown per-iteration (`ExecutionResultsPanel`), using the currently open
+  `editingConfig` when the node's modal is open, falling back to the
+  canvas's last-saved config otherwise. Result renders as a clearly-labeled
+  scratch panel, kept only for the editor session (Question 3).
+- The `sideEffects` boolean on the node module interface and a generic
+  warning in the re-run UI when it's `true` or unset (Question 4). Tagging
+  existing nodes with it is explicitly follow-up work, not blocking.
+
+**Deliberately excluded:**
+- Re-running a node plus everything downstream of it. This is a different
+  and larger feature: it needs a resumable partial graph walk seeded with
+  one real result and continuing through the normal engine, decisions about
+  whether *that* continuation writes a real execution record or another
+  scratch result, and its own side-effect story multiplied across however
+  many downstream nodes are side-effecting. It shares scaffolding with this
+  design (the same `input`-persistence change, the same inline-execution
+  mechanism generalized to more than one node) but is not needed to solve
+  the motivating problem, where each of the three bugs was diagnosable and
+  fixable one node at a time.
+- Tagging the 80-plus existing nodes with `sideEffects`.
+- Dry-run of any kind.
+- A "keep this scratch result" / compare-across-sessions feature.
+- Fixing or implementing `POST /api/v1/executions/:id/retry`
+  (`execution_controller.cpp:445-458`) - unrelated stub, whole-execution
+  scope, left as-is.
+
+## Summary of file:line touch points for implementation
+
+- `src/runner/workflow_engine.hpp:73-107` - `NodeExecutionResult` (input
+  already present in memory).
+- `src/runner/workflow_engine.cpp:271-298` - `toJson()`, add `input` next to
+  `output`.
+- `src/runner/workflow_engine.cpp:319-323`, `1405` on - `executeNode`,
+  callable as-is with a saved input.
+- `src/runner/runner_service.cpp:45-88` - inline-workflow gRPC handling to
+  extend/reuse.
+- `src/webserver/api/node_controller.cpp:26-127` - the pattern to clone for
+  the new rerun endpoint.
+- `src/webserver/api/execution_controller.cpp:327-343` - existing detail
+  fetch, unchanged, now carries `input` once Question 1 ships.
+- `src/runner/node_registry.hpp:29-48` - `NodeDefinition`, add
+  `sideEffects`.
+- `docs/nodes.md:7-12,59-70` - module interface doc, add the optional
+  `sideEffects` field.
+- `webui/src/api/workflows.ts:462-473` - `NodeExecution` type, `input`
+  field already declared, wire it up.
+- `webui/src/pages/WorkflowEditorPage.tsx:377,1901-1923,2337-2362` -
+  `editingConfig`, source of "the fixed node"'s config for a re-run.
+- `webui/src/components/workflow/ExecutionResultsPanel.tsx` - where the
+  re-run action and its scratch result should surface, since it already
+  displays per-iteration node results.