Skip to content

Commit 5ea7077

Browse files
committed
fix(dashboard-agent): read the alerts API's real response shape
create_alert reached for an `alert` envelope the route never sent, so it always reported undefined, and the 403 branch keyed on `reason` where the route sends `code` — the "email isn't set up on this instance" wording was unreachable. The unit fixtures were written against both invented shapes, so they hid it; they now mirror the route's bodies exactly. The subscribe path also inlined its deduplication key instead of calling `watchAlertDeduplicationKey`, the one record of whose channel it is.
1 parent 6244f58 commit 5ea7077

3 files changed

Lines changed: 22 additions & 10 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
import {
1313
canUseDashboardAgentEmailAlerts,
1414
DASHBOARD_AGENT_WATCH_ALERT_TYPE,
15+
watchAlertDeduplicationKey,
1516
} from "~/services/dashboardAgentWatchAlerts.server";
1617
import { logger } from "~/services/logger.server";
1718
import { authenticateUatOrApiRequest } from "~/services/uatRoutePreamble.server";
@@ -179,7 +180,7 @@ export async function action({ request }: ActionFunctionArgs) {
179180
alertTypes: [DASHBOARD_AGENT_WATCH_ALERT_TYPE],
180181
environmentTypes: [environment.type],
181182
// Stable per (email, project), so asking twice re-enables one channel.
182-
deduplicationKey: `dashboard-agent-watch:${email}`,
183+
deduplicationKey: watchAlertDeduplicationKey(email),
183184
channel: { type: "EMAIL", email },
184185
});
185186

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

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2237,11 +2237,20 @@ describe("watch alert tools", () => {
22372237
expect(result).toEqual({ alerts });
22382238
});
22392239

2240+
// The body below is exactly what `api.v1.dashboard-agent.alerts.ts` returns on success:
2241+
// the channel flat, with no envelope around it.
2242+
const CREATED_CHANNEL = {
2243+
id: "alert_2",
2244+
type: "EMAIL",
2245+
target: "so…@example.com",
2246+
enabled: true,
2247+
};
2248+
22402249
it("create_alert posts the email channel and reports the created alert", async () => {
22412250
const { result, requests } = await callAlertTool(
22422251
"create_alert",
22432252
{ email: "someone@example.com" },
2244-
{ body: { ok: true, alert: { id: "alert_2", type: "EMAIL" } } }
2253+
{ body: CREATED_CHANNEL }
22452254
);
22462255

22472256
expect(requests[0]?.url).toBe("http://localhost:3030/api/v1/dashboard-agent/alerts");
@@ -2251,30 +2260,31 @@ describe("watch alert tools", () => {
22512260
channel: "email",
22522261
email: "someone@example.com",
22532262
});
2254-
expect(result).toEqual({ created: true, alert: { id: "alert_2", type: "EMAIL" } });
2263+
expect(result).toEqual({ created: true, alert: CREATED_CHANNEL });
22552264

22562265
// With no email the host defaults to the user's account email, so the body carries
22572266
// only the chat scope and the channel.
2258-
const noEmail = await callAlertTool("create_alert", {}, { body: { ok: true } });
2267+
const noEmail = await callAlertTool("create_alert", {}, { body: CREATED_CHANNEL });
22592268
expect(JSON.parse(String(noEmail.requests[0]?.init?.body))).toEqual({
22602269
chatId: "chat_alerts",
22612270
channel: "email",
22622271
});
22632272
});
22642273

2274+
// `code` is the key both alert routes send a 403 refusal under.
22652275
it("create_alert relays a 403 with the reason the host gave", async () => {
22662276
const noEmailSetup = await callAlertTool(
22672277
"create_alert",
22682278
{},
2269-
{ status: 403, body: { error: "denied", reason: "email_alerts_not_configured" } }
2279+
{ status: 403, body: { error: "denied", code: "email_alerts_not_configured" } }
22702280
);
22712281
expect(noEmailSetup.result.error).toContain("isn't set up on this instance");
22722282
expect(noEmailSetup.result.error).toContain("dashboard");
22732283

22742284
const flag = await callAlertTool(
22752285
"create_alert",
22762286
{},
2277-
{ status: 403, body: { error: "denied", reason: "dashboard_agent_disabled" } }
2287+
{ status: 403, body: { error: "denied", code: "dashboard_agent_disabled" } }
22782288
);
22792289
expect(flag.result.error).toContain("aren't enabled here");
22802290
});

internal-packages/dashboard-agent/src/tool-alerts.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,14 @@ export function buildAlertTools(args: {
3636
}
3737

3838
const data = (await res.json().catch(() => undefined)) as
39-
| { error?: string; reason?: string; code?: string }
39+
| { error?: string; code?: string }
4040
| undefined;
4141

42-
// 403 is a capability refusal and `reason` says which one.
42+
// 403 is a capability refusal and `code` says which one.
4343
if (res.status === 403) {
4444
return {
4545
error:
46-
data?.reason === "email_alerts_not_configured"
46+
data?.code === "email_alerts_not_configured"
4747
? "Email delivery isn't set up on this instance, so an email alert can't be created. Tell the user that, and that watch results still show in the dashboard."
4848
: "Email alerts aren't enabled here. Tell the user that, and that watch results still show in the dashboard.",
4949
};
@@ -86,7 +86,8 @@ export function buildAlertTools(args: {
8686
...(email ? { email } : {}),
8787
});
8888
if ("error" in result) return result;
89-
return { created: true, alert: (result.data as { alert?: unknown } | undefined)?.alert };
89+
// The route's body is the channel itself, not an envelope around one.
90+
return { created: true, alert: result.data };
9091
},
9192
}),
9293

0 commit comments

Comments
 (0)