fix(workflow): preserve saved workflow outputs and metadata

[JPPhoto]

Summary

Saved-workflow execution now preserves child progress routing, returned images, workflow metadata, and graph semantics across queue submission, event delivery, preview replay, and recall.

• Child invocation events preserve queue ancestry and route progress to the visible Call Saved Workflow source node, including nested calls.
• Root call completion owns the visible call node's terminal state and returned thumbnail; child terminal events cannot overwrite it.
• Queue result routing recognizes direct images, collections, tagged output wrappers, legacy wrappers, and every named workflow_return value without walking arbitrary metadata or input fields.
• Queued output images retain recallable invokeai_workflow metadata through the real image service and disk-storage path.
• Saved-workflow invalidation retries and detail fetches are bounded and deduplicated.
• Legacy loop_linkage edges survive copy/paste, node migration, graph compilation, and workflow serialization.

Related Issues / Discussions

• Follow-up to fix(webv2): support dynamic saved workflow fields #274.
• Closes [bug]: Follow-up to PR 274 - saved workflow regressions #303.

QA Instructions

Focused follow-up validation:

• Webv2 output extraction, submission routing, mapper routing, node execution, and coordinator tests: 109 passed.
• Webv2 TypeScript check: passed.
• Legacy node-update and React Flow tests: 10 passed with no type errors.
• Backend event-builder and workflow-call metadata tests: 36 passed.
• Existing PR validation also passed: backend tests (221), webv2 Vitest (455), browser tests (12), legacy-web tests (22), lint:tsc, architecture:check, performance build, Ruff, and byte-identical OpenAPI regeneration.

Manual behavior:

1. Run a call-only or nested saved workflow.
2. Confirm child progress appears on the visible Call Saved Workflow node; child completion/error events do not replace root terminal output.
3. Confirm returned images appear in queue results and Gallery, including return keys named image, collection, or values; confirm no-image returns clear stale thumbnails.
4. Confirm queued PNG metadata contains recallable parent invokeai_workflow data through ImageService and disk storage.
5. Confirm loop-linkage edges survive editing, copy/paste, graph compilation, and workflow serialization.

Review

The PR-298 follow-up review resolved findings 1-12: contract-aware output extraction, nested-input exclusion, direct event-builder coverage, root-owned terminal state, end-to-end metadata storage coverage, workflow JSON caching, request-settling stabilization, redundant-test cleanup, output-adapter boundary documentation, bounded preview gates, and legacy loop_linkage handling.

The final review leaves no merge blockers in the implementation. Separate self-review passes covered correctness/contracts, lifecycle and architecture/performance, and test/product quality; independent review subagents were unavailable.

Compatibility / Rollout

• Event and preview routing fields remain optional for existing consumers and legacy queue snapshots.
• Existing saved workflows and queue snapshots remain readable; no database migration is required.
• Generated legacy API artifacts remain synchronized.
• No invocation input/output contract changed, so no invocation version bump is required.
• The performance baseline records the current initial source graph. The higher browser timings are recorded-only: timing enforcement is disabled and the run is not marked stable; no performance gate was weakened.
• Images produced inside called workflows intentionally carry the parent workflow in invokeai_workflow and the child execution graph in invokeai_graph. Webv2 recalls the parent workflow because it reproduces the image; legacy graph fallback and direct PNG readers can still observe the child graph.

Checklist

• The PR has a short but descriptive title, suitable for a changelog
• Meaningful regression coverage added / updated where needed; obsolete tests/code removed
• Persisted-state and API changes include required migrations / compatibility validation
• Relevant performance/efficiency opportunities considered; material claims have evidence
• Material review findings resolved and relevant checks rerun
• Documentation added / updated (if applicable)
• Updated What's New copy (if doing a release after this PR)

[Pfannkuchensack]

Review — PR #298

Reviewed b58ee4df81 against merge-base c2e4cf6ee6 in a dedicated worktree. Three independent read-only passes (correctness, architecture/performance, test value/product quality), findings below verified by me directly. All checks re-run locally: backend tests (221), webv2 vitest (455), browser test (12), legacy-web tests (22), lint:tsc, architecture:check (70), test:performance:build, ruff, and an OpenAPI regeneration diff. All green, matching the PR's own CI.

The backend side — event ancestry, nested call chains, workflow inheritance — is well designed and genuinely well covered for nesting. The blockers are concentrated in outputImages.ts and in the webv2 routing of child completion events.

---

Blockers

1. getOutputImageNames silently drops workflow return values on a key collision

invokeai/frontend/webv2/src/platform/core/outputImages.ts:41

WorkflowReturnOutput.values is dict[str, Any] keyed by user-typed return names (invokeai/app/invocations/workflow_return.py:20). visitImageValue treats image, collection and values as structural markers, and the generic entry walk at line 41 is gated on none of them being present. So if any return key is literally named image, collection or values, every other named return is skipped.

Executed against the function body:

values {Image, Mask}       -> ["a.png","b.png"]   ok
values {image, mask}       -> ["a.png"]           mask lost
values {collection, mask}  -> ["a.png"]           mask lost
values {values, mask}      -> ["a.png"]           mask lost

image is an entirely natural return name. The lost image never reaches getResultImageNames (submissionApi.ts:155-158), so it never appears in queue results or the gallery. The new tests only use capitalised keys (Image, Images), so this is uncovered.

2. The same function adopts input images as run results

invokeai/frontend/webv2/src/platform/core/outputImages.ts:41-45

The header comment says "Do not walk arbitrary output fields", but once inside values the generic branch descends into every key of every nested plain object at unbounded depth. Executed:

values {meta: MetadataField{control_image}, out: ImageField}
  -> ["input-source.png","generated.png"]
values {Control: ControlField{image}}
  -> ["ctrl-input.png"]

A returned ControlField or MetadataField therefore contributes its input image to the run's result set. Related: the output_meta filter on line 43 is unreachable for real shapes — the top-level walk never descends into an invocation output, and values holds field values rather than outputs. The test "ignores metadata and unused workflow return value images" passes because of the top-level restriction, not because of that filter, so it does not protect the line it appears to.

This module is the most logic-dense addition in the PR and has no colocated test, although platform/core/ colocates them for comparable modules (concurrency.test.ts, json.test.ts).

3. Three of the four event builders have no test

invokeai/app/services/events/events_common.py:223,256,292

workflow_call_parent_source_id appears exactly four times across tests/, all of them via InvocationStartedEvent. tests/app/test_progress_preview_replay_socketio.py:37 bypasses .build entirely with model_copy(update=...). All three fields default to None, so dropping the line from InvocationProgressEvent.build, InvocationCompleteEvent.build or InvocationErrorEvent.build silently routes child progress/completion/error back to the invisible child node id — the exact bug this PR exists to fix — with a fully green suite.

4. The Call Saved Workflow node flickers running↔completed throughout a child run

invokeai/frontend/webv2/src/features/queue/runtime/coordinator.ts:440-443

completed and failed for every child node are rewritten onto the visible call node's source id, and nodeExecutionStore.completed (features/nodes/data/nodeExecutionStore.ts:50-62) unconditionally writes status: 'completed' plus a new outputImageUrl and latestOutput.

A 15-node child workflow therefore cycles the one visible node through ~15 green-check ↔ running-glow transitions (border-color and box-shadow transitions in ui/nodeChrome.tsx), and its displayed output ends up being an arbitrary intermediate child image rather than the workflow_return result. Its output field rows blank out and repopulate throughout the run. If the called workflow returns no image (CallSavedWorkflowInvocation returns WorkflowReturnOutput(values={})), the stale intermediate thumbnail survives via the previous?.outputImageUrl fallback.

Suggested direction: keep progress routed, but do not let a child completed mark the parent terminal — leave it running until the parent's own completion or settleRunning.

Nothing in coordinator.test.ts or nodeExecutionStore.test.ts asserts the parent node's status during a child run, and routeNodeEvent is only exercised for invocation_started and invocation_progress — the completed/failed branches, which are the ones that rewrite the event object, are untested.

5. The metadata test does not establish the QA claim it is named for

tests/app/services/session_queue/test_session_queue_workflow_call_metadata.py:221-307

The first half is sound: a real SQLite child queue item driven through the real ImagesInterface.save → _get_workflow_json, asserting on services.images.create.call_args.kwargs["workflow"].

Lines 302-307 are a separate, disconnected test. They construct a DiskImageFileStorage, hand it the local workflow_json variable — services.images is a MagicMock, so no image was ever stored by the code under test — and assert PIL round-trips invokeai_workflow. That re-tests pre-existing DiskImageFileStorage.save behaviour.

The seam that matters, ImageService.create forwarding workflow to image_files.save, is never exercised. If it regressed, both halves still pass while QA step 4 ("queued PNG metadata contains recallable invokeai_workflow data") would be false. The real-dependency pattern already exists in the repo: tests/app/routers/test_media_copy.py:47-61 and tests/app/services/images/test_images_default.py:80-102 wire a real DiskImageFileStorage and a real ImageService via the public storage.start(invoker), which would also remove the storage._DiskImageFileStorage__invoker poke on line 303.

Also, line 299 (assert ... get_queue_item_workflow_json.call_args.args == (parent_item_id,)) is pure mock choreography, fully redundant given line 300.

---

Worth fixing in the same pass

• Byte-identical duplicated assertion block: invokeai/frontend/webv2/src/workbench/workbenchState.test.ts:3427-3441 repeats the workflow: {…} sub-object already asserted at 3412-3425.
• _get_workflow_json is uncached on a hot path: invokeai/app/services/shared/invocation_context.py:76-82 issues a SELECT workflow per images.save() / videos.save(), taking the process-wide re-entrant DB lock (sqlite_database.py:125-139) and pulling the full workflow document for a value that is immutable for the life of the run. A called workflow in a loop pays that N times. Memoising on InvocationContextData is cheap and in scope. The shape of the accessor is right — get_queue_item(root).workflow would validate the whole session blob and be far worse.
• Mixed provenance in image metadata: _get_workflow_json returns the root queue item's workflow, while graph_ a few lines below still comes from the child session. An image made inside a called workflow is tagged with a workflow that does not contain the node that produced it. This may be the intended product call, but "load workflow from image" then lands the user in a workflow with no matching node — it deserves an explicit line in the PR description.
• settleUntilRequestsStop accepts a single quiet 25 ms sample: CallSavedWorkflowSyncRuntime.browser.test.tsx:89-103 returns as soon as one sample equals the previous one, so a second refetch wave arriving >25 ms later lands after the assertion — a false pass on exactly the failure the test exists to catch. Requiring 3-4 consecutive unchanged samples fixes it.
• Trivial constant assertion: expect(edgeTypes.loop_linkage).toBe(edgeTypes.default) (invokeai/frontend/web/src/features/nodes/store/util/reactFlowUtil.test.ts:41-43) cannot fail for any reason other than editing the literal it imports; tests/AGENTS.md rules these out. The sibling connectionToEdge test above it is genuinely valuable and should stay.
• Layering: outputImages.ts hard-codes backend wire-DTO field names (image_name, collection, values, output_meta) in @platform/core, which ARCHITECTURE.md reserves for domain-neutral infrastructure. architecture:check only enforces import direction, so it passes fail-open here. The constraint is real (Queue↔Nodes is forbidden, so there is no feature-level home), but widening Platform's remit silently is the wrong way to record that.
• rememberFrameGate reads as bounded but is not: coordinator.ts:263-282 only evicts keys absent from waits and returns when every key is a tracked wait, so FRAME_GATE_LIMIT is not a bound, and once exceeded every insert pays a full scan that evicts nothing. Harmless in practice (the backend executes items serially), but a gate is only ever created for an item whose root is in waits — storing the per-child gate map inside the wait record makes it O(1), strictly bounded, and deletes FRAME_GATE_LIMIT, rememberFrameGate and the terminal-status latestFrameGates.delete outright.
• Pre-existing gap now reachable: invokeai/frontend/web/src/features/nodes/hooks/useNodeCopyPaste.ts:167-171 branches on 'collapsed'/'default' and drops anything else with "Invalid edge type, cannot paste". This PR makes the legacy editor create loop_linkage edges, so copy/pasting a for + for_return pair loses the linkage and the backend rejects the next run. The same edge.type === 'default' filter shape also appears in util/node/nodeUpdate.ts:21. Out of scope for this PR, but same feature.
• Baseline re-record: performance/browser-baseline.json records every routeReadyMedianMs higher (e.g. 1471 → 1647) and domContentLoadedMedianMs higher on 8 of 10 routes, plus ~1 KB of otherRawBytes growth that no file in the diff explains. These gate nothing — timingPolicy.enforce is false and the runner is stable: false — so this is not a hidden regression, but re-recording them here adds no value either, and the Compatibility section mentions only the request counts. One sentence on the measurement conditions would close it. The architecture-baseline.json update is required and proportional (+455/+176 raw bytes, well inside the max(1%, 4 KB) allowance).

---

Checked, nothing material

• _get_workflow_call_parent_source_id is correct. create_child_workflow_execution_state (graph.py:4582-4587) appends the parent's frame, so workflow_call_stack[0] is always the frame built in the root session, and build_workflow_call_frame sets source_call_node_id from prepared_source_mapping — the editor's node id. Verified for direct child, grandchild, and non-call runs (empty stack → None).
• Status events cannot settle the root wait early. QueueItemStatusChangedEvent carries no root_item_id, so getTrackedBackendItemId is a no-op there.
• Sibling call nodes do reach error. I reproduced the three-nodes-on-one-deleted-workflow scenario as a browser test: all three are already at error at quiescence, so the detail-fetch dedupe does not leave siblings enqueueable.
• Generated artifacts are in sync. I regenerated openapi.json from the worktree sources and compared parsed JSON: identical. The ProgressPreviewDTO (optional) vs event (required, default null) asymmetry is genuine generator output.
• Compatibility both directions. All three fields default to None server-side and are optional in events.ts / core/types.ts, with root_item_id ?? item_id fallback; ProgressPreviewDTO lives only in MemoryProgressPreviews, so no snapshot migration is needed. Event payload growth is ~80 bytes against an already-embedded AnyInvocation.
• SessionQueueBase.get_queue_item_workflow_json breaks nothing: SqliteSessionQueue is the only implementer, and _DummySessionQueue in the test utils is duck-typed. Returning None for a missing row rather than raising like get_queue_item is the right call — a pruned root must not fail a running child's image save.
• withEnqueueNotification ordering and toast policy are fine: the enqueue notice lands at index 0, the metadata notice at index 1, and only category === 'enqueue' is gated on notifyOnEnqueue.
• Legacy loop_linkage restoration is sound. The backend Edge model really does carry Literal["default","loop_linkage"], the legacy zod schemas already had the variant, validateWorkflow keeps such edges, and the connector-flatten path in buildNodesGraph preserves the type via spread.
• Gallery-refresh amplification is avoided: child terminal events now reach scheduleGalleryRefresh(), but it is timer-coalesced.
• This PR closes a pre-existing unbounded path: before routing, child-workflow source ids accumulated in nodeExecutionStore's keyed store with no editor node to clear them.
• Obsolete code removal is clean — the old getResultImageName and the inlined extraction in submissionApi.ts are both gone, and the edgeTypes literal was moved rather than duplicated. No scratch or planning files in the diff.

---

PR description vs. what is established

• QA step 2 ("child progress and completion appear on the visible Call Saved Workflow node") — completion routing is manual-only (finding 3 above and the untested completed/failed branches).
• QA step 4 ("queued PNG metadata contains recallable invokeai_workflow data") — not established by the automated test (finding 5).
• Checklist item "Meaningful regression coverage added / updated where needed; obsolete tests/code removed" — the duplicated assertion block, the trivial edgeTypes assertion, and the DiskImageFileStorage graft run against it.
• Compatibility section omits the workflow/graph provenance mismatch and the baseline timing re-capture.

[Pfannkuchensack]

Re-review — PR #298 after the follow-up commits

Re-reviewed 9ecaaa1e41 against the previously reviewed b58ee4df81, in the same worktree and against the same running instance (throwaway root, CPU, no models; model-free child workflows built from blank_image / img_blur / workflow_return).

All five blockers from the previous pass are resolved, and I verified the two runtime ones against real runs rather than only by reading. One new user-visible regression was introduced by the fix for blocker 4.

Checks on the updated head: backend 222 passed / 2 xfailed, webv2 444 passed, lint:tsc clean, architecture:check 70 passed, legacy web 29 passed.

---

Resolved

Previous blocker	Fix	Evidence
1 — a return key named image / collection / values dropped every sibling return	outputImages.ts now dispatches on the output type discriminator instead of inferring structure from key names	The workflow_return_output I recorded as the sole result of a root queue item now yields both images. In the running app the same workflow took the gallery from 4 to 6 items; previously only one of the two returned images arrived.
2 — input images adopted as run results	no generic walker any more; a returned MetadataField or ControlField is not opened	Both recorded repros now return only the real output. Regression cases still work: a batched call returning a list under one key, a plain image_output, and a collect_output of image fields.
3 — three of four event builders untested	test_invocation_event_builders_preserve_nested_workflow_ancestry covers started / progress / complete / error	—
4 — call node's thumbnail and latestOutput came from child nodes	solved twice over: child completed/failed no longer reach nodeExecution at all, and nodeExecutionStore.completed no longer falls back to the previous thumbnail	new coordinator and store tests; see the regression below for the cost
5 — metadata test did not establish its own claim	rewritten onto a real DiskImageFileStorage and a real ImageService via the public start(invoker); it now asserts the PNG the code under test actually wrote, plus images.get_workflow	—

Also addressed from the "worth fixing" list: _get_workflow_json is memoised on InvocationContextData; FRAME_GATE_LIMIT is replaced by per-wait gate maps that die with their wait; settleUntilRequestsStop now requires three consecutive unchanged samples; the trivial edgeTypes identity assertion is gone; ARCHITECTURE.md records the adapter's platform ownership together with the constraint that it must not become a recursive walker; and loop_linkage is carried through both the paste path and getConnectedInputNames, each with a test.

The re-recorded browser baseline now moves in both directions and the byte growth is proportional to the source addition, so the earlier concern about it is closed.

---

New: a failing called workflow now leaves the call node with no error state

invokeai/frontend/webv2/src/features/queue/runtime/coordinator.ts:429-435

After a run whose child workflow fails, the visible Call Saved Workflow node ends with no outcome icon and a neutral border — it looks like it never ran. Read from the DOM after a real failing run:

outcomeIcon: "none"    borderColor: oklch(0.44 0.024 264.3)   (neutral)

Root cause, from a socket capture of that run: the backend emits queue_item_status_changed(failed) before invocation_error, for the root item and for a plain non-call node alike.

queue_item_status_changed  item 14  failed
invocation_error           item 14  root 13  src workflow_return_value  call_parent call-node
queue_item_status_changed  item 13  failed
invocation_error           item 13  src call-node

So settleWait runs first, settleRunning(…, 'failed') deletes the node state (nodeExecutionStore.ts:92-105), and the root's own invocation_error then fails the isTrackedEvent guard in handleNodeEvent and is dropped.

Before this update the child's error saved it: it arrived while the root wait was still alive and marked the node failed. Cross-check — I temporarily removed only the case 'failed' early return and repeated the identical run:

outcomeIcon: "Failed"  borderColor: oklch(0.7061 0.0841 19.38)   (red)

The guard is right for completed — that is what was putting intermediate child images on the node. For failed it costs the only failure indication the node has. Two ways out: keep routing child failed (only completed needs the guard), or deliver a settling item's node events before settleRunning clears them.

The new test does not let child terminal events settle the visible call node pins the current behaviour and asserts settleRunning is not called, but nothing covers the state the user is left with.

Worth noting separately: the underlying event ordering is pre-existing and affects every failing node in webv2, not just this one — a node's own invocation_error always arrives after its item's terminal status change, so nodeExecution.failed is effectively unreachable and the node state is deleted instead. The call node was only accidentally immune. That deserves its own issue independent of this PR.

---

Still open, minor

• The byte-identical duplicated assertion block at invokeai/frontend/webv2/src/workbench/workbenchState.test.ts:3427-3441 is still there.
• QA step 2 in the PR description ("child progress and completion appear on the visible Call Saved Workflow node") no longer matches the code — completion is now deliberately withheld from the node.
• The error surfaced on the node is the child's raw message with no attribution to the child node or workflow (verified: the root item carries Workflow return key must not be empty. verbatim). Only relevant once the error is shown again.

Verified working end to end

• Load Workflow on an image produced inside a called workflow loads the calling workflow. Unchanged by this update and still a genuine fix over main, where webv2 would have reported "missing".
• A called workflow's exposed fields render as first-class inputs on the call node, including the legacy exposedFields → form migration, and edited values reach the child run.
• Nested ancestry, progress routing, preview replay, loop_linkage handling, and the regenerated API artifacts all still check out.

[JPPhoto]

@Pfannkuchensack Thanks for the review. The current PR description already says child terminal events do not replace root output. The broader status-before-invocation_error issue for ordinary non-call nodes is pre-existing and outside this PR’s scope and we can make an issue and resolve later. I'll address everything else shortly!