fix(chat): keep unconfirmed legacy sends and share the pane's command list (#1591)

changelog.d/legacy-send-unconfirmed.md

@@ -0,0 +1,7 @@
+### Fixed
+- A message sent to a PC whose Ovis is too old to confirm messages is no longer treated as delivered:
+  the composer, Send now on a held copy and the held queue's Send now keep its text as a
+  "Not confirmed" row to copy or discard, instead of dropping it.
+- A terminal command sent from Chat right after a pane opens now opens the terminal view even when
+  the terminal had not loaded the pane's command list yet, and a chat shown again draws its
+  "Ran in terminal" rows at once instead of as plain messages first.

scripts/skill-menu-cases/claude-terminal.ts

@@ -38,13 +38,20 @@ export const expect = async ({ page, width, scheme, box, menu, scenario, clear,
   await scenario(`terminal group at ${tag}`, `group-${tag}.png`, async (capture) => {
     await box.fill("/");
     await menu.locator(".menu-item").first().waitFor();
+    assert.deepEqual(await menu.locator(".menu-heading").allTextContents(), ["Built in", "Project skills", "Terminal commands"]);
+    // the terminal group is listed last, each row with its badge; the hidden row is nowhere
     const groups = menu.locator(".composer-menu-group");
     const last = groups.nth((await groups.count()) - 1);
     assert.deepEqual(await last.locator(".menu-item-main").allTextContents(), TERMINAL_ROWS.map((name) => `/${name}`));
-    assert.equal(await last.locator(".composer-menu-badge").count(), TERMINAL_ROWS.length);
+    assert.equal(await last.locator(".menu-item .composer-menu-badge").count(), TERMINAL_ROWS.length);
+    assert.deepEqual(await last.locator(".composer-menu-badge").allTextContents(), TERMINAL_ROWS.map(() => "Terminal"));
+    assert.equal(await menu.locator(".composer-menu-badge").count(), TERMINAL_ROWS.length);
     assert.deepEqual((await rowNames()).sort(), commands.filter((command) => command.visibility !== "agent-hidden").map(({ name }) => name).sort());
     await menuFits("terminal group");
     await rowsBrief("terminal group");
+    // the badge stands whole beside the name: neither is cut, whatever the width
+    const cut = await menu.locator(".composer-menu-badge").evaluateAll((badges) => badges.filter((badge) => badge.scrollWidth > badge.clientWidth + 1).length);
+    assert.equal(cut, 0, "no badge is cut");
     await menu.evaluate((node) => { node.scrollTop = node.scrollHeight; });
     await capture();
   });
@@ -53,6 +60,10 @@ export const expect = async ({ page, width, scheme, box, menu, scenario, clear,
     await clear();
     await box.fill("/conf");
     await menu.locator(".menu-item").first().waitFor();
+    // /config by its name first; /terminal-setup follows, found by its description ("Configure ")
+    const listed = await rowNames();
+    assert.equal(listed[0], "config");
+    assert.equal(await menu.locator(".menu-item .composer-menu-badge").count(), listed.filter((name) => TERMINAL_ROWS.includes(name)).length, "a typed query keeps the badge on the rows");
     await box.press("Enter");
     assert.equal(await box.inputValue(), "/config ");
     assert.deepEqual(await submits(), []);

scripts/terminal-autoswitch-demo-regression.ts

@@ -68,18 +68,29 @@ window.WebSocket = function(...args) {
 Object.assign(window.WebSocket, { CONNECTING: Socket.CONNECTING, OPEN: Socket.OPEN, CLOSING: Socket.CLOSING, CLOSED: Socket.CLOSED });
 const fetch = window.fetch;
 window.focusCalls = [];
+// the catalog race (#1591): "later" holds every read of the pane's commands after the first, "all" holds
+// every one, until releaseCommandReads(); commandsParsed counts the answers the page has read
+window.holdCommandReads = sessionStorage.getItem("qa:hold-command-reads");
+window.commandReads = 0;
+window.heldCommandReads = [];
+window.commandsParsed = 0;
+window.releaseCommandReads = () => { window.holdCommandReads = null; window.heldCommandReads.splice(0).forEach(go => go()); };
 // answered prompt reads (each lists the pane's outcomes): a later one has seen what came before it
 window.promptReads = 0;
 window.fetch = (input, init) => {
   const url = typeof input === "string" ? input : input instanceof Request ? input.url : String(input);
   if (url.includes("/pane/focus")) window.focusCalls.push(url);
+  if (url.includes("/pane/commands") && window.holdCommandReads !== null && (++window.commandReads > 1 || window.holdCommandReads === "all")) {
+    return new Promise((resolve, reject) => { window.heldCommandReads.push(() => fetch(input, init).then(resolve, reject)); });
+  }
   const answer = fetch(input, init);
   if (url.includes("/pane/prompt?")) answer.then(() => { window.promptReads += 1; }, () => {});
   return answer;
 };
 const json = Response.prototype.json;
 Response.prototype.json = async function() {
   const value = await json.call(this);
+  if (Array.isArray(value.commands)) window.commandsParsed += 1;
   if (Array.isArray(value.turns)) {
     const history = Array.from({length: 24}, (_, index) => ({
       role: index % 2 ? "assistant" : "user",
@@ -696,6 +707,51 @@ try {
     } finally { await context.close(); }
   }
 
+  // The catalog race (#1591). "later": the message box's read of the pane's commands answers and the
+  // terminal's own is still on its way when the receipt lands; the receipt reads the box's answer.
+  // "all": no read has answered when the message leaves, one answers before its receipt (40f64932).
+  for (const hold of ["later", "all"] as const) {
+    const context = await browser.newContext({ viewport: { width: 1280, height: 844 }, locale: "en-US" });
+    try {
+      await context.addInitScript(({ pane, hold }) => {
+        localStorage.setItem("ovis:settings", JSON.stringify({ language: "en", theme: "light", darkTheme: "amber", lightTheme: "amber", autoSendQueued: true }));
+        localStorage.setItem(`ovis:view:${pane}`, "chat");
+        sessionStorage.setItem("qa:hold-command-reads", hold);
+      }, { pane: owner, hold });
+      const page = await context.newPage();
+      const errors: string[] = [];
+      page.on("pageerror", (error) => { errors.push(error.message); console.error(error.message); });
+      page.setDefaultTimeout(10_000);
+      await page.goto(`${server.url}?pane=${encodeURIComponent(owner)}`);
+      await chat(page).locator(".composer").waitFor();
+      await page.waitForFunction(() => (window as unknown as PeekWindow).promptReads > 0);
+      type HoldWindow = PeekWindow & { heldCommandReads: unknown[]; commandsParsed: number; releaseReceipt: (() => void) | null };
+      // every component asked for the list; all but the first ("later") or all of them ("all") still wait
+      await page.waitForFunction((count) => (window as unknown as HoldWindow).heldCommandReads.length >= count, hold === "later" ? 1 : 2);
+      if (hold === "later") {
+        await page.waitForFunction(() => (window as unknown as HoldWindow).commandsParsed === 1);
+        // the box has the list: its terminal row is in the menu
+        await box(page).fill("/conf");
+        await page.locator('.menu-item[data-terminal]').first().waitFor();
+        await send(page, "/doctor");
+      } else {
+        await page.evaluate(() => { Object.assign(window, { receiptMode: "hold" }); });
+        await send(page, "/doctor");
+        await page.waitForFunction(() => (window as unknown as HoldWindow).releaseReceipt !== null);
+        assert.equal(await peek(page).count(), 0, "no peek before the receipt");
+        await page.evaluate(() => { (window as unknown as { releaseCommandReads: () => void }).releaseCommandReads(); });
+        await page.waitForFunction(() => (window as unknown as HoldWindow).commandsParsed >= 1);
+        await page.evaluate(() => { (window as unknown as HoldWindow).releaseReceipt?.(); });
+      }
+      await peek(page).waitFor();
+      await hint(page).getByText("/doctor ran", { exact: true }).waitFor();
+      await capture(page, `catalog-race-${hold}`);
+      await page.evaluate(() => { (window as unknown as { releaseCommandReads: () => void }).releaseCommandReads(); });
+      assert.deepEqual(errors, []);
+      console.log(`PASS catalog race (${hold}): a receipt reads the command list another read already has, and opens the peek`);
+    } finally { await context.close(); }
+  }
+
   // a split half's own lens button: during a peek it says the chat and takes it back, as the
   // palette's toggle and Mod+Shift+J do, and saves no terminal lens
   const context = await browser.newContext({ viewport: { width: 1280, height: 844 }, locale: "en-US" });

src/components/PaneTerminal.tsx

@@ -19,6 +19,7 @@ import { heldCountShown, heldOpenAtFold, heldOpenOnFocus, heldRefocusDue, heldRo
 import { arrangeNotices, dismissNotice, NO_DISMISSALS, settleDismissed, type DismissedNotices } from "../lib/composerNotices.ts";
 import { MAX_COMPOSER_CHARS, QUEUE_READY_STATUS, agentDisplayLabel, typedTerminalCommand } from "../lib/compose.ts";
 import { usePaneCommands } from "../lib/usePaneCommands.ts";
+import { paneCommandsNow } from "../lib/paneCommandCache.ts";
 import { autoReturnArmed, paneLensFlags, screenReturnArmed, type PeekRequest, type TerminalPeek } from "../lib/terminalPeek.ts";
 import { FrameGate, inputRowState, readScreen, readsInputRow, type InputRowState } from "../lib/inputRow.ts";
 import { outcomeCommand } from "../lib/outcomeChip.ts";
@@ -457,6 +458,8 @@ export function PaneTerminal({
   // read at the receipt, not at the send: a catalog that arrives in between still counts
   const commandsRef = useRef(commands);
   commandsRef.current = commands;
+  // and the list the message box read, when this terminal's own read of it has not answered yet (#1591)
+  const machineIdRef = useRef(machineId); machineIdRef.current = machineId;
   const agentRef = useRef(agent); agentRef.current = agent;
   // The frames of the pane's stream (lib/inputRow.ts FrameGate): the agent's input row is read only
   // off a whole frame drawn for this grid, never mid-frame or between a resize and the redraw for it
@@ -474,7 +477,7 @@ export function PaneTerminal({
   const receiptRowRef = useRef<{ pane: string; text: string; state: InputRowState } | null>(null);
   // `seen`: the agent's empty input row was on screen as the text left (read before it was typed)
   const showSentCommand = useCallback((pane: string, text: string, known: readonly string[] | null, seen: boolean) => {
-    const command = typedTerminalCommand(commandsRef.current, text);
+    const command = typedTerminalCommand(paneCommandsNow(pane, machineIdRef.current, agentRef.current) ?? commandsRef.current, text);
     const open = onPeekRef.current;
     if (command === null || open === undefined || paneRef.current !== pane || !activeRef.current || !presentedRef.current || !shownRef.current
       || !chatMountedRef.current || observeRef.current || heldRef.current || endedRef.current || unsupportedRef.current) return;

src/lib/paneCommandCache.test.ts

@@ -1,7 +1,7 @@
 import { describe, expect, test } from "bun:test";
 
 import type { SlashCommand } from "../../shared/protocol.ts";
-import { cachedPaneCommands, invalidatePaneCommands, onPaneCommandsInvalidated } from "./paneCommandCache.ts";
+import { cachedPaneCommands, invalidatePaneCommands, onPaneCommandsInvalidated, paneCommandsNow } from "./paneCommandCache.ts";
 
 describe("cachedPaneCommands", () => {
   test("answers once a minute per PC, pane and agent, and asks again for another agent in the same pane", async () => {
@@ -63,4 +63,26 @@ describe("cachedPaneCommands", () => {
     expect(await cachedPaneCommands("w3:p1", "local", "omo", fetchCommands)).toBe(freshRows);
     expect(resolvers.length).toBe(2);
   });
+
+  test("paneCommandsNow gives a read still on its way the answer another read already has", async () => {
+    const resolvers: ((commands: SlashCommand[]) => void)[] = [];
+    const fetchCommands = (): Promise<SlashCommand[]> => new Promise((resolve) => { resolvers.push(resolve); });
+    const box = cachedPaneCommands("w4:p1", "local", "claude", fetchCommands);
+    const terminal = cachedPaneCommands("w4:p1", "local", "claude", fetchCommands);
+    expect(paneCommandsNow("w4:p1", "local", "claude")).toBeNull();
+    const rows: SlashCommand[] = [{ name: "config", description: "", source: "builtin", visibility: "terminal" }];
+    resolvers[0]!(rows);
+    expect(await box).toBe(rows);
+    // the terminal's own read, begun later, has not answered: the box's answer is there for it at once
+    expect(paneCommandsNow("w4:p1", "local", "claude")).toBe(rows);
+    expect(paneCommandsNow("w4:p1", "local", "pi")).toBeNull();
+    expect(paneCommandsNow("w4:p1", "studio", "claude")).toBeNull();
+    // the later read's answer replaces it
+    const later: SlashCommand[] = [{ name: "cost", description: "", source: "builtin" }];
+    resolvers[1]!(later);
+    expect(await terminal).toBe(later);
+    expect(paneCommandsNow("w4:p1", "local", "claude")).toBe(later);
+    invalidatePaneCommands("w4:p1", "local");
+    expect(paneCommandsNow("w4:p1", "local", "claude")).toBeNull();
+  });
 });

src/lib/paneCommandCache.ts

@@ -2,11 +2,9 @@ import { paneStorageId } from "../../shared/machines.ts";
 import type { SlashCommand } from "../../shared/protocol.ts";
 
 const COMMAND_CACHE_MS = 60_000;
-const cache = new Map<string, { loadedAt: number; commands: SlashCommand[] }>();
+const cache = new Map<string, { loadedAt: number; commands: SlashCommand[]; read: number }>();
 /** per pane, how many times its lists were invalidated: a read begun before one is answered but not kept */
 const generations = new Map<string, number>();
-/** per pane and agent, the last read started: an older read that answers after it is returned but never cached */
-const latestRead = new Map<string, number>();
 let reads = 0;
 const listeners = new Set<(paneId: string, machineId: string) => void>();
 
@@ -22,13 +20,24 @@ export async function cachedPaneCommands(paneId: string, machineId: string, agen
   if (cached && Date.now() - cached.loadedAt < COMMAND_CACHE_MS) return cached.commands;
   const generation = generations.get(pane) ?? 0;
   const read = ++reads;
-  latestRead.set(key, read);
   const commands = await fetchCommands(paneId);
-  // invalidated while on its way, or overtaken by a later read (a retry beside a refresh): the read begun last is the one kept
-  if ((generations.get(pane) ?? 0) === generation && latestRead.get(key) === read) cache.set(key, { loadedAt: Date.now(), commands });
+  // invalidated while on its way, or answering after a later read's answer was kept (a retry beside a refresh):
+  // never kept. An answer that comes first is kept even while a later read is still on its way, so the other
+  // components of the pane read it at once (paneCommandsNow); that later read replaces it when it answers
+  const kept = cache.get(key);
+  if ((generations.get(pane) ?? 0) === generation && (kept === undefined || kept.read < read)) cache.set(key, { loadedAt: Date.now(), commands, read });
   return commands;
 }
 
+/**
+ * The list a read in the last minute answered for this PC, pane and agent, at once; null: none has. A
+ * component whose own read is still on its way reads the answer another one already has.
+ */
+export function paneCommandsNow(paneId: string, machineId: string, agent: string | null | undefined): SlashCommand[] | null {
+  const cached = cache.get(`${paneStorageId(machineId, paneId)}:${agent ?? ""}`);
+  return cached && Date.now() - cached.loadedAt < COMMAND_CACHE_MS ? cached.commands : null;
+}
+
 /**
  * Forgets a pane's lists, whatever agent they were read for, and tells the listeners: a command
  * that changed the agent's catalog (an outcome's `catalog_changed`) is followed by a fresh read

src/lib/pendingMessages.ts

@@ -8,6 +8,9 @@ const PREFIX = "ovis:pending:";
 const piReceipt = (message: PendingMessage): boolean => message.id.startsWith("pi:")
   && (message.error?.code === "pi_transcript_pending" || message.error?.code === "transcript_unconfirmed");
 
+/** A row's error for a message an older bridge took with no receipt (SubmitResult `unconfirmed`). */
+export const UNCONFIRMED_SEND = { code: "submit_unconfirmed", message: "This PC sent no receipt, so nothing confirmed the pane took this message" };
+
 /** Connection proof stays in memory. A saved copy can never resume automatic delivery. */
 export class PendingMessageStore {
   private rows = new Map<string, PendingMessageView[]>();
@@ -238,6 +241,15 @@ export class PendingMessageStore {
         state: uncertain ? "uncertain" : attemptedGeneration !== undefined ? "held" : message.serverOwned ? message.state : "held" };
     }));
   }
+  /**
+   * Keeps a message an older bridge took with no receipt as a row not confirmed: it may or may not
+   * have reached the pane, so its text stays to copy or discard and is never sent again from here.
+   */
+  keepUnconfirmed(owner: string, text: string): void {
+    const id = `unconfirmed:${[...crypto.getRandomValues(new Uint32Array(2))].map((part) => part.toString(36)).join("-")}`;
+    this.refresh(owner);
+    this.write(owner, [...this.read(owner), { id, request_id: 0, text, created_at: new Date().toISOString(), state: "uncertain", error: UNCONFIRMED_SEND, serverOwned: false }]);
+  }
   removeCopy(owner: string, id: string): void {
     if (this.read(owner).some((message) => message.id === id && message.serverOwned)) return;
     this.scopes.get(owner)?.delete(id);