From 9616fe588dfd600f5e344bd766ee0aba086912c4 Mon Sep 17 00:00:00 2001 From: murashit Date: Fri, 10 Jul 2026 12:25:54 +0900 Subject: [PATCH] Paginate model and MCP server inventories --- src/app-server/services/catalog.ts | 23 +++++++-- src/app-server/services/tool-inventory.ts | 23 ++++++--- tests/app-server/catalog.test.ts | 31 +++++++++++- tests/app-server/tool-inventory.test.ts | 49 ++++++++++++++++++- .../chat/app-server/transports.test.ts | 3 +- 5 files changed, 114 insertions(+), 15 deletions(-) diff --git a/src/app-server/services/catalog.ts b/src/app-server/services/catalog.ts index 24ffa3fc..b624fd17 100644 --- a/src/app-server/services/catalog.ts +++ b/src/app-server/services/catalog.ts @@ -22,11 +22,24 @@ export interface ModelMetadataClient { } export async function listModelMetadata(client: ModelMetadataClient, options: { includeHidden?: boolean } = {}): Promise { - const response = await client.request("model/list", { - includeHidden: options.includeHidden ?? false, - limit: 100, - }); - return modelMetadataFromCatalogModels(response.data); + const models: ClientResponseByMethod["model/list"]["data"] = []; + const seenCursors = new Set(); + let cursor: string | null = null; + + for (;;) { + const response: ClientResponseByMethod["model/list"] = await client.request("model/list", { + includeHidden: options.includeHidden ?? false, + cursor, + limit: 100, + }); + models.push(...response.data); + cursor = response.nextCursor ?? null; + if (!cursor) break; + if (seenCursors.has(cursor)) throw new Error("Codex app-server returned a repeated model list cursor."); + seenCursors.add(cursor); + } + + return modelMetadataFromCatalogModels(models); } export async function listPermissionProfiles(client: AppServerRequestClient, cwd: string): Promise { diff --git a/src/app-server/services/tool-inventory.ts b/src/app-server/services/tool-inventory.ts index b84f9106..330d931c 100644 --- a/src/app-server/services/tool-inventory.ts +++ b/src/app-server/services/tool-inventory.ts @@ -11,6 +11,7 @@ import { mcpServerStatusSummariesFromStatuses, } from "../../domain/server/mcp-status"; import type { ToolInventoryMarketplaceError, ToolInventoryPlugin, ToolInventorySnapshot } from "../../domain/server/tool-inventory"; +import type { ClientResponseByMethod } from "../connection/client"; import { toolInventoryPluginsFromInstalledResponse } from "../protocol/tool-inventory"; import { listSkillCatalog } from "./catalog"; import type { AppServerRequestClient } from "./request-client"; @@ -109,12 +110,22 @@ async function readMcpServers( checkedAt: number, ): Promise<{ items: McpServerStatusSummary[] | null; error: string | null; probe: DiagnosticProbeResult }> { try { - const response = await client.request("mcpServerStatus/list", { - detail: "toolsAndAuthOnly", - limit: 100, - ...(threadId ? { threadId } : {}), - }); - const servers = mcpServerStatusSummariesFromStatuses(response.data); + const servers: McpServerStatusSummary[] = []; + const seenCursors = new Set(); + let cursor: string | null = null; + for (;;) { + const response: ClientResponseByMethod["mcpServerStatus/list"] = await client.request("mcpServerStatus/list", { + detail: "toolsAndAuthOnly", + cursor, + limit: 100, + ...(threadId ? { threadId } : {}), + }); + servers.push(...mcpServerStatusSummariesFromStatuses(response.data)); + cursor = response.nextCursor ?? null; + if (!cursor) break; + if (seenCursors.has(cursor)) throw new Error("Codex app-server returned a repeated MCP server status list cursor."); + seenCursors.add(cursor); + } return { items: servers, error: null, diff --git a/tests/app-server/catalog.test.ts b/tests/app-server/catalog.test.ts index 98e4b11c..413c0c60 100644 --- a/tests/app-server/catalog.test.ts +++ b/tests/app-server/catalog.test.ts @@ -5,7 +5,13 @@ import { modelMetadataFromCatalogModels, skillMetadataFromCatalogSkills, } from "../../src/app-server/protocol/catalog"; -import { listHookCatalog, listSkillCatalog, setHookItemEnabled, trustHookItem } from "../../src/app-server/services/catalog"; +import { + listHookCatalog, + listModelMetadata, + listSkillCatalog, + setHookItemEnabled, + trustHookItem, +} from "../../src/app-server/services/catalog"; import type { AppServerRequestClient } from "../../src/app-server/services/request-client"; import type { HookMetadata } from "../../src/generated/app-server/v2/HookMetadata"; import type { Model } from "../../src/generated/app-server/v2/Model"; @@ -84,6 +90,26 @@ describe("app-server catalog mappers", () => { }); describe("app-server catalog adapters", () => { + it("loads every model page", async () => { + const request = vi + .fn() + .mockResolvedValueOnce({ data: [modelFixture({ id: "first-id", model: "first" })], nextCursor: "next" }) + .mockResolvedValueOnce({ data: [modelFixture({ id: "second-id", model: "second" })], nextCursor: null }); + const client = { request } as unknown as AppServerRequestClient; + + await expect(listModelMetadata(client)).resolves.toMatchObject([{ model: "first" }, { model: "second" }]); + expect(request).toHaveBeenNthCalledWith(1, "model/list", { includeHidden: false, cursor: null, limit: 100 }); + expect(request).toHaveBeenNthCalledWith(2, "model/list", { includeHidden: false, cursor: "next", limit: 100 }); + }); + + it("rejects repeated model cursors", async () => { + const client = { + request: vi.fn().mockResolvedValue({ data: [modelFixture()], nextCursor: "repeat" }), + } as unknown as AppServerRequestClient; + + await expect(listModelMetadata(client)).rejects.toThrow("repeated model list cursor"); + }); + it("returns enabled skill options while preserving total app-server skill count", async () => { const client = { request: vi.fn(async () => ({ @@ -220,7 +246,7 @@ describe("app-server catalog adapters", () => { }); }); -function modelFixture(): Model { +function modelFixture(overrides: Partial = {}): Model { return { id: "gpt-5.5-id", model: "gpt-5.5", @@ -241,6 +267,7 @@ function modelFixture(): Model { serviceTiers: [{ id: "priority", name: "Fast", description: "Fast tier" }], defaultServiceTier: "priority", isDefault: true, + ...overrides, }; } diff --git a/tests/app-server/tool-inventory.test.ts b/tests/app-server/tool-inventory.test.ts index de4d9a7d..c5a5d2e8 100644 --- a/tests/app-server/tool-inventory.test.ts +++ b/tests/app-server/tool-inventory.test.ts @@ -4,6 +4,41 @@ import type { AppServerRequestClient } from "../../src/app-server/services/reque import { readToolInventory } from "../../src/app-server/services/tool-inventory"; describe("tool inventory", () => { + it("loads every MCP server status page", async () => { + const listMcpServers = vi + .fn() + .mockResolvedValueOnce({ data: [mcpServerStatus("first")], nextCursor: "next" }) + .mockResolvedValueOnce({ data: [mcpServerStatus("second")], nextCursor: null }); + const client = toolInventoryClient({ "mcpServerStatus/list": listMcpServers }); + + const result = await readToolInventory(client, "/vault", { threadId: "thread" }); + + expect(result.inventory.mcpServers?.map((server) => server.name)).toEqual(["first", "second"]); + expect(listMcpServers).toHaveBeenNthCalledWith(1, { + detail: "toolsAndAuthOnly", + cursor: null, + limit: 100, + threadId: "thread", + }); + expect(listMcpServers).toHaveBeenNthCalledWith(2, { + detail: "toolsAndAuthOnly", + cursor: "next", + limit: 100, + threadId: "thread", + }); + }); + + it("reports repeated MCP server status cursors", async () => { + const client = toolInventoryClient({ + "mcpServerStatus/list": vi.fn().mockResolvedValue({ data: [mcpServerStatus("github")], nextCursor: "repeat" }), + }); + + const result = await readToolInventory(client, "/vault"); + + expect(result.inventory.mcpServers).toBeNull(); + expect(result.inventory.mcpError).toContain("repeated MCP server status list cursor"); + }); + it("reads installed plugins without loading plugin details", async () => { const readPlugin = vi.fn(); const client = toolInventoryClient({ @@ -91,6 +126,7 @@ function toolInventoryClient( "plugin/read": vi.fn().mockResolvedValue({ plugin: { skills: [], hooks: [], apps: [], appTemplates: [], mcpServers: [] }, }), + "mcpServerStatus/list": vi.fn().mockResolvedValue({ data: [], nextCursor: null }), ...overrides, }; const request = vi.fn(async (method: string, params: unknown) => { @@ -100,7 +136,7 @@ function toolInventoryClient( case "plugin/read": return (handlers["plugin/read"] as unknown as (params: unknown) => Promise)(params); case "mcpServerStatus/list": - return { data: [] }; + return (handlers["mcpServerStatus/list"] as unknown as (params: unknown) => Promise)(params); case "skills/list": return { data: [{ cwd: "/vault", skills: [] }] }; default: @@ -112,6 +148,17 @@ function toolInventoryClient( } as unknown as AppServerRequestClient & { request: ReturnType }; } +function mcpServerStatus(name: string) { + return { + name, + serverInfo: null, + tools: {}, + resources: [], + resourceTemplates: [], + authStatus: "oAuth", + }; +} + function installedPlugin(name: string): { id: string; name: string; diff --git a/tests/features/chat/app-server/transports.test.ts b/tests/features/chat/app-server/transports.test.ts index ec8c376e..959f060b 100644 --- a/tests/features/chat/app-server/transports.test.ts +++ b/tests/features/chat/app-server/transports.test.ts @@ -390,10 +390,11 @@ describe("chat app-server transports", () => { }); expect(snapshot?.resourceProbes.map((probe) => probe.id)).toEqual(["models", "rateLimits"]); - expect(request).toHaveBeenCalledWith("model/list", { includeHidden: false, limit: 100 }); + expect(request).toHaveBeenCalledWith("model/list", { includeHidden: false, cursor: null, limit: 100 }); expect(request).toHaveBeenCalledWith("account/rateLimits/read", undefined); expect(request).toHaveBeenCalledWith("mcpServerStatus/list", { detail: "toolsAndAuthOnly", + cursor: null, limit: 100, threadId: "thread", });