fix(server): a stuck browser action no longer wedges its tab or other tabs

apps/server/src/preview/ServerBrowser.test.ts

@@ -2525,3 +2525,237 @@ it.live("a full server takes back the slot of the longest idle agent tab nobody
     }),
   ).pipe(Effect.provide(layer)),
 );
+
+/** Makes the tab's next color scheme change hang in Playwright; resolves once it has started. */
+const stallColorScheme = (page: ReturnType<typeof makeContext>["page"]) => {
+  const started = Promise.withResolvers<void>();
+  page.emulateMedia.mockImplementationOnce(() => {
+    started.resolve();
+    return new Promise<void>(() => {});
+  });
+  return Effect.promise(() => started.promise);
+};
+
+it.effect("a stuck agent action times out on the server and later calls on its tab fail fast", () =>
+  Effect.scoped(
+    Effect.gen(function* () {
+      const { broker, tabId } = yield* ready;
+      const stalled = stallColorScheme(contexts[0]!.page);
+      const stuck = yield* broker
+        .invoke<void>({
+          scope,
+          tabId,
+          operation: "setColorScheme",
+          input: { colorScheme: "dark" },
+          timeoutMs: 1_000,
+        })
+        .pipe(Effect.flip, Effect.forkScoped);
+      yield* stalled;
+      // The broker evicts a host that has not answered by timeoutMs plus its one second grace.
+      yield* TestClock.adjust("2 seconds");
+      expect(yield* Fiber.join(stuck)).toMatchObject({ _tag: "PreviewAutomationTimeoutError" });
+      // The server answered in time, so the host every tab shares stays connected.
+      const status = yield* broker
+        .invoke({ scope, tabId, operation: "status", input: {} })
+        .pipe(Effect.exit);
+      expect(status).toMatchObject({ _tag: "Success" });
+      const busy = yield* broker
+        .invoke<void>({ scope, tabId, operation: "click", input: { ref: "e1" }, timeoutMs: 1_000 })
+        .pipe(Effect.flip);
+      expect(busy.message).toMatch(/busy|still running/i);
+    }),
+  ).pipe(Effect.provide(layer)),
+);
+
+it.effect("a tab stuck in an agent action does not hold back setting changes for other tabs", () =>
+  Effect.scoped(
+    Effect.gen(function* () {
+      const { broker, tabId } = yield* ready;
+      const manager = yield* Manager.PreviewManager;
+      const other = yield* broker.invoke<PreviewAutomationStatus>({
+        scope,
+        operation: "open",
+        input: { reuseExistingTab: false, show: false },
+      });
+      const stalled = stallColorScheme(contexts[0]!.page);
+      const applied = Promise.withResolvers<void>();
+      contexts[1]!.page.emulateMedia.mockImplementationOnce(async () => applied.resolve());
+      yield* broker
+        .invoke({
+          scope,
+          tabId,
+          operation: "setColorScheme",
+          input: { colorScheme: "dark" },
+          timeoutMs: 1_000,
+        })
+        .pipe(Effect.ignore, Effect.forkScoped);
+      // The stuck action has already published its own tab's resized event.
+      yield* stalled;
+      yield* manager.adjust({
+        threadId: scope.thread.threadId,
+        tabId: PreviewTabId.make(other.tabId!),
+        colorScheme: "dark",
+      });
+      yield* Effect.promise(() => applied.promise);
+      expect(contexts[1]!.page.emulateMedia).toHaveBeenCalledWith({ colorScheme: "dark" });
+    }),
+  ).pipe(Effect.provide(layer)),
+);
+
+it.effect("a dialog on a stuck tab is reported before the tab is busy", () =>
+  Effect.scoped(
+    Effect.gen(function* () {
+      const { broker, tabId } = yield* ready;
+      const page = contexts[0]!.page;
+      yield* broker.invoke({ scope, tabId, operation: "recordingStart", input: {} });
+      const stalled = stallColorScheme(page);
+      const stuck = yield* broker
+        .invoke<void>({
+          scope,
+          tabId,
+          operation: "setColorScheme",
+          input: { colorScheme: "dark" },
+          timeoutMs: 1_000,
+        })
+        .pipe(Effect.flip, Effect.forkScoped);
+      yield* stalled;
+      yield* TestClock.adjust("2 seconds");
+      expect(yield* Fiber.join(stuck)).toMatchObject({ _tag: "PreviewAutomationTimeoutError" });
+      page.emit("dialog", {
+        type: () => "alert",
+        message: () => "Saved",
+        defaultValue: () => "",
+        accept: vi.fn(async () => {}),
+        dismiss: vi.fn(async () => {}),
+      });
+      const blocked = yield* broker
+        .invoke<void>({
+          scope,
+          tabId,
+          operation: "click",
+          input: { ref: "e1" },
+          timeoutMs: 1_000,
+        })
+        .pipe(Effect.flip);
+      expect(blocked).toMatchObject({
+        _tag: "PreviewAutomationControlInterruptedError",
+        reason: "dialogPending",
+      });
+      const stop = yield* broker
+        .invoke<void>({ scope, tabId, operation: "recordingStop", input: {}, timeoutMs: 1_000 })
+        .pipe(Effect.flip);
+      expect(stop).toMatchObject({
+        _tag: "PreviewAutomationControlInterruptedError",
+        reason: "dialogPending",
+      });
+    }),
+  ).pipe(Effect.provide(layer)),
+);
+
+it.effect("an action that fails just after its timeout still reports its own failure", () =>
+  Effect.scoped(
+    Effect.gen(function* () {
+      const { broker, tabId } = yield* ready;
+      const failing = Promise.withResolvers<void>();
+      const started = Promise.withResolvers<void>();
+      contexts[0]!.page.emulateMedia.mockImplementationOnce(() => {
+        started.resolve();
+        return failing.promise;
+      });
+      const failed = yield* broker
+        .invoke<void>({
+          scope,
+          tabId,
+          operation: "setColorScheme",
+          input: { colorScheme: "dark" },
+          timeoutMs: 1_000,
+        })
+        .pipe(Effect.flip, Effect.forkScoped);
+      yield* Effect.promise(() => started.promise);
+      // Operations time themselves out a little after the request arrives; their reason wins.
+      yield* TestClock.adjust("1100 millis");
+      failing.reject(new Error("Target page crashed"));
+      expect(yield* Fiber.join(failed)).toMatchObject({ _tag: "PreviewAutomationExecutionError" });
+    }),
+  ).pipe(Effect.provide(layer)),
+);
+
+it.effect("a recording stop that never finishes leaves its tab busy", () =>
+  Effect.scoped(
+    Effect.gen(function* () {
+      const { broker, tabId } = yield* ready;
+      const context = contexts[0]!;
+      yield* broker.invoke({ scope, tabId, operation: "recordingStart", input: {} });
+      const started = Promise.withResolvers<void>();
+      context.sessions.at(-1)!.detach.mockImplementationOnce(() => {
+        started.resolve();
+        return new Promise<void>(() => {});
+      });
+      const stuck = yield* broker
+        .invoke<void>({ scope, tabId, operation: "recordingStop", input: {}, timeoutMs: 1_000 })
+        .pipe(Effect.flip, Effect.forkScoped);
+      yield* Effect.promise(() => started.promise);
+      yield* TestClock.adjust("2 seconds");
+      expect(yield* Fiber.join(stuck)).toMatchObject({ _tag: "PreviewAutomationTimeoutError" });
+      const busy = yield* broker
+        .invoke<void>({ scope, tabId, operation: "click", input: { ref: "e1" }, timeoutMs: 1_000 })
+        .pipe(Effect.flip, Effect.forkScoped);
+      // The host answers in arrival order, so a refused click has its answer before this one.
+      yield* broker.invoke({ scope, tabId, operation: "status", input: {} });
+      // A click that queued instead of being refused would time out here.
+      yield* TestClock.adjust("2 seconds");
+      expect(yield* Fiber.join(busy)).toMatchObject({
+        _tag: "PreviewAutomationControlInterruptedError",
+        reason: "busy",
+      });
+    }),
+  ).pipe(Effect.provide(layer)),
+);
+
+it.effect("a queued action whose timeout passes while it waits never runs", () =>
+  Effect.scoped(
+    Effect.gen(function* () {
+      const { broker, tabId } = yield* ready;
+      const page = contexts[0]!.page;
+      yield* broker.invoke({ scope, tabId, operation: "recordingStart", input: {} });
+      const stopRan = Promise.withResolvers<void>();
+      contexts[0]!.sessions.at(-1)!.detach.mockImplementationOnce(() => {
+        stopRan.resolve();
+        return new Promise<void>(() => {});
+      });
+      const started = Promise.withResolvers<void>();
+      const finish = Promise.withResolvers<void>();
+      page.emulateMedia.mockImplementationOnce(() => {
+        started.resolve();
+        return finish.promise;
+      });
+      const slow = yield* broker
+        .invoke<void>({
+          scope,
+          tabId,
+          operation: "setColorScheme",
+          input: { colorScheme: "dark" },
+          timeoutMs: 3_000,
+        })
+        .pipe(Effect.forkScoped);
+      yield* Effect.promise(() => started.promise);
+      const queued = yield* broker
+        .invoke<void>({ scope, tabId, operation: "click", input: { ref: "e1" }, timeoutMs: 1_000 })
+        .pipe(Effect.flip, Effect.forkScoped);
+      // The host answers in arrival order, so the click is queued once this answers.
+      yield* broker.invoke({ scope, tabId, operation: "status", input: {} });
+      yield* broker
+        .invoke({ scope, tabId, operation: "recordingStop", input: {}, timeoutMs: 1_000 })
+        .pipe(Effect.ignore, Effect.forkScoped);
+      yield* broker.invoke({ scope, tabId, operation: "status", input: {} });
+      // Past the click's timeout but before the server answers it.
+      yield* TestClock.adjust("1200 millis");
+      finish.resolve();
+      yield* Fiber.join(slow);
+      expect(yield* Fiber.join(queued)).toMatchObject({ _tag: "PreviewAutomationTimeoutError" });
+      expect(page.locator).not.toHaveBeenCalled();
+      // A late stop still runs, or capture would keep going with no recording to stop.
+      yield* Effect.promise(() => stopRan.promise);
+    }),
+  ).pipe(Effect.provide(layer)),
+);

apps/server/src/preview/ServerBrowser.ts

@@ -127,6 +127,8 @@ const EDITABLE_AT_POINT_SCRIPT = `(x, y) => {
 }`;
 const UNATTACHED_FILL_VIEWPORT = { width: 1280, height: 800 } as const;
 const NAVIGATION_TIMEOUT_MS = 15_000;
+// Must stay under the broker's 1 s host response grace.
+const HOST_ANSWER_MARGIN_MS = 500;
 const VIEWER_NAVIGATION_OPTIONS = { waitUntil: "commit", timeout: NAVIGATION_TIMEOUT_MS } as const;
 const ACTION_TIMELINE_LIMIT = 50;
 
@@ -1928,6 +1930,15 @@ const make = Effect.gen(function* () {
     );
   };
 
+  // Work blocked by a dialog outlives its request; point the agent at the dialog, not at `busy`.
+  const requireNoOverdueDialog = (tab: ServerTab, agentSessionId: string) => {
+    if (tab.dialog && tab.control.overdue && tab.control.agentCanActNow(agentSessionId))
+      throw new BrowserControlInterrupted({
+        message: "A browser dialog is pending. Read preview_status and use preview_dialog first.",
+        reason: "dialogPending",
+      });
+  };
+
   const requireTab = async (request: PreviewAutomationRequest) => {
     if (!request.agentSessionId)
       throw new BrowserControlInterrupted({
@@ -1976,7 +1987,7 @@ const make = Effect.gen(function* () {
 
   const runOperation = async (
     request: PreviewAutomationRequest,
-    deadlineMs: number,
+    signal: AbortSignal,
   ): Promise<unknown> => {
     const input = request.input;
     switch (request.operation) {
@@ -2101,7 +2112,7 @@ const make = Effect.gen(function* () {
             }
             return statusWithTitle(tab, request.agentSessionId);
           },
-          deadlineMs,
+          signal,
         );
       }
       case "recordingStop": {
@@ -2135,9 +2146,12 @@ const make = Effect.gen(function* () {
             message: "No recording is active for this thread.",
           });
         }
+        requireNoOverdueDialog(tab, request.agentSessionId ?? "");
         // A stop cannot act twice (a retry finds no recording);
         // dropping a late one would leave capture running.
-        return tab.control.agent(request.agentSessionId ?? "", () => stopRecording(tab));
+        return tab.control.agent(request.agentSessionId ?? "", () => stopRecording(tab), signal, {
+          runIfExpired: true,
+        });
       }
     }
     const tab = await requireTab(request);
@@ -2184,6 +2198,7 @@ const make = Effect.gen(function* () {
         return executeTabOperation(tab, request);
       });
     }
+    requireNoOverdueDialog(tab, agentSessionId);
     return tab.control.agent(
       agentSessionId,
       async () => {
@@ -2204,7 +2219,7 @@ const make = Effect.gen(function* () {
           if (generation !== tab.control.generation) ServerBrowserPage.invalidateRefs(tab.page);
         }
       },
-      deadlineMs,
+      signal,
     );
   };
 
@@ -2302,14 +2317,35 @@ const make = Effect.gen(function* () {
     }
   };
 
-  const handleRequest = (connectionId: string, request: PreviewAutomationRequest) =>
-    Effect.tryPromise({
-      try: () =>
-        runOperation(request, performance.now() + request.timeoutMs).finally(() =>
-          markUsed(request),
-        ),
+  const handleRequest = Effect.fn("ServerBrowser.handleRequest")(function* (
+    connectionId: string,
+    request: PreviewAutomationRequest,
+  ) {
+    // Queued work expires at the request's timeout; a started run past it marks its tab busy.
+    const expiry = yield* Scope.make();
+    const signal = yield* Effect.abortSignal.pipe(Scope.provide(expiry));
+    yield* Effect.sleep(request.timeoutMs).pipe(
+      Effect.andThen(Scope.close(expiry, Exit.void)),
+      Effect.forkChild,
+    );
+    const outcome = yield* Effect.tryPromise({
+      try: () => runOperation(request, signal).finally(() => markUsed(request)),
       catch: ServerBrowserPage.toOperationError,
     }).pipe(
+      // Lets the operation report its own timeout first, yet answers before the broker's
+      // response grace evicts this host. The abandoned run still holds its tab's queue.
+      Effect.timeoutOrElse({
+        duration: request.timeoutMs + HOST_ANSWER_MARGIN_MS,
+        orElse: () =>
+          Effect.fail(
+            new ServerBrowserPage.ServerBrowserOperationError({
+              tag: "PreviewAutomationTimeoutError",
+              message: `The browser did not finish ${request.operation} in ${request.timeoutMs}ms.`,
+              detail: { timeoutMs: request.timeoutMs },
+            }),
+          ),
+      }),
+      Effect.ensuring(Scope.close(expiry, Exit.void)),
       Effect.match({
         onSuccess: (result) => ({ ok: true as const, result }),
         onFailure: (error) => ({
@@ -2321,16 +2357,16 @@ const make = Effect.gen(function* () {
           },
         }),
       }),
-      Effect.flatMap((outcome) =>
-        broker.respond({
-          clientId: SERVER_HOST_CLIENT_ID,
-          connectionId,
-          requestId: request.requestId,
-          ...outcome,
-        }),
-      ),
-      Effect.ignore,
     );
+    yield* broker
+      .respond({
+        clientId: SERVER_HOST_CLIENT_ID,
+        connectionId,
+        requestId: request.requestId,
+        ...outcome,
+      })
+      .pipe(Effect.ignore);
+  });
 
   const mirrorManagerEvent = (event: PreviewEvent) =>
     Effect.promise(async () => {
@@ -2350,8 +2386,9 @@ const make = Effect.gen(function* () {
         closedSessions.delete(key);
         return;
       }
+      // Queued per tab at call time, so one stuck tab cannot hold back the others.
       if (event.type === "resized") {
-        await tab.control
+        void tab.control
           .system(async () => {
             if (
               !viewportSettingsEqual(event.snapshot.viewport ?? FILL_PREVIEW_VIEWPORT, tab.setting)