fix(webapp): the last-chat key stays org-true and a foreign chat heals instead of poisoning it

Author: kathiekiwi

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

@@ -16,6 +16,13 @@ import {
   type DashboardAgentSession,
 } from "./DashboardAgentChat";
 import { createCoalescedReload } from "./coalesced-reload";
+import {
+  forgetLastChat,
+  lastChatStorageKey,
+  readLastChat,
+  shouldPersistLastChat,
+  writeLastChat,
+} from "./last-chat-storage";
 import { DashboardAgentDraft } from "./DashboardAgentDraft";
 import type { TurnActivity } from "./DashboardAgentMessages";
 import { DashboardAgentHeader } from "./DashboardAgentHeader";
@@ -27,23 +34,6 @@ import { agentPageLabel } from "./page-label";
 import { AgentPanelColumn } from "./panel-layout";
 import { concurrencyPath } from "~/utils/pathBuilder";
 
-const lastChatStorageKey = (organizationId: string) =>
-  `tdev:dashboard-agent:last-chat:${organizationId}`;
-
-function readLastChat(storageKey: string): { chatId: string; path: string } | null {
-  if (typeof window === "undefined") return null;
-  try {
-    const raw = window.localStorage.getItem(storageKey);
-    if (!raw) return null;
-    // Pre-path entries were the bare chat id: no page to match, so start fresh.
-    if (!raw.startsWith("{")) return null;
-    const parsed = JSON.parse(raw) as { chatId?: string; path?: string };
-    return parsed.chatId && parsed.path ? { chatId: parsed.chatId, path: parsed.path } : null;
-  } catch {
-    return null;
-  }
-}
-
 function serializePageContext(pageContext: AgentPageContext): string | undefined {
   try {
     return JSON.stringify(pageContext);
@@ -54,6 +44,8 @@ function serializePageContext(pageContext: AgentPageContext): string | undefined
 
 type ActiveChat = {
   chatId: string;
+  // The org the chat belongs to, so a switch can't file it under the new org's key.
+  organizationId: string;
   messages: UIMessage[];
   session: DashboardAgentSession | null;
   pendingFirstMessage?: string;
@@ -158,7 +150,13 @@ export function DashboardAgentPanel({
         const data = res.ok ? ((await res.json()) as OpenedChatResponse) : undefined;
         if (seq !== openChatRequestSeq.current) return;
         const opened = resolveOpenedChat(id, data);
-        setActive(opened.kind === "gone" ? null : opened);
+        if (opened.kind === "gone") {
+          // Deleted, or another org's: drop the pointer so it can't be restored again.
+          setActive(null);
+          forgetLastChat(storageKey);
+          return;
+        }
+        setActive({ ...opened, organizationId: organization.id });
       } catch (error) {
         console.error(`Dashboard agent: failed to open chat ${id}`, error);
         toast.error("We couldn't open that chat. Try again in a moment.");
@@ -167,7 +165,7 @@ export function DashboardAgentPanel({
         if (seq === openChatRequestSeq.current) setLoading(false);
       }
     },
-    [actionPath, toast]
+    [actionPath, organization.id, storageKey, toast]
   );
 
   const createChat = useCallback(
@@ -200,6 +198,7 @@ export function DashboardAgentPanel({
         }
         setActive({
           chatId: data.chatId,
+          organizationId: organization.id,
           messages: data.headStarted ? [userMessage] : [],
           session: { publicAccessToken: data.publicAccessToken },
           pendingFirstMessage: data.headStarted ? undefined : text,
@@ -213,7 +212,7 @@ export function DashboardAgentPanel({
         if (seq === openChatRequestSeq.current) setLoading(false);
       }
     },
-    [actionPath, clientData, toast]
+    [actionPath, clientData, organization.id, toast]
   );
 
   const restored = useRef(false);
@@ -244,16 +243,9 @@ export function DashboardAgentPanel({
   }, [organization.id, loadHistory]);
 
   useEffect(() => {
-    if (!active?.chatId) return;
-    try {
-      window.localStorage.setItem(
-        storageKey,
-        JSON.stringify({ chatId: active.chatId, path: location.pathname })
-      );
-    } catch {
-      /* ignore */
-    }
-  }, [active?.chatId, storageKey, location.pathname]);
+    if (!shouldPersistLastChat(active, organization.id)) return;
+    writeLastChat(storageKey, { chatId: active.chatId, path: location.pathname });
+  }, [active, organization.id, storageKey, location.pathname]);
 
   // Bound to its chat, which remounts with a fresh guard ref on every switch.
   const [prefill, setPrefill] = useState<{ text: string; seq: number; chatId: string } | undefined>(

apps/webapp/app/components/dashboard-agent/last-chat-storage.test.ts

@@ -0,0 +1,74 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import {
+  forgetLastChat,
+  lastChatStorageKey,
+  readLastChat,
+  shouldPersistLastChat,
+  writeLastChat,
+} from "./last-chat-storage";
+
+const ORG_A = "org_a";
+const ORG_B = "org_b";
+const KEY_A = lastChatStorageKey(ORG_A);
+const KEY_B = lastChatStorageKey(ORG_B);
+
+const chatOfA = { chatId: "chat_a1", organizationId: ORG_A };
+
+let store: Map<string, string>;
+
+beforeEach(() => {
+  store = new Map();
+  vi.stubGlobal("window", {
+    localStorage: {
+      getItem: (key: string) => store.get(key) ?? null,
+      setItem: (key: string, value: string) => void store.set(key, value),
+      removeItem: (key: string) => void store.delete(key),
+    },
+  });
+});
+
+afterEach(() => {
+  vi.unstubAllGlobals();
+});
+
+describe("shouldPersistLastChat", () => {
+  it("persists a chat under its own org", () => {
+    expect(shouldPersistLastChat(chatOfA, ORG_A)).toBe(true);
+  });
+
+  // The org-reset effect clears `active` in a later flush, so the persistence effect runs
+  // once with the previous org's chat and the new org's key.
+  it("does not persist the previous org's chat once the org has switched", () => {
+    expect(shouldPersistLastChat(chatOfA, ORG_B)).toBe(false);
+  });
+
+  it("persists nothing when there is no chat", () => {
+    expect(shouldPersistLastChat(null, ORG_A)).toBe(false);
+  });
+});
+
+describe("last chat storage across an org switch", () => {
+  it("leaves the new org's key untouched when the panel still holds the old org's chat", () => {
+    writeLastChat(KEY_A, { chatId: chatOfA.chatId, path: "/orgs/a/runs" });
+    if (shouldPersistLastChat(chatOfA, ORG_B)) {
+      writeLastChat(KEY_B, { chatId: chatOfA.chatId, path: "/orgs/b/runs" });
+    }
+
+    expect(readLastChat(KEY_B)).toBeNull();
+    expect(readLastChat(KEY_A)).toEqual({ chatId: chatOfA.chatId, path: "/orgs/a/runs" });
+  });
+
+  it("forgets a pointer to a chat that is gone", () => {
+    writeLastChat(KEY_A, { chatId: chatOfA.chatId, path: "/orgs/a/runs" });
+    forgetLastChat(KEY_A);
+
+    expect(readLastChat(KEY_A)).toBeNull();
+    expect(store.has(KEY_A)).toBe(false);
+  });
+
+  it("ignores a pre-path entry that was just the chat id", () => {
+    store.set(KEY_A, "chat_a1");
+
+    expect(readLastChat(KEY_A)).toBeNull();
+  });
+});

apps/webapp/app/components/dashboard-agent/last-chat-storage.ts

@@ -0,0 +1,45 @@
+export const lastChatStorageKey = (organizationId: string) =>
+  `tdev:dashboard-agent:last-chat:${organizationId}`;
+
+export function readLastChat(storageKey: string): { chatId: string; path: string } | null {
+  if (typeof window === "undefined") return null;
+  try {
+    const raw = window.localStorage.getItem(storageKey);
+    if (!raw) return null;
+    // Pre-path entries were the bare chat id: no page to match, so start fresh.
+    if (!raw.startsWith("{")) return null;
+    const parsed = JSON.parse(raw) as { chatId?: string; path?: string };
+    return parsed.chatId && parsed.path ? { chatId: parsed.chatId, path: parsed.path } : null;
+  } catch {
+    return null;
+  }
+}
+
+export function writeLastChat(storageKey: string, entry: { chatId: string; path: string }) {
+  if (typeof window === "undefined") return;
+  try {
+    window.localStorage.setItem(storageKey, JSON.stringify(entry));
+  } catch {
+    /* ignore */
+  }
+}
+
+export function forgetLastChat(storageKey: string) {
+  if (typeof window === "undefined") return;
+  try {
+    window.localStorage.removeItem(storageKey);
+  } catch {
+    /* ignore */
+  }
+}
+
+/**
+ * The chat's own org, never the panel's: an org switch re-keys the storage entry in the same
+ * effect flush that still holds the previous org's chat, which would file it under the new key.
+ */
+export function shouldPersistLastChat<T extends { chatId: string; organizationId: string }>(
+  active: T | null | undefined,
+  organizationId: string
+): active is T {
+  return Boolean(active?.chatId) && active?.organizationId === organizationId;
+}

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts

@@ -110,7 +110,10 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
       getChatMessages(dashboardAgentDb, { chatId, userId, organizationId: project.organizationId }),
       getSession(dashboardAgentDb, { chatId, userId, organizationId: project.organizationId }),
     ]);
-    return json({ messages: messages ?? [], session });
+    // Null is not an empty transcript: the chat is deleted or another org's, and a 200 would
+    // read as a real, empty chat.
+    if (messages === null) return json({ error: "Chat not found" }, { status: 404 });
+    return json({ messages, session });
   }
 
   const chats = await listChats(dashboardAgentDb, {

apps/webapp/test/dashboardAgentForeignChat.test.ts

@@ -0,0 +1,101 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+
+const mocks = vi.hoisted(() => ({
+  getChatMessages: vi.fn(),
+  getSession: vi.fn(),
+}));
+
+vi.mock("~/db.server", () => ({ $replica: {}, prisma: {} }));
+vi.mock("~/env.server", () => ({ env: { SESSION_SECRET: "test-session-secret" } }));
+vi.mock("~/services/session.server", () => ({
+  requireUser: async () => ({ id: "usr_real", admin: false, isImpersonating: false }),
+}));
+vi.mock("~/v3/canAccessDashboardAgent.server", () => ({
+  canAccessDashboardAgent: async () => true,
+}));
+vi.mock("~/models/project.server", () => ({
+  findProjectBySlug: async () => ({
+    id: "proj_real",
+    organizationId: "org_real",
+    externalRef: "proj_ref_real",
+  }),
+}));
+vi.mock("~/models/runtimeEnvironment.server", () => ({ findEnvironmentBySlug: vi.fn() }));
+vi.mock("~/services/dashboardAgent.server", () => ({
+  dashboardAgentApiOrigin: () => "https://api.trigger.dev",
+  isDashboardAgentConfigured: () => true,
+  mintDashboardAgentToken: vi.fn(),
+  mintDashboardAgentUserActorToken: vi.fn(),
+  resolveDashboardAgentRepoSnapshot: async () => null,
+  startDashboardAgentSession: vi.fn(),
+}));
+vi.mock("~/services/dashboardAgentHeadStart.server", () => ({
+  startDashboardAgentHeadStart: vi.fn(),
+}));
+vi.mock("~/services/dashboardAgentDb.server", () => ({ dashboardAgentDb: {} }));
+vi.mock("~/services/resolveTriggerUri.server", () => ({ resolveTriggerUri: () => null }));
+vi.mock("@internal/dashboard-agent-db", () => ({
+  chatExists: vi.fn(),
+  countUserMessages: vi.fn(),
+  createChat: vi.fn(),
+  getChatMessages: mocks.getChatMessages,
+  getSession: mocks.getSession,
+  listChatIdsWithOpenInvestigations: vi.fn(),
+  listChats: vi.fn(),
+  renameChat: vi.fn(),
+  setChatPinned: vi.fn(),
+  softDeleteChat: vi.fn(),
+}));
+vi.mock("~/services/logger.server", () => ({
+  logger: { debug: vi.fn(), error: vi.fn(), warn: vi.fn(), info: vi.fn() },
+}));
+
+import { resolveOpenedChat } from "~/components/dashboard-agent/opened-chat";
+import { loader } from "~/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent";
+
+function openChatRequest(chatId: string) {
+  return loader({
+    request: new Request(
+      `https://app.trigger.dev/resources/orgs/acme/projects/api/env/dev/dashboard-agent?chatId=${chatId}`
+    ),
+    params: { organizationSlug: "acme", projectParam: "api", envParam: "dev" },
+    context: {},
+  } as any);
+}
+
+// `getChatMessages` returns null for a chat this org cannot see, and [] for one it can that
+// simply has no messages yet. The route must keep those apart.
+describe("dashboard agent loader — opening a chat", () => {
+  beforeEach(() => {
+    mocks.getChatMessages.mockReset();
+    mocks.getSession.mockReset().mockResolvedValue(null);
+  });
+
+  it("reports a chat belonging to another org as not found", async () => {
+    mocks.getChatMessages.mockResolvedValue(null);
+
+    const response = await openChatRequest("chat_from_another_org");
+
+    expect(response.status).toBe(404);
+    expect(await response.json()).toMatchObject({ error: "Chat not found" });
+  });
+
+  it("still returns an empty transcript for a chat of this org that has no messages", async () => {
+    mocks.getChatMessages.mockResolvedValue([]);
+
+    const response = await openChatRequest("chat_mine");
+
+    expect(response.status).toBe(200);
+    expect(await response.json()).toMatchObject({ messages: [] });
+  });
+
+  // What the client makes of the 404: a foreign chat is gone, not an empty chat to keep.
+  it("resolves the not-found response as a gone chat", async () => {
+    mocks.getChatMessages.mockResolvedValue(null);
+
+    const response = await openChatRequest("chat_from_another_org");
+    const data = response.ok ? await response.json() : undefined;
+
+    expect(resolveOpenedChat("chat_from_another_org", data as any)).toEqual({ kind: "gone" });
+  });
+});