fix(execution): keep terminal reconnect off runs a Sim run tool owns
Summary
Running a workflow from Chat on staging produced a "Run Error: Execution state is no longer available after reconnect" entry (0ms, last in the group) on a run whose blocks all succeeded. The entry is synthetic: the terminal's reconnect effect wrote it after a reconnect GET .../executions/{id}/stream returned 404. Tracing it end to end surfaced three gaps in how the Chat run tool and the terminal reconnect flow share a run, and this PR closes all three at the level they live.
Root cause of the reported error
doExecuteRunTool(apps/sim/lib/copilot/tools/client/run-tool-execution.ts) setscurrentExecutionIdand writes the sessionStorage execution pointer before sending the execute POST. It is the only run path that writes the pointer before the server has created the run buffer; the manual path andexecuteWorkflowWithFullLoggingwrite it on theX-Execution-Idheader.- Starting the run adds and activates the workflow tab in Chat, which mounts
useWorkflowExecution(embeddedworkflow.tsxandEmbeddedWorkflowActions), andsetActiveWorkflowflipsactiveWorkflowIdafter the workflow fetch. Both re-run the hook's reconnect effect mid-run. - The effect's claim check only knows "pointer present and the current execution id matches", which is exactly the state a live run tool creates.
cancelExecuteis a no-op for the tool's raw fetch, so the effect opened a reconnect GET in parallel with the live POST. - That GET races the POST's
initializeExecutionStreamMeta; when it reads first, the stream route answers 404, which the effect treats as non-retryable: it logs the Run Error with the live run's execution id, setsisExecutingfalse, nulls the current execution id, and clears the pointer the tool keeps for reload recovery. The POST headers then restore the id and the blocks stream in normally, which is why the run still looked successful. The error entry getsexecutionOrder: MAX_SAFE_INTEGER, so it always sorts last.
Changes
1. The reconnect effect defers to a live run tool, and the tool hands an interrupted run back (commit 1)
isRunToolActiveForWorkflow(workflowId)exposes the run tool's existing per-workflow ownership map. The reconnect effect returns early while it is set, before loading the pointer and without clearing it.- When the tool gives up an interrupted run it notifies
subscribeToRunToolReleasesubscribers after releasing ownership; the hook subscribes and bumpsreconnectAttemptNonce, so the terminal re-attaches from the last persisted event, matching what the manual path already does on interruption. Runs the tool observed to completion never notify, so a failed completion report still leaves the pointer forbindRunToolToExecutionto re-report after a reload.
2. One stream-interruption classifier for every live stream (commit 2)
- Only
useExecutionStream.executewrapped a transport failure asSSEStreamInterruptedError;executeWorkflowWithFullLoggingrethrew the rawTypeError. So on the Chat path a mid-run network drop took the generic branch: the tool reportederror, the confirm route marked the row failed, and the pointer was cleared while the execute route kept running the workflow (the streaming branch never aborts on client disconnect). toStreamInterruptedErroris now the single exported classifier, used by both hook execute paths (pure extraction, no behaviour change) and by the shared executor's post-acknowledgement catch. The run tool therefore reaches its recoverable branch: reportsbackground(truthful, the server continues), keeps the pointer, and releases the run to the terminal reconnect via change 1.- Review rounds hardened the classifier in place: it matches the browsers' transport messages as patterns (so Firefox's
NetworkError when attempting to fetch resource.counts), tolerates nullish or non-object rejections, and never classifies the stream layer's own typed errors (ExecutionStreamHttpError,SSEEventHandlerError) by their message text.
3. Async launches no longer write the terminal pointer (commit 2)
- The async path wrote the pointer only so
bindRunToolToExecutionwould find something after a reload, but an async run has no reconnectable stream, so any reconnect against that pointer 404'd into the same synthetic Run Error. - The tab-local pending completion report already carries the execution id. Async launches now write only that, and recovery answers from the pending report first, falling back to the pointer only for a live run this tab was observing. The legacy
clearExecutionPointerAfterReportflag is still honoured so pointers older clients left behind get cleaned up.
Traced end to end across both commits: manual runs and run tools cannot overlap on a workflow, reload recovery is unchanged for live runs, async runs were never stoppable from the terminal so nothing loses the pointer, no import cycle, nothing new in the editor bundle. Three independent adversarial reviews over the diffs; their findings that survived are the two accepted notes below.
Tests
- Hook: the 404 path on an unowned pointer still logs the Run Error (proves the guard test is not vacuous); an owned pointer skips reconnect, logs nothing, and keeps store and pointer intact; a release for the active workflow reconnects from the pointer's
lastEventId, one for another workflow does not, and unmount unsubscribes. - Run tool: ownership is set before the pointer write and held exactly while the run is in flight; both interruption kinds release only after ownership and store teardown, keep the pointer, and send a
backgroundconfirm; a completed run whose report fails does not release; async launches never touch the pointer and recover from the pending report alone; a legacy async pointer is cleaned up after its pending report is re-sent. - Shared executor: a transport drop after the server acknowledged the run rejects with
SSEStreamInterruptedErrorcarrying the server execution id; an AbortError, the run tool's string abort reason, and a non-transport error are rethrown untouched. - Mutation-tested: removing the guard, the nonce bump, the notify, the
streamInterruptedcondition, the ownership write, the executor wrap, the classifier, the pointer-free async launch, or the pending-first recovery each fails exactly the test written for it. bun run teston the four files pluschat.test.tsx,bun run type-check(all packages), biome on every changed path, andbun run check:audits(44/44) pass.
Accepted notes from review
- A server-initiated stream teardown (buffer write failure, which aborts the run and errors the stream) now reads as a transport drop on the Chat path and is reported as
background; the same classification the manual path already makes, and the confirm route resolves against the trusted execution on any later confirm. - A tab running the previous client with a stuck async pointer that reloads onto this build can log the synthetic Run Error once before
bindRunToolToExecutioncleans the pointer up, exactly as the previous client did to itself.
🤖 Generated with Claude Code