chore(webapp,dashboard-agent): review nitpicks — tolerant test assertions, single-sourced cadence options, tx-narrowed advisory lock

Author: kathiekiwi

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/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 () => {

internal-packages/dashboard-agent-contracts/src/watch.ts

@@ -588,20 +588,28 @@ export type WatchExternalNotificationStatus = WatchExternalNotification["status"
 /** The window lengths the card offers, in hours. Capped by {@link WATCH_MAX_HOURS}. */
 export const WATCH_WINDOW_HOURS_OPTIONS = [0.5, 1, 2, 6, 12, 24] as const;
 
-const RUN_STATE_KINDS = ["run_start", "run_finished", "run_failed"] as const;
+/** Lifecycle order: the three questions anyone can ask about one run. */
+const RUN_STATE_KINDS = watchSpecSchema.options[0].shape.kind.options;
 
 export function isRunStateWatchKind(kind: WatchKind): boolean {
   return (RUN_STATE_KINDS as readonly string[]).includes(kind);
 }
 
-/** Must stay in step with the cadence schemas, or the picker offers invalid options. */
+function cadenceOptionsOf(
+  schema: typeof runStateCadenceSchema | typeof standardCadenceSchema
+): readonly number[] {
+  return schema.shape.checkEveryMinutes.options.map((option) => option.value);
+}
+
+const RUN_STATE_CADENCE_OPTIONS = cadenceOptionsOf(runStateCadenceSchema);
+const STANDARD_CADENCE_OPTIONS = cadenceOptionsOf(standardCadenceSchema);
+
+/** Read off the cadence schemas, so the picker can never offer an option they reject. */
 export function watchCadenceOptions(kind: WatchKind): readonly number[] {
-  return isRunStateWatchKind(kind) ? [1, 5, 15, 60] : [5, 15, 60];
+  return isRunStateWatchKind(kind) ? RUN_STATE_CADENCE_OPTIONS : STANDARD_CADENCE_OPTIONS;
 }
 
 // One family per array, in the order the picker lists them.
-// Lifecycle order: the three questions anyone can ask about one run.
-const RUN_CONDITION_VARIANTS = ["run_start", "run_finished", "run_failed"] as const;
 const QUEUE_CONDITION_VARIANTS = [
   "backlog_drain",
   "queue_depth_above",
@@ -611,7 +619,7 @@ const QUEUE_CONDITION_VARIANTS = [
 ] as const;
 
 export function watchConditionVariants(kind: WatchKind): readonly WatchKind[] {
-  if ((RUN_CONDITION_VARIANTS as readonly string[]).includes(kind)) return RUN_CONDITION_VARIANTS;
+  if ((RUN_STATE_KINDS as readonly string[]).includes(kind)) return RUN_STATE_KINDS;
   if ((QUEUE_CONDITION_VARIANTS as readonly string[]).includes(kind)) {
     return QUEUE_CONDITION_VARIANTS;
   }

internal-packages/dashboard-agent-db/src/internal.ts

@@ -3,15 +3,18 @@ import type { DashboardAgentDb } from "./client.js";
 
 // Shared by `queries.ts` and `watch-queries.ts`. Not part of the package's surface.
 
-export type DashboardAgentDbOrTx =
-  | DashboardAgentDb
-  | Parameters<Parameters<DashboardAgentDb["transaction"]>[0]>[0];
+export type DashboardAgentTx = Parameters<Parameters<DashboardAgentDb["transaction"]>[0]>[0];
+
+export type DashboardAgentDbOrTx = DashboardAgentDb | DashboardAgentTx;
 
 /** Advisory-lock namespace (ASCII `watc`), so keys can't collide with another lock. */
 const WATCH_CHAT_LOCK_NAMESPACE = 0x77617463;
 
-/** Serializes creating a watch against deleting the chat under it. Transaction-scoped. */
-export function lockChatForWatches(tx: DashboardAgentDbOrTx, chatId: string) {
+/**
+ * Serializes creating a watch against deleting the chat under it. The lock releases with the
+ * enclosing transaction, so a plain `Db` handle would drop it immediately — hence tx only.
+ */
+export function lockChatForWatches(tx: DashboardAgentTx, chatId: string) {
   return tx.execute(
     sql`select pg_advisory_xact_lock(${WATCH_CHAT_LOCK_NAMESPACE}, hashtext(${chatId}))`
   );