Skip to content

fix(web): clone subworkflowContexts tree in processEvent (#307) - #308

Merged
Jason Robert (jrob5756) merged 2 commits into
mainfrom
fix/307-subworkflow-node-stuck-running
Jul 16, 2026
Merged

fix(web): clone subworkflowContexts tree in processEvent (#307)#308
Jason Robert (jrob5756) merged 2 commits into
mainfrom
fix/307-subworkflow-node-stuck-running

Conversation

@jrob5756

Copy link
Copy Markdown
Collaborator

Summary

Fixes#307: an agent node adjacent to a type: workflow sub-workflow step
could render stuck showing "running" even though the underlying store data
was already completed — a pure rendering/reactivity desync, not an engine
data bug (confirmed correct via the detail panel per the issue report).

Root cause

processEvent in workflow-store.ts clones nodes/groupProgress/
eventLog/activityLog on every event so React/Zustand reference-equality
checks pick up changes (per replaceNode's own doc comment: "Create a new
reference for a node to ensure React/ReactFlow detects the change"
) — but
it never cloned subworkflowContexts. Handlers reached through
activeTarget()/resolveContext()/resolveSlotPath() (subworkflow_started,
subworkflow_completed, agent events carrying a subworkflow_path, etc.)
mutate nested ctx.nodes/groupProgress/routes/highlightedEdges in
place, and subworkflow_started pushes onto subworkflowContexts/
children arrays in place. Nothing forced a fresh reference anywhere in
that tree, so any selector/useMemo keyed on subworkflowContexts (e.g.
useViewedNodes/useViewedGraphData in use-viewed-context.ts) could miss
an update depending on render/batching timing.

Fix

  • Added cloneSubworkflowContexts(): a structural clone of the
    SubworkflowContext tree (each context plus its own nodes/
    groupProgress/routes/highlightedEdges/children/etc.), wired into
    processEvent alongside the existing clones.
  • Audited replayState: it already rebuilds subworkflowContexts fresh
    from {} per replay and commits once at the end, so it needed no change.

Testing

There were previously zero frontend automated tests. Added a minimal
Vitest setup (vitest devDependency, vitest.config.ts, npm run test,
and a make test-frontend target) plus a regression test in
workflow-store.test.ts that:

  • Simulates the reported repro (an agent looping back once, then an
    adjacent subworkflow_started/subworkflow_completed pair) and asserts
    the agent's node stays completed and subworkflowContexts gets a fresh
    reference on every mutating event.
  • Covers a second case: a node update nested inside a running child
    sub-workflow context (via subworkflow_path), asserting the same
    reference-identity behavior.
  • Verified fails without the fix: temporarily reverted the
    cloneSubworkflowContexts call and confirmed both tests fail with the
    exact Object.is reference-equality assertion; restored the fix and
    confirmed both pass again.
make test-frontend # 2 passed
npx tsc -b # clean
uv run ruff check src tests # All checks passed!
uv run ty check src # 1 pre-existing warning (dialog_evaluator.py), unrelated

Closes#307.

processEvent already cloned nodes/groupProgress/eventLog/activityLog on
every event so React/Zustand reference-equality checks pick up changes,
but subworkflowContexts was never cloned. Handlers routed through
activeTarget()/resolveContext()/resolveSlotPath() (subworkflow_started,
subworkflow_completed, agent events carrying a subworkflow_path, etc.)
mutate nested ctx.nodes/groupProgress/routes/highlightedEdges and push
onto subworkflowContexts/children in place, so nothing forced a fresh
reference anywhere in that tree. Any selector/useMemo keyed on
subworkflowContexts (e.g. useViewedNodes/useViewedGraphData) could
therefore miss an update, producing the reported symptom: an agent node
adjacent to a sub-workflow step visually stuck on "running" even though
the store's underlying data was already correct.
Adds cloneSubworkflowContexts(), a structural clone of the
SubworkflowContext tree, wired into processEvent. replayState already
rebuilds subworkflowContexts fresh per replay and commits once, so it
needed no change.
Also adds a minimal Vitest setup for the frontend (previously zero
frontend tests existed) plus a regression test exercising both the
root-adjacent-to-subworkflow case and a node update nested inside a
running child context; verified the test fails without the fix and
passes with it.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a7be2391-c5bd-44d9-a978-12fe7832735e
…ests into CI
Addresses code-review feedback on #308:
- Replace the whole-tree clone of subworkflowContexts with a path-scoped
clone-on-write (cloneContextShallow + resolveMutableContext/
resolveMutableSlotPath). Only the top-level array and the ancestor chain
down to the context actually being mutated get a new reference; untouched
sibling subtrees (including their own descendants) keep their existing
reference. This avoids deep-cloning the entire sub-workflow tree on every
single event (including root-level events that never touch a
sub-workflow), and avoids triggering re-renders in components viewing an
unrelated branch.
- Add regression tests for: nested (grandchild) sub-workflow propagation,
concurrent for_each siblings not cloning each other, root-only events
never touching subworkflowContexts at all, and field preservation across
a clone-and-mutate cycle. Verified each new test fails when the
corresponding part of the fix is reverted.
- Switch resetStore() to a shared beforeEach.
- Add a `frontend` CI job (typecheck + Vitest + build) so the regression
suite actually runs on every PR instead of only when invoked manually.
- Trim doc-comment cross-references that were prone to drift (pointing at
another comment / naming specific hook consumers / citing the issue
number as load-bearing).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a7be2391-c5bd-44d9-a978-12fe7832735e
@jrob5756

Copy link
Copy Markdown
CollaboratorAuthor

Ran a multi-agent code review (code-reviewer, pr-test-analyzer, comment-analyzer) against the original commit and addressed the findings in a follow-up commit:

  • Performance/re-render fix: replaced the whole-tree clone of subworkflowContexts with a path-scoped clone-on-write (resolveMutableContext/resolveMutableSlotPath). Only the mutated branch's ancestor chain gets a fresh reference now; untouched siblings (and root-level events with no active sub-workflow) keep their existing reference, so this no longer forces every subworkflowContexts-dependent component to re-render on unrelated events.
  • New regression tests: nested (grandchild) sub-workflow propagation, concurrent for_each siblings not cross-cloning each other, root-only events never touching subworkflowContexts, and field preservation across a clone-and-mutate cycle. Verified each fails when the corresponding part of the fix is reverted.
  • CI: added a frontend job (typecheck + Vitest + build) so this regression suite runs on every PR instead of relying on someone remembering make test-frontend.
  • Comment hygiene: trimmed doc-comment cross-references prone to drift (pointing at another comment, naming specific hook consumers, citing the issue number as load-bearing).

All 6 frontend tests pass, tsc -b is clean, and ruff check / ty check are unaffected.

@jrob5756
Jason Robert (jrob5756) merged commit 2ad20c4 into mainJul 16, 2026
10 checks passed
@jrob5756
Jason Robert (jrob5756) deleted the fix/307-subworkflow-node-stuck-running branch July 16, 2026 20:38
Sign up for freeto 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.

Web dashboard: agent node stuck showing 'running' after completion when adjacent to a sub-workflow step

1 participant

@jrob5756