chore: merge fix/watch-mode-keepalive-tri-13065 (review fixes round 3)

Author: kathiekiwi

apps/webapp/app/components/dashboard-agent/DashboardAgentComposer.tsx

@@ -2,6 +2,7 @@ import { ArrowUpIcon, StopIcon } from "@heroicons/react/20/solid";
 import { useEffect, useRef } from "react";
 import { Button } from "~/components/primitives/Buttons";
 import { cn } from "~/utils/cn";
+import { composerKeepsEscape } from "./composer-escape";
 import {
   MAX_MESSAGE_CHARS,
   MESSAGE_CHARS_WARN_AT,
@@ -95,6 +96,10 @@ export function DashboardAgentComposer({
                 e.preventDefault();
                 onSubmit();
               }
+              // Keeping Escape from the panel's close handler, which skips a prevented event.
+              if (e.key === "Escape" && composerKeepsEscape(value)) {
+                e.preventDefault();
+              }
               // Only while empty, so with text present Tab keeps its normal focus behavior.
               if (e.key === "Tab" && !e.shiftKey && placeholderSuggestion && value === "") {
                 e.preventDefault();

apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx

@@ -45,6 +45,8 @@ export type DashboardAgentMessagesProps = {
 
 // Cached so a stripped message keeps its identity across renders and memoization holds:
 // rebuilding it re-renders every tool-calling turn on each streamed token.
+// Relies on @ai-sdk/react cloning a message per update: an SDK mutating one in place would
+// keep serving the cached copy of its earlier state.
 const strippedMessages = new WeakMap<UIMessage, UIMessage>();
 
 export function stripStepParts(message: UIMessage): UIMessage {

apps/webapp/app/components/dashboard-agent/DashboardAgentPanel.tsx

@@ -37,7 +37,12 @@ import type { AgentPageContext } from "./page-context-types";
 import { agentPageLabel } from "./page-label";
 import { explicitPromptTarget } from "./explicit-prompt";
 import { escapeClosesPanel } from "./panel-escape";
-import { markChatListRead, unreadWorkCount } from "./unread-counts";
+import {
+  markChatListRead,
+  nextVisibleChat,
+  settleReadChats,
+  unreadWorkCount,
+} from "./unread-counts";
 import { AgentPanelColumn } from "./panel-layout";
 import { markerAfterActiveChat, markerAfterActivity } from "./thinking-marker";
 import { concurrencyPath } from "~/utils/pathBuilder";
@@ -148,6 +153,10 @@ export function DashboardAgentPanel({
   // The read POST and its reload can land out of order, so mask the next list.
   const justRead = useRef<Set<string>>(new Set());
 
+  // Read when the response lands, not when it was requested, so a chat switched to mid-flight
+  // is the one the list settles against.
+  const visibleChatId = useRef<string | null>(null);
+
   // Ordering-safe: if the new chat has not reported yet, its own report re-sets the marker.
   useEffect(() => {
     setThinkingChatId((previous) => markerAfterActiveChat(previous, active?.chatId));
@@ -168,9 +177,7 @@ export function DashboardAgentPanel({
           const pending = chats.some((chat) => chat.hasActiveWatch || chat.hasUnreadWake);
           if (pending) rememberWatchActivity(organization.id);
           else forgetWatchActivity(organization.id);
-          const settled = chats.map((chat) =>
-            read.has(chat.id) ? { ...chat, hasUnreadWake: false, hasUnreadWork: false } : chat
-          );
+          const settled = settleReadChats(chats, read, visibleChatId.current);
           setChats(settled);
           setChatsLoaded(true);
         } catch (error) {
@@ -320,11 +327,13 @@ export function DashboardAgentPanel({
     if (!active?.chatId) return;
     const chatId = active.chatId;
     onChatRead?.(chatId, { leaving: false });
+    visibleChatId.current = nextVisibleChat(chatId, { leaving: false });
     justRead.current.add(chatId);
     setChats((previous) => markChatListRead(previous, chatId));
     // Read again on the way out: a wake can land while the chat is open.
     return () => {
       onChatRead?.(chatId, { leaving: true });
+      visibleChatId.current = nextVisibleChat(chatId, { leaving: true });
       justRead.current.add(chatId);
       setChats((previous) => markChatListRead(previous, chatId));
     };

apps/webapp/app/components/dashboard-agent/composer-escape.test.ts

@@ -0,0 +1,16 @@
+import { describe, expect, it } from "vitest";
+import { composerKeepsEscape } from "./composer-escape";
+
+describe("Escape while the composer has focus", () => {
+  it("is kept by the composer while there is a draft, so the panel stays open", () => {
+    expect(composerKeepsEscape("half a question about a failing run")).toBe(true);
+  });
+
+  it("closes the panel when there is nothing to lose", () => {
+    expect(composerKeepsEscape("")).toBe(false);
+  });
+
+  it("reads whitespace as nothing to lose, matching what Send accepts", () => {
+    expect(composerKeepsEscape("  \n ")).toBe(false);
+  });
+});

apps/webapp/app/components/dashboard-agent/composer-escape.ts

@@ -0,0 +1,8 @@
+/**
+ * The panel closes on Escape unless a child has already prevented the event's default —
+ * `defaultPrevented` is how a child vetoes the close. A composer holding a draft takes the
+ * first Escape for itself, so the draft survives; an empty one lets Escape close the panel.
+ */
+export function composerKeepsEscape(value: string): boolean {
+  return value.trim() !== "";
+}

apps/webapp/app/components/dashboard-agent/settled-transcript.test.ts

@@ -239,6 +239,36 @@ describe("an already-open panel when a turn is exhausted", () => {
     expect(reads).toBe(2);
   });
 
+  it("keeps re-reading a stream that died mid-tool with no card open", async () => {
+    // No investigation anywhere: only the dangling `get_report` says the turn is unfinished.
+    const DANGLING = {
+      id: "msg_step",
+      role: "assistant",
+      parts: [{ type: "tool-get_report", toolCallId: "call_1", state: "input-available" }],
+    };
+    const FINISHED = {
+      id: "msg_step",
+      role: "assistant",
+      parts: [
+        { type: "tool-get_report", toolCallId: "call_1", state: "output-available", output: {} },
+      ],
+    };
+
+    const responses = [[DANGLING], [DANGLING], [FINISHED]];
+    let rendered: (typeof DANGLING)[] = [DANGLING];
+    let reads = 0;
+
+    await pollSettledTranscript({
+      fetchTranscript: async () => responses[reads++] ?? null,
+      apply: (merge) => void (rendered = merge(rendered)),
+      wait: async () => {},
+    });
+
+    expect(reads).toBe(3);
+    expect(rendered).toEqual([FINISHED]);
+    expect(transcriptLooksUnfinished(rendered)).toBe(false);
+  });
+
   it("stops on a failed re-read instead of hammering the endpoint", async () => {
     let reads = 0;
     await pollSettledTranscript<typeof OPEN>({

apps/webapp/app/components/dashboard-agent/settled-transcript.ts

@@ -86,8 +86,9 @@ export async function pollSettledTranscript<T extends Identified>(deps: {
     const fetched = await deps.fetchTranscript();
     if (!fetched) return;
     deps.apply((current) => mergeSettledMessages(current, fetched));
-    // The stored transcript is the authority on whether anything is still open.
-    if (!hasOpenInvestigation(fetched)) return;
+    // The stored transcript is the authority on whether anything is still open. Same test that
+    // starts the poll, so a stream that died mid-tool is followed until it settles too.
+    if (!transcriptLooksUnfinished(fetched)) return;
   }
 }
 

apps/webapp/app/components/dashboard-agent/unread-counts.test.ts

@@ -1,6 +1,11 @@
 import { readFileSync } from "node:fs";
 import { describe, expect, it } from "vitest";
-import { markChatListRead, nextVisibleChat, unreadWorkCount } from "./unread-counts";
+import {
+  markChatListRead,
+  nextVisibleChat,
+  settleReadChats,
+  unreadWorkCount,
+} from "./unread-counts";
 
 const list = () => [
   { id: "chat_a", hasUnreadWake: true, hasUnreadWork: true },
@@ -71,6 +76,28 @@ describe("the chat on screen", () => {
   });
 });
 
+/**
+ * The list is refreshed after every turn, and the server's lastReadAt trails the turn that just
+ * landed — so the chat being read has to settle on its own, not wait for the next read to land.
+ */
+describe("settleReadChats", () => {
+  it("settles the chat on screen, however fresh the turn that just landed in it", () => {
+    const settled = settleReadChats(list(), new Set(), "chat_a");
+    expect(settled[0]).toEqual({ id: "chat_a", hasUnreadWake: false, hasUnreadWork: false });
+    expect(settled.slice(1)).toEqual(list().slice(1));
+  });
+
+  it("settles the chats just read", () => {
+    const settled = settleReadChats(list(), new Set(["chat_b"]), null);
+    expect(settled[1]).toEqual({ id: "chat_b", hasUnreadWake: false, hasUnreadWork: false });
+    expect(unreadWorkCount(settled)).toBe(1);
+  });
+
+  it("leaves every other chat exactly as the server reported it", () => {
+    expect(settleReadChats(list(), new Set(), null)).toEqual(list());
+  });
+});
+
 describe("what the panel and the layout actually do with it", () => {
   const panel = readFileSync(new URL("./DashboardAgentPanel.tsx", import.meta.url), "utf8");
   const layout = readFileSync(new URL("./DashboardAgent.tsx", import.meta.url), "utf8");
@@ -87,6 +114,16 @@ describe("what the panel and the layout actually do with it", () => {
     expect(layout).toContain("visibleChat.current = nextVisibleChat(chatId, options);");
   });
 
+  /**
+   * Structural: the reload is memoised without `active`, so the chat on screen has to reach the
+   * settle through a ref — the closure's copy is whatever it was when the reload was created.
+   */
+  it("settles the refreshed list against the chat on screen, read from a ref", () => {
+    expect(panel).toContain("settleReadChats(chats, read, visibleChatId.current)");
+    expect(panel).toContain("visibleChatId.current = nextVisibleChat(chatId, { leaving: false });");
+    expect(panel).toContain("visibleChatId.current = nextVisibleChat(chatId, { leaving: true });");
+  });
+
   /**
    * Structural: there is no DOM here to open a panel in. The poll runs for as long as this tab
    * is watching, so the chat on screen has to be read from a ref at request time — `open` in the

apps/webapp/app/components/dashboard-agent/unread-counts.ts

@@ -24,6 +24,23 @@ export function markChatListRead<T extends UnreadChat>(chats: T[], chatId: strin
   );
 }
 
+/**
+ * The list as the panel renders it. The chat on screen settles alongside the ones just read:
+ * the server's lastReadAt still trails the turn that just landed in it, so a refresh would
+ * otherwise mark the chat its owner is reading right now as unread.
+ */
+export function settleReadChats<T extends UnreadChat>(
+  chats: T[],
+  read: Set<string>,
+  visibleChatId: string | null
+): T[] {
+  return chats.map((chat) =>
+    read.has(chat.id) || chat.id === visibleChatId
+      ? { ...chat, hasUnreadWake: false, hasUnreadWork: false }
+      : chat
+  );
+}
+
 /**
  * How many chats still hold work their owner hasn't seen. The chat on screen is being read
  * right now, so a turn landing in it is not work anyone is waiting on — every count of this,