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

Author: kathiekiwi

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

@@ -126,7 +126,7 @@ export function showWatchWakesSummaryToast(count: number, onOpenChat: () => void
       <WakeToastUI
         t={t}
         title="Watch updates"
-        message={`${count} watch updates — open the chat panel.`}
+        message={`${count} watch update${count === 1 ? "" : "s"} — open the chat panel.`}
         onOpenChat={onOpenChat}
       />
     ),

apps/webapp/app/components/dashboard-agent/turn-navigation.test.ts

@@ -127,6 +127,6 @@ describe("the chat scopes a turn's navigation to the page it started on", () =>
   });
 
   it("hands the persistent handled-set in, so drops are recorded across commits", () => {
-    expect(chat).toContain("handled: navigatedRef.current!");
+    expect(chat).toMatch(/handled:\s*navigatedRef\.current!/);
   });
 });

apps/webapp/app/components/dashboard-agent/watch-activity.test.ts

@@ -1,5 +1,8 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 
+/** The key the module under test writes; kept here so a rename fails loudly in one place. */
+const STORAGE_KEY = "tdev:dashboard-agent:watching";
+
 type StorageListener = (event: { key: string | null }) => void;
 
 const store = new Map<string, string>();
@@ -27,8 +30,8 @@ const {
 
 /** What another tab writing the key looks like here. */
 function otherTabWrote(organizationId: string) {
-  store.set("tdev:dashboard-agent:watching", JSON.stringify([organizationId]));
-  for (const listener of storageListeners) listener({ key: "tdev:dashboard-agent:watching" });
+  store.set(STORAGE_KEY, JSON.stringify([organizationId]));
+  for (const listener of storageListeners) listener({ key: STORAGE_KEY });
 }
 
 describe("watch activity", () => {
@@ -98,20 +101,20 @@ describe("watch activity", () => {
 
   describe("a corrupt key", () => {
     it("reads as nothing known when the value is not an array", () => {
-      store.set("tdev:dashboard-agent:watching", JSON.stringify({ org_1: true }));
+      store.set(STORAGE_KEY, JSON.stringify({ org_1: true }));
 
       expect(hasWatchActivity("org_1")).toBe(false);
       expect(shouldPollWakeFeed({ serverUnreadWakes: 0, organizationId: "org_1" })).toBe(false);
     });
 
     it("keeps the ids out of an array holding other things", () => {
-      store.set("tdev:dashboard-agent:watching", JSON.stringify([{ id: "org_1" }, "org_2", 7]));
+      store.set(STORAGE_KEY, JSON.stringify([{ id: "org_1" }, "org_2", 7]));
 
       expect(hasWatchActivity("org_1")).toBe(false);
       expect(hasWatchActivity("org_2")).toBe(true);
 
       rememberWatchActivity("org_3");
-      expect(store.get("tdev:dashboard-agent:watching")).toBe(JSON.stringify(["org_2", "org_3"]));
+      expect(store.get(STORAGE_KEY)).toBe(JSON.stringify(["org_2", "org_3"]));
     });
   });
 

apps/webapp/app/components/dashboard-agent/watch-card-state.test.ts

@@ -94,8 +94,9 @@ describe("every chat change goes through one door", () => {
   it("bumps the open sequence in exactly one place, next to the card reset", () => {
     const bumps = panel.match(/openChatRequestSeq\.current\s*(\+\+|\+=)|\+\+openChatRequestSeq/g);
     expect(bumps).toHaveLength(1);
-    expect(panel).toContain(
-      'dispatchWatchCard({ type: "chat-changed" });\n    return ++openChatRequestSeq.current;'
+    // Whitespace-tolerant: the formatter is free to reindent or rewrap these two lines.
+    expect(panel).toMatch(
+      /dispatchWatchCard\(\{\s*type:\s*"chat-changed",?\s*\}\);\s*return \+\+openChatRequestSeq\.current;/
     );
   });
 

apps/webapp/app/routes/api.v1.dashboard-agent.watches.ts

@@ -3,6 +3,7 @@ import { watchSpecSchema } from "@internal/dashboard-agent-contracts";
 import { z } from "zod";
 import { logger } from "~/services/logger.server";
 import { resolveWatchEmailAlertsState } from "~/services/dashboardAgentWatchAlerts.server";
+import { watchErrorStatus } from "~/services/dashboardAgentWatchErrorStatus.server";
 import {
   authorizeWatchEnvironmentById,
   createDashboardAgentWatch,
@@ -110,24 +111,13 @@ export async function action({ request }: ActionFunctionArgs) {
     });
 
     if (!result.ok) {
-      const status =
-        result.code === "limit_reached" || result.code === "duplicate"
-          ? 409
-          : result.code === "invalid_target"
-            ? 404
-            : // The chat was deleted while the create was in flight.
-              result.code === "chat_not_found"
-              ? 404
-              : result.code === "not_configured"
-                ? 501
-                : 500;
       return json(
         {
           error: result.error,
           code: result.code,
           ...(result.existingId ? { existingId: result.existingId } : {}),
         },
-        { status }
+        { status: watchErrorStatus(result.code) }
       );
     }
 

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

@@ -50,6 +50,7 @@ import {
   startDashboardAgentSession,
 } from "~/services/dashboardAgent.server";
 import { dashboardAgentEnvironmentAddress } from "~/services/dashboardAgentEnvironmentAddress.server";
+import { watchErrorStatus } from "~/services/dashboardAgentWatchErrorStatus.server";
 import { startDashboardAgentHeadStart } from "~/services/dashboardAgentHeadStart.server";
 import { dashboardAgentDb } from "~/services/dashboardAgentDb.server";
 import { logger } from "~/services/logger.server";
@@ -525,23 +526,13 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
     });
 
     if (!result.ok) {
-      const status =
-        result.code === "limit_reached" ||
-        result.code === "duplicate" ||
-        result.code === "request_conflict"
-          ? 409
-          : result.code === "invalid_target" || result.code === "chat_not_found"
-            ? 404
-            : result.code === "not_configured"
-              ? 501
-              : 500;
       return json(
         {
           error: result.error,
           code: result.code,
           ...(result.existingId ? { existingId: result.existingId } : {}),
         },
-        { status }
+        { status: watchErrorStatus(result.code) }
       );
     }
 

apps/webapp/app/services/dashboardAgentWatchErrorStatus.server.ts

@@ -0,0 +1,19 @@
+import type { SubmitWatchErrorCode } from "./dashboardAgentWatches.server";
+
+/**
+ * One HTTP status per watch error code, so the resources route and the API route can't
+ * drift on what a refusal means.
+ */
+const STATUS_BY_CODE: Record<SubmitWatchErrorCode, number> = {
+  limit_reached: 409,
+  duplicate: 409,
+  request_conflict: 409,
+  invalid_target: 404,
+  chat_not_found: 404,
+  not_configured: 501,
+  internal: 500,
+};
+
+export function watchErrorStatus(code: SubmitWatchErrorCode): number {
+  return STATUS_BY_CODE[code];
+}

apps/webapp/test/dashboardAgentAlertAdminPreview.test.ts

@@ -5,7 +5,7 @@
  * preview the agent honours has to be read off the user row.
  */
 
-import { beforeEach, describe, expect, test, vi } from "vitest";
+import { afterAll, beforeEach, describe, expect, test, vi } from "vitest";
 
 const ctx = vi.hoisted(() => ({ admin: false }));
 
@@ -18,10 +18,10 @@ vi.mock("~/db.server", () => {
   return { prisma: db, $replica: db, sqlDatabaseSchema: undefined };
 });
 
-process.env.SESSION_SECRET = "test-session-secret-for-alert-admin-preview";
+vi.stubEnv("SESSION_SECRET", "test-session-secret-for-alert-admin-preview");
 // The install this is previewed on: the flag is off for everyone else.
-process.env.DASHBOARD_AGENT_ADMIN_PREVIEW = "1";
-delete process.env.DASHBOARD_AGENT_ENABLED;
+vi.stubEnv("DASHBOARD_AGENT_ADMIN_PREVIEW", "1");
+vi.stubEnv("DASHBOARD_AGENT_ENABLED", undefined);
 
 const { canUseDashboardAgentAlerts } = await import("~/services/dashboardAgentWatchAlerts.server");
 
@@ -31,6 +31,10 @@ beforeEach(() => {
   ctx.admin = false;
 });
 
+afterAll(() => {
+  vi.unstubAllEnvs();
+});
+
 describe("watch alerts during the admin preview", () => {
   test("let an admin's watch alert, exactly as the agent lets them create it", async () => {
     ctx.admin = true;

apps/webapp/test/dashboardAgentLastReadBackfill.test.ts

@@ -6,9 +6,9 @@ import {
 } from "@internal/dashboard-agent-db";
 import { postgresTest } from "@internal/testcontainers";
 import type { PrismaClient } from "@trigger.dev/database";
-import { readFileSync } from "node:fs";
+import { readdirSync, readFileSync } from "node:fs";
 import path from "node:path";
-import { afterEach, describe, expect } from "vitest";
+import { afterEach, describe, expect, it } from "vitest";
 
 /**
  * `chats.last_read_at` is nullable and every reader treats NULL as unread, so without a
@@ -82,6 +82,16 @@ async function readLastReadAt(prisma: PrismaClient): Promise<Record<string, Date
   return Object.fromEntries(rows.map((row) => [row.id, row.last_read_at]));
 }
 
+describe("the replayed migration list", () => {
+  it("is the first migrations on disk, so a renamed or inserted one fails here", () => {
+    const onDisk = readdirSync(DRIZZLE)
+      .filter((file) => file.endsWith(".sql"))
+      .sort();
+
+    expect(onDisk.slice(0, MIGRATIONS.length)).toEqual(MIGRATIONS);
+  });
+});
+
 let agentDbClient: DashboardAgentDbClient | undefined;
 
 afterEach(async () => {

apps/webapp/test/dashboardAgentQueriesTenantIsolation.test.ts

@@ -1,8 +1,15 @@
 import {
+  claimWatchDelivery,
   createChat,
   createDashboardAgentDb,
+  createWatch,
+  deleteTerminalWatchesOlderThan,
+  getWatch,
   listChats,
+  releaseWatchDelivery,
   softDeleteChat,
+  softDeleteChatsForOrganization,
+  transitionWatchCondition,
   type DashboardAgentDb,
   type DashboardAgentDbClient,
 } from "@internal/dashboard-agent-db";
@@ -98,3 +105,97 @@ describe("softDeleteChat tenant isolation", () => {
     30_000
   );
 });
+
+/**
+ * Both delivery sweeps skip a watch whose chat is deleted, and retention only deletes rows
+ * whose delivery is settled — so a wake still owed when the chat goes would keep its snapshot
+ * until the chat's hard delete (30 days) instead of the much shorter watch retention cutoff.
+ */
+describe("deleting a chat settles the wakes it still owes", () => {
+  async function firedWatch(db: DashboardAgentDb, chatId: string) {
+    const created = await createWatch(db, {
+      chatId,
+      identity: `run_finished:run_${chatId}`,
+      spec: {
+        kind: "run_finished",
+        runId: `run_${chatId}`,
+        checkEveryMinutes: 1,
+        maxHours: 2,
+      } as never,
+      organizationId: ORG,
+      projectId: "proj_1",
+      environmentId: "env_1",
+      userId: USER,
+      expiresAt: new Date(Date.now() + 60 * 60 * 1000),
+    });
+    if (!created.ok) throw new Error(`the watch wasn't created: ${created.error}`);
+
+    const fired = await transitionWatchCondition(db, {
+      id: created.watch.id,
+      resolution: "condition_met",
+      lastResult: { runs: 1 },
+    });
+    expect(fired?.deliveryStatus).toBe("pending");
+    return created.watch.id;
+  }
+
+  postgresTest(
+    "softDeleteChat settles a fired watch's pending delivery, so retention can reclaim it",
+    async ({ prisma, postgresContainer }) => {
+      const db = await boot(prisma, postgresContainer.getConnectionUri());
+      await createChat(db, { id: "chat_1", organizationId: ORG, userId: USER });
+      const watchId = await firedWatch(db, "chat_1");
+
+      await softDeleteChat(db, { chatId: "chat_1", userId: USER, organizationId: ORG });
+
+      expect((await getWatch(db, { id: watchId }))?.deliveryStatus).toBe("not_required");
+      expect(
+        await deleteTerminalWatchesOlderThan(db, { before: new Date(Date.now() + 60_000) })
+      ).toBe(1);
+    },
+    30_000
+  );
+
+  postgresTest(
+    "a delivery claimed in flight is settled too, claim and all",
+    async ({ prisma, postgresContainer }) => {
+      const db = await boot(prisma, postgresContainer.getConnectionUri());
+      await createChat(db, { id: "chat_1", organizationId: ORG, userId: USER });
+      const watchId = await firedWatch(db, "chat_1");
+
+      // Deleted while a deliverer holds the claim: no sweep can reach the row afterwards, so
+      // it would sit in `delivering` for as long as the chat does.
+      const claim = await claimWatchDelivery(db, {
+        id: watchId,
+        staleBefore: new Date(Date.now() - 60_000),
+      });
+      expect(claim?.watch.deliveryStatus).toBe("delivering");
+
+      await softDeleteChat(db, { chatId: "chat_1", userId: USER, organizationId: ORG });
+
+      const settled = await getWatch(db, { id: watchId });
+      expect(settled?.deliveryStatus).toBe("not_required");
+      expect(settled?.deliveryClaimId).toBeNull();
+      expect(settled?.deliveryClaimedAt).toBeNull();
+
+      // The deliverer that still holds the old claim can no longer move the row either way.
+      await releaseWatchDelivery(db, { id: watchId, claimId: claim!.claimId });
+      expect((await getWatch(db, { id: watchId }))?.deliveryStatus).toBe("not_required");
+    },
+    30_000
+  );
+
+  postgresTest(
+    "softDeleteChatsForOrganization settles them too",
+    async ({ prisma, postgresContainer }) => {
+      const db = await boot(prisma, postgresContainer.getConnectionUri());
+      await createChat(db, { id: "chat_1", organizationId: ORG, userId: USER });
+      const watchId = await firedWatch(db, "chat_1");
+
+      expect(await softDeleteChatsForOrganization(db, { organizationId: ORG })).toBe(1);
+
+      expect((await getWatch(db, { id: watchId }))?.deliveryStatus).toBe("not_required");
+    },
+    30_000
+  );
+});