Skip to content

Commit f1f589d

Browse files
committed
chore: merge review-fix packet (chore(webapp,dashboard-agent): review nitpicks — tolerant )
2 parents 4a00424 + ecd293d commit f1f589d

9 files changed

Lines changed: 69 additions & 44 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,6 @@ describe("the chat scopes a turn's navigation to the page it started on", () =>
127127
});
128128

129129
it("hands the persistent handled-set in, so drops are recorded across commits", () => {
130-
expect(chat).toContain("handled: navigatedRef.current!");
130+
expect(chat).toMatch(/handled:\s*navigatedRef\.current!/);
131131
});
132132
});

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
22

3+
/** The key the module under test writes; kept here so a rename fails loudly in one place. */
4+
const STORAGE_KEY = "tdev:dashboard-agent:watching";
5+
36
type StorageListener = (event: { key: string | null }) => void;
47

58
const store = new Map<string, string>();
@@ -27,8 +30,8 @@ const {
2730

2831
/** What another tab writing the key looks like here. */
2932
function otherTabWrote(organizationId: string) {
30-
store.set("tdev:dashboard-agent:watching", JSON.stringify([organizationId]));
31-
for (const listener of storageListeners) listener({ key: "tdev:dashboard-agent:watching" });
33+
store.set(STORAGE_KEY, JSON.stringify([organizationId]));
34+
for (const listener of storageListeners) listener({ key: STORAGE_KEY });
3235
}
3336

3437
describe("watch activity", () => {
@@ -98,20 +101,20 @@ describe("watch activity", () => {
98101

99102
describe("a corrupt key", () => {
100103
it("reads as nothing known when the value is not an array", () => {
101-
store.set("tdev:dashboard-agent:watching", JSON.stringify({ org_1: true }));
104+
store.set(STORAGE_KEY, JSON.stringify({ org_1: true }));
102105

103106
expect(hasWatchActivity("org_1")).toBe(false);
104107
expect(shouldPollWakeFeed({ serverUnreadWakes: 0, organizationId: "org_1" })).toBe(false);
105108
});
106109

107110
it("keeps the ids out of an array holding other things", () => {
108-
store.set("tdev:dashboard-agent:watching", JSON.stringify([{ id: "org_1" }, "org_2", 7]));
111+
store.set(STORAGE_KEY, JSON.stringify([{ id: "org_1" }, "org_2", 7]));
109112

110113
expect(hasWatchActivity("org_1")).toBe(false);
111114
expect(hasWatchActivity("org_2")).toBe(true);
112115

113116
rememberWatchActivity("org_3");
114-
expect(store.get("tdev:dashboard-agent:watching")).toBe(JSON.stringify(["org_2", "org_3"]));
117+
expect(store.get(STORAGE_KEY)).toBe(JSON.stringify(["org_2", "org_3"]));
115118
});
116119
});
117120

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,9 @@ describe("every chat change goes through one door", () => {
9494
it("bumps the open sequence in exactly one place, next to the card reset", () => {
9595
const bumps = panel.match(/openChatRequestSeq\.current\s*(\+\+|\+=)|\+\+openChatRequestSeq/g);
9696
expect(bumps).toHaveLength(1);
97-
expect(panel).toContain(
98-
'dispatchWatchCard({ type: "chat-changed" });\n return ++openChatRequestSeq.current;'
97+
// Whitespace-tolerant: the formatter is free to reindent or rewrap these two lines.
98+
expect(panel).toMatch(
99+
/dispatchWatchCard\(\{\s*type:\s*"chat-changed",?\s*\}\);\s*return \+\+openChatRequestSeq\.current;/
99100
);
100101
});
101102

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

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { watchSpecSchema } from "@internal/dashboard-agent-contracts";
33
import { z } from "zod";
44
import { logger } from "~/services/logger.server";
55
import { resolveWatchEmailAlertsState } from "~/services/dashboardAgentWatchAlerts.server";
6+
import { watchErrorStatus } from "~/services/dashboardAgentWatchErrorStatus.server";
67
import {
78
authorizeWatchEnvironmentById,
89
createDashboardAgentWatch,
@@ -110,24 +111,13 @@ export async function action({ request }: ActionFunctionArgs) {
110111
});
111112

112113
if (!result.ok) {
113-
const status =
114-
result.code === "limit_reached" || result.code === "duplicate"
115-
? 409
116-
: result.code === "invalid_target"
117-
? 404
118-
: // The chat was deleted while the create was in flight.
119-
result.code === "chat_not_found"
120-
? 404
121-
: result.code === "not_configured"
122-
? 501
123-
: 500;
124114
return json(
125115
{
126116
error: result.error,
127117
code: result.code,
128118
...(result.existingId ? { existingId: result.existingId } : {}),
129119
},
130-
{ status }
120+
{ status: watchErrorStatus(result.code) }
131121
);
132122
}
133123

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
startDashboardAgentSession,
5151
} from "~/services/dashboardAgent.server";
5252
import { dashboardAgentEnvironmentAddress } from "~/services/dashboardAgentEnvironmentAddress.server";
53+
import { watchErrorStatus } from "~/services/dashboardAgentWatchErrorStatus.server";
5354
import { startDashboardAgentHeadStart } from "~/services/dashboardAgentHeadStart.server";
5455
import { dashboardAgentDb } from "~/services/dashboardAgentDb.server";
5556
import { logger } from "~/services/logger.server";
@@ -525,23 +526,13 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
525526
});
526527

527528
if (!result.ok) {
528-
const status =
529-
result.code === "limit_reached" ||
530-
result.code === "duplicate" ||
531-
result.code === "request_conflict"
532-
? 409
533-
: result.code === "invalid_target" || result.code === "chat_not_found"
534-
? 404
535-
: result.code === "not_configured"
536-
? 501
537-
: 500;
538529
return json(
539530
{
540531
error: result.error,
541532
code: result.code,
542533
...(result.existingId ? { existingId: result.existingId } : {}),
543534
},
544-
{ status }
535+
{ status: watchErrorStatus(result.code) }
545536
);
546537
}
547538

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import type { SubmitWatchErrorCode } from "./dashboardAgentWatches.server";
2+
3+
/**
4+
* One HTTP status per watch error code, so the resources route and the API route can't
5+
* drift on what a refusal means.
6+
*/
7+
const STATUS_BY_CODE: Record<SubmitWatchErrorCode, number> = {
8+
limit_reached: 409,
9+
duplicate: 409,
10+
request_conflict: 409,
11+
invalid_target: 404,
12+
chat_not_found: 404,
13+
not_configured: 501,
14+
internal: 500,
15+
};
16+
17+
export function watchErrorStatus(code: SubmitWatchErrorCode): number {
18+
return STATUS_BY_CODE[code];
19+
}

apps/webapp/test/dashboardAgentLastReadBackfill.test.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ import {
66
} from "@internal/dashboard-agent-db";
77
import { postgresTest } from "@internal/testcontainers";
88
import type { PrismaClient } from "@trigger.dev/database";
9-
import { readFileSync } from "node:fs";
9+
import { readdirSync, readFileSync } from "node:fs";
1010
import path from "node:path";
11-
import { afterEach, describe, expect } from "vitest";
11+
import { afterEach, describe, expect, it } from "vitest";
1212

1313
/**
1414
* `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
8282
return Object.fromEntries(rows.map((row) => [row.id, row.last_read_at]));
8383
}
8484

85+
describe("the replayed migration list", () => {
86+
it("is the first migrations on disk, so a renamed or inserted one fails here", () => {
87+
const onDisk = readdirSync(DRIZZLE)
88+
.filter((file) => file.endsWith(".sql"))
89+
.sort();
90+
91+
expect(onDisk.slice(0, MIGRATIONS.length)).toEqual(MIGRATIONS);
92+
});
93+
});
94+
8595
let agentDbClient: DashboardAgentDbClient | undefined;
8696

8797
afterEach(async () => {

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -596,20 +596,28 @@ export type WatchExternalNotificationStatus = WatchExternalNotification["status"
596596
/** The window lengths the card offers, in hours. Capped by {@link WATCH_MAX_HOURS}. */
597597
export const WATCH_WINDOW_HOURS_OPTIONS = [0.5, 1, 2, 6, 12, 24] as const;
598598

599-
const RUN_STATE_KINDS = ["run_start", "run_finished", "run_failed"] as const;
599+
/** Lifecycle order: the three questions anyone can ask about one run. */
600+
const RUN_STATE_KINDS = watchSpecSchema.options[0].shape.kind.options;
600601

601602
export function isRunStateWatchKind(kind: WatchKind): boolean {
602603
return (RUN_STATE_KINDS as readonly string[]).includes(kind);
603604
}
604605

605-
/** Must stay in step with the cadence schemas, or the picker offers invalid options. */
606+
function cadenceOptionsOf(
607+
schema: typeof runStateCadenceSchema | typeof standardCadenceSchema
608+
): readonly number[] {
609+
return schema.shape.checkEveryMinutes.options.map((option) => option.value);
610+
}
611+
612+
const RUN_STATE_CADENCE_OPTIONS = cadenceOptionsOf(runStateCadenceSchema);
613+
const STANDARD_CADENCE_OPTIONS = cadenceOptionsOf(standardCadenceSchema);
614+
615+
/** Read off the cadence schemas, so the picker can never offer an option they reject. */
606616
export function watchCadenceOptions(kind: WatchKind): readonly number[] {
607-
return isRunStateWatchKind(kind) ? [1, 5, 15, 60] : [5, 15, 60];
617+
return isRunStateWatchKind(kind) ? RUN_STATE_CADENCE_OPTIONS : STANDARD_CADENCE_OPTIONS;
608618
}
609619

610620
// One family per array, in the order the picker lists them.
611-
// Lifecycle order: the three questions anyone can ask about one run.
612-
const RUN_CONDITION_VARIANTS = ["run_start", "run_finished", "run_failed"] as const;
613621
const QUEUE_CONDITION_VARIANTS = [
614622
"backlog_drain",
615623
"queue_depth_above",
@@ -619,7 +627,7 @@ const QUEUE_CONDITION_VARIANTS = [
619627
] as const;
620628

621629
export function watchConditionVariants(kind: WatchKind): readonly WatchKind[] {
622-
if ((RUN_CONDITION_VARIANTS as readonly string[]).includes(kind)) return RUN_CONDITION_VARIANTS;
630+
if ((RUN_STATE_KINDS as readonly string[]).includes(kind)) return RUN_STATE_KINDS;
623631
if ((QUEUE_CONDITION_VARIANTS as readonly string[]).includes(kind)) {
624632
return QUEUE_CONDITION_VARIANTS;
625633
}

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,18 @@ import type { DashboardAgentDb } from "./client.js";
33

44
// Shared by `queries.ts` and `watch-queries.ts`. Not part of the package's surface.
55

6-
export type DashboardAgentDbOrTx =
7-
| DashboardAgentDb
8-
| Parameters<Parameters<DashboardAgentDb["transaction"]>[0]>[0];
6+
export type DashboardAgentTx = Parameters<Parameters<DashboardAgentDb["transaction"]>[0]>[0];
7+
8+
export type DashboardAgentDbOrTx = DashboardAgentDb | DashboardAgentTx;
99

1010
/** Advisory-lock namespace (ASCII `watc`), so keys can't collide with another lock. */
1111
const WATCH_CHAT_LOCK_NAMESPACE = 0x77617463;
1212

13-
/** Serializes creating a watch against deleting the chat under it. Transaction-scoped. */
14-
export function lockChatForWatches(tx: DashboardAgentDbOrTx, chatId: string) {
13+
/**
14+
* Serializes creating a watch against deleting the chat under it. The lock releases with the
15+
* enclosing transaction, so a plain `Db` handle would drop it immediately — hence tx only.
16+
*/
17+
export function lockChatForWatches(tx: DashboardAgentTx, chatId: string) {
1518
return tx.execute(
1619
sql`select pg_advisory_xact_lock(${WATCH_CHAT_LOCK_NAMESPACE}, hashtext(${chatId}))`
1720
);

0 commit comments

Comments
 (0)