Keep a Bot's browser closed until its reset has finished, so a reset always signs it out

CHANGELOG.md

@@ -8,6 +8,16 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### Resetting a Bot's computer while the Bot is acting signs it out
+
+On a deployment with one shared computer, which is what the published image and the Helm chart run by
+default, a reset takes about two seconds to close the browser before it deletes the profile. A Bot
+action that arrived in that window started a new browser from the profile about to be deleted, so the
+Bot stayed signed in to everything until that browser next closed, while the reset reported success
+and the audit trail recorded the saved state as deleted. A Bot's browser is no longer reopened while
+it is being closed: the action waits for the reset to finish and starts signed out. Computers the
+supervisor makes per Bot were not affected.
+
 ### A long control name or value in a page snapshot is cut between characters
 
 The computer's page snapshot keeps the first 200 UTF-16 code units of each control's accessible

agent-computer/src/profiles.ts

@@ -248,6 +248,25 @@ export function createProfiles(root: string, onClosed: BrowserClosed) {
   const live = new Map<string, LiveBrowser>();
   /** Launches in flight, so a cold computer is started once however many callers ask at once. */
   const starting = new Map<string, Promise<Page>>();
+  /** Closes and resets in flight, so a Bot's browser is never reopened from a profile being closed or deleted. */
+  const closing = new Map<string, Promise<void>>();
+
+  const whileClosing = async (
+    botId: string,
+    work: () => Promise<void>,
+  ): Promise<void> => {
+    const { promise: done, resolve } = Promise.withResolvers<void>();
+    const held = Promise.all([closing.get(botId), done]).then(() => undefined);
+    closing.set(botId, held);
+    try {
+      await work();
+    } finally {
+      resolve();
+      void held.then(() => {
+        if (closing.get(botId) === held) closing.delete(botId);
+      });
+    }
+  };
 
   // Checked, not joined. `join(root, botId)` normalizes `..` away, so a Bot id of `../workspace`
   // used to resolve outside the root and `reset` would delete whatever was there.
@@ -273,8 +292,10 @@ export function createProfiles(root: string, onClosed: BrowserClosed) {
     // Bounded, because this now sits on the launch path: `enforceCap` evicts from inside another
     // Bot's launch, so a teardown that never answers would pin that launch and every caller waiting
     // on it. The close is the thing that must happen; being told about it is best effort.
-    await settleWithin(Promise.resolve(onClosed(botId)), ANNOUNCE_BUDGET_MS);
-    await closeAndWait(running.context).catch(() => undefined);
+    await whileClosing(botId, async () => {
+      await settleWithin(Promise.resolve(onClosed(botId)), ANNOUNCE_BUDGET_MS);
+      await closeAndWait(running.context).catch(() => undefined);
+    });
     return true;
   };
 
@@ -352,6 +373,9 @@ export function createProfiles(root: string, onClosed: BrowserClosed) {
      * container restarts. This turns that into one slow request instead of an outage.
      */
     async page(botId: string): Promise<Page> {
+      // Only when something is closing: an unconditional await would let a stop slip in ahead of the launch.
+      const closingNow = closing.get(botId);
+      if (closingNow) await closingNow;
       /*
        * One launch at a time per Bot. Calls that arrive during a launch wait for that launch instead
        * of starting another browser against the same profile directory.
@@ -476,8 +500,10 @@ export function createProfiles(root: string, onClosed: BrowserClosed) {
      */
     async reset(botId: string): Promise<void> {
       // Its own reason rather than borrowing stop's, so the trail says which of the two happened.
-      await closeOnRequest(botId, "it was reset");
-      await rm(directoryFor(botId), { recursive: true, force: true });
+      await whileClosing(botId, async () => {
+        await closeOnRequest(botId, "it was reset");
+        await rm(directoryFor(botId), { recursive: true, force: true });
+      });
     },
 
     /**

agent-computer/tests/browser-close-announcement.test.ts

@@ -133,6 +133,39 @@ describe.skipIf(!asked)(
       await profiles.closeAll();
     }, 60_000);
 
+    test("a page asked for while a reset is closing the browser starts signed out", async () => {
+      process.env.COMPUTER_BROWSER_IDLE_MS = String(30 * 60_000);
+      const { createProfiles } = (await import(
+        `../src/profiles?reopening=${Date.now()}`
+      )) as typeof import("../src/profiles");
+      const root3 = join(root, "reopening");
+      const profiles = createProfiles(root3, () => undefined);
+      const site = "https://example.test";
+
+      const first = await profiles.page("keeper");
+      await first.context().addCookies([
+        {
+          name: "session",
+          value: "signed-in",
+          url: site,
+          expires: Math.floor(Date.now() / 1000) + 86_400,
+        },
+      ]);
+      await profiles.stop("keeper");
+      const reopened = await profiles.page("keeper");
+      expect(await reopened.context().cookies(site)).toHaveLength(1);
+
+      const resetting = profiles.reset("keeper");
+      await new Promise((resolve) => setTimeout(resolve, 100));
+      const during = await profiles.page("keeper");
+      // Read at once, as an action would, since Chromium loads cookies on first use.
+      await during.context().cookies(site);
+      await resetting;
+
+      expect(await during.context().cookies(site)).toEqual([]);
+      await profiles.closeAll();
+    }, 60_000);
+
     test("the cap tells the Bot whose browser it closed", async () => {
       // One browser allowed, so the second Bot's launch is what closes the first Bot's browser. The
       // person watching the first one never asked for anything and is owed the message just the same.