From fbf88fbf8a800e20d5632bc619870b9d1b69e1f9 Mon Sep 17 00:00:00 2001 From: m4 Date: Thu, 30 Jul 2026 11:43:50 +0800 Subject: [PATCH] fix(webui): stop probing the scope registry for unscoped threads in the list Every conversation-list load fanned out getByThread per visible thread; legacy threads without a scope each logged a 404 warning and added a round-trip. A thread whose metadata claims no workspace_scope_id can never pass the scope assertion, so skip the probe and filter it directly. Co-Authored-By: Claude Opus 4.7 --- src/app/api/conversations/route.test.ts | 103 ++++++++++++++++++++++++ src/app/api/conversations/route.ts | 8 +- 2 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 src/app/api/conversations/route.test.ts diff --git a/src/app/api/conversations/route.test.ts b/src/app/api/conversations/route.test.ts new file mode 100644 index 0000000..2e7dc8f --- /dev/null +++ b/src/app/api/conversations/route.test.ts @@ -0,0 +1,103 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { NextRequest } from "next/server"; + +const mocks = vi.hoisted(() => ({ + getActiveDeployment: vi.fn(), + assertCutoverIsIdle: vi.fn(), + assertThreadMatchesScope: vi.fn(), +})); + +vi.mock("server-only", () => ({})); +vi.mock("@/lib/server/activeDeployment", () => ({ + getActiveDeployment: mocks.getActiveDeployment, + assertCutoverIsIdle: mocks.assertCutoverIsIdle, + CutoverInProgressError: class CutoverInProgressError extends Error {}, +})); +vi.mock("@/lib/server/conversationResponse", () => ({ + assertThreadMatchesScope: mocks.assertThreadMatchesScope, + sanitizeConversationThread: (value: T) => value, +})); + +const routes = await import("./route"); + +const SCOPE = { + deployment_id: "deployment-a", + scope_id: "00000000-0000-4000-8000-000000000010", + primary_thread_id: "thread-scoped", + primary_owner_id: "owner-1", + state: "active", + revision: 1, +}; + +const SCOPED_THREAD = { + thread_id: "thread-scoped", + metadata: { + graph_id: "EvoScientist", + workspace_schema_version: 1, + workspace_scope_id: SCOPE.scope_id, + workspace_scope_owner_id: "owner-1", + workspace_scope_revision: 1, + workspace_deployment_id: "deployment-a", + workspace_status: "active", + }, +}; + +const LEGACY_THREAD = { + thread_id: "thread-legacy", + metadata: { graph_id: "EvoScientist" }, +}; + +function scopedDeployment(threads: unknown[]) { + return { + assistantId: "EvoScientist", + isolationMode: "required" as const, + scopeRegistry: { + getByThread: vi.fn().mockResolvedValue(SCOPE), + }, + threadClient: { + threads: { search: vi.fn().mockResolvedValue(threads) }, + }, + }; +} + +function listRequest() { + return new NextRequest("http://localhost/api/conversations", { + method: "GET", + }); +} + +describe("conversations list route", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("does not probe the scope registry for threads that claim no scope", async () => { + const deployment = scopedDeployment([LEGACY_THREAD, SCOPED_THREAD]); + mocks.getActiveDeployment.mockResolvedValue(deployment); + + const response = await routes.GET(listRequest()); + expect(response.status).toBe(200); + + // The legacy thread claims no workspace_scope_id: it can never pass the + // scope assertion, so probing the registry for it is pure 404 log spam. + expect(deployment.scopeRegistry.getByThread).toHaveBeenCalledTimes(1); + expect(deployment.scopeRegistry.getByThread).toHaveBeenCalledWith( + "thread-scoped" + ); + + const body = (await response.json()) as { threads: unknown[] }; + expect(body.threads).toEqual([SCOPED_THREAD]); + }); + + it("skips probing entirely when every visible thread is unscoped", async () => { + const deployment = scopedDeployment([LEGACY_THREAD]); + mocks.getActiveDeployment.mockResolvedValue(deployment); + + const response = await routes.GET(listRequest()); + expect(response.status).toBe(200); + expect(deployment.scopeRegistry.getByThread).not.toHaveBeenCalled(); + + const body = (await response.json()) as { threads: unknown[] }; + expect(body.threads).toEqual([]); + }); +}); diff --git a/src/app/api/conversations/route.ts b/src/app/api/conversations/route.ts index d0ed98a..202d333 100644 --- a/src/app/api/conversations/route.ts +++ b/src/app/api/conversations/route.ts @@ -57,12 +57,16 @@ export async function GET(request: NextRequest) { } const verified = await Promise.all( visible.map(async (thread) => { + const metadata = + (thread.metadata as Record | undefined) ?? {}; + // A thread that claims no scope can never pass the assertion — + // probing the registry for it only spams a 404 warning per legacy + // thread on every list load. + if (typeof metadata.workspace_scope_id !== "string") return null; try { const scope = await deployment.scopeRegistry!.getByThread( thread.thread_id ); - const metadata = - (thread.metadata as Record | undefined) ?? {}; assertThreadMatchesScope(thread.thread_id, metadata, scope); return sanitizeConversationThread(thread); } catch {