diff --git a/src/app-server/protocol/tool-inventory.ts b/src/app-server/protocol/tool-inventory.ts index eb2a4b70..ac56d8d5 100644 --- a/src/app-server/protocol/tool-inventory.ts +++ b/src/app-server/protocol/tool-inventory.ts @@ -63,8 +63,6 @@ function toolInventoryPluginFromSummary( enabled: plugin.enabled, availability: plugin.availability, source: pluginSourceLabel(plugin.source), - details: null, - detailsError: null, }; } diff --git a/src/app-server/services/tool-inventory.ts b/src/app-server/services/tool-inventory.ts index 755f1239..800b1284 100644 --- a/src/app-server/services/tool-inventory.ts +++ b/src/app-server/services/tool-inventory.ts @@ -8,13 +8,10 @@ import { mcpServerStatusSummariesFromStatuses, } from "../../domain/server/diagnostics"; import type { ToolInventoryMarketplaceError, ToolInventoryPlugin, ToolInventorySnapshot } from "../../domain/server/tool-inventory"; -import type { ClientRequestParams } from "../connection/rpc-messages"; import { toolInventoryPluginsFromInstalledResponse } from "../protocol/tool-inventory"; import { listSkillCatalog } from "./catalog"; import type { AppServerRequestClient } from "./request-client"; -const PLUGIN_DETAILS_CONCURRENCY = 4; - export interface ReadToolInventoryOptions { readonly threadId?: string | null; readonly mcpDiagnostics?: readonly McpServerDiagnostic[]; @@ -87,9 +84,8 @@ async function readPlugins( try { const response = await client.request("plugin/installed", { cwds: [cwd] }); const { plugins, marketplaceErrors } = toolInventoryPluginsFromInstalledResponse(response); - const pluginsWithDetails = await mapLimit(plugins, PLUGIN_DETAILS_CONCURRENCY, (plugin) => readPluginDetails(client, plugin)); return { - items: pluginsWithDetails, + items: plugins, marketplaceErrors, error: null, probe: diagnosticProbeOk("plugin/installed", `${String(plugins.length)} plugins`, checkedAt), @@ -104,60 +100,6 @@ async function readPlugins( } } -async function mapLimit(items: readonly T[], concurrency: number, fn: (item: T, index: number) => Promise): Promise { - const results = new Array(items.length); - let nextIndex = 0; - - async function worker(): Promise { - for (;;) { - const index = nextIndex; - nextIndex += 1; - if (index >= items.length) return; - results[index] = await fn(items[index] as T, index); - } - } - - const workers = Array.from({ length: Math.min(Math.max(concurrency, 1), items.length) }, () => worker()); - await Promise.all(workers); - return results; -} - -async function readPluginDetails(client: AppServerRequestClient, plugin: ToolInventoryPlugin): Promise { - if (!isUsablePlugin(plugin)) return plugin; - try { - const response = await client.request("plugin/read", pluginReadParams(plugin)); - return { - ...plugin, - details: { - skillCount: response.plugin.skills.length, - hookCount: response.plugin.hooks.length, - appCount: response.plugin.apps.length, - mcpServerCount: response.plugin.mcpServers.length, - }, - detailsError: null, - }; - } catch (error) { - return { ...plugin, details: null, detailsError: shortErrorMessage(error) }; - } -} - -function pluginReadParams(plugin: ToolInventoryPlugin): ClientRequestParams<"plugin/read"> { - if (plugin.marketplacePath) { - return { - pluginName: plugin.name, - marketplacePath: plugin.marketplacePath, - }; - } - return { - pluginName: plugin.name, - remoteMarketplaceName: plugin.marketplaceName, - }; -} - -function isUsablePlugin(plugin: ToolInventoryPlugin): boolean { - return plugin.enabled && plugin.installed; -} - async function readMcpServers( client: AppServerRequestClient, threadId: string | null, diff --git a/src/domain/server/diagnostics.ts b/src/domain/server/diagnostics.ts index 5b1f672a..3056ac97 100644 --- a/src/domain/server/diagnostics.ts +++ b/src/domain/server/diagnostics.ts @@ -45,6 +45,7 @@ export interface McpServerStatusSummary { readonly toolCount: number; readonly resourceCount: number; readonly resourceTemplateCount: number; + readonly codexAppIds?: readonly string[]; } export interface Diagnostics { @@ -153,6 +154,7 @@ function mcpServerStatusSummaryFromStatus(server: McpServerStatus): McpServerSta toolCount: Object.keys(server.tools).length, resourceCount: server.resources.length, resourceTemplateCount: server.resourceTemplates.length, + codexAppIds: server.name === "codex_apps" ? codexAppIdsFromTools(server.tools) : [], }; } @@ -166,6 +168,23 @@ function shortErrorMessage(error: unknown, maxLength = 160): string { return compact.length > maxLength ? `${compact.slice(0, maxLength - 3)}...` : compact; } +function codexAppIdsFromTools(tools: Readonly>): string[] { + const appIds = new Set(); + for (const [toolKey, tool] of Object.entries(tools)) { + const toolName = toolNameFromStatusTool(tool) ?? toolKey; + const prefixSeparator = toolName.indexOf("."); + if (prefixSeparator <= 0) continue; + appIds.add(toolName.slice(0, prefixSeparator)); + } + return [...appIds].sort((left, right) => left.localeCompare(right)); +} + +function toolNameFromStatusTool(tool: unknown): string | null { + if (!tool || typeof tool !== "object") return null; + const name = (tool as { readonly name?: unknown }).name; + return typeof name === "string" && name.length > 0 ? name : null; +} + function mcpServerDiagnosticFromStatus(server: McpServerStatusSummary): McpServerDiagnostic { return { name: server.name, diff --git a/src/domain/server/tool-inventory.ts b/src/domain/server/tool-inventory.ts index 5ee27c6a..74a5810f 100644 --- a/src/domain/server/tool-inventory.ts +++ b/src/domain/server/tool-inventory.ts @@ -12,13 +12,6 @@ export interface ToolInventoryPlugin { readonly enabled: boolean; readonly availability: string; readonly source: string; - readonly details: { - readonly skillCount: number; - readonly hookCount: number; - readonly appCount: number; - readonly mcpServerCount: number; - } | null; - readonly detailsError: string | null; } export interface ToolInventoryMarketplaceError { @@ -41,12 +34,14 @@ export interface ToolInventorySnapshot { export function cloneToolInventorySnapshot(snapshot: ToolInventorySnapshot): ToolInventorySnapshot { return { ...snapshot, - plugins: snapshot.plugins - ? snapshot.plugins.map((plugin) => ({ ...plugin, details: plugin.details ? { ...plugin.details } : null })) - : null, + plugins: snapshot.plugins ? snapshot.plugins.map((plugin) => ({ ...plugin })) : null, pluginMarketplaceErrors: snapshot.pluginMarketplaceErrors.map((error) => ({ ...error })), - mcpServers: snapshot.mcpServers ? snapshot.mcpServers.map((server) => ({ ...server })) : null, + mcpServers: snapshot.mcpServers ? snapshot.mcpServers.map(cloneMcpServerStatusSummary) : null, mcpDiagnostics: snapshot.mcpDiagnostics.map((diagnostic) => ({ ...diagnostic })), skills: snapshot.skills ? snapshot.skills.map((skill) => ({ ...skill })) : null, }; } + +function cloneMcpServerStatusSummary(server: McpServerStatusSummary): McpServerStatusSummary { + return server.codexAppIds ? { ...server, codexAppIds: [...server.codexAppIds] } : { ...server }; +} diff --git a/src/features/chat/application/composer/slash-commands.ts b/src/features/chat/application/composer/slash-commands.ts index 30087cc8..4e17811a 100644 --- a/src/features/chat/application/composer/slash-commands.ts +++ b/src/features/chat/application/composer/slash-commands.ts @@ -141,7 +141,7 @@ export const SLASH_COMMANDS = [ usage: "/tools", argsKind: "none", surface: "diagnostic", - detail: "Show Codex plugins, MCP servers, and skills reported by App Server.", + detail: "Show Codex plugins, tool providers, and skills reported by App Server.", }, { command: "/model", diff --git a/src/features/chat/application/connection/connection-controller.ts b/src/features/chat/application/connection/connection-controller.ts index a0136a55..59dc3314 100644 --- a/src/features/chat/application/connection/connection-controller.ts +++ b/src/features/chat/application/connection/connection-controller.ts @@ -29,6 +29,8 @@ export interface ChatConnectionControllerHost { diagnostics: ChatConnectionDiagnosticsActions; invalidateThreadWork: () => void; refreshSharedThreads: () => Promise; + scheduleDeferredDiagnostics: () => void; + clearDeferredDiagnostics: () => void; refreshTabHeader: () => void; resetThreadTurnPresence: (hadTurns: boolean) => void; setStatus: (statusText: string, phase?: ChatConnectionPhase) => void; @@ -112,8 +114,10 @@ async function refreshDiagnostics( host: ChatConnectionControllerHost, controller: Pick, ): Promise { + host.clearDeferredDiagnostics(); await controller.ensureConnected(); if (!host.connection.isConnected()) return; + host.clearDeferredDiagnostics(); await host.metadata.refreshAppServerMetadata(); await host.diagnostics.refreshServerDiagnostics({ appServerMetadataSnapshot: true }); } @@ -141,6 +145,7 @@ async function initializeConnection(host: ChatConnectionControllerHost, connecti if (host.connectionWork.isStale(connection)) return; await host.refreshSharedThreads(); if (host.connectionWork.isStale(connection)) return; + host.scheduleDeferredDiagnostics(); host.refreshTabHeader(); host.setStatus(STATUS_CONNECTED, { kind: "connected" }); } catch (error) { diff --git a/src/features/chat/application/connection/reconnect-actions.ts b/src/features/chat/application/connection/reconnect-actions.ts index f8ab67a5..b5119d99 100644 --- a/src/features/chat/application/connection/reconnect-actions.ts +++ b/src/features/chat/application/connection/reconnect-actions.ts @@ -7,6 +7,7 @@ export interface ChatReconnectActionsHost { stateStore: ChatStateStore; invalidateConnectionWork: () => void; invalidateThreadWork: () => void; + clearDeferredDiagnostics: () => void; resetConnection: () => void; setStatus: (statusText: string, phase?: ChatConnectionPhase) => void; ensureConnected: () => Promise; @@ -19,6 +20,7 @@ export async function reconnectPanel(host: ChatReconnectActionsHost): Promise [server.name, server])); @@ -79,6 +80,8 @@ function mcpToolProviderRows(inventory: ToolInventorySnapshot): DiagnosticRow[] } function mcpToolProviderStatusRow(server: McpServerStatusSummary, diagnostic: McpServerDiagnostic | undefined): DiagnosticRow { + if (server.name === "codex_apps") return codexAppsToolProviderRow(server, diagnostic); + const startup = diagnostic?.startupStatus && diagnostic.startupStatus !== "unknown" ? diagnostic.startupStatus : "available"; const parts = [ "MCP server", @@ -96,6 +99,14 @@ function mcpToolProviderStatusRow(server: McpServerStatusSummary, diagnostic: Mc }; } +function codexAppsToolProviderRow(server: McpServerStatusSummary, diagnostic: McpServerDiagnostic | undefined): DiagnosticRow { + return { + label: server.name, + value: server.codexAppIds && server.codexAppIds.length > 0 ? listSummary(server.codexAppIds) : "(none)", + level: mcpToolProviderLevel(diagnostic, server.authStatus), + }; +} + function mcpToolProviderDiagnosticRow(name: string, diagnostic: McpServerDiagnostic | undefined): DiagnosticRow { const startup = diagnostic?.startupStatus ?? "unknown"; const auth = diagnostic?.authStatus ? `auth ${diagnostic.authStatus}` : "auth unknown"; @@ -142,21 +153,7 @@ function skillRows(inventory: ToolInventorySnapshot): DiagnosticRow[] { } function pluginBundleSummary(plugin: ToolInventoryPlugin): string { - if (plugin.detailsError) return "details unavailable"; - if (!plugin.details) return "details not loaded"; - - const parts = [ - countPart(plugin.details.skillCount, "skill"), - countPart(plugin.details.hookCount, "hook"), - countPart(plugin.details.appCount, "app"), - countPart(plugin.details.mcpServerCount, "MCP server"), - ].filter((part): part is string => part !== null); - return parts.length > 0 ? parts.join(", ") : "no listed items"; -} - -function countPart(count: number, singular: string): string | null { - if (count === 0) return null; - return `${String(count)} ${singular}${count === 1 ? "" : "s"}`; + return plugin.localVersion ? `version ${plugin.localVersion}` : "version unknown"; } function countLabel(count: number, singular: string): string { diff --git a/src/features/chat/host/connection-bundle.ts b/src/features/chat/host/connection-bundle.ts index c994e64d..d3999be9 100644 --- a/src/features/chat/host/connection-bundle.ts +++ b/src/features/chat/host/connection-bundle.ts @@ -67,6 +67,22 @@ export interface ChatPanelConnectionBundle { refreshSharedThreads: () => Promise; } +export interface DeferredDiagnosticsRefreshHost { + scheduleDiagnostics(callback: () => void): void; + isConnected(): boolean; + refreshServerDiagnostics(options: { appServerMetadataSnapshot: true }): Promise; + addSystemMessage(text: string): void; +} + +export function scheduleDeferredDiagnosticsRefresh(host: DeferredDiagnosticsRefreshHost): void { + host.scheduleDiagnostics(() => { + if (!host.isConnected()) return; + void host.refreshServerDiagnostics({ appServerMetadataSnapshot: true }).catch((error: unknown) => { + host.addSystemMessage(error instanceof Error ? error.message : String(error)); + }); + }); +} + function respondToCurrentServerRequest(currentClient: CurrentAppServerClient, requestId: RespondRequestId, result: unknown): boolean { try { const client = currentClient(); @@ -205,6 +221,21 @@ export function createConnectionBundle( refreshServerDiagnostics: (options) => serverDiagnostics.refreshServerDiagnostics(options), }, refreshSharedThreads, + scheduleDeferredDiagnostics: () => { + scheduleDeferredDiagnosticsRefresh({ + scheduleDiagnostics: (callback) => { + host.deferredTasks.scheduleDiagnostics(callback); + }, + isConnected: () => connection.isConnected(), + refreshServerDiagnostics: (options) => serverDiagnostics.refreshServerDiagnostics(options), + addSystemMessage: (text) => { + status.addSystemMessage(text); + }, + }); + }, + clearDeferredDiagnostics: () => { + host.deferredTasks.clearDiagnostics(); + }, refreshTabHeader: () => { host.refreshTabHeader(); }, diff --git a/src/features/chat/host/deferred-work.ts b/src/features/chat/host/deferred-work.ts index e9f731f9..74d8cde0 100644 --- a/src/features/chat/host/deferred-work.ts +++ b/src/features/chat/host/deferred-work.ts @@ -1,15 +1,26 @@ import { DeferredTask, type DeferredTaskWindow } from "../../../shared/lifecycle/deferred-task"; export interface ChatViewDeferredTasks { + scheduleDiagnostics(callback: () => void): void; + clearDiagnostics(): void; scheduleAppServerWarmup(callback: () => void): void; clearAppServerWarmup(): void; clearAll(): void; } export function createChatViewDeferredTasks(getWindow: () => DeferredTaskWindow): ChatViewDeferredTasks { + const diagnosticsTask = new DeferredTask(getWindow, 1_000); const appServerWarmupTask = new DeferredTask(getWindow, 0); return { + scheduleDiagnostics(callback): void { + diagnosticsTask.schedule(callback); + }, + + clearDiagnostics(): void { + diagnosticsTask.clear(); + }, + scheduleAppServerWarmup(callback): void { appServerWarmupTask.schedule(callback); }, @@ -20,6 +31,7 @@ export function createChatViewDeferredTasks(getWindow: () => DeferredTaskWindow) clearAll(): void { appServerWarmupTask.clear(); + diagnosticsTask.clear(); }, }; } diff --git a/src/features/chat/host/turn-bundle.ts b/src/features/chat/host/turn-bundle.ts index 28c372b8..ec34d483 100644 --- a/src/features/chat/host/turn-bundle.ts +++ b/src/features/chat/host/turn-bundle.ts @@ -113,6 +113,9 @@ export function createTurnBundle(host: ChatPanelTurnHost, input: ChatPanelTurnIn invalidateThreadWork: () => { invalidateThreadWork(); }, + clearDeferredDiagnostics: () => { + host.deferredTasks.clearDiagnostics(); + }, resetConnection: () => { connection.resetConnection(); }, @@ -151,6 +154,9 @@ export function createTurnBundle(host: ChatPanelTurnHost, input: ChatPanelTurnIn effortStatusLines: runtimeProjection.effortStatusLines, statusSummaryLines: runtimeProjection.statusSummaryLines, toolInventoryDetails: async () => { + if (host.stateStore.getState().connection.serverDiagnostics.toolInventory) { + return runtimeProjection.toolInventoryDetails(); + } await refreshDiagnostics(); return runtimeProjection.toolInventoryDetails(); }, diff --git a/tests/app-server/app-server-diagnostics.test.ts b/tests/app-server/app-server-diagnostics.test.ts index b84667c1..40a70430 100644 --- a/tests/app-server/app-server-diagnostics.test.ts +++ b/tests/app-server/app-server-diagnostics.test.ts @@ -4,6 +4,7 @@ import { createServerDiagnostics, diagnosticProbeError, diagnosticProbeOk, + mcpServerStatusSummariesFromStatuses, serverIdentity, serverPlatform, upsertMcpServerDiagnostic, @@ -99,4 +100,41 @@ describe("app-server diagnostics", () => { message: null, }); }); + + it("derives codex app ids from MCP tool prefixes", () => { + const summaries = mcpServerStatusSummariesFromStatuses([ + { + name: "codex_apps", + authStatus: "oAuth", + tools: { + "github.fetch_issue": { name: "github.fetch_issue" }, + "google_drive.get_document_text": { name: "google_drive.get_document_text" }, + "apple_music.get-track-details-batch": {}, + malformed_tool: { name: "malformed_tool" }, + }, + resources: [], + resourceTemplates: [], + }, + { + name: "github", + authStatus: "oAuth", + tools: { + "github.fetch_issue": { name: "github.fetch_issue" }, + }, + resources: [], + resourceTemplates: [], + }, + ]); + + expect(summaries).toMatchObject([ + { + name: "codex_apps", + codexAppIds: ["apple_music", "github", "google_drive"], + }, + { + name: "github", + codexAppIds: [], + }, + ]); + }); }); diff --git a/tests/app-server/tool-inventory.test.ts b/tests/app-server/tool-inventory.test.ts index dffe0f0b..74ce3ff9 100644 --- a/tests/app-server/tool-inventory.test.ts +++ b/tests/app-server/tool-inventory.test.ts @@ -2,19 +2,10 @@ import { describe, expect, it, vi } from "vitest"; import type { AppServerRequestClient } from "../../src/app-server/services/request-client"; import { readToolInventory } from "../../src/app-server/services/tool-inventory"; -import type { PluginReadParams } from "../../src/generated/app-server/v2/PluginReadParams"; describe("tool inventory", () => { - it("reads plugin details with exactly one marketplace locator", async () => { - const readPlugin = vi.fn(async (params: PluginReadParams) => ({ - plugin: { - skills: params.pluginName === "local-plugin" ? [{ name: "local-skill" }] : [], - hooks: [], - apps: [], - appTemplates: [], - mcpServers: params.pluginName === "remote-plugin" ? ["remote-server"] : [], - }, - })); + it("reads installed plugins without loading plugin details", async () => { + const readPlugin = vi.fn(); const client = toolInventoryClient({ "plugin/installed": vi.fn().mockResolvedValue({ marketplaces: [ @@ -58,21 +49,8 @@ describe("tool inventory", () => { const result = await readToolInventory(client, "/vault"); - expect(readPlugin).toHaveBeenCalledWith({ - pluginName: "local-plugin", - marketplacePath: "/marketplaces/local.json", - }); - expect(readPlugin).toHaveBeenCalledWith({ - pluginName: "remote-plugin", - remoteMarketplaceName: "remote-marketplace", - }); - expect(readPlugin).not.toHaveBeenCalledWith( - expect.objectContaining({ marketplacePath: expect.any(String), remoteMarketplaceName: expect.any(String) }), - ); - expect(result.inventory.plugins?.map((plugin) => [plugin.name, plugin.details, plugin.detailsError])).toEqual([ - ["local-plugin", { skillCount: 1, hookCount: 0, appCount: 0, mcpServerCount: 0 }, null], - ["remote-plugin", { skillCount: 0, hookCount: 0, appCount: 0, mcpServerCount: 1 }, null], - ]); + expect(readPlugin).not.toHaveBeenCalled(); + expect(result.inventory.plugins?.map((plugin) => plugin.name)).toEqual(["local-plugin", "remote-plugin"]); }); it("skips app catalog loading during diagnostics", async () => { @@ -84,25 +62,9 @@ describe("tool inventory", () => { expect(result.probes.some((probe) => probe.method === "app/list")).toBe(false); }); - it("limits plugin detail reads while preserving plugin order", async () => { - let activeReads = 0; - let maxActiveReads = 0; + it("preserves plugin order from installed plugin summaries", async () => { const plugins = Array.from({ length: 10 }, (_, index) => installedPlugin(`plugin-${String(index)}`)); - const readPlugin = vi.fn(async (params: PluginReadParams) => { - activeReads += 1; - maxActiveReads = Math.max(maxActiveReads, activeReads); - await Promise.resolve(); - activeReads -= 1; - return { - plugin: { - skills: [{ name: `${params.pluginName}-skill` }], - hooks: [], - apps: [], - appTemplates: [], - mcpServers: [], - }, - }; - }); + const readPlugin = vi.fn(); const client = toolInventoryClient({ "plugin/installed": vi.fn().mockResolvedValue({ marketplaces: [{ name: "local-marketplace", path: "/marketplaces/local.json", plugins }], @@ -113,9 +75,8 @@ describe("tool inventory", () => { const result = await readToolInventory(client, "/vault"); - expect(maxActiveReads).toBe(4); + expect(readPlugin).not.toHaveBeenCalled(); expect(result.inventory.plugins?.map((plugin) => plugin.name)).toEqual(plugins.map((plugin) => plugin.name)); - expect(result.inventory.plugins?.every((plugin) => plugin.details?.skillCount === 1)).toBe(true); }); }); diff --git a/tests/features/chat/app-server/actions/actions.test.ts b/tests/features/chat/app-server/actions/actions.test.ts index 6842407f..b73c5671 100644 --- a/tests/features/chat/app-server/actions/actions.test.ts +++ b/tests/features/chat/app-server/actions/actions.test.ts @@ -618,10 +618,12 @@ describe("chat app-server actions", () => { await diagnostics.refreshServerDiagnostics({ appServerMetadataSnapshot: true }); const sections = toolInventoryDiagnosticSections(stateStore.getState().connection.serverDiagnostics); - const mcpRows = sections.find((section) => section.title === "MCP servers")?.rows ?? []; + const toolProviderRows = sections.find((section) => section.title === "Tool providers")?.rows ?? []; - expect(sections.map((section) => section.title)).toEqual(["Plugins", "MCP servers", "Skills"]); - expect(mcpRows.map((row) => `${row.label}: ${row.value}`)).toEqual(["github: MCP server, ready, auth oAuth, 1 tool, 0 resources"]); + expect(sections.map((section) => section.title)).toEqual(["Plugins", "Tool providers", "Skills"]); + expect(toolProviderRows.map((row) => `${row.label}: ${row.value}`)).toEqual([ + "github: MCP server, ready, auth oAuth, 1 tool, 0 resources", + ]); expect(listMcpServerStatus).toHaveBeenCalledWith({ detail: "toolsAndAuthOnly", limit: 100, diff --git a/tests/features/chat/application/connection/connection-controller.test.ts b/tests/features/chat/application/connection/connection-controller.test.ts index f540fbb6..847b75e0 100644 --- a/tests/features/chat/application/connection/connection-controller.test.ts +++ b/tests/features/chat/application/connection/connection-controller.test.ts @@ -39,6 +39,8 @@ function createController({ connected = false } = {}) { diagnostics, invalidateThreadWork: vi.fn(), refreshSharedThreads: vi.fn().mockResolvedValue(undefined), + scheduleDeferredDiagnostics: vi.fn(), + clearDeferredDiagnostics: vi.fn(), refreshTabHeader: vi.fn(), resetThreadTurnPresence: vi.fn(), setStatus: vi.fn(), @@ -74,14 +76,16 @@ describe("ChatConnectionController", () => { }); expect(refreshAppServerMetadata).toHaveBeenCalledOnce(); expect(host.refreshSharedThreads).toHaveBeenCalledOnce(); + expect(host.scheduleDeferredDiagnostics).toHaveBeenCalledOnce(); expect(host.setStatus).toHaveBeenCalledWith("Connected.", { kind: "connected" }); }); it("refreshes metadata before server diagnostics", async () => { - const { controller, refreshAppServerMetadata, refreshServerDiagnostics } = createController({ connected: true }); + const { controller, host, refreshAppServerMetadata, refreshServerDiagnostics } = createController({ connected: true }); await controller.refreshDiagnostics(); + expect(host.clearDeferredDiagnostics).toHaveBeenCalledTimes(2); expect(refreshAppServerMetadata).toHaveBeenCalledOnce(); expect(refreshServerDiagnostics).toHaveBeenCalledWith({ appServerMetadataSnapshot: true }); }); diff --git a/tests/features/chat/application/connection/diagnostic-sections.test.ts b/tests/features/chat/application/connection/diagnostic-sections.test.ts index a1e63c3c..7d035971 100644 --- a/tests/features/chat/application/connection/diagnostic-sections.test.ts +++ b/tests/features/chat/application/connection/diagnostic-sections.test.ts @@ -121,8 +121,6 @@ describe("connection diagnostics", () => { enabled: true, availability: "AVAILABLE", source: "remote", - details: { skillCount: 2, hookCount: 1, appCount: 1, mcpServerCount: 1 }, - detailsError: null, }, { id: "installable-plugin", @@ -135,8 +133,6 @@ describe("connection diagnostics", () => { enabled: true, availability: "AVAILABLE", source: "remote", - details: null, - detailsError: null, }, { id: "disabled-plugin", @@ -149,8 +145,18 @@ describe("connection diagnostics", () => { enabled: false, availability: "AVAILABLE", source: "remote", - details: null, - detailsError: null, + }, + { + id: "unversioned-plugin", + name: "unversioned-plugin", + displayName: "Unversioned Plugin", + marketplaceName: "personal", + marketplacePath: null, + localVersion: null, + installed: true, + enabled: true, + availability: "AVAILABLE", + source: "remote", }, ], pluginMarketplaceErrors: [], @@ -162,6 +168,7 @@ describe("connection diagnostics", () => { toolCount: 219, resourceCount: 0, resourceTemplateCount: 0, + codexAppIds: ["apple_music", "github", "google_drive"], }, { name: "github", @@ -231,13 +238,16 @@ describe("connection diagnostics", () => { const sections = toolInventoryDiagnosticSections(diagnosticsWithToolInventory(inventory)); const pluginRows = sections.find((section) => section.title === "Plugins")?.rows ?? []; - const mcpRows = sections.find((section) => section.title === "MCP servers")?.rows ?? []; + const toolProviderRows = sections.find((section) => section.title === "Tool providers")?.rows ?? []; const skillRows = sections.find((section) => section.title === "Skills")?.rows ?? []; - expect(sections.map((section) => section.title)).toEqual(["Plugins", "MCP servers", "Skills"]); - expect(pluginRows.map((row) => `${row.label}: ${row.value}`)).toEqual(["Usable Plugin: 2 skills, 1 hook, 1 app, 1 MCP server"]); - expect(mcpRows.map((row) => `${row.label}: ${row.value}`)).toEqual([ - "codex_apps: MCP server, ready, auth oAuth, 219 tools, 0 resources", + expect(sections.map((section) => section.title)).toEqual(["Plugins", "Tool providers", "Skills"]); + expect(pluginRows.map((row) => `${row.label}: ${row.value}`)).toEqual([ + "Usable Plugin: version 1.2.3", + "Unversioned Plugin: version unknown", + ]); + expect(toolProviderRows.map((row) => `${row.label}: ${row.value}`)).toEqual([ + "codex_apps: apple_music, github, google_drive", "github: MCP server, ready, auth oAuth, 2 tools, 0 resources", ]); expect(skillRows.map((row) => `${row.label}: ${row.value}`)).toEqual([ @@ -280,7 +290,7 @@ describe("connection diagnostics", () => { toolInventory: inventory, }; - const mcpRows = toolInventoryDiagnosticSections(diagnostics).find((section) => section.title === "MCP servers")?.rows ?? []; + const mcpRows = toolInventoryDiagnosticSections(diagnostics).find((section) => section.title === "Tool providers")?.rows ?? []; expect(mcpRows.map((row) => `${row.label}: ${row.value}`)).toEqual(["github: MCP server, ready, auth oAuth, 1 tool, 0 resources"]); }); @@ -307,8 +317,8 @@ describe("connection diagnostics", () => { }; const mcpRows = - toolInventoryDiagnosticSections(diagnosticsWithToolInventory(inventory)).find((section) => section.title === "MCP servers")?.rows ?? - []; + toolInventoryDiagnosticSections(diagnosticsWithToolInventory(inventory)).find((section) => section.title === "Tool providers") + ?.rows ?? []; expect(mcpRows.map((row) => `${row.label}: ${row.value}`)).toEqual([ "figma: MCP server, failed, auth unknown, tools unknown, command not found", diff --git a/tests/features/chat/application/connection/reconnect-actions.test.ts b/tests/features/chat/application/connection/reconnect-actions.test.ts index 92c161dc..51db5876 100644 --- a/tests/features/chat/application/connection/reconnect-actions.test.ts +++ b/tests/features/chat/application/connection/reconnect-actions.test.ts @@ -19,6 +19,7 @@ function createHost(overrides: Partial = {}) { stateStore, invalidateConnectionWork: vi.fn(), invalidateThreadWork: vi.fn(), + clearDeferredDiagnostics: vi.fn(), resetConnection: vi.fn(), setStatus: vi.fn(), ensureConnected: vi.fn().mockResolvedValue(undefined), @@ -38,6 +39,7 @@ describe("reconnectPanel", () => { expect(stateStore.getState().ui.toolbarPanel).toBeNull(); expect(host.invalidateConnectionWork).toHaveBeenCalledOnce(); expect(host.invalidateThreadWork).toHaveBeenCalledOnce(); + expect(host.clearDeferredDiagnostics).toHaveBeenCalledOnce(); expect(host.resetConnection).toHaveBeenCalledOnce(); expect(host.setStatus).toHaveBeenCalledWith("Reconnecting...", { kind: "connecting" }); expect(host.ensureConnected).toHaveBeenCalledOnce(); diff --git a/tests/features/chat/application/conversation/slash-command-execution.test.ts b/tests/features/chat/application/conversation/slash-command-execution.test.ts index ce8117f8..19fe8290 100644 --- a/tests/features/chat/application/conversation/slash-command-execution.test.ts +++ b/tests/features/chat/application/conversation/slash-command-execution.test.ts @@ -47,7 +47,7 @@ function context(overrides: Partial = {}): SlashCo }, statusSummaryLines: () => ["status"], connectionDiagnosticDetails: () => [{ title: "Process", rows: [{ key: "connection", value: "connected" }] }], - toolInventoryDetails: vi.fn(() => [{ title: "MCP servers", auditFacts: [{ key: "codex_apps", value: "GitHub" }] }]), + toolInventoryDetails: vi.fn(() => [{ title: "Tool providers", auditFacts: [{ key: "codex_apps", value: "github" }] }]), modelStatusLines: () => ["model"], effortStatusLines: () => ["effort"], ...overrides, @@ -676,14 +676,14 @@ describe("slash commands", () => { it("shows Codex capabilities", async () => { const toolInventoryDetails = vi .fn() - .mockResolvedValue([{ title: "MCP servers", auditFacts: [{ key: "codex_apps", value: "GitHub" }] }]); + .mockResolvedValue([{ title: "Tool providers", auditFacts: [{ key: "codex_apps", value: "github" }] }]); const ctx = context({ toolInventoryDetails }); await executeSlashCommand("tools", "", ctx); expect(toolInventoryDetails).toHaveBeenCalledOnce(); expect(ctx.addStructuredSystemMessage).toHaveBeenCalledWith("Codex capabilities", [ - { title: "MCP servers", auditFacts: [{ key: "codex_apps", value: "GitHub" }] }, + { title: "Tool providers", auditFacts: [{ key: "codex_apps", value: "github" }] }, ]); }); @@ -706,7 +706,7 @@ describe("slash commands", () => { }); it("documents Codex capabilities", () => { - expect(slashCommandHelpValue("/tools")).toBe("Show Codex plugins, MCP servers, and skills reported by App Server."); + expect(slashCommandHelpValue("/tools")).toBe("Show Codex plugins, tool providers, and skills reported by App Server."); }); }); diff --git a/tests/features/chat/host/connection-bundle.test.ts b/tests/features/chat/host/connection-bundle.test.ts new file mode 100644 index 00000000..73e519da --- /dev/null +++ b/tests/features/chat/host/connection-bundle.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it, vi } from "vitest"; + +import { scheduleDeferredDiagnosticsRefresh } from "../../../../src/features/chat/host/connection-bundle"; + +describe("connection bundle deferred diagnostics", () => { + it("reports deferred diagnostics failures without returning a blocking promise", async () => { + const callbacks: Array<() => void> = []; + const error = new Error("diagnostics failed"); + const refreshServerDiagnostics = vi.fn().mockRejectedValue(error); + const addSystemMessage = vi.fn(); + + scheduleDeferredDiagnosticsRefresh({ + scheduleDiagnostics: (scheduled) => { + callbacks.push(scheduled); + }, + isConnected: () => true, + refreshServerDiagnostics, + addSystemMessage, + }); + + expect(callbacks).toHaveLength(1); + const scheduled = callbacks[0]; + if (!scheduled) throw new Error("Expected deferred diagnostics callback."); + scheduled(); + await Promise.resolve(); + + expect(refreshServerDiagnostics).toHaveBeenCalledWith({ appServerMetadataSnapshot: true }); + expect(addSystemMessage).toHaveBeenCalledWith("diagnostics failed"); + }); + + it("does not refresh deferred diagnostics after disconnect", () => { + const callbacks: Array<() => void> = []; + const refreshServerDiagnostics = vi.fn().mockResolvedValue(undefined); + + scheduleDeferredDiagnosticsRefresh({ + scheduleDiagnostics: (scheduled) => { + callbacks.push(scheduled); + }, + isConnected: () => false, + refreshServerDiagnostics, + addSystemMessage: vi.fn(), + }); + + const scheduled = callbacks[0]; + if (!scheduled) throw new Error("Expected deferred diagnostics callback."); + scheduled(); + + expect(refreshServerDiagnostics).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/features/chat/host/deferred-work.test.ts b/tests/features/chat/host/deferred-work.test.ts index e5214915..e8feb2d3 100644 --- a/tests/features/chat/host/deferred-work.test.ts +++ b/tests/features/chat/host/deferred-work.test.ts @@ -11,13 +11,16 @@ describe("createChatViewDeferredTasks", () => { it("clears scheduled deferred work", async () => { const tasks = createChatViewDeferredTasks(() => window); + const diagnostics = vi.fn(); const warmup = vi.fn(); + tasks.scheduleDiagnostics(diagnostics); tasks.scheduleAppServerWarmup(warmup); tasks.clearAll(); await vi.advanceTimersByTimeAsync(1_500); + expect(diagnostics).not.toHaveBeenCalled(); expect(warmup).not.toHaveBeenCalled(); }); }); diff --git a/tests/features/chat/host/turn-bundle.test.ts b/tests/features/chat/host/turn-bundle.test.ts new file mode 100644 index 00000000..e1d5e415 --- /dev/null +++ b/tests/features/chat/host/turn-bundle.test.ts @@ -0,0 +1,120 @@ +import { describe, expect, it, vi } from "vitest"; + +import { createServerDiagnostics, diagnosticsWithToolInventory } from "../../../../src/domain/server/diagnostics"; +import type { ToolInventorySnapshot } from "../../../../src/domain/server/tool-inventory"; +import { createChatStateStore } from "../../../../src/features/chat/application/state/store"; +import { createTurnBundle } from "../../../../src/features/chat/host/turn-bundle"; +import { ConnectionWorkTracker } from "../../../../src/shared/lifecycle/connection-work"; + +describe("createTurnBundle", () => { + it("uses cached tool inventory for /tools without refreshing diagnostics", async () => { + const stateStore = createChatStateStore(); + stateStore.dispatch({ + type: "connection/metadata-applied", + serverDiagnostics: diagnosticsWithToolInventory(createServerDiagnostics(), toolInventory()), + }); + const fixture = turnBundleFixture({ stateStore }); + + await fixture.bundle.turnActions.composerSubmit.submit(); + + expect(fixture.refreshDiagnostics).not.toHaveBeenCalled(); + expect(fixture.runtimeProjection.toolInventoryDetails).toHaveBeenCalledOnce(); + expect(fixture.status.addStructuredSystemMessage).toHaveBeenCalledWith("Codex capabilities", [ + { title: "Tool providers", auditFacts: [{ key: "codex_apps", value: "github, gmail" }] }, + ]); + }); + + it("refreshes diagnostics for /tools when tool inventory is not loaded", async () => { + const fixture = turnBundleFixture(); + + await fixture.bundle.turnActions.composerSubmit.submit(); + + expect(fixture.refreshDiagnostics).toHaveBeenCalledOnce(); + expect(fixture.runtimeProjection.toolInventoryDetails).toHaveBeenCalledOnce(); + }); +}); + +function turnBundleFixture(options: { stateStore?: ReturnType } = {}) { + const stateStore = options.stateStore ?? createChatStateStore(); + const status = { + set: vi.fn(), + addSystemMessage: vi.fn(), + addStructuredSystemMessage: vi.fn(), + }; + const runtimeProjection = { + connectionDiagnosticDetails: vi.fn(() => []), + modelStatusLines: vi.fn(() => []), + effortStatusLines: vi.fn(() => []), + statusSummaryLines: vi.fn(() => []), + toolInventoryDetails: vi.fn(() => [{ title: "Tool providers", auditFacts: [{ key: "codex_apps", value: "github, gmail" }] }]), + }; + const refreshDiagnostics = vi.fn().mockResolvedValue(undefined); + const bundle = createTurnBundle( + { + environment: { + plugin: { + settingsRef: { + vaultPath: "/vault", + }, + }, + }, + stateStore, + deferredTasks: {}, + connectionWork: new ConnectionWorkTracker(), + messageScrollController: { + showLatest: vi.fn(), + }, + } as never, + { + connection: { resetConnection: vi.fn() }, + localItemIds: { next: vi.fn(() => "local-id") }, + ensureConnected: vi.fn().mockResolvedValue(undefined), + connectedClient: vi.fn().mockResolvedValue({}), + currentClient: vi.fn(() => ({})), + status, + inboundHandler: {}, + threadLifecycle: { + restoration: { ensureLoaded: vi.fn().mockResolvedValue(true) }, + resume: { resumeThread: vi.fn() }, + }, + threadActions: {}, + navigation: { + startNewThread: vi.fn(), + selectThread: vi.fn(), + }, + composerController: { + get trimmedDraft() { + return "/tools"; + }, + setDraft: vi.fn(), + codexInput: vi.fn(), + hasFocus: vi.fn(() => false), + focusComposer: vi.fn(), + }, + runtimeSettings: {}, + serverThreads: {}, + goals: {}, + autoTitleCoordinator: { resetThreadTurnPresence: vi.fn() }, + invalidateThreadWork: vi.fn(), + runtimeProjection, + refreshDiagnostics, + refreshLiveState: vi.fn(), + notifyActiveThreadIdentityChanged: vi.fn(), + } as never, + ); + return { bundle, refreshDiagnostics, runtimeProjection, status }; +} + +function toolInventory(): ToolInventorySnapshot { + return { + checkedAt: 1, + plugins: [], + pluginMarketplaceErrors: [], + pluginsError: null, + mcpServers: [], + mcpDiagnostics: [], + mcpError: null, + skills: [], + skillsError: null, + }; +} diff --git a/tests/features/chat/ui/toolbar.test.ts b/tests/features/chat/ui/toolbar.test.ts index 8c99bd6a..375fd352 100644 --- a/tests/features/chat/ui/toolbar.test.ts +++ b/tests/features/chat/ui/toolbar.test.ts @@ -169,7 +169,7 @@ describe("Toolbar decisions", () => { expect(parent.querySelector(".codex-panel__connection-diagnostics-title")?.textContent).toBe("Connection"); expect(parent.textContent).toContain("Codex capabilities"); - expect(parent.textContent).toContain("MCP servers"); + expect(parent.textContent).toContain("Tool providers"); expect(parent.textContent).toContain("Process"); expect(parent.textContent).toContain("Runtime Checks"); expect(parent.textContent).toContain("Copy debug details"); @@ -363,7 +363,7 @@ function toolbarModel(overrides: Partial = {}): ToolbarViewMod threads: [{ title: "Thread", threadId: "thread", selected: true, disabled: false, canArchive: true, rename: null }], connectLabel: "Reconnect", diagnostics: [{ title: "Process", rows: [{ label: "Codex App Server", value: "codex-cli/test" }] }], - toolInventory: [{ title: "MCP servers", rows: [{ label: "MCP servers", value: "not loaded", level: "warning" }] }], + toolInventory: [{ title: "Tool providers", rows: [{ label: "Tool providers", value: "not loaded", level: "warning" }] }], ...overrides, }; }