diff --git a/apps/web/src/domains/chat/api/messages.ts b/apps/web/src/domains/chat/api/messages.ts index 3f794ac27f6..cfdc108027a 100644 --- a/apps/web/src/domains/chat/api/messages.ts +++ b/apps/web/src/domains/chat/api/messages.ts @@ -24,6 +24,7 @@ import { type PreChatOnboardingContext, } from "@/domains/onboarding/prechat.js"; import { persistPreChatOnboardingProfile } from "@/domains/onboarding/prechat-profile.js"; +import { pickConversationIdWireField } from "@/lib/backwards-compat/conversation-id-wire-field.js"; const POLL_INTERVAL_MS = 1000; const POLL_TIMEOUT_MS = 120_000; @@ -485,11 +486,12 @@ export async function postChatMessage( attachmentIds: string[] = [], onboarding?: PreChatOnboardingContext, ): Promise { + // Daemon 0.8.5+ accepts `conversationId` on this endpoint as a direct + // internal-id lookup; older daemons only understand the legacy + // `conversationKey` (external-key path). The gate that picks between + // them lives in `lib/backwards-compat/conversation-id-wire-field.ts`. const body: Record = { - // Daemon's send-message endpoint reads `body.conversationKey` only - // (see assistant/src/runtime/routes/conversation-routes.ts handleSendMessage). - // The web-side parameter is conversationId; map to the wire field here. - conversationKey: conversationId, + [pickConversationIdWireField()]: conversationId, content, sourceChannel: "vellum", interface: "vellum", diff --git a/apps/web/src/domains/chat/api/post-chat-message.test.ts b/apps/web/src/domains/chat/api/post-chat-message.test.ts index 0ce1b5979a5..19f7a0bb352 100644 --- a/apps/web/src/domains/chat/api/post-chat-message.test.ts +++ b/apps/web/src/domains/chat/api/post-chat-message.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; import { postChatMessage } from "@/domains/chat/api/messages.js"; +import { useAssistantIdentityStore } from "@/stores/assistant-identity-store.js"; describe("postChatMessage onboarding payload", () => { let originalFetch: typeof fetch; @@ -10,6 +11,11 @@ describe("postChatMessage onboarding payload", () => { beforeEach(() => { originalFetch = globalThis.fetch; capturedRequests = []; + // Reset the assistant-identity store so version-gated wire-field + // selection in `postChatMessage` defaults to the conservative legacy + // `conversationKey` path. Individual tests that exercise the new + // `conversationId` path opt in explicitly via `setIdentity(...)`. + useAssistantIdentityStore.getState().clearIdentity(); // The vellum-api client request interceptor calls ensureCsrfCookie() on // mutating requests, which reads `document.cookie`. Stub a minimal // `document` so the bun test (Node) environment doesn't throw. @@ -177,3 +183,93 @@ describe("postChatMessage onboarding payload", () => { expect(onboarding).not.toHaveProperty("assistantName"); }); }); + +describe("postChatMessage wire-field bilingual cutover", () => { + // Verifies the daemon-version gate selects the right wire field on + // `POST /v1/messages`. See `apps/web/src/assistant/version-compat.ts` + // for the cutover policy (legacy `conversationKey` for daemons older + // than 0.8.5, canonical `conversationId` for 0.8.5+). + let originalFetch: typeof fetch; + let originalDocument: unknown; + let capturedRequests: Array<{ url: string; body: string }> = []; + + beforeEach(() => { + originalFetch = globalThis.fetch; + capturedRequests = []; + useAssistantIdentityStore.getState().clearIdentity(); + originalDocument = (globalThis as { document?: unknown }).document; + (globalThis as { document?: unknown }).document = { cookie: "csrftoken=test" }; + globalThis.fetch = mock(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input instanceof Request ? input.url : String(input); + let bodyText: string | undefined; + if (input instanceof Request) { + bodyText = await input.clone().text(); + } else if (typeof init?.body === "string") { + bodyText = init.body; + } + capturedRequests.push({ url, body: bodyText ?? "" }); + return new Response( + JSON.stringify({ accepted: true, messageId: "msg-1" }), + { status: 200, headers: { "Content-Type": "application/json" } }, + ); + }) as unknown as typeof fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + if (originalDocument === undefined) { + delete (globalThis as { document?: unknown }).document; + } else { + (globalThis as { document?: unknown }).document = originalDocument; + } + useAssistantIdentityStore.getState().clearIdentity(); + }); + + function getMessageBody(): Record { + const requests = capturedRequests.filter((r) => r.url.includes("/messages/")); + expect(requests).toHaveLength(1); + return JSON.parse(requests[0]!.body) as Record; + } + + test("uses conversationId wire field when daemon version >= 0.8.5", async () => { + useAssistantIdentityStore.getState().setIdentity("Vel", "0.8.5"); + + await postChatMessage("asst-1", "conv-internal-1", "hi"); + + const body = getMessageBody(); + expect(body.conversationId).toBe("conv-internal-1"); + expect(body).not.toHaveProperty("conversationKey"); + }); + + test("uses conversationId wire field for newer daemons (e.g. 0.9.0)", async () => { + useAssistantIdentityStore.getState().setIdentity("Vel", "0.9.0"); + + await postChatMessage("asst-1", "conv-internal-2", "hi"); + + const body = getMessageBody(); + expect(body.conversationId).toBe("conv-internal-2"); + expect(body).not.toHaveProperty("conversationKey"); + }); + + test("uses conversationKey wire field for daemons older than 0.8.5", async () => { + useAssistantIdentityStore.getState().setIdentity("Vel", "0.8.4"); + + await postChatMessage("asst-1", "conv-internal-3", "hi"); + + const body = getMessageBody(); + expect(body.conversationKey).toBe("conv-internal-3"); + expect(body).not.toHaveProperty("conversationId"); + }); + + test("falls back to conversationKey when daemon version is unknown (identity not yet hydrated)", async () => { + // Default store state: version === null. This is the window + // between page load and the identity fetch resolving. + expect(useAssistantIdentityStore.getState().version).toBeNull(); + + await postChatMessage("asst-1", "conv-internal-4", "hi"); + + const body = getMessageBody(); + expect(body.conversationKey).toBe("conv-internal-4"); + expect(body).not.toHaveProperty("conversationId"); + }); +}); diff --git a/apps/web/src/domains/chat/api/stream.test.ts b/apps/web/src/domains/chat/api/stream.test.ts index 5c2b988ef16..5e8272975b6 100644 --- a/apps/web/src/domains/chat/api/stream.test.ts +++ b/apps/web/src/domains/chat/api/stream.test.ts @@ -54,6 +54,7 @@ import { } from "@/domains/messaging/turn-store.js"; import { parseAssistantEvent } from "@/domains/chat/api/event-parser.js"; import { subscribeChatEvents, type ChatStreamReconnectCause } from "@/domains/chat/api/stream.js"; +import { useAssistantIdentityStore } from "@/stores/assistant-identity-store.js"; describe("polling reconciliation with state machine", () => { /** @@ -305,6 +306,10 @@ describe("subscribeChatEvents idle watchdog", () => { // path but keeps the bun (Node) test env consistent. originalDocument = (globalThis as { document?: unknown }).document; (globalThis as { document?: unknown }).document = { cookie: "csrftoken=test" }; + // Reset the version-gating store so subscribeChatEvents defaults to + // the legacy `conversationKey` wire field. Tests that exercise the + // newer `conversationId` path opt in explicitly via setIdentity(). + useAssistantIdentityStore.getState().clearIdentity(); }); afterEach(() => { @@ -314,9 +319,10 @@ describe("subscribeChatEvents idle watchdog", () => { } else { (globalThis as { document?: unknown }).document = originalDocument; } + useAssistantIdentityStore.getState().clearIdentity(); }); - test("omits conversationKey query when subscribing to all assistant events", async () => { + test("omits any conversation query param when subscribing to all assistant events", async () => { const requestedUrls: string[] = []; globalThis.fetch = mock( async (input: RequestInfo | URL) => { @@ -344,7 +350,124 @@ describe("subscribeChatEvents idle watchdog", () => { await new Promise((r) => setTimeout(r, 50)); expect(requestedUrls).toHaveLength(1); expect(requestedUrls[0]).toContain("/v1/assistants/asst-1/events/"); + // Neither the legacy nor the canonical wire field should appear when + // subscribing to all assistant events (no conversation filter). expect(requestedUrls[0]).not.toContain("conversationKey"); + expect(requestedUrls[0]).not.toContain("conversationId"); + } finally { + stream.cancel(); + } + }); + + test("uses conversationId query when daemon version >= 0.8.5", async () => { + useAssistantIdentityStore.getState().setIdentity("Vel", "0.8.5"); + + const requestedUrls: string[] = []; + globalThis.fetch = mock( + async (input: RequestInfo | URL) => { + requestedUrls.push(input instanceof Request ? input.url : String(input)); + return new Response( + new ReadableStream({ + start(controller) { + controller.close(); + }, + }), + { status: 200, headers: { "Content-Type": "text/event-stream" } }, + ); + }, + ) as unknown as typeof fetch; + + const stream = subscribeChatEvents( + "asst-1", + "conv-internal-1", + () => {}, + () => {}, + { idleTimeoutMs: 5_000, reconnectBaseDelayMs: 10_000 }, + ); + + try { + await new Promise((r) => setTimeout(r, 50)); + expect(requestedUrls).toHaveLength(1); + const url = new URL(requestedUrls[0]!); + expect(url.searchParams.get("conversationId")).toBe("conv-internal-1"); + expect(url.searchParams.get("conversationKey")).toBeNull(); + } finally { + stream.cancel(); + } + }); + + test("uses conversationKey query when daemon version is older than 0.8.5", async () => { + useAssistantIdentityStore.getState().setIdentity("Vel", "0.8.4"); + + const requestedUrls: string[] = []; + globalThis.fetch = mock( + async (input: RequestInfo | URL) => { + requestedUrls.push(input instanceof Request ? input.url : String(input)); + return new Response( + new ReadableStream({ + start(controller) { + controller.close(); + }, + }), + { status: 200, headers: { "Content-Type": "text/event-stream" } }, + ); + }, + ) as unknown as typeof fetch; + + const stream = subscribeChatEvents( + "asst-1", + "conv-key-legacy", + () => {}, + () => {}, + { idleTimeoutMs: 5_000, reconnectBaseDelayMs: 10_000 }, + ); + + try { + await new Promise((r) => setTimeout(r, 50)); + expect(requestedUrls).toHaveLength(1); + const url = new URL(requestedUrls[0]!); + expect(url.searchParams.get("conversationKey")).toBe("conv-key-legacy"); + expect(url.searchParams.get("conversationId")).toBeNull(); + } finally { + stream.cancel(); + } + }); + + test("falls back to conversationKey when daemon version is unknown", async () => { + // Default store state: identity not yet hydrated. Conservative + // fallback to the legacy field is the only safe choice — older + // daemons silently ignore `conversationId`. + expect(useAssistantIdentityStore.getState().version).toBeNull(); + + const requestedUrls: string[] = []; + globalThis.fetch = mock( + async (input: RequestInfo | URL) => { + requestedUrls.push(input instanceof Request ? input.url : String(input)); + return new Response( + new ReadableStream({ + start(controller) { + controller.close(); + }, + }), + { status: 200, headers: { "Content-Type": "text/event-stream" } }, + ); + }, + ) as unknown as typeof fetch; + + const stream = subscribeChatEvents( + "asst-1", + "conv-unknown-version", + () => {}, + () => {}, + { idleTimeoutMs: 5_000, reconnectBaseDelayMs: 10_000 }, + ); + + try { + await new Promise((r) => setTimeout(r, 50)); + expect(requestedUrls).toHaveLength(1); + const url = new URL(requestedUrls[0]!); + expect(url.searchParams.get("conversationKey")).toBe("conv-unknown-version"); + expect(url.searchParams.get("conversationId")).toBeNull(); } finally { stream.cancel(); } diff --git a/apps/web/src/domains/chat/api/stream.ts b/apps/web/src/domains/chat/api/stream.ts index c2b8d0fbf71..7fdd261d07d 100644 --- a/apps/web/src/domains/chat/api/stream.ts +++ b/apps/web/src/domains/chat/api/stream.ts @@ -12,6 +12,7 @@ import { client, SDK_BASE_OPTIONS } from "@/domains/chat/api/client.js"; import { recordChatDiagnostic, resolvePlatformTag } from "@/domains/chat/utils/diagnostics.js"; import { parseAssistantEvent, readEventConversationId } from "@/domains/chat/api/event-parser.js"; import type { AssistantEvent } from "@/domains/chat/api/event-types.js"; +import { pickConversationIdWireField } from "@/lib/backwards-compat/conversation-id-wire-field.js"; import { getClientRegistrationHeaders } from "@/lib/telemetry/client-identity.js"; import { markClientEstablished, @@ -308,15 +309,21 @@ export function subscribeChatEvents( dataFramesReceivedSinceConnect = 0; let streamError: Error | null = null; try { + // Daemon 0.8.5+ accepts `conversationId` on this endpoint as a + // direct internal-id lookup; older daemons only understand the + // legacy `conversationKey` (external-key path). The gate that + // picks between them lives in + // `lib/backwards-compat/conversation-id-wire-field.ts`. const { stream } = await client.sse.get | string>({ ...SDK_BASE_OPTIONS, url: "/v1/assistants/{assistant_id}/events/", path: { assistant_id: assistantId }, - // SSE endpoint `GET /v1/assistants/{id}/events/` accepts only - // `conversationKey` as its query param (see events-routes.ts). - // Map the internal conversationId variable onto the wire field. ...(requestedConversationId - ? { query: { conversationKey: requestedConversationId } } + ? { + query: { + [pickConversationIdWireField()]: requestedConversationId, + }, + } : {}), headers: { Accept: "text/event-stream, application/json", diff --git a/apps/web/src/lib/backwards-compat/conversation-id-wire-field.test.ts b/apps/web/src/lib/backwards-compat/conversation-id-wire-field.test.ts new file mode 100644 index 00000000000..70cd031fba6 --- /dev/null +++ b/apps/web/src/lib/backwards-compat/conversation-id-wire-field.test.ts @@ -0,0 +1,61 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; + +import { pickConversationIdWireField } from "@/lib/backwards-compat/conversation-id-wire-field.js"; +import { useAssistantIdentityStore } from "@/stores/assistant-identity-store.js"; + +function setVersion(version: string | null) { + useAssistantIdentityStore.getState().setIdentity("test-asst", version); +} + +beforeEach(() => { + useAssistantIdentityStore.getState().clearIdentity(); +}); + +afterEach(() => { + useAssistantIdentityStore.getState().clearIdentity(); +}); + +// Exhaustive truth-table for the underlying semver gate lives in +// `utils.test.ts` (covers null/empty, unparseable, pre-release, `v` +// prefix, etc). Here we verify the wire-field branch on each side +// of the 0.8.5 boundary plus the conservative-on-unknown policy. +describe("pickConversationIdWireField", () => { + test("returns conversationKey when version is unknown", () => { + setVersion(null); + expect(pickConversationIdWireField()).toBe("conversationKey"); + }); + + test("returns conversationKey for assistants on 0.8.4 and older", () => { + setVersion("0.8.4"); + expect(pickConversationIdWireField()).toBe("conversationKey"); + setVersion("0.7.0"); + expect(pickConversationIdWireField()).toBe("conversationKey"); + }); + + test("returns conversationId for assistants on 0.8.5+", () => { + setVersion("0.8.5"); + expect(pickConversationIdWireField()).toBe("conversationId"); + setVersion("0.8.6"); + expect(pickConversationIdWireField()).toBe("conversationId"); + setVersion("0.9.0"); + expect(pickConversationIdWireField()).toBe("conversationId"); + setVersion("1.0.0"); + expect(pickConversationIdWireField()).toBe("conversationId"); + }); + + test("treats RC builds of the cutover patch as supporting the new field", () => { + // 0.8.5-rc.1 ships with the same bilingual handlers as 0.8.5, + // so RC testers must get the new wire field. + setVersion("0.8.5-rc.1"); + expect(pickConversationIdWireField()).toBe("conversationId"); + setVersion("0.8.5-beta"); + expect(pickConversationIdWireField()).toBe("conversationId"); + }); + + test("returns conversationKey for unparseable versions", () => { + setVersion("garbage"); + expect(pickConversationIdWireField()).toBe("conversationKey"); + setVersion("0.8"); + expect(pickConversationIdWireField()).toBe("conversationKey"); + }); +}); diff --git a/apps/web/src/lib/backwards-compat/conversation-id-wire-field.ts b/apps/web/src/lib/backwards-compat/conversation-id-wire-field.ts new file mode 100644 index 00000000000..5553135c108 --- /dev/null +++ b/apps/web/src/lib/backwards-compat/conversation-id-wire-field.ts @@ -0,0 +1,50 @@ +/** + * Backwards-compat gate: conversation-id wire field on POST /v1/messages + * and GET /v1/events. + * + * Vellum Assistant 0.8.5 (PR #31922) made the daemon bilingual on + * conversation routing: `handleSendMessage` and + * `handleSubscribeAssistantEvents` accept either `conversationKey` + * (legacy external-key lookup; materializes a row on first use) or + * `conversationId` (direct internal-id lookup; 404 on miss). The web + * client always has the assistant-minted internal id, so we prefer the + * canonical `conversationId` field when the assistant supports it. + * + * Assistants on 0.8.4 or older only understand `conversationKey`. + * + * NOTE: this helper reads the version snapshot via + * `useAssistantIdentityStore.getState()` rather than the `use.version()` + * hook selector, so it's safe to call from non-hook contexts (event + * handlers, async ops like `postChatMessage` / `subscribeChatEvents`). + * For React-render paths that should re-render when the version flips, + * use `useAssistantSupports(MIN_VERSION)` from `./utils.ts` directly. + */ +import { useAssistantIdentityStore } from "@/stores/assistant-identity-store.js"; +import { compareParsed, parseSemver } from "@/utils/semver.js"; + +const MIN_VERSION = "0.8.5"; + +export type ConversationIdWireField = "conversationId" | "conversationKey"; + +/** + * Picks the conversation-id wire field for the currently-active + * assistant. + * + * - Returns `"conversationKey"` when the assistant version is unknown + * (identity not yet hydrated) or unparseable. Old daemons silently + * ignore `conversationId`, so falling back to the legacy field is + * strictly safer than guessing. + * - Pre-release suffixes on the patch version are ignored: + * `0.8.5-rc.1` counts as `0.8.5`. Testers on RCs get the new path + * the moment the patch version bumps. + */ +export function pickConversationIdWireField(): ConversationIdWireField { + const version = useAssistantIdentityStore.getState().version; + if (!version) return "conversationKey"; + const parsed = parseSemver(version); + const min = parseSemver(MIN_VERSION); + if (!parsed || !min) return "conversationKey"; + return compareParsed({ ...parsed, pre: null }, min) >= 0 + ? "conversationId" + : "conversationKey"; +}