From 5744c3015b5e4a2376628e6739a553894b2945cd Mon Sep 17 00:00:00 2001 From: RohanReddy44 Date: Thu, 24 Sep 2026 14:55:51 +0530 Subject: [PATCH 1/4] fix: reject a started event without ids `started` was the one event read without checking its shape: a missing assistantMessageId or traceId was coerced with String(), so the store got the literal text "undefined" as the bubble id. A second such turn would then overwrite the first turn's bubble in place, and "undefined" would be kept as the trace id for support. It is now held to the same rule as completed and error: a started event without both ids is a broken stream, reported as upstream_error before any handler runs. Co-Authored-By: Claude Fable 5.1 --- src/__tests__/api-service.test.ts | 10 ++++++++++ src/lib/api-service.ts | 10 +++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/__tests__/api-service.test.ts b/src/__tests__/api-service.test.ts index a84ca58..b266ff8 100644 --- a/src/__tests__/api-service.test.ts +++ b/src/__tests__/api-service.test.ts @@ -108,6 +108,16 @@ describe("sendUserQuery", () => { expect(answer.error).toMatchObject({ code: "provider_unavailable", retryable: true }); }); + it("fails as upstream_error when started has no ids, rather than passing 'undefined' on", async () => { + fetchOnce(streamResponse('event: started\ndata: {"sequence":1,"sessionId":"s","messageId":"m"}\n\n' + COMPLETED)); + const onStarted = vi.fn(); + + const error = await failure(apiService.sendUserQuery(turn, { onStarted })); + + expect(error).toMatchObject({ code: "upstream_error", retryable: true }); + expect(onStarted).not.toHaveBeenCalled(); + }); + it("skips events it does not know", async () => { fetchOnce(streamResponse(STARTED + 'event: heartbeat\ndata: {"sequence":2}\n\n' + COMPLETED)); diff --git a/src/lib/api-service.ts b/src/lib/api-service.ts index 656759f..c15d05c 100644 --- a/src/lib/api-service.ts +++ b/src/lib/api-service.ts @@ -163,11 +163,11 @@ const readAnswer = async ( switch (frame.event) { case 'started': - if (typeof data.traceId === 'string') traceId = data.traceId; - handlers.onStarted?.({ - assistantMessageId: String(data.assistantMessageId), - traceId: String(data.traceId), - }); + if (typeof data.assistantMessageId !== 'string' || typeof data.traceId !== 'string') { + throw upstreamError('started carried no assistantMessageId or traceId'); + } + traceId = data.traceId; + handlers.onStarted?.({ assistantMessageId: data.assistantMessageId, traceId: data.traceId }); break; case 'delta': if (typeof data.text === 'string') handlers.onDelta?.(data.text); From 532609bf5dad6f1d18c90fe30df407f2ed132d39 Mon Sep 17 00:00:00 2001 From: RohanReddy44 Date: Thu, 24 Sep 2026 14:56:40 +0530 Subject: [PATCH 2/4] fix: accept api.baseUrl with a trailing slash `api.baseUrl` is used as written and every path is appended as `/v1/chat`, so a value of `/experience/api/` produced `/experience/api//v1/chat`. The API treats that as a different, unknown route and answers 404, which the client showed as an unknown error on every turn with nothing pointing at the cause. The dev proxy had the matching problem: its key matched, but the stripped path lost its leading slash. Trailing slashes are now dropped where the value is read, in the client and in vite.config.ts alike. The old default ended in a slash, so this is a natural thing to type. Co-Authored-By: Claude Fable 5.1 --- src/__tests__/api-service.test.ts | 17 ++++++++++++++--- src/lib/api-service.ts | 5 ++++- vite.config.ts | 3 ++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/__tests__/api-service.test.ts b/src/__tests__/api-service.test.ts index b266ff8..bc15621 100644 --- a/src/__tests__/api-service.test.ts +++ b/src/__tests__/api-service.test.ts @@ -1,8 +1,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -vi.mock("@/lib/config/runtime-config", () => ({ - getConfig: () => ({ api: { baseUrl: "/api" }, stubs: { enabled: false } }) -})); +const mockConfig = vi.hoisted(() => ({ api: { baseUrl: "/api" }, stubs: { enabled: false } })); +vi.mock("@/lib/config/runtime-config", () => ({ getConfig: () => mockConfig })); import apiService, { ApiError, type ChatTurn } from "@/lib/api-service"; @@ -79,6 +78,18 @@ describe("sendUserQuery", () => { expect(JSON.parse(second.body as string).location).toEqual({ latitude: 20.0059, longitude: 73.7898 }); }); + it("tolerates a trailing slash on api.baseUrl", async () => { + mockConfig.api.baseUrl = "/experience/api/"; + try { + const fetchMock = fetchOnce(streamResponse(STARTED + COMPLETED)); + await apiService.sendUserQuery(turn); + + expect(fetchMock.mock.calls[0]?.[0]).toBe("/experience/api/v1/chat"); + } finally { + mockConfig.api.baseUrl = "/api"; + } + }); + it("reports started and each delta, then resolves with the completed answer", async () => { fetchOnce(streamResponse(STARTED + DELTAS + COMPLETED)); const onStarted = vi.fn(); diff --git a/src/lib/api-service.ts b/src/lib/api-service.ts index c15d05c..81477aa 100644 --- a/src/lib/api-service.ts +++ b/src/lib/api-service.ts @@ -217,8 +217,11 @@ interface ImageUploadResponse { * * Read on each call rather than at import. Configuration exists only once * loadRuntimeConfig() has resolved, and this module is imported by tests. + * + * A trailing slash is dropped: every path is appended as `/v1/chat`, and + * `/api//v1/chat` is a different, unknown route to the API. */ -const apiBaseUrl = (): string => getConfig().api.baseUrl; +const apiBaseUrl = (): string => getConfig().api.baseUrl.replace(/\/+$/, ''); class ApiService { private locationData: LocationData | null = null; diff --git a/vite.config.ts b/vite.config.ts index 932c539..db350b9 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -25,7 +25,8 @@ const virtualRouteFileChangeReloadPlugin: PluginOption = { * plugin puts it on the web root in both dev and build from that one source. */ const APP_CONFIG_SOURCE = path.resolve(__dirname, "src/config/app-config.json"); -const apiBaseUrl: string = JSON.parse(fs.readFileSync(APP_CONFIG_SOURCE, "utf-8")).api.baseUrl; +// Without its trailing slash, if any, as the client reads it. +const apiBaseUrl: string = JSON.parse(fs.readFileSync(APP_CONFIG_SOURCE, "utf-8")).api.baseUrl.replace(/\/+$/, ""); const appConfigPlugin: PluginOption = { name: "serve-app-config", From 2d36080c8c73a2813d660c293b70953e7a3eaed8 Mon Sep 17 00:00:00 2001 From: RohanReddy44 Date: Thu, 24 Sep 2026 14:57:23 +0530 Subject: [PATCH 3/4] fix: validate the completed answer before trusting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `completed` was accepted once `content` was an array. Two malformed shapes then reached the store: an `error` field that was not the contract's object, which made the turn count as failed with Retry silently hidden because `retryable` was undefined; and empty `content` with no `error`, which drew an empty assistant bubble with a Copy row and counted as answered. Both are now a broken stream, reported as upstream_error, the same as a completed with no content array. The rule is §5.2: content is a list, error when present has our shape, and empty content only comes with error. Co-Authored-By: Claude Fable 5.1 --- src/__tests__/api-service.test.ts | 10 ++++++++++ src/lib/api-service.ts | 8 ++++++++ 2 files changed, 18 insertions(+) diff --git a/src/__tests__/api-service.test.ts b/src/__tests__/api-service.test.ts index bc15621..a94c9ce 100644 --- a/src/__tests__/api-service.test.ts +++ b/src/__tests__/api-service.test.ts @@ -129,6 +129,16 @@ describe("sendUserQuery", () => { expect(onStarted).not.toHaveBeenCalled(); }); + it("fails as upstream_error when completed is malformed", async () => { + const stringError = COMPLETED.replace('"sources":[]', '"sources":[],"error":"provider down"'); + fetchOnce(streamResponse(STARTED + stringError)); + expect(await failure(apiService.sendUserQuery(turn))).toMatchObject({ code: "upstream_error" }); + + const nothingToShow = COMPLETED.replace(/"content":\[.*?\],"sources"/, '"content":[],"sources"'); + fetchOnce(streamResponse(STARTED + nothingToShow)); + expect(await failure(apiService.sendUserQuery(turn))).toMatchObject({ code: "upstream_error" }); + }); + it("skips events it does not know", async () => { fetchOnce(streamResponse(STARTED + 'event: heartbeat\ndata: {"sequence":2}\n\n' + COMPLETED)); diff --git a/src/lib/api-service.ts b/src/lib/api-service.ts index 81477aa..1e06a4b 100644 --- a/src/lib/api-service.ts +++ b/src/lib/api-service.ts @@ -173,7 +173,15 @@ const readAnswer = async ( if (typeof data.text === 'string') handlers.onDelta?.(data.text); break; case 'completed': + // Held to §5.2 before the store acts on it: content is a list, error + // when present has our shape, and empty content only comes with error. if (!Array.isArray(data.content)) throw upstreamError('completed carried no content array', traceId); + if (data.error !== undefined && !isErrorBody(data.error)) { + throw upstreamError('completed carried an error without the contract shape', traceId); + } + if (data.content.length === 0 && data.error === undefined) { + throw upstreamError('completed carried neither content nor error', traceId); + } return data as unknown as FinalAnswer; case 'error': if (!isErrorBody(data.error)) throw upstreamError('error event carried no error body', traceId); From 2c79e1c765e8e353558cb0bc2be2d42e6eb89bba Mon Sep 17 00:00:00 2001 From: RohanReddy44 Date: Thu, 24 Sep 2026 14:58:06 +0530 Subject: [PATCH 4/4] refactor: fetch suggestions in one place The turn runner removed the two copies of the streaming and error code from sendText and sendImage, but each still carried its own copy of "fetch suggestions and map them" afterwards. One helper now, so a change to the suggestion shape is made once. Co-Authored-By: Claude Fable 5.1 --- src/hooks/store/chat/index.ts | 39 +++++++++++++++++------------------ 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/hooks/store/chat/index.ts b/src/hooks/store/chat/index.ts index 0363897..9f02d5f 100644 --- a/src/hooks/store/chat/index.ts +++ b/src/hooks/store/chat/index.ts @@ -298,6 +298,23 @@ async function streamTurn( } } +/** After an answered turn, fetch follow-up suggestions, when that surface is on. */ +async function refreshSuggestions( + set: StoreApi["setState"], + sessionId: string, + language: string +): Promise { + if (!FEATURES.suggestions) return; + const suggestions = await apiService.getSuggestions(sessionId, language); + set({ + suggestions: suggestions.map((s) => ({ + id: crypto.randomUUID(), + text: s.question, + label: s.question + })) + }); +} + function makeImageMessage(imageUrl: string, caption?: string): ChatMessage { return { id: crypto.randomUUID(), @@ -482,16 +499,7 @@ export const useChatStore = create((set, get) => ({ } ); - if (answered && FEATURES.suggestions) { - const suggestions = await apiService.getSuggestions(currentSession, language); - set({ - suggestions: suggestions.map((s) => ({ - id: crypto.randomUUID(), - text: s.question, - label: s.question - })) - }); - } + if (answered) await refreshSuggestions(set, currentSession, language); }, sendImage: async (imageFile, language, t) => { @@ -583,16 +591,7 @@ export const useChatStore = create((set, get) => ({ } ); - if (answered && FEATURES.suggestions) { - const suggestions = await apiService.getSuggestions(currentSession, language); - set({ - suggestions: suggestions.map((s) => ({ - id: crypto.randomUUID(), - text: s.question, - label: s.question - })) - }); - } + if (answered) await refreshSuggestions(set, currentSession, language); }, sendAudio: async (blob, sessionId, language) => {