From 516a1519485cdb65780b3871d5e1afdad0017d3f Mon Sep 17 00:00:00 2001 From: echobt <154886644+echobt@users.noreply.github.com> Date: Fri, 9 Oct 2026 17:11:20 +0000 Subject: [PATCH] fix: coalesce remote Chat catalog reads for sharing --- docs/providers.md | 6 ++ packages/app/src/screens/chat/AGENTS.md | 11 +++ .../app/src/screens/chat/live-features.tsx | 4 +- packages/app/src/screens/chat/remote-chat.tsx | 4 +- .../app/src/screens/chat/remote-owner.test.ts | 22 +++++- packages/app/src/screens/chat/remote-owner.ts | 5 ++ tests/e2e/chat-share-catalog.spec.ts | 72 +++++++++++++++++++ 7 files changed, 118 insertions(+), 6 deletions(-) create mode 100644 packages/app/src/screens/chat/AGENTS.md create mode 100644 tests/e2e/chat-share-catalog.spec.ts diff --git a/docs/providers.md b/docs/providers.md index a11126cd..0143707d 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -27,6 +27,12 @@ The model list is the public models.dev catalog, `https://models.dev/api.json` Routes: `GET /api/catalog/providers`, `GET /api/catalog/providers/:id/models`, `GET /api/catalog/search`. +Signed-in Chat and its feature screens use the main-owned remote Chat catalog through +`remoteChatModels()` in `screens/chat/remote-owner.ts`. Concurrent reads share one +`api.remoteSessions.models()` request. The Code catalog endpoint does not populate the +Chat core catalog; it must not be substituted under the same shared key. Completed reads +are not reused indefinitely: the next call reloads against the current owner. + ## Supported providers A provider is usable when its catalog `npm` maps to a bundled AI SDK factory diff --git a/packages/app/src/screens/chat/AGENTS.md b/packages/app/src/screens/chat/AGENTS.md new file mode 100644 index 00000000..65c974cd --- /dev/null +++ b/packages/app/src/screens/chat/AGENTS.md @@ -0,0 +1,11 @@ +# Desktop Chat catalog ownership + +Remote Chat and signed-in feature screens call `remoteChatModels()` from +`remote-owner.ts`. It coalesces in-flight `api.remoteSessions.models()` reads +under `chat-feature-models`. Capability discovery must use this same loader. +Do not put `api.code.models()` under that key: it does not populate the Chat +core catalog, so a later Chat create can refuse `catalog_unavailable`. + +Only concurrent requests are shared; settled reads are refreshed on the next +call. Existing route and epoch fences still decide whether a response may render. +No backend cancellation or native integration is implied by this renderer fix. diff --git a/packages/app/src/screens/chat/live-features.tsx b/packages/app/src/screens/chat/live-features.tsx index c851cbcb..9326ef38 100644 --- a/packages/app/src/screens/chat/live-features.tsx +++ b/packages/app/src/screens/chat/live-features.tsx @@ -6,6 +6,7 @@ import { api } from "../../api"; import { useT } from "../../i18n"; import { Icon } from "../../kit/ui"; import { useQuery } from "../../state/live"; +import { remoteChatModels } from "./remote-owner"; import { ImagesTool, ResearchTool, SearchTool, TempTool } from "./tools-live"; type Conversation = { id: string; title: string; last_message_at: string; message_count: number }; @@ -19,7 +20,7 @@ function useFeature(epoch: string, op: ChatFeatureCall["op"] | "", params: Re export function LiveChatFeature({ feature, local }: { feature: string; local: React.ReactNode }) { const connection = useQuery(() => api.connection.get(), []); - const catalog = useQuery(() => api.code.models(), []); + const catalog = useQuery(() => remoteChatModels(), []); if (connection.state !== "ready") return null; if (connection.data.mode === "local" || !connection.data.signedIn) return
{local}
; return ; @@ -100,7 +101,6 @@ function CanvasLive({ epoch }: { epoch: string }) { - function VoiceLive({ epoch }: { epoch: string }) { const t = useT(); const caps = useFeature<{ stt: boolean; tts: boolean; live: boolean }>(epoch, "audio.capabilities"); diff --git a/packages/app/src/screens/chat/remote-chat.tsx b/packages/app/src/screens/chat/remote-chat.tsx index 14258051..4c4052ed 100644 --- a/packages/app/src/screens/chat/remote-chat.tsx +++ b/packages/app/src/screens/chat/remote-chat.tsx @@ -12,7 +12,7 @@ import { navigation, readHash, useNav } from "../../shell/nav"; import { Att, BotRow, Paras } from "./shared"; import type { ComposerLeaveGuard } from "./model-composer"; import { RemoteModelMenu, type Effort } from "./remote-model-menu"; -import { ownsRemoteSession as matches } from "./remote-owner"; +import { ownsRemoteSession as matches, remoteChatModels } from "./remote-owner"; type Draft = { text: string; files: File[]; oneOff: string }; type Submitted = { draft: Draft; attachmentIDs: string[]; id: string; epoch: string }; @@ -193,7 +193,7 @@ function RemoteChat({ id, epoch }: { id?: string; epoch?: string }) { }); void (async () => { try { - const catalog = await api.remoteSessions.models(); + const catalog = await remoteChatModels(); if (!valid()) return; if (binding.current && binding.current !== catalog.epoch) fail(); binding.current = catalog.epoch; diff --git a/packages/app/src/screens/chat/remote-owner.test.ts b/packages/app/src/screens/chat/remote-owner.test.ts index 24b2b644..c08f311b 100644 --- a/packages/app/src/screens/chat/remote-owner.test.ts +++ b/packages/app/src/screens/chat/remote-owner.test.ts @@ -1,6 +1,9 @@ -import { expect, it } from "vitest"; +import { expect, it, vi } from "vitest"; import { RemoteSessionView } from "@cortex/schema"; -import { ownsRemoteSession } from "./remote-owner"; +vi.mock("../../api", () => ({ api: { remoteSessions: { models: vi.fn() } } })); +import { api } from "../../api"; +import { forget } from "../../state/shared"; +import { ownsRemoteSession, remoteChatModels } from "./remote-owner"; const view = (scope: "process" | "account") => RemoteSessionView.parse({ id: "ses_1", source: "remote", epoch: "epoch-1", scope, title: "", modelSlug: "cortex-1-mini", effort: "medium", state: "ready", time: { created: 1, updated: 1 }, @@ -12,3 +15,18 @@ it("owns process and account scoped sessions of the current binding only", () => expect(ownsRemoteSession(view("account"), "ses_2", "epoch-1")).toBe(false); expect(ownsRemoteSession(view("account"), "ses_1", "epoch-2")).toBe(false); }); + +it("coalesces concurrent Chat catalogues but rereads after settlement", async () => { + forget("chat-feature-models"); + const catalog = { epoch: "epoch-1", models: [] }; + let release!: (value: typeof catalog) => void; + const pending = new Promise(resolve => { release = resolve; }); + vi.mocked(api.remoteSessions.models).mockReturnValueOnce(pending).mockResolvedValueOnce(catalog); + const chat = remoteChatModels(), share = remoteChatModels(); + expect(share).toBe(chat); + expect(api.remoteSessions.models).toHaveBeenCalledTimes(1); + release(catalog); + expect(await Promise.all([chat, share])).toEqual([catalog, catalog]); + expect(await remoteChatModels()).toEqual(catalog); + expect(api.remoteSessions.models).toHaveBeenCalledTimes(2); +}); diff --git a/packages/app/src/screens/chat/remote-owner.ts b/packages/app/src/screens/chat/remote-owner.ts index 5ef9f35b..bd665201 100644 --- a/packages/app/src/screens/chat/remote-owner.ts +++ b/packages/app/src/screens/chat/remote-owner.ts @@ -1,4 +1,9 @@ import type { RemoteSessionView } from "@cortex/schema"; +import { api } from "../../api"; +import { shared } from "../../state/shared"; + +// All Chat catalogue readers must load the same endpoint: concurrent core reads retire one another. +export const remoteChatModels = () => shared("chat-feature-models", () => api.remoteSessions.models()); // Either scope belongs to the current binding: "account" when main verified `/v1/me.id`, "process" otherwise. export const ownsRemoteSession = (value: RemoteSessionView, id: string, epoch: string) => diff --git a/tests/e2e/chat-share-catalog.spec.ts b/tests/e2e/chat-share-catalog.spec.ts new file mode 100644 index 00000000..a8a52abc --- /dev/null +++ b/tests/e2e/chat-share-catalog.spec.ts @@ -0,0 +1,72 @@ +import { test, expect } from "@playwright/test"; +import { createServer } from "node:http"; +import type { AddressInfo } from "node:net"; +import { launch } from "./fixtures"; + +const conversation = "cnv_01ARZ3NDEKTSV4RRFFQ69G5FAV"; +const shareID = "11111111-1111-4111-8111-111111111111"; + +for (const theme of ["light", "dark"]) test(`authenticated text-model Share survives catalog loading — ${theme}`, async () => { + const requests: string[] = []; + let shares: unknown[] = []; + const row = { id: shareID, slug: "test", url_path: "/s/test", conversation_id: conversation, created_at: "2026-10-09T00:00:00Z" }; + const server = createServer((req, res) => void (async () => { + for await (const chunk of req) void chunk; + const route = new URL(req.url!, "http://localhost").pathname; + requests.push(`${req.method} ${route}`); + const json = (body: unknown, status = 200) => { res.writeHead(status, { "content-type": "application/json" }); res.end(JSON.stringify(body)); }; + if (route === "/readyz") { res.end("ok"); return; } + if (route === "/v1/instance") return json({ mode: "self_host", version: "test", auth: { mode: "cortex", required: true, providers: ["cortex"] }, registry: { enabled: true } }); + if (route === "/v1/auth/magic-auth") return json({}, 204); + if (route === "/v1/auth/magic-auth/verify") return json({ status: "session", access_token: "test-only-share" }); + if (route === "/v1/registry/models") return json({ items: [{ id: "fixture", name: "Cortex Fixture", configured: true, capabilities: { reasoning: false, image: false, tools: false, context_tokens: 8192, output_tokens: 1024 } }], source: "cache", has_more: false }); + if (route === "/v1/conversations") return json({ items: [{ id: conversation, title: "Saved conversation", model_slug: "fixture", last_message_at: "2026-10-09T00:00:00Z", message_count: 2 }], has_more: false }); + if (route === "/v1/conversation-shares" && req.method === "GET") return json({ items: shares }); + if (route === `/v1/conversations/${conversation}/shares`) { shares = [row]; return json(row); } + if (route === `/v1/conversation-shares/${shareID}` && req.method === "DELETE") { shares = []; return json({}, 204); } + if (route === "/v1/mascots" || route === "/v1/notifications") return json({ items: [], has_more: false }); + if (route === "/v1/bot/inbox") return json({ items: [], working_mascot_ids: [] }); + return json({}, 404); + })().catch(() => { res.statusCode = 500; res.end(); })); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const { app, page } = await launch({ hash: `#/home?theme=${theme}`, locale: "en", env: { CORTEX_CATALOG_URL: "data:application/json,{}" } }); + try { + await page.emulateMedia({ reducedMotion: "reduce" }); + await page.getByTestId("composer-input").waitFor(); + await page.evaluate(async url => { + const call = async (route: string, method = "GET", body?: unknown) => { + const response = await (window as unknown as { __bridgeFetch: typeof fetch }).__bridgeFetch(`cortex://local${route}`, { method, headers: { "content-type": "application/json" }, body: body === undefined ? undefined : JSON.stringify(body) }); + if (!response.ok) throw new Error(`${route}: ${response.status}`); + return response.json(); + }; + await call("/api/connection", "PUT", { mode: "selfhost", url, signedIn: false }); + const { owner } = await call("/api/connection/auth"); + const pending = await call("/api/connection/auth", "POST", { action: "email", owner, email: "share@example.test" }); + await call("/api/connection/auth", "POST", { action: "code", owner: pending.owner, code: "123456" }); + }, `http://127.0.0.1:${(server.address() as AddressInfo).port}`); + await page.reload(); + await expect(page.getByTestId("remote-model-trigger")).toBeEnabled(); + await page.getByTestId("composer-input").fill("Retained draft"); + await expect(page.getByTestId("composer-send")).toBeEnabled(); + await expect(page.locator(".chat-err")).toHaveCount(0); + await page.evaluate(() => history.pushState(null, "", `#/share?theme=${document.documentElement.dataset.theme}`)); + await expect(page.getByTestId("screen-share")).toHaveAttribute("data-state", "ready"); + await expect(page.getByTestId("chat-share-create")).toBeEnabled(); + await page.getByTestId("chat-share-create").click(); + await expect(page.getByTestId("chat-share-row")).toContainText("/s/test"); + for (const width of [1440, 960]) { + await app.evaluate(({ BrowserWindow }, width) => BrowserWindow.getAllWindows()[0]!.setSize(width, width === 960 ? 640 : 900), width); + const screenshot = test.info().outputPath(`share-${theme}-${width}.png`); + await page.screenshot({ path: screenshot }); + await test.info().attach(`share-${width}`, { path: screenshot, contentType: "image/png" }); + } + await page.getByTestId("chat-share-row").getByRole("button").click(); + await expect(page.getByTestId("chat-share-row")).toHaveCount(0); + expect(requests).toContain(`POST /v1/conversations/${conversation}/shares`); + expect(requests).toContain(`DELETE /v1/conversation-shares/${shareID}`); + await test.info().attach("requests", { body: JSON.stringify(requests, null, 2), contentType: "application/json" }); + } finally { + await app.close(); server.closeAllConnections(); + await new Promise(resolve => server.close(() => resolve())); + } +});