Skip to content

Harden the async node kinds: per-node runs, and failures on the node - #389

Merged
dovvnloading merged 1 commit into
mainfrom
ux/node-async-hardening
Sep 1, 2026
Merged

dovvnloading merged 1 commit into
mainfrom
ux/node-async-hardening

Conversation

@dovvnloading

Copy link
Copy Markdown
Owner

Problem

Three related gaps in how the plugin node kinds handle asynchronous work.

Artifact could not report a failure. ArtifactState carried exactly one field, artifact_content, so a failed generation surfaced only through a session-wide notification toast - with two artifact nodes on a canvas there was no way to tell which one failed. Web Research already keeps research_error on the node, and Gitlink and Code Sandbox render their own in-card banners.

Artifact and Web Research allowed one run at a time across the whole canvas. RunRegistry.is_busy() is kind-scoped and session-wide - RunHandle's own docstring states node_id is "informational only … NEVER consulted" - so a second research node could not start while an unrelated one was in flight, on a canvas whose premise is parallel branches, and the refusal could not name the node holding the slot. Gitlink and Code Sandbox already guard per node.

Five plugins demanded a selection they never used. Only HTML Renderer genuinely needs a parent: its factory seeds the new node's content from document.nodes[parent_node_id].content. The other five used parent_node_id as a place_child anchor and an edge, nothing more - so requiring a selection made the picker a dead end on an empty canvas.

Change

Failures on the node - artifact_error joins artifactContent on the wire, rendered in the card with the same banner treatment the sandbox uses. Live-wire only: research_error appears in neither session_save.py nor session_load.py, so a reloaded session does not resurrect a stale failure. Cleared when a new instruction is sent and when a turn lands.

Per-node runs - both kinds move onto the node.pending_request_id guard Gitlink and Code Sandbox established, and name the busy node. Two details that guard shape demands:

  • The stamp moves out of _run() to immediately after the claim, and the WS wrapper claims a placeholder synchronously before its first await. The field is the gate now, not UI bookkeeping, and a value written after an await leaves a window where a second click on one node sees an idle node. Same mechanism as _GITLINK_RUN_CLAIM_PLACEHOLDER.
  • Busy means "the registry still holds this run", not "the node has a request id". Cancelling releases the slot at once while the worker unwinds in its own time, and during that window the node still carries the dead id - reading the field alone refused the very next Run clicked after a cancel. AgentDispatcher.is_node_run_live asks the registry.

test_dispatch_claim_ordering.py's AST guard now covers both guard shapes rather than only the is_busy() one, so the no-await-between-guard-and-claim property stays enforced for the two converted kinds instead of silently going unchecked.

Parentless creation - make_simple_child_node_handler gains an opt-in standalone path that spawns at the picker's reported viewport centre and leaves the node unconnected. The five add_*_node methods already tolerated parent_id=None internally, so only their annotations widened. HTML Renderer is unchanged.

Also removes the Validation & Delivery picker category, which carried a full name and description, zero plugins, and was skipped at render.

Test plan

  • Full backend suite on a clean checkout of this branch: 3092 passed, 19 skipped. ruff clean. python contracts/codegen.py --check: 13 artifact sets up to date after regeneration.
  • npm run check (schema drift, typecheck, lint, vitest, build, bundle size): green.
  • npx playwright test: 5/5 - the specs bootstrap through the Plugins picker, which the parentless change touches.
  • New backend coverage: fail_artifact_generation records the message, no-ops on a deleted node, and is cleared by both a new instruction and a landed turn; two web-research nodes hold in-flight runs simultaneously; a second run on the same node is bounced without overwriting its query, with the node-named message; cancelling frees the node for an immediate re-run (the regression the liveness check exists for); each of the five plugins creates an unconnected node at the reported spawn point and treats a stale selection as no selection, while HTML Renderer still refuses.
  • Per-builtin parent behavior is now pinned once, exhaustively, in test_plugin_builtin_migration.py's parametrized suite; ten near-identical per-plugin duplicates in test_plugins.py were replaced by a single test asserting the split itself.
  • Frontend: the artifact card renders the failure banner with role="alert" and renders none when there is no error.

Three related gaps, all in how the plugin node kinds handle asynchronous
work.

Artifact could not report a failure. ArtifactState carried exactly one
field, artifact_content, so a failed generation surfaced only through a
session-wide notification toast - with two artifact nodes on a canvas
there was no way to tell which one failed. Web Research already keeps
research_error on the node, and gitlink/code_sandbox render their own
in-card banners. artifact_error joins them, live-wire only: research_error
appears in neither session_save.py nor session_load.py, so a reloaded
session does not resurrect a stale failure. It is cleared when a new
instruction is sent and when a turn lands.

Artifact and Web Research allowed one run at a time across the WHOLE
canvas. RunRegistry.is_busy() is kind-scoped and session-wide - RunHandle's
own docstring says node_id is "informational only ... NEVER consulted" -
so a second research node could not start while an unrelated one was in
flight, on a canvas whose premise is parallel branches, and the refusal
could not name the node holding the slot. Both now use the per-node guard
gitlink_run/code_sandbox established, and say which node is busy.

Two details that guard shape demands, neither of them optional:

- The stamp moved out of _run() to immediately after the claim, and the
  WS wrapper claims a placeholder synchronously before its first await.
  pending_request_id is the gate now, not UI bookkeeping, and a value
  written after an await leaves a window where a second click on one node
  sees an idle node. This is the same mechanism, and the same reasoning,
  behind _GITLINK_RUN_CLAIM_PLACEHOLDER.
- Busy means "the registry still holds this run", not "the node has a
  request id". Cancelling releases the slot at once while the worker
  unwinds in its own time, and during that window the node still carries
  the dead id - reading the field alone refused the very next Run clicked
  after a cancel. AgentDispatcher.is_node_run_live asks the registry.

test_dispatch_claim_ordering.py's AST guard covers both shapes now rather
than only the is_busy() one, so the ordering stays enforced for the two
converted kinds instead of silently going unchecked.

Five plugins become creatable with nothing selected. Only HTML Renderer
genuinely needs a parent - its factory seeds content from
document.nodes[parent_node_id].content. The other five used
parent_node_id as a place_child anchor and an edge, nothing more, so
requiring a selection made the picker a dead end on an empty canvas.
make_simple_child_node_handler gains an opt-in standalone path; the five
add_*_node methods already tolerated parent_id=None internally, so only
their annotations widened.

Also removes the "Validation & Delivery" picker category, which carried a
full name and description and zero plugins, and was skipped at render.

Co-Authored-By: Claude <noreply@anthropic.com>
@dovvnloading
dovvnloading merged commit a4ea7c0 into main Sep 1, 2026
4 checks passed
@dovvnloading
dovvnloading deleted the ux/node-async-hardening branch September 1, 2026 16:17
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