fix: block stuck on "running" (and a dead Stop button) when another block interrupts it

docs/cspell.json

@@ -34,6 +34,7 @@
     "prek",
     "prereq",
     "pyenv",
+    "refreshable",
     "resourcemanager",
     "runbook",
     "runbooks",

web/src/hooks/useApiExec.test.ts

@@ -25,6 +25,10 @@ function createMockWindowApi() {
   const listeners = new Map<string, Set<EventCallback>>()
   let invokeResolve: ((value?: unknown) => void) | null = null
   let invokeReject: ((err: Error) => void) | null = null
+  // Every pending exec:run resolver, in call order. Interleaved-run tests need
+  // to settle an *earlier* block's invoke after a later one has started, which
+  // the single `invokeResolve` slot above can't express.
+  const invokeResolvers: Array<(value?: unknown) => void> = []
 
   const api = {
     invoke: vi.fn((channel: string, ..._args: unknown[]) => {
@@ -36,6 +40,7 @@ function createMockWindowApi() {
       return new Promise<unknown>((resolve, reject) => {
         invokeResolve = resolve
         invokeReject = reject
+        invokeResolvers.push(resolve)
       })
     }),
     on: vi.fn((channel: string, callback: EventCallback) => {
@@ -67,6 +72,10 @@ function createMockWindowApi() {
     rejectInvoke(err: Error) {
       invokeReject?.(err)
     },
+    /** Resolve the nth invoke('exec:run') call (0-based, in call order) */
+    resolveInvokeNth(index: number, value?: unknown) {
+      invokeResolvers[index]?.(value)
+    },
   }
 }
 
@@ -226,6 +235,98 @@ describe('useApiExec state machine', () => {
     expect(result.current.state.exitCode).toBe(0)
   })
 
+  // ---------------------------------------------------------------------------
+  // Interrupted runs (main aborts every in-flight execution when a new one
+  // starts, resolving the aborted invoke as { status: null, cancelled: true }).
+  // ---------------------------------------------------------------------------
+
+  it('an aborted run leaves "running" instead of spinning forever', async () => {
+    const { result } = renderHook(() => useApiExec())
+
+    act(() => {
+      result.current.execute('long-running-script')
+    })
+    expect(result.current.state.status).toBe('running')
+
+    await act(async () => {
+      mock.resolveInvoke({ status: null, cancelled: true })
+    })
+
+    await waitFor(() => expect(result.current.state.status).toBe('pending'))
+    const lastLog = result.current.state.logs[result.current.state.logs.length - 1]
+    expect(lastLog.line).toContain('another block was run')
+  })
+
+  it('a block interrupted by a second block does not stay stuck on running', async () => {
+    const first = renderHook(() => useApiExec())
+    const second = renderHook(() => useApiExec())
+
+    act(() => {
+      first.result.current.execute('slow-script')
+    })
+    expect(first.result.current.state.status).toBe('running')
+
+    // Second block starts before the first finishes. The main process aborts
+    // the first run and kills its process group, then resolves its invoke with
+    // no status — and the first block's listeners are already ignoring events
+    // because the newer run owns activeExecId.
+    act(() => {
+      second.result.current.execute('other-script')
+    })
+    expect(second.result.current.state.status).toBe('running')
+
+    await act(async () => {
+      mock.resolveInvokeNth(0, { status: null, cancelled: true })
+    })
+
+    await waitFor(() => expect(first.result.current.state.status).toBe('pending'))
+    expect(second.result.current.state.status).toBe('running')
+  })
+
+  it('does not explain the stop twice when the user cancelled it', async () => {
+    const { result } = renderHook(() => useApiExec())
+
+    act(() => {
+      result.current.execute('long-running-script')
+    })
+    act(() => {
+      result.current.cancel()
+    })
+
+    await act(async () => {
+      mock.resolveInvoke({ status: null, cancelled: true })
+    })
+
+    const lines = result.current.state.logs.map((l) => l.line)
+    expect(lines.filter((l) => l.includes('cancelled by user'))).toHaveLength(1)
+    expect(lines.some((l) => l.includes('another block was run'))).toBe(false)
+  })
+
+  it('cancel still targets this run after its invoke has already resolved', async () => {
+    const { result } = renderHook(() => useApiExec())
+
+    act(() => {
+      result.current.execute('long-running-script')
+    })
+    const runCalls = vi
+      .mocked(mock.api.invoke)
+      .mock.calls.filter(([channel]) => channel === 'exec:run')
+    expect(runCalls).toHaveLength(1)
+    const { executionId } = runCalls[0][1] as { executionId: string }
+
+    // The invoke settles (aborted by a newer run) — which used to clear the id
+    // Stop depends on, leaving the button wired to nothing.
+    await act(async () => {
+      mock.resolveInvoke({ status: null, cancelled: true })
+    })
+
+    act(() => {
+      result.current.cancel()
+    })
+
+    expect(mock.api.invoke).toHaveBeenCalledWith('exec:cancel', { executionId })
+  })
+
   it('reset: clears all state back to initial', async () => {
     const { result } = renderHook(() => useApiExec())
 

web/src/hooks/useApiExec.ts

@@ -110,11 +110,25 @@ export function useApiExec(options?: UseApiExecOptions): UseApiExecReturn {
   // still cancel a run whose listeners have already been detached — and so the
   // decision to cancel doesn't depend on listener lifecycle timing.
   const runningExecIdRef = useRef<string | null>(null)
+  // The last execution id this hook started, kept even after `exec:run`
+  // resolves. `runningExecIdRef` is cleared the moment the invoke settles, but
+  // a run aborted by the main process settles while its child may still be
+  // winding down — and while the UI still shows "running". Falling back to this
+  // id keeps Stop working in that window. It always names THIS hook's own run,
+  // never "whatever ran last", so Stop can't reach into another block's script.
+  const lastExecIdRef = useRef<string | null>(null)
+  // Set when this hook asked for the cancellation, so the completion handler
+  // doesn't explain the stop a second time (cancel() already logged it).
+  const selfCancelledRef = useRef(false)
 
   const cancel = useCallback(() => {
-    const execId = runningExecIdRef.current
+    const execId = runningExecIdRef.current ?? lastExecIdRef.current
+    selfCancelledRef.current = true
 
     // Signal the backend to interrupt + kill *this* run's child process group.
+    // Cancelling a run the main process has already finished with is a no-op
+    // there (the id is dropped when the handler returns), so it's safe to send
+    // whenever we have one.
     if (execId !== null) {
       window.api.invoke('exec:cancel', { executionId: execId }).catch(() => {})
       runningExecIdRef.current = null
@@ -126,14 +140,18 @@ export function useApiExec(options?: UseApiExecOptions): UseApiExecReturn {
       cleanupRef.current = null
     }
 
-    // Only add cancellation log and update state if there was actually an active execution
-    if (execId !== null) {
-      setState((prev) => ({
-        ...prev,
-        status: 'pending',
-        logs: [...prev.logs, createLogEntry('Execution cancelled by user')],
-      }))
-    }
+    // Log the cancellation only when the block was actually showing a run in
+    // progress. Keyed on the rendered status rather than the id bookkeeping,
+    // so a stuck-looking block reports the stop and an idle one stays quiet.
+    setState((prev) =>
+      prev.status === 'running'
+        ? {
+            ...prev,
+            status: 'pending',
+            logs: [...prev.logs, createLogEntry('Execution cancelled by user')],
+          }
+        : prev,
+    )
   }, [])
 
   const reset = useCallback(() => {
@@ -170,6 +188,8 @@ export function useApiExec(options?: UseApiExecOptions): UseApiExecReturn {
     // can target this specific execution. Held in a ref for cancel() to read.
     const executionId = String(execId)
     runningExecIdRef.current = executionId
+    lastExecIdRef.current = executionId
+    selfCancelledRef.current = false
 
     // Reset state for new execution
     setState({
@@ -261,6 +281,32 @@ export function useApiExec(options?: UseApiExecOptions): UseApiExecReturn {
               ? { ...prev, status: finalStatus.status as ExecState['status'], exitCode: finalStatus.exitCode }
               : prev,
           )
+        } else {
+          // No status means the run was interrupted before it could report one:
+          // the main process aborts every in-flight execution when a new one
+          // starts, and an aborted run resolves as { status: null, cancelled:
+          // true }. Its `exec:status` event never arrives either — main stops
+          // sending after the abort, and these listeners are already ignoring
+          // events now that a newer run owns `activeExecId`. Without this the
+          // block spins on "running" forever, over a child process that was
+          // killed. Self-cancellation is already reported by cancel().
+          const explain = !selfCancelledRef.current
+          setState((prev) =>
+            prev.status === 'running' || prev.status === 'pending'
+              ? {
+                  ...prev,
+                  status: 'pending',
+                  logs: explain
+                    ? [
+                        ...prev.logs,
+                        createLogEntry(
+                          'Execution stopped: another block was run before this one finished.',
+                        ),
+                      ]
+                    : prev.logs,
+                }
+              : prev,
+          )
         }
       }
       // Schedule listener cleanup on the next macrotask so any IPC events