fix(dashboard): a /chat tab's attach token is per tab, and one token maps to one PTY

Two tabs on the dashboard /chat collapsed into one session: ChatPage kept its ?attach= token in localStorage, which every tab on the origin shares, and the PTY registry maps one token to exactly one PTY — so the second tab closed the first one with 4409 and both showed one conversation/model.

The token now lives per tab in sessionStorage and is claimed with a Web Lock, so a second tab (including a Chrome "Duplicate tab", which clones sessionStorage) mints its own instead of sharing one; a reload finds its own token free again and still reattaches to the living PTY.

PtySessionRegistry.attach_or_spawn also spanned awaits, so two connections racing one token both forked a PTY: the token then mapped to whichever registered last while the other tab's live session fell out of the registry (never reaped, invisible to close_all), and a reattach landed on the other tab's terminal. The get-or-spawn decision is serialized now.

Issue #115304.

hermes_cli/pty_session.py

@@ -173,24 +173,35 @@ class PtySessionRegistry:
         self._buffer_cap = buffer_cap
         self._read_timeout = read_timeout
         self._sessions: Dict[str, PtySession] = {}
+        # The get-or-spawn decision spans awaits (reap_idle, the spawn thread,
+        # session.start), so two connections racing one attach token both saw
+        # "no session" and forked a PTY each: the token then mapped to whichever
+        # registered last while the other tab's live session fell out of the
+        # registry — never reaped, and a reattach landed on the wrong terminal
+        # (#115304). Serialize the decision so a token maps to one PTY.
+        # ponytail: one registry-wide lock, not per key — argv resolution is
+        # already serialized globally for the same reason, and a spawn only
+        # delays NEW chats. Per-key locks if spawn throughput ever matters.
+        self._attach_lock = asyncio.Lock()
 
     async def attach_or_spawn(self, key: str, *, spawn: Callable[[], object]) -> Tuple[PtySession, bool]:
         await self.reap_idle()
-        existing = self._sessions.get(key)
-        if existing is not None and existing.alive:
-            return existing, False
-        if existing is not None:                       # dead remnant
-            await existing.close()
-            self._sessions.pop(key, None)
-        if len(self._sessions) >= self._max:
-            self._reap_one_idle_or_raise()
-        # PTY spawn does blocking fork/exec work — keep it off the event loop.
-        # See #53227.
-        bridge = await asyncio.to_thread(spawn)
-        session = PtySession(key, bridge, buffer_cap=self._buffer_cap, read_timeout=self._read_timeout)
-        await session.start()
-        self._sessions[key] = session
-        return session, True
+        async with self._attach_lock:
+            existing = self._sessions.get(key)
+            if existing is not None and existing.alive:
+                return existing, False
+            if existing is not None:                       # dead remnant
+                await existing.close()
+                self._sessions.pop(key, None)
+            if len(self._sessions) >= self._max:
+                self._reap_one_idle_or_raise()
+            # PTY spawn does blocking fork/exec work — keep it off the event loop.
+            # See #53227.
+            bridge = await asyncio.to_thread(spawn)
+            session = PtySession(key, bridge, buffer_cap=self._buffer_cap, read_timeout=self._read_timeout)
+            await session.start()
+            self._sessions[key] = session
+            return session, True
 
     def detach(self, key: str, ws) -> None:
         s = self._sessions.get(key)

tests/hermes_cli/test_pty_session.py

@@ -324,6 +324,48 @@ async def test_new_key_at_capacity_raises_when_none_reapable():
     await reg.close_all()
 
 
+@pytest.mark.asyncio
+async def test_concurrent_attach_on_one_token_forks_one_pty():
+    """Two connections racing one attach token must share ONE registered PTY.
+
+    The get-or-spawn decision spans awaits (reap, the spawn thread, start()), so
+    both racing callers used to see "no session" and fork their own: the token
+    then mapped to whichever registered last, the other tab's live session fell
+    out of the registry (never reaped) and a reattach landed on the wrong
+    terminal (#115304).
+    """
+    from hermes_cli.pty_session import WS_CLOSE_SUPERSEDED
+
+    reg = make_registry()
+    spawned = []
+
+    def spawn():
+        bridge = FakeBridge([b"", b""])
+        spawned.append(bridge)
+        return bridge
+
+    (s1, created1), (s2, created2) = await asyncio.gather(
+        reg.attach_or_spawn("tok", spawn=spawn),
+        reg.attach_or_spawn("tok", spawn=spawn),
+    )
+
+    assert len(spawned) == 1                    # one token, one PTY
+    assert (s1, created1) == (s2, True)
+    assert created2 is False
+    assert s1.bridge is spawned[0]
+    assert list(reg._sessions.values()) == [s1]  # every handed-out session is tracked
+
+    # Whichever socket attached last owns the terminal; the loser is superseded
+    # by contract, so no viewer is left writing into an untracked PTY.
+    ws_a, ws_b = FakeWS(), FakeWS()
+    await s1.attach(ws_a)
+    await s2.attach(ws_b)
+    assert reg._sessions["tok"] is s1
+    assert s1._ws is ws_b and ws_b.close_code is None
+    assert ws_a.close_code == WS_CLOSE_SUPERSEDED
+    await reg.close_all()
+
+
 @pytest.mark.asyncio
 async def test_reaper_loop_invokes_reap(monkeypatch):
     from hermes_cli.pty_session import run_reaper

web/src/lib/pty-attach-token.test.ts

@@ -0,0 +1,143 @@
+// @vitest-environment jsdom
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+/**
+ * One "browser tab": its own sessionStorage, its own module state (the claimed
+ * token) and the pages share the fake Web Locks manager, exactly like tabs on
+ * one origin do.
+ */
+type Tab = {
+  ptyAttachToken: (rotate?: boolean) => Promise<string>;
+  storage: Storage;
+};
+
+const KEY = "hermes.pty.token.chat";
+
+function fakeStorage(seed: Record<string, string> = {}): Storage {
+  const store: Record<string, string> = { ...seed };
+  return {
+    clear: () => {
+      for (const key of Object.keys(store)) delete store[key];
+    },
+    getItem: (key: string) => store[key] ?? null,
+    key: (index: number) => Object.keys(store)[index] ?? null,
+    get length() {
+      return Object.keys(store).length;
+    },
+    removeItem: (key: string) => {
+      delete store[key];
+    },
+    setItem: (key: string, value: string) => {
+      store[key] = String(value);
+    },
+  } as Storage;
+}
+
+/** Web Locks, as far as attach-token claiming uses them. */
+function fakeLocks(held: Set<string>, requests: string[] = []) {
+  return {
+    request: (
+      name: string,
+      _options: { ifAvailable?: boolean },
+      callback: (lock: { name: string } | null) => unknown,
+    ) => {
+      requests.push(name);
+      if (held.has(name)) {
+        callback(null);
+        return Promise.resolve();
+      }
+      held.add(name);
+      return Promise.resolve(callback({ name }));
+    },
+  };
+}
+
+/** The lock a tab holds disappears when the document unloads (reload/close). */
+function simulateUnload(held: Set<string>) {
+  held.clear();
+}
+
+async function openTab(
+  held: Set<string>,
+  seed: Record<string, string> = {},
+  requests: string[] = [],
+): Promise<Tab> {
+  vi.resetModules(); // a new document has its own claimed-token state
+  const storage = fakeStorage(seed);
+  Object.defineProperty(window, "sessionStorage", { configurable: true, value: storage });
+  Object.defineProperty(window.navigator, "locks", {
+    configurable: true,
+    value: fakeLocks(held, requests),
+  });
+  const { ptyAttachToken } = await import("./pty-attach-token");
+  return { ptyAttachToken, storage };
+}
+
+beforeEach(() => {
+  vi.stubGlobal("crypto", {
+    getRandomValues: (values: Uint8Array) => {
+      values.fill(Math.floor(Math.random() * 256));
+      return values;
+    },
+  });
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+  vi.restoreAllMocks();
+  delete (window.navigator as { locks?: unknown }).locks;
+});
+
+describe("ptyAttachToken", () => {
+  it("mints its own token for a duplicate tab instead of sharing the live one", async () => {
+    const held = new Set<string>();
+    const first = await openTab(held);
+    const tokenA = await first.ptyAttachToken();
+
+    // Chrome's "Duplicate tab" clones sessionStorage into the new tab.
+    const duplicate = await openTab(held, { [KEY]: tokenA });
+    const tokenB = await duplicate.ptyAttachToken();
+
+    expect(tokenB).not.toBe(tokenA);
+    expect(duplicate.storage.getItem(KEY)).toBe(tokenB);
+  });
+
+  it("reuses the stored token after a reload, once the old document released it", async () => {
+    const held = new Set<string>();
+    const before = await openTab(held, { [KEY]: "tab-token" });
+    expect(await before.ptyAttachToken()).toBe("tab-token");
+
+    simulateUnload(held); // reload: the previous document's lock is gone
+    const reloaded = await openTab(held, { [KEY]: "tab-token" });
+
+    expect(await reloaded.ptyAttachToken()).toBe("tab-token");
+  });
+
+  it("claims the token once per document so a reconnect never re-locks it", async () => {
+    const held = new Set<string>();
+    const requests: string[] = [];
+    const tab = await openTab(held, { [KEY]: "tab-token" }, requests);
+
+    expect(await tab.ptyAttachToken()).toBe("tab-token");
+    expect(await tab.ptyAttachToken()).toBe("tab-token");
+
+    expect(requests).toEqual([`hermes.pty.attach.tab-token`]);
+  });
+
+  it("rotates the token for an explicit fresh session", async () => {
+    const held = new Set<string>();
+    const tab = await openTab(held, { [KEY]: "tab-token" });
+
+    const rotated = await tab.ptyAttachToken(true);
+
+    expect(rotated).not.toBe("tab-token");
+    expect(tab.storage.getItem(KEY)).toBe(rotated);
+  });
+
+  it("still isolates tabs on sessionStorage when the browser has no Web Locks", async () => {
+    const tab = await openTab(new Set(), { [KEY]: "tab-token" });
+    Object.defineProperty(window.navigator, "locks", { configurable: true, value: undefined });
+
+    expect(await tab.ptyAttachToken()).toBe("tab-token");
+  });
+});

web/src/lib/pty-attach-token.ts

@@ -0,0 +1,76 @@
+/**
+ * Identity of THIS browser tab's keep-alive PTY, sent as `?attach=`.
+ *
+ * The dashboard's PTY registry maps one attach token to exactly one PTY, so a
+ * token two tabs present makes the second tab take the first one's terminal
+ * over (the first is closed 4409 and goes dead — the reported multi-tab
+ * interference). localStorage is shared per ORIGIN, so every tab read the same
+ * value, and Chrome's "Duplicate tab" clones sessionStorage into the new tab,
+ * so neither storage alone isolates the tabs. The token therefore lives in
+ * sessionStorage *and* is claimed with a Web Lock: a tab whose token is
+ * already claimed by a live document mints its own instead of sharing one.
+ * See #115304.
+ */
+
+const PTY_ATTACH_TOKEN_KEY = "hermes.pty.token.chat";
+
+/** The token this document claimed — a reconnect must not re-request the lock
+ *  (Web Locks are not reentrant, so asking twice in one document deadlocks). */
+let claimedToken: string | null = null;
+
+function tabStorage(): Storage | null {
+  try {
+    return typeof window === "undefined" ? null : window.sessionStorage;
+  } catch {
+    return null; /* private mode / storage blocked */
+  }
+}
+
+function mint(): string {
+  const a = new Uint8Array(16);
+  crypto.getRandomValues(a);
+  return Array.from(a, (b) => b.toString(16).padStart(2, "0")).join("");
+}
+
+/** Never settles, so the lock stays held for this document's lifetime. */
+const HOLD: Promise<void> = new Promise<void>(() => {});
+
+/**
+ * Claim `token` for this document. True when we now hold it; the browser
+ * releases it when the document unloads, so a reload of this tab finds the
+ * token free again while a live second tab does not. Browsers without the Web
+ * Locks API fall back to sessionStorage isolation only.
+ */
+async function claim(token: string): Promise<boolean> {
+  const locks = typeof navigator === "undefined" ? undefined : navigator.locks;
+  if (!locks) return true;
+  return new Promise<boolean>((resolve) => {
+    void locks.request(`hermes.pty.attach.${token}`, { ifAvailable: true }, (lock) => {
+      resolve(lock !== null);
+      return lock ? HOLD : Promise.resolve();
+    });
+  });
+}
+
+/**
+ * This tab's `?attach=` token, minted when `rotate` starts a fresh session (the
+ * old keep-alive PTY must NOT be reattached) or when another live tab already
+ * claimed the stored one.
+ */
+export async function ptyAttachToken(rotate = false): Promise<string> {
+  const stored =
+    claimedToken ?? tabStorage()?.getItem(PTY_ATTACH_TOKEN_KEY) ?? "";
+  if (!rotate && stored && (stored === claimedToken || (await claim(stored)))) {
+    claimedToken = stored;
+    return stored;
+  }
+  const token = mint();
+  try {
+    tabStorage()?.setItem(PTY_ATTACH_TOKEN_KEY, token);
+  } catch {
+    /* ignore */
+  }
+  await claim(token);
+  claimedToken = token;
+  return token;
+}

web/src/pages/ChatPage.tsx

@@ -92,39 +92,16 @@ import {
   ptyRejectionBanner,
   type PtyBannerAction,
 } from "@/lib/pty-close-copy";
+import { ptyAttachToken } from "@/lib/pty-attach-token";
 import { loseWebglContexts } from "@/lib/xterm-webgl-release";
 import { PluginSlot } from "@/plugins";
 import { useTheme } from "@/themes";
 import { useProfileScope } from "@/contexts/useProfileScope";
 import { errorMessage } from "@/lib/api-error";
 
-// Stable per-browser token identifying THIS chat tab's keep-alive PTY session.
-// Sent as ?attach=; lets a refresh/disconnect reattach to the same live process
-// instead of spawning a fresh one. Per-localStorage, so other devices can't grab it.
-// ``rotate`` mints a new token — used when the user explicitly starts a fresh
-// session so the old keep-alive PTY is NOT reattached (the registry reaps it).
-const PTY_ATTACH_TOKEN_KEY = "hermes.pty.token.chat";
-function ptyAttachToken(rotate = false): string {
-  let t = "";
-  if (!rotate) {
-    try {
-      t = window.localStorage.getItem(PTY_ATTACH_TOKEN_KEY) ?? "";
-    } catch {
-      /* private mode / storage blocked */
-    }
-  }
-  if (!t) {
-    const a = new Uint8Array(16);
-    crypto.getRandomValues(a);
-    t = Array.from(a, (b) => b.toString(16).padStart(2, "0")).join("");
-    try {
-      window.localStorage.setItem(PTY_ATTACH_TOKEN_KEY, t);
-    } catch {
-      /* ignore */
-    }
-  }
-  return t;
-}
+// Per-tab keep-alive identity (`?attach=`): lives in pty-attach-token.ts so a
+// second tab — including a Chrome "Duplicate tab" — gets its own PTY instead of
+// taking over this one. See #115304.
 
 // Channel id ties this chat tab's PTY child (publisher) to its sidebar
 // (subscriber).  Generated once per mount so a tab refresh starts a fresh
@@ -1254,7 +1231,7 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) {
       // Keep-alive identity: reattach to this tab's living PTY across
       // refresh/transient drops. A forced-fresh start rotates the token so
       // the previous keep-alive PTY is not reattached (registry reaps it).
-      params.attach = ptyAttachToken(forceFresh);
+      params.attach = await ptyAttachToken(forceFresh);
       // Profile-scoped chat: the PTY child gets HERMES_HOME pointed at the
       // selected profile, so the conversation runs with that profile's model,
       // skills, memory, and sessions (see web_server._resolve_chat_argv).