fix(roster): list a pane's new agent at once, not at the next 5 s read

CHANGELOG.md

@@ -79,6 +79,11 @@ between releases do not reach them. Remote-PC runtime bundles are versioned sepa
   checkouts too. ([#521](https://github.com/devswha/herdr-web-ui/pull/521))
 
 ### Fixed
+- A pane whose agent has just started is listed as an agent's pane within a moment, where the
+  list on this PC could take up to 5 seconds to say so. A device that opened the pane in that
+  time took it for a shell: it opened the terminal instead of the chat, and resized the
+  terminal shared with the other devices.
+  ([#537](https://github.com/devswha/herdr-web-ui/pull/537))
 - A workspace row's state rolls up as herdr's does: a workspace with one finished agent and one
   still running shows DONE, where it showed RUN.
   ([#521](https://github.com/devswha/herdr-web-ui/pull/521))

scripts/chat-size-regression.ts

@@ -3,6 +3,7 @@ import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import type { Browser, BrowserContext, Page } from "playwright-core";
+import type { Machine } from "../shared/machines.ts";
 import { herdrRpc, paneRead, paneSendKeys, paneSendText, workspaceClose, workspaceCreate } from "../server/herdr/client.ts";
 
 /** how long a resize that should not happen gets to show up */
@@ -54,6 +55,21 @@ async function openRecording(browser: Browser, contexts: BrowserContext[], origi
   return page;
 }
 
+/**
+ * The roster pages are handed names the pane's agent. A page opened before that takes the pane for
+ * a shell: with chat as its lens for every pane it still opens the terminal there, and attaching in
+ * the terminal lens fits the shared grid to that page, which is what these checks say does not happen.
+ */
+async function agentListed(origin: string, paneId: string): Promise<void> {
+  const deadline = Date.now() + 15_000;
+  while (Date.now() < deadline) {
+    const roster = await (await fetch(`${origin}/api/machines`)).json() as { machines: Machine[] };
+    if (roster.machines.some((machine) => machine.snapshot?.panes.some((pane) => pane.pane_id === paneId && pane.agent === "claude"))) return;
+    await Bun.sleep(50);
+  }
+  throw new Error(`the roster never named the agent in ${paneId}`);
+}
+
 // the server answers an attach with input-ready, and resizes in the same step: once the
 // `nth` one is in, that attach has done whatever it does to the grid
 const attached = (page: Page, nth = 1) => page.waitForFunction((count) => (window as unknown as { frames_: { dir: string; type: string }[] }).frames_.filter((f) => f.dir === "in" && f.type === "input-ready").length >= count, nth, { timeout: 15_000 });
@@ -74,6 +90,7 @@ export async function checkChatKeepsTerminalSize(browser: Browser, origin: strin
   try {
     // an agent pane opens in the chat lens on a phone; the shell under it answers `stty size`
     await herdrRpc("pane.report_agent", { pane_id: paneId, source: "manual", agent: "claude", state: "idle" });
+    await agentListed(origin, paneId);
     const size = shellSize(paneId);
     const open = (options: Parameters<Browser["newContext"]>[0], settings: object) => openRecording(browser, contexts, origin, paneId, options, settings);
     const sent = (page: Page) => page.evaluate(() => (window as unknown as { frames_: { dir: string; type: string; keep_size?: boolean }[] }).frames_.filter((f) => f.dir === "out" && (f.type === "attach" || f.type === "resize")));
@@ -143,6 +160,7 @@ export async function checkPaneSwitchKeepsTerminalSize(browser: Browser, origin:
     }
     const [agentPane, shellPane] = panes as [string, string];
     await herdrRpc("pane.report_agent", { pane_id: agentPane, source: "manual", agent: "claude", state: "idle" });
+    await agentListed(origin, agentPane);
     const size = shellSize(agentPane);
 
     const desktop = await openRecording(browser, contexts, origin, agentPane, { viewport: { width: 1280, height: 800 } }, { language: "en", defaultView: "terminal" });

scripts/ui-regression.ts

@@ -430,10 +430,23 @@ try {
     if (hidden) Object.defineProperty(document, "visibilityState", { configurable: true, get: () => "hidden" });
     else delete (document as unknown as Record<string, unknown>).visibilityState;
     document.dispatchEvent(new Event("visibilitychange"));
+    // The app takes the change in a task it posts during the event. A hide and a show sent one
+    // after the other both land before that task, and are one render in which nothing changed:
+    // a task of the page's own, posted after the app's, has run once the app has taken it.
+    return new Promise<void>((resolve) => {
+      const channel = new MessageChannel();
+      channel.port1.onmessage = () => resolve();
+      channel.port2.postMessage(null);
+    });
   }, hidden);
   // the idle status poll runs every 30 s: a hide and a show restart it at once, onto the fake
-  await setPageHidden(true);
-  await setPageHidden(false);
+  for (const deadline = Date.now() + 10_000; ;) {
+    const restarted = page.waitForRequest((request) => new URL(request.url()).pathname === "/api/updates", { timeout: 2_000 }).then(() => true, () => false);
+    await setPageHidden(true);
+    await setPageHidden(false);
+    if (await restarted) break;
+    assert.ok(Date.now() < deadline, "a hide and a show restart the update status poll");
+  }
   await page.keyboard.press("ControlOrMeta+Shift+Comma");
   const checkUpdates = page.getByRole("dialog", { name: "Settings" }).getByRole("button", { name: "Check for updates", exact: true });
   await checkUpdates.waitFor();

server/collector.test.ts

@@ -136,10 +136,16 @@ function recorder() {
     resyncs: [] as { panes: string[]; newer: string[] }[],
     structure: 0,
     statuses: [] as string[],
+    /** statuses and reconciled snapshots, in the order they were told */
+    order: [] as string[],
   };
   const handlers: StatusCollectorHandlers = {
-    onStatus: (paneId, status, _agent, replay) => log.statuses.push(`${paneId}:${status}${replay ? ` (was ${replay.before})` : ""}`),
+    onStatus: (paneId, status, _agent, replay) => {
+      log.statuses.push(`${paneId}:${status}${replay ? ` (was ${replay.before})` : ""}`);
+      log.order.push(`${paneId}:${status}`);
+    },
     onBaseline: () => { log.baselines += 1; },
+    onReconciled: (panes) => log.order.push(`reconciled ${panes.map((p) => `${p.pane_id}:${p.agent_status}`).join(" ")}`),
     onResync: (panes, newer) => log.resyncs.push({ panes: panes.map((p) => p.pane_id), newer: [...newer].sort() }),
     onPaneEnded: () => {},
     onStructureChange: () => { log.structure += 1; },
@@ -180,19 +186,25 @@ describe("startStatusCollector recovery", () => {
     expect(second.paneIds).toEqual(["w1:p1", "w2:p1"]);
     herdr.setPanes([paneOf("w1:p1", "idle"), paneOf("w2:p1", "idle")]);
     expect(log.statuses).toEqual([]);
+    log.order.length = 0;
     second.start();
     await tick(20);
     // once, as the event would have been; a pane first seen is no change; nothing was "lost"
     expect(log.statuses).toEqual(["w1:p1:idle (was working)"]);
     expect(log.resyncs).toEqual([]);
+    // the snapshot is handed on as reconciled only after what it replays has been told
+    expect(log.order).toEqual(["w1:p1:idle", "reconciled w1:p1:idle w2:p1:idle"]);
     // the same snapshot again tells nothing more, and an event since outranks an older snapshot
     herdr.hold();
     herdr.lifecycle().emit({ event: "pane_closed", data: { type: "pane_closed" } });
     await tick(20);
+    log.order.length = 0;
     second.emit(statusFrame("w1:p1", "working"));
     herdr.answerAll();
     await tick();
     expect(log.statuses).toEqual(["w1:p1:idle (was working)", "w1:p1:working"]);
+    // and that snapshot is not handed on for the pane the event spoke of
+    expect(log.order).toEqual(["w1:p1:working", "reconciled w2:p1:idle"]);
     collector.stop();
   });
 

server/collector.ts

@@ -77,6 +77,12 @@ export interface StatusCollectorHandlers {
    * truth for every pane but the `newer` ones, which had an event since it was asked for.
    */
   onResync?: (panes: readonly HerdrPane[], newer: ReadonlySet<string>) => void;
+  /**
+   * The panes of a reconcile's snapshot, once what that snapshot replays has been told: for a
+   * consumer whose own reading of herdr must not run ahead of those status events. A pane with
+   * an event, an end or a focus since the snapshot was asked for is left out: it is newer.
+   */
+  onReconciled?: (panes: readonly HerdrPane[]) => void;
   onPaneEnded: (paneId: string) => void;
   onStructureChange: () => void;
   /** herdr's focus moved onto this pane: whoever is at its terminal has it in front */
@@ -368,6 +374,8 @@ export function startStatusCollector(handlers: StatusCollectorHandlers, override
           handlers.onStatus(pane.pane_id, pane.agent_status, agent, { before: before.status, agent: before.agent });
         }
       }
+      // a pane heard of since the snapshot was asked for is newer than what it shows of it
+      handlers.onReconciled?.(snapshot.panes.filter((pane) => !newerThan(pane.pane_id, askedAt)));
       // gone before this snapshot; a pane heard of since may be too new for it
       for (const [paneId, seq] of lastEventOf) if (seq <= askedAt && !paneIds.includes(paneId)) lastEventOf.delete(paneId);
       for (const paneId of [...heard.keys()]) if (!paneIds.includes(paneId) && (lastEventOf.get(paneId) ?? 0) <= askedAt) heard.delete(paneId);

server/index.ts

@@ -1124,6 +1124,9 @@ export function createServer(
       if (replay && (omo.runs(paneId) || !completions.replayed(paneId, raw, replay))) return;
       // another agent took an OmO pane: what OmO worked on there is not that agent's to finish
       if (omo.named(paneId, agent)) completions.forget(paneId);
+      // the frame below names no agent: one herdr names anew is read into the roster now. An OmO
+      // pane is `omo` in every snapshot, whatever herdr calls it in an event
+      machines?.localAgents([{ pane_id: paneId, agent: omo.runs(paneId) ? "omo" : agent }]);
       // herdr says `claude/idle` for an OmO pane whatever it does: its own status stands
       if (omo.tracks(paneId)) return;
       // back at work, the agent has had its answer, maybe from a terminal: the same prompt on
@@ -1155,6 +1158,9 @@ export function createServer(
         pending.status(pane.pane_id, status); drainPending(pane.pane_id);
       }
     },
+    // a pane created a moment ago got its agent after the roster's own read of it. After the
+    // snapshot's replays: a roster read before them could show a finish ahead of its status frame
+    onReconciled: (panes) => machines?.localAgents(panes),
     // the tracker first: what it makes of each pane (a finish after work is done, not idle) is
     // what the alerts are measured against from here, or the next event would alert of it
     onResync: (panes, newer) => {

server/machines-refresh.test.ts

@@ -181,6 +181,47 @@ describe("machine snapshot refresh races", () => {
     expect(manager.list()[0]?.snapshot?.panes).toEqual([]);
   });
 
+  it("reads the roster again when herdr names a local pane's agent anew, not at the next 5 s read", async () => {
+    const { manager, drive, local } = await fixture();
+    await manager.refreshLocal();
+    // what the collector has heard so far is what the roster shows
+    manager.localAgents(snapshot("idle").panes);
+    await until(() => !drive.localBusy && manager.list()[0]?.snapshot?.panes[0]?.agent === "codex");
+    const before = local.calls();
+    // another agent starts there: herdr names it in a status event, and the frame that tells pages of it names none
+    local.set({ ...snapshot("idle"), panes: [{ ...snapshot("idle").panes[0]!, agent: "opencode" }] });
+    manager.localAgents([{ pane_id: "fixture:p1", agent: "opencode" }]);
+    await until(() => manager.list()[0]?.snapshot?.panes[0]?.agent === "opencode");
+    expect(local.calls()).toBe(before + 1);
+    // said again, in an event or a snapshot, it is no news: nothing is read
+    manager.localAgents([{ pane_id: "fixture:p1", agent: "opencode" }]);
+    manager.localAgents([{ pane_id: "fixture:p1", agent: "opencode" }, { pane_id: "fixture:p2", agent: null }]);
+    await until(() => !drive.localBusy);
+    expect(local.calls()).toBe(before + 1);
+  });
+
+  it("hears a pane's agent anew once the pane has gone: its id used again by the same agent is read at once", async () => {
+    const { manager, drive, local } = await fixture();
+    await manager.refreshLocal();
+    manager.localAgents(snapshot("idle").panes);
+    await until(() => !drive.localBusy);
+    // the pane closes: the roster no longer lists it
+    local.set({ ...snapshot("idle"), panes: [] });
+    await manager.refreshLocal();
+    const closed = local.calls();
+    // the id comes back with the same agent: news again
+    local.set(snapshot("idle"));
+    manager.localAgents(snapshot("idle").panes);
+    await until(() => manager.list()[0]?.snapshot?.panes[0]?.agent === "codex" && !drive.localBusy);
+    expect(local.calls()).toBe(closed + 1);
+    // so is one that ended, before any roster was read without it
+    manager.localMessage({ type: "pane-exited", pane_id: "fixture:p1" });
+    await until(() => !drive.localBusy);
+    const ended = local.calls();
+    manager.localAgents(snapshot("idle").panes);
+    await until(() => local.calls() === ended + 1 && !drive.localBusy);
+  });
+
   it("keeps a newer remote status and retries the invalidated HTTP snapshot", async () => {
     const { drive, rosters, remote, runtime, send } = await remoteFixture();
     remote.hold();

server/machines.ts

@@ -85,6 +85,8 @@ export class MachineManager {
   private localBusy = false;
   private localRefreshQueued = false;
   private localRevision = 0;
+  /** each local pane's agent as herdr last named it to the status collector */
+  private heardAgents = new Map<string, string | null>();
   private localTimer: ReturnType<typeof setInterval>;
   private saveTimer?: ReturnType<typeof setTimeout>;
   private statePath: string;
@@ -137,11 +139,31 @@ export class MachineManager {
       if (this.localBusy) this.localRefreshQueued = true;
     }
     this.emit({ type: "machine-message", machine_id: LOCAL_MACHINE, message });
+    // its id used again is a pane heard of for the first time
+    if (message.type === "pane-exited") this.heardAgents.delete(message.pane_id);
     if (message.type === "pane-status" && this.local.snapshot) {
       this.local.snapshot = { ...this.local.snapshot, panes: this.local.snapshot.panes.map((p: HerdrPane) => p.pane_id === message.pane_id ? paneAfterStatus(p, message) : p) };
     }
     if (message.type === "session-changed" || message.type === "pane-exited") void this.refreshLocal();
   }
+  /**
+   * Which agent local panes run, as the status collector heard it: in an event (that pane), or
+   * in the snapshot it reconciles from. The roster is read on a timer and patched by status
+   * frames, which name no agent, so an agent herdr names anew would show up to 5 s late. A page
+   * that opens the pane meanwhile takes it for a shell: it opens the terminal lens, and attaching
+   * there fits the shared grid to that page. Read again now.
+   */
+  localAgents(panes: readonly Pick<HerdrPane, "pane_id" | "agent">[]): void {
+    let news = false;
+    for (const pane of panes) {
+      const agent = pane.agent ?? null;
+      const before = this.heardAgents.get(pane.pane_id);
+      this.heardAgents.set(pane.pane_id, agent);
+      // a pane first heard of as a shell is what the roster's own read of it showed
+      if (before !== agent && !(before === undefined && agent === null)) news = true;
+    }
+    if (news) void this.refreshLocal();
+  }
   async refreshLocal(): Promise<void> {
     if (this.stopped) return;
     if (this.localBusy) { this.localRefreshQueued = true; return; }
@@ -156,6 +178,8 @@ export class MachineManager {
           // A newer event already patched the roster. Never publish this older load.
           if (revision !== this.localRevision) { this.localRefreshQueued = true; continue; }
           this.local.snapshot = snapshot; this.local.state = "connected"; this.local.error = null;
+          // a pane the roster no longer lists is gone: the same id later is a pane heard of anew
+          for (const paneId of [...this.heardAgents.keys()]) if (!snapshot.panes.some((pane: HerdrPane) => pane.pane_id === paneId)) this.heardAgents.delete(paneId);
         } catch (e) {
           if (this.stopped) break;
           if (revision !== this.localRevision) { this.localRefreshQueued = true; continue; }