Skip to content

Commit 739b193

Browse files
ericallamTrigger.dev RepoOps
authored andcommitted
fix(webapp): authorize dashboard agent feedback without the agent's chat store
Fixes the dashboard agent's `submit_feedback` reports failing when the request is served by a deployment where the API runs separately from the dashboard. The feedback route no longer looks the chat up in the agent's own database; it authorizes the report from the agent's token and the environment it was issued for. Mono-RevId: 0784bed04b292bca83f097297f7020f05953577d
1 parent b983e06 commit 739b193

2 files changed

Lines changed: 17 additions & 22 deletions

File tree

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
import { json, type ActionFunctionArgs } from "@remix-run/server-runtime";
22
import { DASHBOARD_AGENT_FEEDBACK_LIMITS } from "@internal/dashboard-agent-contracts";
33
import { z } from "zod";
4-
import { resolveAgentAlertContext } from "~/services/dashboardAgentAlertContext.server";
4+
import { authorizeWatchEnvironmentById } from "~/services/dashboardAgentWatches.server";
55
import { logger } from "~/services/logger.server";
66
import { telemetry } from "~/services/telemetry.server";
77
import { authenticateUatOrApiRequest } from "~/services/uatRoutePreamble.server";
88

99
/**
1010
* `POST` records the agent's report of a problem with its own tools or the docs. Only the
1111
* agent's delegated user-actor token is accepted, and the user, organization, project and
12-
* environment all come from it and the chat, never from the body.
12+
* environment all come from it, never from the body. The chat id is only a property on the
13+
* event: this route is served by the API service too, which has no agent chat store.
1314
*/
1415

1516
const FeedbackBodySchema = z.object({
@@ -40,20 +41,19 @@ export async function action({ request }: ActionFunctionArgs) {
4041
return json({ error: "Invalid request", code: "invalid_request" }, { status: 400 });
4142
}
4243

43-
const context = await resolveAgentAlertContext({
44+
const environment = await authorizeWatchEnvironmentById({
4445
userId: actor.userId,
4546
environmentId: actor.environmentId,
46-
chatId: body.data.chatId,
4747
});
48-
if (!context.ok) {
49-
return json({ error: context.error, code: context.code }, { status: 404 });
48+
if (!environment) {
49+
return json({ error: "Environment not found", code: "invalid_target" }, { status: 404 });
5050
}
5151

5252
const recorded = telemetry.dashboardAgent.feedback({
5353
userId: actor.userId,
54-
organizationId: context.environment.organizationId,
55-
projectId: context.environment.project.id,
56-
environmentId: context.environment.id,
54+
organizationId: environment.organizationId,
55+
projectId: environment.project.id,
56+
environmentId: environment.id,
5757
chatId: body.data.chatId,
5858
message: body.data.message,
5959
toolName: body.data.toolName,

‎apps/webapp/test/dashboardAgentFeedbackRoute.test.ts‎

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

33
const mocks = vi.hoisted(() => ({
44
authenticate: vi.fn(),
5-
resolveContext: vi.fn(),
5+
authorizeEnvironment: vi.fn(),
66
feedback: vi.fn(),
77
}));
88

99
vi.mock("~/services/uatRoutePreamble.server", () => ({
1010
authenticateUatOrApiRequest: mocks.authenticate,
1111
}));
12-
vi.mock("~/services/dashboardAgentAlertContext.server", () => ({
13-
resolveAgentAlertContext: mocks.resolveContext,
12+
vi.mock("~/services/dashboardAgentWatches.server", () => ({
13+
authorizeWatchEnvironmentById: mocks.authorizeEnvironment,
1414
}));
1515
vi.mock("~/services/telemetry.server", () => ({
1616
telemetry: { dashboardAgent: { feedback: mocks.feedback } },
@@ -38,7 +38,7 @@ function post(body: unknown) {
3838

3939
beforeEach(() => {
4040
mocks.authenticate.mockReset().mockResolvedValue({ userActor: AGENT_ACTOR });
41-
mocks.resolveContext.mockReset().mockResolvedValue({ ok: true, environment: ENVIRONMENT });
41+
mocks.authorizeEnvironment.mockReset().mockResolvedValue(ENVIRONMENT);
4242
mocks.feedback.mockReset().mockReturnValue(true);
4343
});
4444

@@ -53,10 +53,9 @@ describe("POST /api/v1/dashboard-agent/feedback", () => {
5353
});
5454

5555
expect(response.status).toBe(200);
56-
expect(mocks.resolveContext).toHaveBeenCalledWith({
56+
expect(mocks.authorizeEnvironment).toHaveBeenCalledWith({
5757
userId: "user_1",
5858
environmentId: "env_1",
59-
chatId: "chat_1",
6059
});
6160
expect(mocks.feedback).toHaveBeenCalledWith({
6261
userId: "user_1",
@@ -81,14 +80,10 @@ describe("POST /api/v1/dashboard-agent/feedback", () => {
8180
expect(mocks.feedback).not.toHaveBeenCalled();
8281
});
8382

84-
it("records nothing for a chat the user doesn't own", async () => {
85-
mocks.resolveContext.mockResolvedValue({
86-
ok: false,
87-
code: "chat_not_found",
88-
error: "Chat not found",
89-
});
83+
it("records nothing for an environment the user can no longer reach", async () => {
84+
mocks.authorizeEnvironment.mockResolvedValue(null);
9085

91-
const response = await post({ chatId: "chat_other", message: "hi" });
86+
const response = await post({ chatId: "chat_1", message: "hi" });
9287

9388
expect(response.status).toBe(404);
9489
expect(mocks.feedback).not.toHaveBeenCalled();

0 commit comments

Comments
 (0)