fix: end a stopped execute's pending navigation so later executes don't hang

README.md

@@ -243,10 +243,11 @@ connection itself isn't encrypted by browse; use a network that is (Tailscale or
 | `status` | Connection status |
 
 `execute` takes an optional `timeoutMs` (`--timeout-ms` on the CLI), the budget for the whole script: 60000
-milliseconds by default, 0 disables it, and time spent in `handoff()` doesn't count. An explicit `timeoutMs` (or
-`OPENCODE_BROWSER_EXECUTE_TIMEOUT_MS`) is also how long Playwright actions and navigations may take in that call, at
-least 30 seconds; without one they keep Playwright's 30-second default. At 0 their timeout is off too, so a stuck
-wait only ends when the call is stopped.
+milliseconds by default, 0 disables it, and time spent in `handoff()` doesn't count. It starts when the call does,
+so time spent waiting behind another execute on the same session counts, and that other call's handoff doesn't
+pause it. An explicit `timeoutMs` (or `OPENCODE_BROWSER_EXECUTE_TIMEOUT_MS`) is also how long Playwright actions
+and navigations may take in that call, at least 30 seconds; without one they keep Playwright's 30-second default.
+At 0 their timeout is off too, so a stuck wait only ends when the call is stopped.
 
 `snapshot()` and `ariaSnapshot()` hide the values of editable fields and of password, one-time-code and card
 fields (`autocomplete` `cc-number`, `cc-csc`, `cc-exp*`). Other readonly fields show their value, so a generated

cli/skills/browse/SKILL.md

@@ -244,6 +244,8 @@ about 2 seconds; split heavy loops with
 
 `execute`'s `timeoutMs` (MCP) / `--timeout-ms` (CLI) is the whole-script
 deadline, 60 seconds by default and paused during `handoff()`; `0` disables it.
+It starts with the call, so time queued behind another execute on the session
+counts, and that other call's handoff doesn't pause it.
 When you set it (or the relay sets `OPENCODE_BROWSER_EXECUTE_TIMEOUT_MS`),
 Playwright actions and navigations get the same budget, at least 30 seconds;
 without either they keep Playwright's 30-second default.

cli/src/cli/execute.ts

@@ -122,7 +122,7 @@ const execute = Command.make(
     ),
     timeoutMs: Flag.Int("timeout-ms").pipe(
       Flag.optional,
-      Flag.withDescription("Overall script timeout in milliseconds (paused during handoff); defaults to 60000, 0 disables. When set, it is also the timeout for Playwright actions and navigations, at least 30 seconds; 0 removes those too"),
+      Flag.withDescription("Overall script timeout in milliseconds (paused during handoff); defaults to 60000, 0 disables. It includes time spent waiting behind another execute on the same session, and that call's handoff does not pause it. When set, it is also the timeout for Playwright actions and navigations, at least 30 seconds; 0 removes those too"),
     ),
     targetUrl: Flag.String("target-url").pipe(
       Flag.optional,

cli/src/execute.ts

@@ -1,4 +1,4 @@
-import { Effect } from "effect"
+import { Effect, Fiber } from "effect"
 import type { Browser, BrowserContext, Page } from "patchright-core"
 import { registerAriaSnapshotSelector } from "./snapshot/aria-snapshot.ts"
 import { registerRefSelector } from "./snapshot/ref-selector.ts"
@@ -418,13 +418,12 @@ export class ExecuteSandbox extends DefaultPageHost {
   }
 
   /**
-   * Stops a running script from outside (the toolbar's stop, session delete/reset): closes its browser connection so
-   * every Playwright call it makes rejects, and disposes its realm (pending runs reject, timers are revoked; `state` resets).
-   * The session manager relies on this hook; keep it when the realm changes.
+   * Stops a running script's code at once (the toolbar's stop, session delete/reset): pending runs reject and timers
+   * are revoked. Synchronous on purpose, so a stop takes effect before anything the caller does next. The session
+   * manager relies on this hook; keep it synchronous when the realm changes.
    */
-  abortExecution(): Effect.Effect<void, Error> {
+  stopRealm(): void {
     this.realm?.dispose(new Error("Execute stopped: the session was stopped, deleted or reset"))
-    return this.disconnectSettled()
   }
 
   disconnectSettled(): Effect.Effect<void, Error> {
@@ -435,20 +434,24 @@ export class ExecuteSandbox extends DefaultPageHost {
       sandbox.state = sandbox.realm?.state ?? {}
       // Routes, listeners, init scripts and CDP sessions belong to the browser connection closed below.
       const browser = sandbox.browser
+      // Start the close before the first yield, while the reference is still held: an interrupted disconnect must not
+      // leave a connection open with nothing pointing at it. Cancel does no CDP work of its own (it discards pending
+      // capture work and waits only for finalizers already running, whose failures are caught), so the two can overlap.
+      const closing = yield* (browser
+        ? runPlaywrightOperation({
+            label: "Disconnect sandbox browser after handoff cancellation",
+            timeoutMs: playwrightCloseTimeoutMs,
+            run: () => browser.close(),
+          }).pipe(Effect.ignore)
+        : Effect.void).pipe(Effect.forkDetach({ startImmediately: true }))
       sandbox.browser = undefined
       sandbox.unbindDefaultPage(sandbox.defaultPageTargetId, {
         ownsPage: sandbox.ownsPage,
         pageCrashed: sandbox.pageCrashed,
         pageProtectedUi: sandbox.pageProtectedUi,
       })
       yield* sandbox.networkCapture.cancel()
-      if (browser) {
-        yield* runPlaywrightOperation({
-          label: "Disconnect sandbox browser after handoff cancellation",
-          timeoutMs: playwrightCloseTimeoutMs,
-          run: () => browser.close(),
-        }).pipe(Effect.ignore)
-      }
+      yield* Fiber.join(closing)
     })
   }
 

cli/src/mcp/execute.ts

@@ -37,7 +37,7 @@ registerMcpTools({
             type: "integer",
             minimum: 0,
             description:
-              "Optional overall script timeout in milliseconds (paused while handoff() waits on the user); defaults to 60000, 0 disables. When set, it is also the timeout for Playwright actions and navigations in the code, at least 30 seconds; 0 removes those too.",
+              "Optional overall script timeout in milliseconds (paused while handoff() waits on the user); defaults to 60000, 0 disables. It includes time spent waiting behind another execute on the same session, and that call's handoff does not pause it. When set, it is also the timeout for Playwright actions and navigations in the code, at least 30 seconds; 0 removes those too.",
           },
         },
         ["code"],

cli/src/relay.ts

@@ -299,6 +299,20 @@ const makeRelay = Effect.fnUntraced(function* (options: {
       getUserAttachedPageUrls: (session) => registry.listRootTargets()
         .filter((target) => target.owner === "user" && browsers.ownerKey(target.tabId) === session.browser?.key)
         .map((target) => target.targetInfo.url || "about:blank"),
+      // A stopped script may be awaiting a navigation whose server never answers. The caller and the execute lock
+      // are released at once, but Chromium still has Page.navigate in flight on the tab and queues every
+      // renderer-bound command behind it (including the next execute's connectOverCDP), so end it browser-side.
+      // Only tabs the session owns: a tab it merely drove through target selection may be another session's or the user's.
+      onStoppingExecute: (session) => {
+        if (session.readOnly) return Effect.void
+        const tabs = registry.listRootTargets().filter((target) => target.opencodeBrowserSessionId === session.id)
+        if (tabs.length === 0) return Effect.void
+        return Effect.forEach(tabs, (target) =>
+          sendDebuggerCommand({ tabId: target.tabId, method: "Page.stopLoading", params: {} }).pipe(
+            Effect.timeoutOption("2 seconds"),
+            Effect.ignore,
+          ), { concurrency: "unbounded", discard: true })
+      },
       ...(executeTimeoutMs === undefined ? {} : { executeTimeoutMs }),
       onExecuteStateChange: (sessionId, executing) => Effect.gen(function* () {
         yield* presentation.onSessionExecuteStateChange(sessionId, executing)

cli/src/relay/types.ts

@@ -51,8 +51,8 @@ export interface ExecuteSandboxLike {
   execute(code: string, options?: ExecuteOptions): Effect.Effect<ExecuteResult>
   authenticatedJson(request: Omit<AuthenticatedJsonRequest, "sessionId">): Effect.Effect<AuthenticatedJsonOutcome, Error>
   adoptPage(targetId: string): Effect.Effect<void, Error>
-  /** Stop a running script: its Playwright calls reject and its timers stop; the default tab is kept. */
-  abortExecution(): Effect.Effect<void, Error>
+  /** Stop a running script at once: its pending runs reject and its timers stop; the default tab is kept. */
+  stopRealm(): void
   /** Shutdown and handoff cancellation await disconnection without closing or forgetting the default tab. */
   disconnectSettled(): Effect.Effect<void, Error>
   /** Adoption rollback cleanup does not settle before started Playwright close promises settle. */

cli/src/session-manager.ts

@@ -702,9 +702,23 @@ export class OpenCodeBrowserSessions {
 
   private endStoppedScript(session: OpenCodeBrowserSession): Effect.Effect<void> {
     this.cancelSessionHandoff(session.id)
-    return session.sandbox.abortExecution().pipe(
-      Effect.timeoutOption(this.stopGraceMs()),
-      Effect.catchCause((cause) => Effect.sync(() => console.error("Abort stopped execute", cause))),
+    // User code stops first, synchronously, as before: nothing the script does runs after its caller was told it stopped.
+    session.sandbox.stopRealm()
+    // Then end a navigation it left pending, and only then close its browser connection. One stop grace covers both:
+    // the cancel may take at most half of it (never more than 2s), so the disconnect always starts within the grace.
+    const graceMs = this.stopGraceMs()
+    const cancelNavigation = this.hooks.onStoppingExecute?.(session)
+    const cancel = cancelNavigation === undefined
+      ? Effect.void
+      : cancelNavigation.pipe(
+          Effect.timeoutOption(Math.min(2_000, Math.floor(graceMs / 2))),
+          Effect.catchCause((cause) => Effect.sync(() => console.error("Cancel stopped execute navigation", cause))),
+          Effect.asVoid,
+        )
+    return cancel.pipe(
+      Effect.andThen(Effect.suspend(() => session.sandbox.disconnectSettled())),
+      Effect.timeoutOption(graceMs),
+      Effect.catchCause((cause) => Effect.sync(() => console.error("Disconnect stopped execute", cause))),
       Effect.asVoid,
     )
   }

cli/src/session-manager/execution.ts

@@ -46,12 +46,58 @@ export function executeInSession(
     const resolved = yield* ctx.resolveWorkSession(options.sessionId, options.createIfMissing, options.browser)
     const session = resolved.session
     const response = yield* Deferred.make<ExecutionResponse, Error>()
+    const stopSignal = Deferred.makeUnsafe<{ readonly message: string; readonly diagnostic: string }>()
     let started = false
+    // True while this call waits for the session's execute lock; its deadline uses it to tell a queued call from one
+    // that is already setting up its browser.
+    let queued = true
     let cancelled = false
+    let stopped: { readonly message: string; readonly diagnostic: string } | undefined
+    const stoppedExecution = (stop: { readonly message: string; readonly diagnostic: string }): ExecutionResponse => ({
+      result: stoppedExecuteResult(stop.message, stop.diagnostic),
+      session: createdSummary(session, resolved.created),
+    })
+    const chosenTimeoutMs = options.timeoutMs ?? ctx.hooks.executeTimeoutMs
+    const effectiveTimeoutMs = chosenTimeoutMs ?? defaultExecuteTimeoutMs
+    // Only an explicit per-call or relay timeout raises Playwright's 30s action timeout; the built-in default leaves it alone.
+    const actionTimeoutMs = chosenTimeoutMs === undefined
+      ? undefined
+      : chosenTimeoutMs === 0 ? 0 : Math.max(30_000, chosenTimeoutMs)
+    let deadline: ExecutionDeadline | undefined
+    const stopRunning = (message: string, diagnostic: string) => {
+      // A timeout and a toolbar stop can race; the caller gets whichever stopped it first.
+      if (stopped !== undefined) return
+      stopped = { message, diagnostic }
+      deadline?.cancel()
+      Effect.runSync(Deferred.succeed(stopSignal, { message, diagnostic }))
+      Effect.runSync(Deferred.succeed(response, stoppedExecution({ message, diagnostic })))
+    }
+    // Armed before the session's execute lock: the timeout covers time spent waiting behind another execute on the
+    // session, so a queued call gets its own answer instead of waiting for a lock a wedged tab may still hold.
+    if (effectiveTimeoutMs > 0) {
+      deadline = new ExecutionDeadline(effectiveTimeoutMs, () => {
+        if (queued) {
+          stopRunning(
+            `Execute timed out after ${effectiveTimeoutMs}ms while this session was still busy with another operation; this code did not run.`,
+            "execute-queued-timeout",
+          )
+          return
+        }
+        stopRunning(
+          `Execute timed out after ${effectiveTimeoutMs}ms. Pass --timeout-ms <ms> (or timeoutMs in MCP) for longer runs, or 0 to disable.`,
+          "execute-timeout",
+        )
+      })
+    }
     const operation = Effect.gen(function* () {
+      queued = false
       if (cancelled) {
         return yield* sessionError("inactive", `Session execute was cancelled before starting: ${session.id}`, session.id)
       }
+      // The deadline expired while this call waited for the execute lock; its caller already has the answer.
+      if (stopped !== undefined) {
+        return stoppedExecution(stopped)
+      }
       if (ctx.sessions.get(session.id) !== session) {
         return yield* sessionError("inactive", `Session is no longer active: ${session.id}`, session.id)
       }
@@ -60,40 +106,22 @@ export function executeInSession(
       if (session.browser?.key && ctx.hooks.checkBrowserResponsive) yield* ctx.hooks.checkBrowserResponsive(session.browser.key).pipe(
         Effect.mapError(error => sessionError("browser-unavailable", error.message, session.id)),
       )
+      // The deadline can expire while that check waits; its caller already has the answer, so the code never runs.
+      if (stopped !== undefined) {
+        return stoppedExecution(stopped)
+      }
       started = true
       session.updatedAt = new Date().toISOString()
-      yield* ctx.setExecuting(session.id, true)
-      const stopSignal = Deferred.makeUnsafe<{ readonly message: string; readonly diagnostic: string }>()
       const released = Deferred.makeUnsafe<{ readonly forced: boolean }>()
-      const chosenTimeoutMs = options.timeoutMs ?? ctx.hooks.executeTimeoutMs
-      const effectiveTimeoutMs = chosenTimeoutMs ?? defaultExecuteTimeoutMs
-      // Only an explicit per-call or relay timeout raises Playwright's 30s action timeout; the built-in default leaves it alone.
-      const actionTimeoutMs = chosenTimeoutMs === undefined
-        ? undefined
-        : chosenTimeoutMs === 0 ? 0 : Math.max(30_000, chosenTimeoutMs)
-      let deadline: ExecutionDeadline | undefined
-      const stopRunning = (message: string, diagnostic: string) => {
-        deadline?.cancel()
-        Effect.runSync(Deferred.succeed(stopSignal, { message, diagnostic }))
-        Effect.runSync(Deferred.succeed(response, {
-          result: stoppedExecuteResult(message, diagnostic),
-          session: createdSummary(session, resolved.created),
-        }))
-      }
-      if (effectiveTimeoutMs > 0) {
-        deadline = new ExecutionDeadline(effectiveTimeoutMs, () => {
-          stopRunning(
-            `Execute timed out after ${effectiveTimeoutMs}ms. Pass --timeout-ms <ms> (or timeoutMs in MCP) for longer runs, or 0 to disable.`,
-            "execute-timeout",
-          )
-        })
-      }
       const running: RunningExecution = {
         released,
         stop: stopRunning,
         pauseTimeout: () => deadline ? deadline.pause() : () => {},
       }
+      // Registered before setExecuting yields: a caller that disconnects while the state hook runs still finds the
+      // execution to stop instead of leaving its script running with nobody waiting for it.
       ctx.runningExecutions.set(session.id, running)
+      yield* ctx.setExecuting(session.id, true)
       let forced = false
       const startedAt = Date.now()
       // The script runs in its own fiber so a stop can release the execute lock without waiting for it: a stopped
@@ -149,12 +177,14 @@ export function executeInSession(
       return { result: resultWithHint, session: createdSummary(session, resolved.created) }
     })
     const worker = session.executeSemaphore.withPermit(operation.pipe(Effect.matchEffect({
-      onFailure: (error) => (resolved.created
-        ? ctx.cleanupCreatedSession(session)
-        : Effect.void).pipe(Effect.andThen(Deferred.fail(response, error))),
-      onSuccess: (value) => (options.removeAfterExecute
-        ? ctx.cleanupCreatedSession(session)
-        : Effect.void).pipe(Effect.andThen(Deferred.succeed(response, value))),
+      onFailure: (error) => Effect.sync(() => deadline?.cancel()).pipe(
+        Effect.andThen(resolved.created ? ctx.cleanupCreatedSession(session) : Effect.void),
+        Effect.andThen(Deferred.fail(response, error)),
+      ),
+      onSuccess: (value) => Effect.sync(() => deadline?.cancel()).pipe(
+        Effect.andThen(options.removeAfterExecute ? ctx.cleanupCreatedSession(session) : Effect.void),
+        Effect.andThen(Deferred.succeed(response, value)),
+      ),
     })))
     const workerFiber = yield* ctx.admission.forkTracked(worker, session.id)
     return yield* restore(Deferred.await(response)).pipe(
@@ -164,6 +194,7 @@ export function executeInSession(
           return Effect.void
         }
         cancelled = true
+        deadline?.cancel()
         return Fiber.interrupt(workerFiber).pipe(
           Effect.asVoid,
           Effect.andThen(resolved.created

cli/src/session-manager/types.ts

@@ -80,6 +80,12 @@ export type SessionHooks = {
   readonly onSessionsChanged?: (sessions: readonly PersistedSession[]) => unknown | Promise<unknown>
   /** Cancel the session's pending handoff so its waiter fails; true when one was pending. */
   readonly onCancelSessionHandoff?: (sessionId: string) => boolean
+  /**
+   * End any navigation still pending on the session's tabs before the stopped script's browser connection closes.
+   * Chromium handles `Page.stopLoading` in the browser process, so it answers (and releases queued commands) even
+   * while a `Page.navigate` to a server that never responds is still in flight.
+   */
+  readonly onStoppingExecute?: (session: OpenCodeBrowserSession) => Effect.Effect<void>
   /** How long a stopped script may take to settle before its sandbox is abandoned and the execute lock released. */
   readonly stopGraceMs?: number
 }