Prevent duplicate session browser controllers

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Fixed
 
+- Prevent concurrent browser controllers from mutating the same ask-pro session.
 - Preserve mutable recovery files when a replacement write fails.
 - Require state-bearing Temporary Chat evidence, delegate manual-login restart
   cleanup correctly, and persist captured answers before browser cleanup.

src/ask-pro/browserRunner.ts

@@ -25,6 +25,7 @@ import {
   writeAskProBrowserMetadata,
 } from "./session.js";
 import { harvestLatestAssistantZip, writeResponseZipManifest } from "./responseZip.js";
+import { withSessionControllerLease } from "./sessionControllerLease.js";
 
 const DEFAULT_TIMEOUT_MS = 180 * 60 * 1000;
 const MANUAL_LOGIN_WAIT_MS = 10 * 60 * 1000;
@@ -44,7 +45,14 @@ export interface RunAskProBrowserSessionOptions {
   verbose?: boolean;
 }
 
-export async function runAskProBrowserSession({
+export async function runAskProBrowserSession(
+  options: RunAskProBrowserSessionOptions,
+): Promise<BrowserRunResult> {
+  const { dir } = getAskProSessionPaths(options.cwd, options.sessionId);
+  return withSessionControllerLease(dir, () => runAskProBrowserSessionWithLease(options));
+}
+
+async function runAskProBrowserSessionWithLease({
   cwd,
   sessionId,
   temporary,
@@ -239,7 +247,7 @@ export async function runAskProBrowserSession({
           chromeMode: "launched",
         },
       });
-      return runAskProBrowserSession({
+      return runAskProBrowserSessionWithLease({
         cwd,
         sessionId,
         temporary: false,
@@ -299,7 +307,14 @@ export async function runAskProBrowserSession({
   }
 }
 
-export async function resumeAskProBrowserSession({
+export async function resumeAskProBrowserSession(
+  options: RunAskProBrowserSessionOptions,
+): Promise<void> {
+  const { dir } = getAskProSessionPaths(options.cwd, options.sessionId);
+  return withSessionControllerLease(dir, () => resumeAskProBrowserSessionWithLease(options));
+}
+
+async function resumeAskProBrowserSessionWithLease({
   cwd,
   sessionId,
   temporary,
@@ -334,7 +349,7 @@ export async function resumeAskProBrowserSession({
       sessionId,
       "Retrying Temporary Chat session in normal ChatGPT; opening managed browser submission.",
     );
-    await runAskProBrowserSession({
+    await runAskProBrowserSessionWithLease({
       cwd,
       sessionId,
       temporary: false,
@@ -359,7 +374,7 @@ export async function resumeAskProBrowserSession({
     const shouldPreserveUrl =
       effectiveTemporary !== undefined ||
       (metadata.url !== undefined && !storedUrlIsDefaultTemporary);
-    await runAskProBrowserSession({
+    await runAskProBrowserSessionWithLease({
       cwd,
       sessionId,
       temporary: effectiveTemporary,

src/ask-pro/sessionControllerLease.ts

@@ -0,0 +1,68 @@
+import { randomUUID } from "node:crypto";
+import { mkdir, readdir, rename, rm, stat, writeFile } from "node:fs/promises";
+import path from "node:path";
+import { isProcessAlive } from "../browser/profileState.js";
+
+const LEASE_FILENAME = ".controller.lease";
+const OWNER_PATTERN = /^(\d+)-(.+)\.owner$/;
+const REMOVE_OPTIONS = { recursive: true, force: true, maxRetries: 5, retryDelay: 20 } as const;
+
+export async function withSessionControllerLease<T>(
+  sessionDir: string,
+  action: () => Promise<T>,
+): Promise<T> {
+  const id = randomUUID();
+  const leasePath = path.join(sessionDir, LEASE_FILENAME);
+  const candidatePath = `${leasePath}.${id}.candidate`;
+  const ownerName = `${process.pid}-${id}.owner`;
+  await mkdir(candidatePath);
+
+  let acquired = false;
+  try {
+    await writeFile(path.join(candidatePath, ownerName), "");
+    for (;;) {
+      try {
+        await rename(candidatePath, leasePath);
+        acquired = true;
+        break;
+      } catch (error) {
+        if (!(await stat(leasePath).catch(() => null))) throw error;
+      }
+
+      const existingOwner = (await readdir(leasePath)).find((entry) => OWNER_PATTERN.test(entry));
+      const existingPid = Number(existingOwner?.match(OWNER_PATTERN)?.[1]);
+      if (!existingOwner || !Number.isInteger(existingPid) || existingPid <= 0) {
+        throw new Error("ask-pro session controller lease is unreadable");
+      }
+      if (isProcessAlive(existingPid)) {
+        throw new Error(`ask-pro session controller is already running (pid ${existingPid})`);
+      }
+
+      try {
+        await rename(path.join(leasePath, existingOwner), path.join(leasePath, ownerName));
+      } catch (error) {
+        if ((error as NodeJS.ErrnoException).code === "ENOENT") continue;
+        throw error;
+      }
+      const stalePath = `${leasePath}.${id}.stale`;
+      await rename(leasePath, stalePath);
+      await rm(stalePath, REMOVE_OPTIONS);
+    }
+
+    try {
+      return await action();
+    } finally {
+      const ownsLease = (await readdir(leasePath).catch((): string[] => [])).includes(ownerName);
+      if (ownsLease) {
+        const retiredPath = `${leasePath}.${id}.retired`;
+        const retired = await rename(leasePath, retiredPath).then(
+          () => true,
+          () => false,
+        );
+        if (retired) await rm(retiredPath, REMOVE_OPTIONS).catch(() => undefined);
+      }
+    }
+  } finally {
+    if (!acquired) await rm(candidatePath, REMOVE_OPTIONS).catch(() => undefined);
+  }
+}

tests/ask-pro/browserRunner.test.ts

@@ -106,6 +106,14 @@ const { AskProNeedsAuthError, resumeAskProBrowserSession, runAskProBrowserSessio
 
 const tempDirs: string[] = [];
 
+function deferred(): { promise: Promise<void>; resolve: () => void } {
+  let resolve!: () => void;
+  const promise = new Promise<void>((done) => {
+    resolve = done;
+  });
+  return { promise, resolve };
+}
+
 beforeEach(async () => {
   managedProfileState.root = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-managed-root-"));
   managedProfileState.legacyRoot = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-legacy-root-"));
@@ -134,6 +142,119 @@ afterEach(async () => {
 });
 
 describe("ask-pro browser runner", () => {
+  test("rejects a live same-session controller and releases the lease in finally", async () => {
+    const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-controller-live-"));
+    tempDirs.push(cwd);
+    const session = await createAskProSession({
+      cwd,
+      question: "Keep one controller per session.",
+      filePatterns: [],
+      dryRun: false,
+    });
+    const entered = deferred();
+    const finish = deferred();
+    runBrowserModeMock.mockImplementationOnce(async () => {
+      entered.resolve();
+      await finish.promise;
+      return {
+        answerText: "agent answer",
+        answerMarkdown: "# Agent\n",
+        browserTransport: "launched",
+      };
+    });
+
+    const first = runAskProBrowserSession({ cwd, sessionId: session.id });
+    await entered.promise;
+    await expect(resumeAskProBrowserSession({ cwd, sessionId: session.id })).rejects.toThrow(
+      `ask-pro session controller is already running (pid ${process.pid})`,
+    );
+    finish.resolve();
+    await first;
+
+    runBrowserModeMock.mockRejectedValueOnce(new Error("browser failed"));
+    await expect(runAskProBrowserSession({ cwd, sessionId: session.id })).rejects.toThrow(
+      "browser failed",
+    );
+    await runAskProBrowserSession({ cwd, sessionId: session.id });
+    await expect(fs.stat(path.join(session.dir, ".controller.lease"))).rejects.toMatchObject({
+      code: "ENOENT",
+    });
+  });
+
+  test("recovers a stale session-controller owner", async () => {
+    const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-controller-stale-"));
+    tempDirs.push(cwd);
+    const session = await createAskProSession({
+      cwd,
+      question: "Recover the stale controller.",
+      filePatterns: [],
+      dryRun: false,
+    });
+    const leaseDir = path.join(session.dir, ".controller.lease");
+    await fs.mkdir(leaseDir);
+    await fs.writeFile(path.join(leaseDir, "2147483647-stale.owner"), "");
+    const entered = deferred();
+    const finish = deferred();
+    runBrowserModeMock.mockImplementation(async () => {
+      entered.resolve();
+      await finish.promise;
+      return {
+        answerText: "agent answer",
+        answerMarkdown: "# Agent\n",
+        browserTransport: "launched",
+      };
+    });
+
+    const runs = [
+      runAskProBrowserSession({ cwd, sessionId: session.id }),
+      runAskProBrowserSession({ cwd, sessionId: session.id }),
+    ];
+    const rejected = deferred();
+    for (const run of runs) void run.catch(() => rejected.resolve());
+    const outcomesPromise = Promise.allSettled(runs);
+    await entered.promise;
+    await rejected.promise;
+    finish.resolve();
+    const outcomes = await outcomesPromise;
+
+    expect(runBrowserModeMock).toHaveBeenCalledTimes(1);
+    expect(outcomes.map(({ status }) => status).sort()).toEqual(["fulfilled", "rejected"]);
+  });
+
+  test("allows browser controllers for different sessions to run concurrently", async () => {
+    const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-controller-concurrent-"));
+    tempDirs.push(cwd);
+    const sessions = await Promise.all(
+      ["first", "second"].map((name) =>
+        createAskProSession({
+          cwd,
+          question: `${name} concurrent session.`,
+          filePatterns: [],
+          dryRun: false,
+        }),
+      ),
+    );
+    const bothEntered = deferred();
+    const finish = deferred();
+    let entered = 0;
+    runBrowserModeMock.mockImplementation(async () => {
+      if (++entered === 2) bothEntered.resolve();
+      await finish.promise;
+      return {
+        answerText: "agent answer",
+        answerMarkdown: "# Agent\n",
+        browserTransport: "launched",
+      };
+    });
+
+    const runs = sessions.map((session) => runAskProBrowserSession({ cwd, sessionId: session.id }));
+    await bothEntered.promise;
+    finish.resolve();
+    await Promise.all(runs);
+
+    expect(runBrowserModeMock).toHaveBeenCalledTimes(2);
+  });
+
   test("runs ask-pro sessions with the explicit agent profile", async () => {
     const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "ask-pro-run-agent-"));
     tempDirs.push(cwd);