chore: merge feat/query-safety-tri-11165 (review fixes)

Author: kathiekiwi

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

@@ -23,9 +23,10 @@ import {
 } from "./panel-layout";
 import { nextPendingTurnChatId } from "./pending-turn";
 import { nextVisibleChat } from "./unread-counts";
-import { planWakeToasts, startWakePolling, wakesToToast } from "./wake-poll";
+import { createWakePendingCount, startWakePolling, wakesToToast } from "./wake-poll";
 import { shouldPollWakeFeed, subscribeWatchActivity } from "./watch-activity";
 import {
+  dismissWatchWakesSummaryToast,
   showWatchWakesSummaryToast,
   showWatchWakeToast,
   WAKE_TOAST_MAX_INDIVIDUAL,
@@ -101,8 +102,8 @@ export function DashboardAgent({
 
   // The count the still-visible grouped toast claims. Consecutive polls add to it so a
   // later batch grows the summary instead of overwriting it with only its own count;
-  // reset when the user opens the panel from that toast.
-  const summaryPending = useRef(0);
+  // reset when the user opens the panel, whichever route they took.
+  const wakePending = useRef(createWakePendingCount());
 
   // Switching environment re-runs the layout loader but does not remount it, so the seeds
   // above would keep the old environment's counts.
@@ -146,35 +147,58 @@ export function DashboardAgent({
     undefined
   );
 
-  const setPanelOpen = useCallback((next: boolean) => {
-    setOpen(next);
-    // Pending requests must be dropped or a stale one re-applies on the next open.
-    if (!next) {
+  // The single entry point for opening the panel — every open route must go through it.
+  // Opening acknowledges the wakes counted so far, and the visible summary goes with the
+  // count it was claiming.
+  const openPanel = useCallback(() => {
+    wakePending.current.acknowledge();
+    dismissWatchWakesSummaryToast();
+    setOpen(true);
+  }, []);
+
+  const setPanelOpen = useCallback(
+    (next: boolean) => {
+      if (next) {
+        openPanel();
+        return;
+      }
+      setOpen(false);
+      // Pending requests must be dropped or a stale one re-applies on the next open.
       visibleChat.current = null;
       setFullscreen(false);
       writeAgentFullscreen(false);
       setRequestedMessage(undefined);
       setOpenChatRequest(undefined);
       setWatchRequest(undefined);
-    }
-  }, []);
+    },
+    [openPanel]
+  );
 
-  const openChat = useCallback((chatId: string) => {
-    setOpen(true);
-    setOpenChatRequest((current) => ({ chatId, seq: (current?.seq ?? 0) + 1 }));
-  }, []);
+  const openChat = useCallback(
+    (chatId: string) => {
+      openPanel();
+      setOpenChatRequest((current) => ({ chatId, seq: (current?.seq ?? 0) + 1 }));
+    },
+    [openPanel]
+  );
 
-  const openWith = useCallback((text: string) => {
-    const trimmed = text.trim();
-    if (!trimmed) return;
-    setOpen(true);
-    setRequestedMessage((current) => ({ text: trimmed, seq: (current?.seq ?? 0) + 1 }));
-  }, []);
+  const openWith = useCallback(
+    (text: string) => {
+      const trimmed = text.trim();
+      if (!trimmed) return;
+      openPanel();
+      setRequestedMessage((current) => ({ text: trimmed, seq: (current?.seq ?? 0) + 1 }));
+    },
+    [openPanel]
+  );
 
-  const openWithWatch = useCallback((spec: WatchSpec) => {
-    setOpen(true);
-    setWatchRequest((current) => ({ spec, seq: (current?.seq ?? 0) + 1 }));
-  }, []);
+  const openWithWatch = useCallback(
+    (spec: WatchSpec) => {
+      openPanel();
+      setWatchRequest((current) => ({ spec, seq: (current?.seq ?? 0) + 1 }));
+    },
+    [openPanel]
+  );
 
   // Nothing to be woken about means nothing to poll for. The page load's unread count and
   // active-watch flag are the ungated signals; the browser's own memory of a watch starts the
@@ -230,17 +254,9 @@ export function DashboardAgent({
         for (const wake of fresh) rememberToasted(wake.watchId);
 
         if (fresh.length > 0) {
-          const { plan, pending } = planWakeToasts(
-            fresh,
-            summaryPending.current,
-            WAKE_TOAST_MAX_INDIVIDUAL
-          );
-          summaryPending.current = pending;
+          const plan = wakePending.current.plan(fresh, WAKE_TOAST_MAX_INDIVIDUAL);
           if (plan.mode === "summary") {
-            showWatchWakesSummaryToast(plan.count, () => {
-              summaryPending.current = 0;
-              setPanelOpen(true);
-            });
+            showWatchWakesSummaryToast(plan.count, () => setPanelOpen(true));
           } else {
             for (const wake of [...plan.wakes].reverse()) {
               showWatchWakeToast(wake, openChat);

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

@@ -39,6 +39,7 @@ import { explicitPromptTarget } from "./explicit-prompt";
 import { escapeClosesPanel } from "./panel-escape";
 import { markChatListRead, unreadWorkCount } from "./unread-counts";
 import { AgentPanelColumn } from "./panel-layout";
+import { markerAfterActiveChat, markerAfterActivity } from "./thinking-marker";
 import { concurrencyPath } from "~/utils/pathBuilder";
 
 function serializePageContext(pageContext: AgentPageContext): string | undefined {
@@ -138,9 +139,7 @@ export function DashboardAgentPanel({
   const [thinkingChatId, setThinkingChatId] = useState<string | null>(null);
   const handleActivityChange = useCallback(
     (chatId: string, activity: TurnActivity | null) => {
-      setThinkingChatId((previous) =>
-        activity !== null ? chatId : previous === chatId ? null : previous
-      );
+      setThinkingChatId((previous) => markerAfterActivity(previous, chatId, activity));
       onTurnActivityChange?.(chatId, activity !== null);
     },
     [onTurnActivityChange]
@@ -149,6 +148,11 @@ 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());
 
+  // Ordering-safe: if the new chat has not reported yet, its own report re-sets the marker.
+  useEffect(() => {
+    setThinkingChatId((previous) => markerAfterActiveChat(previous, active?.chatId));
+  }, [active?.chatId]);
+
   const loadHistory = useMemo(
     () =>
       createCoalescedReload(async () => {

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

@@ -115,6 +115,10 @@ export function showWatchWakeToast(wake: WatchWake, onOpenChat: (chatId: string)
   );
 }
 
+// One id for all summaries: a later poll rewrites the count in place instead of stacking a
+// second never-expiring toast on top of the first.
+const WAKES_SUMMARY_TOAST_ID = "watch-wakes-summary";
+
 /** One persistent toast standing in for a batch too large to narrate one by one. */
 export function showWatchWakesSummaryToast(count: number, onOpenChat: () => void) {
   show(
@@ -126,8 +130,14 @@ export function showWatchWakesSummaryToast(count: number, onOpenChat: () => void
         onOpenChat={onOpenChat}
       />
     ),
-    // One id for all summaries: a later poll rewrites the count in place instead
-    // of stacking a second never-expiring toast on top of the first.
-    "watch-wakes-summary"
+    WAKES_SUMMARY_TOAST_ID
   );
 }
+
+/**
+ * Takes the summary off screen. Its count only means anything until the user opens the
+ * panel; left up, a later poll would rewrite it to a smaller number.
+ */
+export function dismissWatchWakesSummaryToast() {
+  toast.dismiss(WAKES_SUMMARY_TOAST_ID);
+}

apps/webapp/app/components/dashboard-agent/demo/demo.test.ts

@@ -14,6 +14,7 @@ import {
 import { readdirSync, readFileSync, statSync } from "node:fs";
 import { join } from "node:path";
 import { describe, expect, it } from "vitest";
+import { resolveSuggestedPrompts } from "../suggested-prompts";
 import * as fixtures from "./fixtures";
 import { DEMO_ID_PREFIX, DEMO_MARKER } from "./ids";
 
@@ -256,6 +257,15 @@ describe("page context and prompt fixtures", () => {
       expect(fixtures.demoPromptsAfterDismissal.some((prompt) => prompt.id === id)).toBe(false);
     }
   });
+
+  it("dismisses a chip the resolver actually emits", () => {
+    const full = resolveSuggestedPrompts(fixtures.demoFailedRunPageContext);
+    const after = resolveSuggestedPrompts(fixtures.demoFailedRunPageContext, {
+      dismissedIds: fixtures.demoResolvedDismissedPromptIds,
+    });
+    expect(full.map((prompt) => prompt.id)).toContain(fixtures.demoResolvedDismissedPromptIds[0]);
+    expect(after.map((prompt) => prompt.id)).not.toEqual(full.map((prompt) => prompt.id));
+  });
 });
 
 describe("report fixtures", () => {

apps/webapp/app/components/dashboard-agent/demo/fixtures/page-context.ts

@@ -238,6 +238,12 @@ export const demoPromptSets: Record<DemoPageContextKey, SuggestedPrompt[]> = {
 
 export const demoDismissedPromptIds: string[] = [demoId("prompt-watch-retry")];
 
+/**
+ * Ids the resolver itself emits, for dismissing against a live resolve. Dismissing the
+ * fresh-failure chip on `demoFailedRunPageContext` falls back to `sp:run-investigate`.
+ */
+export const demoResolvedDismissedPromptIds: string[] = ["sp:fresh-failure"];
+
 export const demoPromptsAfterDismissal: SuggestedPrompt[] = demoPromptSets.failedRun
   .filter((p) => !demoDismissedPromptIds.includes(p.id))
   .slice(0, SUGGESTED_PROMPT_CAP);

apps/webapp/app/components/dashboard-agent/thinking-marker.test.ts

@@ -0,0 +1,24 @@
+import { describe, expect, it } from "vitest";
+import { markerAfterActiveChat, markerAfterActivity } from "./thinking-marker";
+
+describe("thinking marker", () => {
+  it("marks the chat that is working and clears it when the turn settles", () => {
+    const working = markerAfterActivity(null, "chat_1", "working");
+    expect(working).toBe("chat_1");
+    expect(markerAfterActivity(working, "chat_1", null)).toBe(null);
+  });
+
+  it("ignores a settled report from another chat", () => {
+    expect(markerAfterActivity("chat_1", "chat_2", null)).toBe("chat_1");
+  });
+
+  it("clears the marker when the user switches away mid-turn", () => {
+    // The streaming chat unmounts without reporting null, so only the switch clears it.
+    expect(markerAfterActiveChat("chat_1", "chat_2")).toBe(null);
+    expect(markerAfterActiveChat("chat_1", undefined)).toBe(null);
+  });
+
+  it("keeps the marker the chat just reported for itself", () => {
+    expect(markerAfterActiveChat("chat_1", "chat_1")).toBe("chat_1");
+  });
+});

apps/webapp/app/components/dashboard-agent/thinking-marker.ts

@@ -0,0 +1,20 @@
+import type { TurnActivity } from "./DashboardAgentMessages";
+
+// Which chat the history list shows as busy. Only the mounted chat reports.
+
+export function markerAfterActivity(
+  previous: string | null,
+  chatId: string,
+  activity: TurnActivity | null
+): string | null {
+  return activity !== null ? chatId : previous === chatId ? null : previous;
+}
+
+// A streaming chat unmounts on a switch without reporting null — the turn carries on
+// server-side — so the marker is dropped once another chat (or the draft) is active.
+export function markerAfterActiveChat(
+  previous: string | null,
+  activeChatId: string | undefined
+): string | null {
+  return previous === activeChatId ? previous : null;
+}

apps/webapp/app/components/dashboard-agent/wake-poll.test.ts

@@ -1,5 +1,6 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import {
+  createWakePendingCount,
   planWakeToasts,
   startWakePolling,
   UNREAD_POLL_INTERVAL_MS,
@@ -168,3 +169,37 @@ describe("planWakeToasts", () => {
     expect(second.pending).toBe(7);
   });
 });
+
+describe("createWakePendingCount", () => {
+  const MAX = 3;
+  const batch = (n: number) => Array.from({ length: n }, (_, i) => i);
+
+  it("carries unacknowledged wakes into the summary", () => {
+    const count = createWakePendingCount();
+
+    expect(count.plan(batch(2), MAX)).toEqual({ mode: "individual", wakes: [0, 1] });
+    expect(count.plan(batch(2), MAX)).toEqual({ mode: "summary", count: 4 });
+  });
+
+  it("does not count wakes the user already opened", () => {
+    const count = createWakePendingCount();
+
+    // Two individual toasts, both opened — from the toast, ⌘J, anywhere.
+    expect(count.plan(batch(2), MAX).mode).toBe("individual");
+    count.acknowledge();
+
+    // Only the two new wakes are waiting, so they toast individually rather than
+    // claiming "4 watch updates".
+    expect(count.plan(batch(2), MAX)).toEqual({ mode: "individual", wakes: [0, 1] });
+  });
+
+  it("starts the next summary from the wakes that arrived after the open", () => {
+    const count = createWakePendingCount();
+
+    expect(count.plan(batch(4), MAX)).toEqual({ mode: "summary", count: 4 });
+    count.acknowledge();
+
+    expect(count.plan(batch(2), MAX).mode).toBe("individual");
+    expect(count.plan(batch(2), MAX)).toEqual({ mode: "summary", count: 4 });
+  });
+});

apps/webapp/app/components/dashboard-agent/wake-poll.ts

@@ -45,6 +45,27 @@ export function planWakeToasts<T>(
   return { plan: { mode: "individual", wakes: fresh }, pending: total };
 }
 
+/**
+ * The running pending count, owned by one holder so the poll and the panel cannot drift.
+ * Every wake counts until the user opens the panel — by whichever route, including a single
+ * wake toast — and opening it clears the count so a later grouped toast claims only wakes
+ * still waiting.
+ */
+export function createWakePendingCount() {
+  let pending = 0;
+
+  return {
+    plan<T>(fresh: T[], max: number): WakeToastPlan<T> {
+      const result = planWakeToasts(fresh, pending, max);
+      pending = result.pending;
+      return result.plan;
+    },
+    acknowledge() {
+      pending = 0;
+    },
+  };
+}
+
 export type WakePollOptions = {
   load: () => Promise<void>;
   isHidden: () => boolean;

apps/webapp/app/routes/storybook.agent-ui/route.tsx

@@ -174,9 +174,9 @@ const promotedPrompt: SuggestedPrompt = {
   source: "promoted",
 };
 
-// Pinned, not resolved: the fixture's signal has a fixed timestamp, so a live resolve would
-// dismiss a different chip once that timestamp aged out of the freshness window.
-const dismissedPromptIds = demoFixtures.demoDismissedPromptIds;
+// Resolver-minted ids: the panel resolves its own chips, so a demo-namespaced id would
+// match nothing and the state would render undismissed.
+const dismissedPromptIds = demoFixtures.demoResolvedDismissedPromptIds;
 
 function toWatchChip(watch: (typeof demoWatches.row)[number]): WatchChip {
   return {