diff --git a/packages/junior-dashboard/e2e/conversation-send.spec.ts b/packages/junior-dashboard/e2e/conversation-send.spec.ts index 83a39b3e9b..5a2d279e40 100644 --- a/packages/junior-dashboard/e2e/conversation-send.spec.ts +++ b/packages/junior-dashboard/e2e/conversation-send.spec.ts @@ -1,4 +1,7 @@ -import { webMessageId } from "@sentry/junior/api/schema"; +import { + CONVERSATION_DETAIL_VERSION_HEADER, + webMessageId, +} from "@sentry/junior/api/schema"; import type { ConversationPendingMessage, InputImage, @@ -178,7 +181,10 @@ test("starts and continues conversations from the dashboard", async ({ if (holdDetailRefresh) await detailRefreshHeld; // Request the full body because this test changes it. const response = await route.fetch({ - headers: { ...route.request().headers(), "if-none-match": "" }, + headers: { + ...route.request().headers(), + [CONVERSATION_DETAIL_VERSION_HEADER]: "", + }, }); await route.fulfill({ response, @@ -331,7 +337,10 @@ test("hands web and external messages from queue to history without gaps", async await historyHeld; // Request the full body because this test changes it. const response = await route.fetch({ - headers: { ...route.request().headers(), "if-none-match": "" }, + headers: { + ...route.request().headers(), + [CONVERSATION_DETAIL_VERSION_HEADER]: "", + }, }); const detail = await response.json(); await route.fulfill({ @@ -678,7 +687,10 @@ test("stops the active Turn and returns queued text to the composer", async ({ }); await page.route(path, async (route) => { const response = await route.fetch({ - headers: { ...route.request().headers(), "if-none-match": "" }, + headers: { + ...route.request().headers(), + [CONVERSATION_DETAIL_VERSION_HEADER]: "", + }, }); const detail = await response.json(); await route.fulfill({ @@ -795,7 +807,10 @@ test("edits a queued message back into the composer", async ({ }); await page.route(path, async (route) => { const response = await route.fetch({ - headers: { ...route.request().headers(), "if-none-match": "" }, + headers: { + ...route.request().headers(), + [CONVERSATION_DETAIL_VERSION_HEADER]: "", + }, }); const detail = await response.json(); await route.fulfill({ diff --git a/packages/junior-dashboard/e2e/review-desk.spec.ts b/packages/junior-dashboard/e2e/review-desk.spec.ts index d9141cbed2..bcd5d51d86 100644 --- a/packages/junior-dashboard/e2e/review-desk.spec.ts +++ b/packages/junior-dashboard/e2e/review-desk.spec.ts @@ -1,3 +1,4 @@ +import { CONVERSATION_DETAIL_VERSION_HEADER } from "@sentry/junior/api/schema"; import type { Page } from "@playwright/test"; import { screenshot } from "./screenshot"; import { expect, test } from "./test"; @@ -279,7 +280,10 @@ test("review item changes show as rows, not as context for other items", async ( `**/api/conversations/${encodeURIComponent(conversationId)}`, async (route) => { const response = await route.fetch({ - headers: { ...route.request().headers(), "if-none-match": "" }, + headers: { + ...route.request().headers(), + [CONVERSATION_DETAIL_VERSION_HEADER]: "", + }, }); const detail = await response.json(); const askedAt = Date.parse(rollout.createdAt); @@ -455,7 +459,10 @@ test("a message asks before it drops unsent picks", async ({ // Show the tool call that added the item, so its card is in the transcript. await page.route(path, async (route) => { const response = await route.fetch({ - headers: { ...route.request().headers(), "if-none-match": "" }, + headers: { + ...route.request().headers(), + [CONVERSATION_DETAIL_VERSION_HEADER]: "", + }, }); const detail = await response.json(); const last = detail.events.at(-1); diff --git a/packages/junior-dashboard/src/app.ts b/packages/junior-dashboard/src/app.ts index 7b76f6c677..42d1207fc1 100644 --- a/packages/junior-dashboard/src/app.ts +++ b/packages/junior-dashboard/src/app.ts @@ -537,7 +537,7 @@ export function createDashboardApp( }; app.use("*", requireAuth); - // Core detail reads set a version ETag before they build the report. Mock + // Core detail reads set a version token before they build the report. Mock // reporting has no stored version, so it revalidates on content instead. if (options.mockConversations) { app.use("/api/conversations/:conversationId", revalidateConversation); diff --git a/packages/junior-dashboard/src/client/conversations/queries.ts b/packages/junior-dashboard/src/client/conversations/queries.ts index e44c9bbd36..e5de666444 100644 --- a/packages/junior-dashboard/src/client/conversations/queries.ts +++ b/packages/junior-dashboard/src/client/conversations/queries.ts @@ -16,6 +16,7 @@ import type { } from "@sentry/junior/api/schema"; import { acceptedConversationMessageSchema, + CONVERSATION_DETAIL_VERSION_HEADER, webMessageId, cancelConversationPendingMessagesResponseSchema, conversationDetailReportSchema, @@ -748,11 +749,17 @@ export async function readConversationData( const response = await readDashboardResponse( `/api/conversations/${encodeURIComponent(conversationId)}`, signal, - previous?.detailEtag, + previous?.detailEtag + ? { [CONVERSATION_DETAIL_VERSION_HEADER]: previous.detailEtag } + : undefined, ); if (response.status === 304 && previous) return previous; const report = conversationDetailReportSchema.parse(await response.json()); - return { ...report, detailEtag: response.headers.get("etag") ?? undefined }; + return { + ...report, + detailEtag: + response.headers.get(CONVERSATION_DETAIL_VERSION_HEADER) ?? undefined, + }; } /** Read accepted mailbox messages that have not reached durable history yet. */ diff --git a/packages/junior-dashboard/src/client/http.ts b/packages/junior-dashboard/src/client/http.ts index 72c761cdaa..fb002a198b 100644 --- a/packages/junior-dashboard/src/client/http.ts +++ b/packages/junior-dashboard/src/client/http.ts @@ -174,21 +174,21 @@ export async function fetchDashboardJson( return schema.parse(await response.json()); } -/** Read an authenticated response, allowing 304 only for an explicit validator. */ +/** Read an authenticated response, allowing 304 only for explicit validator headers. */ export async function readDashboardResponse( path: string, signal?: AbortSignal, - etag?: string, + validator?: Record, ): Promise { const response = await fetchDashboard(path, { credentials: "same-origin", ...(signal ? { signal } : undefined), - ...(etag - ? { cache: "no-store" as const, headers: { "if-none-match": etag } } + ...(validator + ? { cache: "no-store" as const, headers: validator } : undefined), }); if (response.status === 401) restartDashboardSignIn(); - if (!response.ok && !(etag && response.status === 304)) { + if (!response.ok && !(validator && response.status === 304)) { await throwDashboardApiError(path, response); } return response; diff --git a/packages/junior-dashboard/src/conversation-cache.ts b/packages/junior-dashboard/src/conversation-cache.ts index 8852f52160..96fba0b60e 100644 --- a/packages/junior-dashboard/src/conversation-cache.ts +++ b/packages/junior-dashboard/src/conversation-cache.ts @@ -1,4 +1,5 @@ import { createHash } from "node:crypto"; +import { CONVERSATION_DETAIL_VERSION_HEADER } from "@sentry/junior/api/schema"; import type { MiddlewareHandler } from "hono"; /** Revalidate mock detail reads on content, after the route checks viewer access. */ @@ -15,15 +16,12 @@ export const revalidateConversation: MiddlewareHandler = async ( // generatedAt is the read time, not a change to the Conversation. The mailbox // has its own live watermark and is never handled by this middleware. const { generatedAt: _generatedAt, ...content } = report; - const etag = `W/"${createHash("sha256").update(JSON.stringify(content)).digest("hex")}"`; + const version = createHash("sha256") + .update(JSON.stringify(content)) + .digest("hex"); context.header("cache-control", "private, no-store"); - context.header("etag", etag); - const validators = context.req.header("if-none-match")?.split(","); - if ( - validators?.some( - (value) => value.trim().replace(/^W\//, "") === etag.slice(2), - ) - ) { + context.header(CONVERSATION_DETAIL_VERSION_HEADER, version); + if (context.req.header(CONVERSATION_DETAIL_VERSION_HEADER) === version) { context.res = new Response(null, { status: 304, headers: context.res.headers, diff --git a/packages/junior-dashboard/tests/client-api.test.ts b/packages/junior-dashboard/tests/client-api.test.ts index 7c6cc1f3f1..85430da41f 100644 --- a/packages/junior-dashboard/tests/client-api.test.ts +++ b/packages/junior-dashboard/tests/client-api.test.ts @@ -1,3 +1,4 @@ +import { CONVERSATION_DETAIL_VERSION_HEADER } from "@sentry/junior/api/schema"; import { createHash } from "node:crypto"; import { QueryClient } from "@tanstack/react-query"; import { webMessageId } from "@sentry/junior/api/schema"; @@ -167,7 +168,9 @@ describe("dashboard client API", () => { signedIn = false; const denied = await app.fetch( new Request(`http://localhost${detailPath}`, { - headers: { "if-none-match": changed.detailEtag! }, + headers: { + [CONVERSATION_DETAIL_VERSION_HEADER]: changed.detailEtag!, + }, }), ); expect(denied.status).toBe(401); diff --git a/packages/junior/src/api/conversations/detail-etag.ts b/packages/junior/src/api/conversations/detail-etag.ts index 2e1f08ab69..52de29a144 100644 --- a/packages/junior/src/api/conversations/detail-etag.ts +++ b/packages/junior/src/api/conversations/detail-etag.ts @@ -82,15 +82,5 @@ export async function readConversationDetailEtag( subtree, viewerMarks: viewerMarks ?? null, }); - return `W/"${createHash("sha256").update(basis).digest("hex")}"`; -} - -/** Report whether an `If-None-Match` header names the current ETag. */ -export function etagMatches(header: string | undefined, etag: string): boolean { - const opaque = etag.replace(/^W\//, ""); - return Boolean( - header - ?.split(",") - .some((value) => value.trim().replace(/^W\//, "") === opaque), - ); + return createHash("sha256").update(basis).digest("hex"); } diff --git a/packages/junior/src/api/conversations/routes.ts b/packages/junior/src/api/conversations/routes.ts index f7cd2cf936..259eda70e1 100644 --- a/packages/junior/src/api/conversations/routes.ts +++ b/packages/junior/src/api/conversations/routes.ts @@ -12,6 +12,7 @@ import { steerConversationPendingMessagesResponseSchema, conversationAttachmentParamsSchema, conversationDetailQuerySchema, + CONVERSATION_DETAIL_VERSION_HEADER, conversationDetailReportSchema, conversationEventPageSchema, conversationEventsQuerySchema, @@ -56,7 +57,7 @@ import { type WebInputServices, } from "./create"; import { readConversationDetail } from "./detail"; -import { etagMatches, readConversationDetailEtag } from "./detail-etag"; +import { readConversationDetailEtag } from "./detail-etag"; import { forkConversationForViewer, keepQuickQuestionForViewer } from "./fork"; import { readConversationEvents } from "./event-list"; import { readConversationFeed } from "./list"; @@ -494,17 +495,23 @@ export function createConversationRoutes( const { conversationId } = context.req.valid("param"); const query = context.req.valid("query"); const viewer = context.get("viewer"); - // Polls send the last ETag. Compare it before the report is built, - // because building the report is the expensive part of this route. + // Polls send the last version token. Compare it before the report is + // built, because building the report is the expensive part of this route. const etag = await readConversationDetailEtag(conversationId, { limit: query.limit, ...(viewer ? { viewer } : undefined), }); - if (etag && etagMatches(context.req.header("if-none-match"), etag)) { - return new Response(null, { - status: 304, - headers: { "cache-control": "private, no-store", etag }, - }); + const headers = etag + ? { + "cache-control": "private, no-store", + [CONVERSATION_DETAIL_VERSION_HEADER]: etag, + } + : undefined; + if ( + headers && + context.req.header(CONVERSATION_DETAIL_VERSION_HEADER) === etag + ) { + return new Response(null, { status: 304, headers }); } const report = await readConversationDetail(conversationId, { ...query, @@ -514,9 +521,7 @@ export function createConversationRoutes( return jsonResponse( conversationDetailReportSchema, report, - etag - ? { headers: { "cache-control": "private, no-store", etag } } - : undefined, + headers ? { headers } : undefined, ); }, ); diff --git a/packages/junior/src/api/schema.ts b/packages/junior/src/api/schema.ts index 67e08dc110..cdd8695d48 100644 --- a/packages/junior/src/api/schema.ts +++ b/packages/junior/src/api/schema.ts @@ -55,6 +55,7 @@ export { steerConversationPendingMessagesResponseSchema, conversationAuxiliaryCostsSchema, conversationDetailQuerySchema, + CONVERSATION_DETAIL_VERSION_HEADER, conversationDetailReportSchema, conversationEventPageSchema, conversationEventHistorySchema, diff --git a/packages/junior/src/api/schema/conversation.ts b/packages/junior/src/api/schema/conversation.ts index 030e65d63d..497340eb7d 100644 --- a/packages/junior/src/api/schema/conversation.ts +++ b/packages/junior/src/api/schema/conversation.ts @@ -847,6 +847,13 @@ export type ConversationAgentTreeNode = z.infer< typeof conversationAgentTreeNodeSchema >; +/** + * Carries the detail version token in both directions. A custom header, not + * `ETag` / `If-None-Match`: in production the Vercel CDN answered standard + * validators itself, after the function had already built the full report. + */ +export const CONVERSATION_DETAIL_VERSION_HEADER = "x-junior-detail-version"; + export const conversationDetailReportSchema = conversationSummaryReportSchema .extend({ /** diff --git a/packages/junior/tests/integration/api/conversations/archive.test.ts b/packages/junior/tests/integration/api/conversations/archive.test.ts index 49894ef323..e1eb8ba9b1 100644 --- a/packages/junior/tests/integration/api/conversations/archive.test.ts +++ b/packages/junior/tests/integration/api/conversations/archive.test.ts @@ -1,3 +1,4 @@ +import { CONVERSATION_DETAIL_VERSION_HEADER } from "@/api/schema"; import { Hono } from "hono"; import { afterEach, describe, expect, it } from "vitest"; import { defineJuniorPlugin } from "@sentry/junior-plugin-api"; @@ -346,7 +347,9 @@ describe("conversation archive API", () => { const { app } = await authenticatedApi(viewer.email); const detailUrl = `http://localhost/api/conversations/${optedOutId}`; - const beforeEtag = (await app.request(detailUrl)).headers.get("etag")!; + const beforeEtag = (await app.request(detailUrl)).headers.get( + CONVERSATION_DETAIL_VERSION_HEADER, + )!; const optOut = await app.request( `http://localhost/api/conversations/${optedOutId}/auto-archive`, { @@ -363,7 +366,7 @@ describe("conversation archive API", () => { expect( ( await app.request(detailUrl, { - headers: { "if-none-match": beforeEtag }, + headers: { [CONVERSATION_DETAIL_VERSION_HEADER]: beforeEtag }, }) ).status, ).toBe(200); diff --git a/packages/junior/tests/integration/api/conversations/detail.test.ts b/packages/junior/tests/integration/api/conversations/detail.test.ts index e556451bfb..c3db9803fd 100644 --- a/packages/junior/tests/integration/api/conversations/detail.test.ts +++ b/packages/junior/tests/integration/api/conversations/detail.test.ts @@ -1,3 +1,4 @@ +import { CONVERSATION_DETAIL_VERSION_HEADER } from "@/api/schema"; import { afterEach, describe, expect, it } from "vitest"; import { avatarPath } from "../../../fixtures/avatar"; import { testViewer } from "../../../fixtures/user"; @@ -134,13 +135,12 @@ describe("conversation detail API", () => { const app = createJuniorApi(); const poll = (etag?: string) => app.request(`http://localhost/api/conversations/${conversationId}`, { - headers: etag ? { "if-none-match": etag } : {}, + headers: etag ? { [CONVERSATION_DETAIL_VERSION_HEADER]: etag } : {}, }); const first = await poll(); - const firstEtag = first.headers.get("etag")!; + const firstEtag = first.headers.get(CONVERSATION_DETAIL_VERSION_HEADER)!; expect(first.status).toBe(200); - expect(firstEtag).toMatch(/^W\/"/); const unchanged = await poll(firstEtag); expect(unchanged.status).toBe(304); expect(await unchanged.text()).toBe(""); @@ -150,7 +150,9 @@ describe("conversation detail API", () => { parentConversationId: conversationId, }); const withChild = await poll(firstEtag); - const childEtag = withChild.headers.get("etag")!; + const childEtag = withChild.headers.get( + CONVERSATION_DETAIL_VERSION_HEADER, + )!; expect(withChild.status).toBe(200); expect(childEtag).not.toBe(firstEtag); @@ -167,7 +169,9 @@ describe("conversation detail API", () => { }, ]); const childChanged = await poll(childEtag); - const childChangedEtag = childChanged.headers.get("etag")!; + const childChangedEtag = childChanged.headers.get( + CONVERSATION_DETAIL_VERSION_HEADER, + )!; expect(childChanged.status).toBe(200); await createPluginAnnotations({ diff --git a/packages/junior/tests/integration/api/conversations/pin.test.ts b/packages/junior/tests/integration/api/conversations/pin.test.ts index 292b033f0b..960792eda6 100644 --- a/packages/junior/tests/integration/api/conversations/pin.test.ts +++ b/packages/junior/tests/integration/api/conversations/pin.test.ts @@ -1,3 +1,4 @@ +import { CONVERSATION_DETAIL_VERSION_HEADER } from "@/api/schema"; import { Hono } from "hono"; import { afterEach, describe, expect, it } from "vitest"; import { createJuniorApi } from "@/api"; @@ -98,7 +99,9 @@ describe("conversation pin API", () => { } const detailPath = `http://localhost/api/conversations/${older}`; - const unpinnedEtag = (await app.request(detailPath)).headers.get("etag")!; + const unpinnedEtag = (await app.request(detailPath)).headers.get( + CONVERSATION_DETAIL_VERSION_HEADER, + )!; const pin = await setPin(app, older, true); expect(pin.status).toBe(200); @@ -117,7 +120,7 @@ describe("conversation pin API", () => { ]); // A pin is viewer state, so an open detail page revalidates. const pinnedDetail = await app.request(detailPath, { - headers: { "if-none-match": unpinnedEtag }, + headers: { [CONVERSATION_DETAIL_VERSION_HEADER]: unpinnedEtag }, }); expect(pinnedDetail.status).toBe(200); expect(