diff --git a/src/app-server/connection/client-access.ts b/src/app-server/connection/client-access.ts index 960d3d52..88144c55 100644 --- a/src/app-server/connection/client-access.ts +++ b/src/app-server/connection/client-access.ts @@ -1,5 +1,11 @@ import type { AppServerClient } from "./client"; -export interface AppServerClientAccess { - withClient(operation: (client: AppServerClient) => Promise, options?: { unhandledServerRequestMessage?: string }): Promise; +export type AppServerClientRequestPolicy = { kind: "interactive" } | { kind: "reject"; message: string }; + +export interface AppServerClientAccessOptions { + serverRequests?: AppServerClientRequestPolicy; +} + +export interface AppServerClientAccess { + withClient(operation: (client: AppServerClient) => Promise, options?: AppServerClientAccessOptions): Promise; } diff --git a/src/app-server/connection/client.ts b/src/app-server/connection/client.ts index 62d27cc8..fe60cad9 100644 --- a/src/app-server/connection/client.ts +++ b/src/app-server/connection/client.ts @@ -203,8 +203,8 @@ export class AppServerClient { } async connect(): Promise { - if (this.activeTransport()?.isRunning()) { - throw new Error("Codex app-server is already running."); + if (this.lifecycle.kind !== "disconnected") { + throw new Error("Codex app-server client is already connecting or connected."); } const transportRef: { current: AppServerTransport | null } = { current: null }; @@ -232,9 +232,10 @@ export class AppServerClient { if (!transport) return; if (!this.isActiveTransport(transport)) return; const intentional = this.intentionallyStoppedTransports.has(transport); + const wasInitialized = this.lifecycle.kind === "initialized"; this.lifecycle = { kind: "disconnected" }; this.rpc.rejectAll(new Error(`Codex app-server exited: ${String(code ?? signal ?? "unknown")}`)); - if (intentional) return; + if (intentional || !wasInitialized) return; this.handlers.onExit(code, signal); }, }; @@ -243,22 +244,32 @@ export class AppServerClient { : new StdioAppServerTransport(this.codexPath, this.cwd, transportHandlers); transportRef.current = transport; this.lifecycle = { kind: "starting", transport }; - transport.start(); - - const init = await this.request("initialize", { - clientInfo: { - name: "obsidian_codex_panel", - title: "Codex Panel", - version: CLIENT_VERSION, - }, - capabilities: { - experimentalApi: true, - requestAttestation: false, - }, - }); - this.notify({ method: "initialized" }); - this.lifecycle = { kind: "initialized", transport, initializeResponse: init }; - return init; + try { + transport.start(); + const init = await this.request("initialize", { + clientInfo: { + name: "obsidian_codex_panel", + title: "Codex Panel", + version: CLIENT_VERSION, + }, + capabilities: { + experimentalApi: true, + requestAttestation: false, + }, + }); + this.notify({ method: "initialized" }); + this.lifecycle = { kind: "initialized", transport, initializeResponse: init }; + return init; + } catch (error) { + if (this.isActiveTransport(transport)) { + const normalized = error instanceof Error ? error : new Error(String(error)); + this.lifecycle = { kind: "disconnected" }; + this.rpc.rejectAll(normalized); + this.intentionallyStoppedTransports.add(transport); + transport.stop(); + } + throw error; + } } disconnect(): void { diff --git a/src/app-server/connection/short-lived-client.ts b/src/app-server/connection/short-lived-client.ts index 2a62be4b..c1a25392 100644 --- a/src/app-server/connection/short-lived-client.ts +++ b/src/app-server/connection/short-lived-client.ts @@ -1,14 +1,11 @@ import { AppServerClient } from "./client"; - -interface ShortLivedAppServerClientOptions { - unhandledServerRequestMessage?: string; -} +import type { AppServerClientAccessOptions } from "./client-access"; export async function withShortLivedAppServerClient( codexPath: string, cwd: string, operation: (client: AppServerClient) => Promise, - options: ShortLivedAppServerClientOptions = {}, + options: AppServerClientAccessOptions = {}, ): Promise { let client!: AppServerClient; client = new AppServerClient(codexPath, cwd, { @@ -17,7 +14,9 @@ export async function withShortLivedAppServerClient( client.rejectServerRequest( request.id, -32601, - options.unhandledServerRequestMessage ?? "This Codex Panel view does not handle server requests.", + options.serverRequests?.kind === "reject" + ? options.serverRequests.message + : "This Codex Panel view does not handle server requests.", ); }, onLog: () => undefined, diff --git a/src/app-server/query/cache.ts b/src/app-server/query/cache.ts index e986ea2f..442cd156 100644 --- a/src/app-server/query/cache.ts +++ b/src/app-server/query/cache.ts @@ -1,6 +1,7 @@ import { QueryClient, QueryObserver, type QueryObserverResult } from "@tanstack/query-core"; import type { AppServerClient } from "../connection/client"; +import type { AppServerClientAccessOptions } from "../connection/client-access"; import { listModelMetadata } from "../catalog"; import { readRateLimitMetadataProbe, readSkillMetadataProbe } from "./metadata-probes"; import { runtimeConfigSnapshotFromAppServerConfig } from "../protocol/runtime-config"; @@ -29,7 +30,7 @@ export interface AppServerQueryClientRunner { runWithClient( context: AppServerQueryContext, operation: (client: AppServerClient) => Promise, - options?: { unhandledServerRequestMessage?: string }, + options?: AppServerClientAccessOptions, ): Promise; } @@ -386,7 +387,7 @@ export class AppServerQueryCache { queryFn: async (): Promise => { return cloneModelMetadata( await this.runWithClient(refreshContext, (client) => listModelMetadata(client), { - unhandledServerRequestMessage: "Codex model list refresh does not handle server requests.", + serverRequests: { kind: "reject", message: "Codex model list refresh does not handle server requests." }, }), ); }, @@ -452,7 +453,7 @@ export class AppServerQueryCache { private runWithClient( context: AppServerQueryContext, operation: (client: AppServerClient) => Promise, - options: { unhandledServerRequestMessage?: string } = {}, + options: AppServerClientAccessOptions = {}, ): Promise { if (!this.clientRunner) { throw new Error("Codex app-server query client runner is not configured."); diff --git a/src/app-server/services/ephemeral-structured-turn.ts b/src/app-server/services/ephemeral-structured-turn.ts index e8cedf87..fb4bc913 100644 --- a/src/app-server/services/ephemeral-structured-turn.ts +++ b/src/app-server/services/ephemeral-structured-turn.ts @@ -4,6 +4,7 @@ import { type AppServerStartEphemeralThreadOptions, type AppServerStartStructuredTurnOptions, } from "../connection/client"; +import type { AppServerClientRequestPolicy } from "../connection/client-access"; import type { RequestId, ServerNotification } from "../connection/rpc-messages"; import type { ModelMetadataClient } from "../catalog"; import { lastAgentMessageTextFromTurnRecord, type TurnItem, type TurnRecord } from "../protocol/turn"; @@ -51,7 +52,7 @@ export interface RunEphemeralStructuredTurnOptions { prompt: string; outputSchema: StructuredTurnOutputSchema; timeoutMs: number; - unhandledServerRequestMessage: string; + serverRequests: Extract; exitedMessage: string; timedOutMessage: string; abortMessage?: string; @@ -105,7 +106,7 @@ export async function runEphemeralStructuredTurn(options: RunEphemeralStructured handleNotification(notification); }, onServerRequest: (request) => { - client.rejectServerRequest(request.id, -32601, options.unhandledServerRequestMessage); + client.rejectServerRequest(request.id, -32601, options.serverRequests.message); }, onLog: () => undefined, onExit: () => { diff --git a/src/app-server/services/thread-title-generation.ts b/src/app-server/services/thread-title-generation.ts index c81b7537..3455d011 100644 --- a/src/app-server/services/thread-title-generation.ts +++ b/src/app-server/services/thread-title-generation.ts @@ -57,7 +57,7 @@ export async function generateThreadTitleWithCodex( prompt: threadTitlePrompt(context), outputSchema: TITLE_OUTPUT_SCHEMA, timeoutMs: THREAD_TITLE_TIMEOUT_MS, - unhandledServerRequestMessage: "Thread title generation does not handle server requests.", + serverRequests: { kind: "reject", message: "Thread title generation does not handle server requests." }, exitedMessage: "Codex title generation app-server exited.", timedOutMessage: "Timed out while generating a Codex thread title.", resolveRuntime: (client) => threadTitleRuntimeOverrideForClient(client, runtimeSettings), diff --git a/src/app-server/tool-inventory.ts b/src/app-server/tool-inventory.ts index 8a304e45..9dbdbfc3 100644 --- a/src/app-server/tool-inventory.ts +++ b/src/app-server/tool-inventory.ts @@ -19,6 +19,7 @@ import { toolInventoryAppsFromAppInfos, toolInventoryPluginsFromInstalledRespons const APP_PAGE_LIMIT = 100; const APP_PAGE_LOOP_LIMIT = 20; +const PLUGIN_DETAILS_CONCURRENCY = 4; export interface ReadToolInventoryOptions { readonly threadId?: string | null; @@ -101,6 +102,7 @@ async function listAllApps( threadId: string | null, ): Promise>["data"]> { let cursor: string | null = null; + const seenCursors = new Set(); const apps: Awaited>["data"] = []; for (let page = 0; page < APP_PAGE_LOOP_LIMIT; page += 1) { const response = await client.listApps({ @@ -111,8 +113,12 @@ async function listAllApps( apps.push(...response.data); cursor = response.nextCursor; if (!cursor) return apps; + if (seenCursors.has(cursor)) { + throw new Error("Codex app-server returned a repeated app list cursor."); + } + seenCursors.add(cursor); } - return apps; + throw new Error("Codex app-server returned too many app list pages."); } async function readPlugins( @@ -128,7 +134,7 @@ async function readPlugins( try { const response = await client.listInstalledPlugins(cwd); const { plugins, marketplaceErrors } = toolInventoryPluginsFromInstalledResponse(response); - const pluginsWithDetails = await Promise.all(plugins.map((plugin) => readPluginDetails(client, plugin))); + const pluginsWithDetails = await mapLimit(plugins, PLUGIN_DETAILS_CONCURRENCY, (plugin) => readPluginDetails(client, plugin)); return { data: pluginsWithDetails, marketplaceErrors, @@ -145,6 +151,24 @@ 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: AppServerClient, plugin: ToolInventoryPlugin): Promise { if (!isUsablePlugin(plugin)) return plugin; try { diff --git a/src/features/selection-rewrite/runner.ts b/src/features/selection-rewrite/runner.ts index 4457a960..4975feb1 100644 --- a/src/features/selection-rewrite/runner.ts +++ b/src/features/selection-rewrite/runner.ts @@ -50,7 +50,7 @@ export async function runSelectionRewrite(options: RunSelectionRewriteOptions): prompt: options.prompt, outputSchema: SELECTION_REWRITE_OUTPUT_SCHEMA, timeoutMs: SELECTION_REWRITE_TIMEOUT_MS, - unhandledServerRequestMessage: "Selection rewrite does not handle server requests.", + serverRequests: { kind: "reject", message: "Selection rewrite does not handle server requests." }, exitedMessage: "Selection rewrite app-server exited.", timedOutMessage: "Timed out while rewriting the selection.", abortMessage: "Selection rewrite cancelled.", diff --git a/src/plugin-runtime.ts b/src/plugin-runtime.ts index f434ef0c..363d23d5 100644 --- a/src/plugin-runtime.ts +++ b/src/plugin-runtime.ts @@ -4,7 +4,7 @@ import { VIEW_TYPE_CODEX_THREADS, VIEW_TYPE_CODEX_TURN_DIFF } from "./constants" import { AppServerQueryCache } from "./app-server/query/cache"; import { AppServerSharedQueries } from "./app-server/query/shared-queries"; import type { AppServerClient } from "./app-server/connection/client"; -import type { AppServerClientAccess } from "./app-server/connection/client-access"; +import type { AppServerClientAccess, AppServerClientAccessOptions } from "./app-server/connection/client-access"; import { withShortLivedAppServerClient } from "./app-server/connection/short-lived-client"; import { appServerQueryContextIsComplete, type AppServerQueryContext } from "./app-server/query/keys"; import type { ChatTurnDiffViewState } from "./features/chat/domain/turn-diff"; @@ -169,7 +169,7 @@ export class CodexPanelRuntime implements AppServerClientAccess { }; } - withClient(operation: (client: AppServerClient) => Promise, options: { unhandledServerRequestMessage?: string } = {}): Promise { + withClient(operation: (client: AppServerClient) => Promise, options: AppServerClientAccessOptions = {}): Promise { return this.runWithAppServerClient(this.appServerQueryContext(), operation, options); } @@ -224,12 +224,12 @@ export class CodexPanelRuntime implements AppServerClientAccess { private async runWithAppServerClient( context: AppServerQueryContext, operation: (client: AppServerClient) => Promise, - options: { unhandledServerRequestMessage?: string } = {}, + options: AppServerClientAccessOptions = {}, ): Promise { if (!appServerQueryContextIsComplete(context)) { throw new Error("Codex app-server query context is incomplete."); } - if (options.unhandledServerRequestMessage !== undefined) { + if (options.serverRequests?.kind === "reject") { return withShortLivedAppServerClient(context.codexPath, context.vaultPath, operation, options); } const chatSurface = this.connectedClientSurface(); diff --git a/src/settings/archived-section.tsx b/src/settings/archived-section.tsx index 1d060b3d..599bf538 100644 --- a/src/settings/archived-section.tsx +++ b/src/settings/archived-section.tsx @@ -2,18 +2,10 @@ import type { ComponentChild as UiNode } from "preact"; import type { Thread } from "../domain/threads/model"; import { threadArchiveDisplayTitle } from "../domain/threads/title"; +import { ObsidianExtraButton, ObsidianTextInput, ObsidianToggle } from "../shared/ui/components"; import { shortThreadId } from "../utils"; import type { ArchivedThreadSectionState } from "./section-state"; -import { - SettingRow, - SettingsGroup, - SettingsHeading, - SettingsIconButton, - SettingsItems, - SettingsStatusRow, - TextControl, - ToggleControl, -} from "./setting-components"; +import { SettingRow, SettingsGroup, SettingsHeading, SettingsItems, SettingsStatusRow } from "./setting-components"; export function ArchivedThreadSection({ state }: { state: ArchivedThreadSectionState }): UiNode { return ( @@ -38,7 +30,7 @@ function ArchiveExportSettings({ state }: { state: ArchivedThreadSectionState }) return ( - { state.onExportEnabledChange(checked); @@ -46,7 +38,7 @@ function ArchiveExportSettings({ state }: { state: ArchivedThreadSectionState }) /> - { @@ -55,7 +47,7 @@ function ArchiveExportSettings({ state }: { state: ArchivedThreadSectionState }) /> - { @@ -64,7 +56,7 @@ function ArchiveExportSettings({ state }: { state: ArchivedThreadSectionState }) /> - { @@ -104,7 +96,7 @@ function ArchivedThreadRow({ thread, state }: { thread: Thread; state: ArchivedT } > {!deleteConfirming ? ( - ) : null} - 0) { this.callbacks.notify("Could not refresh all Codex data."); @@ -289,7 +276,9 @@ export class SettingsDynamicDataController { } settingsDataLoading(): boolean { - return this.settingsDataRefreshLifecycle.kind === "loading"; + return ( + this.modelsLifecycle.kind === "loading" || this.hooksLifecycle.kind === "loading" || this.archivedThreadsLifecycle.kind === "loading" + ); } snapshot(): SettingsDynamicDataSnapshot { @@ -475,7 +464,7 @@ export class SettingsDynamicDataController { private async withSettingsConnection(operation: (client: AppServerClient) => Promise): Promise { return this.host.clientAccess.withClient(operation, { - unhandledServerRequestMessage: "Codex Panel settings does not handle server requests.", + serverRequests: { kind: "reject", message: "Codex Panel settings does not handle server requests." }, }); } diff --git a/src/settings/helper-section.tsx b/src/settings/helper-section.tsx index 04f084f5..ece3639b 100644 --- a/src/settings/helper-section.tsx +++ b/src/settings/helper-section.tsx @@ -2,8 +2,9 @@ import type { ComponentChild as UiNode } from "preact"; import type { ModelMetadata, ReasoningEffort } from "../domain/catalog/metadata"; import { findModelMetadataByIdOrName, supportedEffortsForModelMetadata } from "../domain/catalog/metadata"; +import { ObsidianDropdown } from "../shared/ui/components"; import type { HelperSettingsState } from "./section-state"; -import { SelectControl, SettingRow, SettingsGroup, SettingsHeading, SettingsItems } from "./setting-components"; +import { SettingRow, SettingsGroup, SettingsHeading, SettingsItems } from "./setting-components"; const CODEX_DEFAULT_VALUE = "__codex-default__"; @@ -56,14 +57,14 @@ function ModelEffortSetting({ const efforts = reasoningEffortsForSelectedModel(models, modelValue); return ( - { onModelChange(value === CODEX_DEFAULT_VALUE ? null : value); }} options={modelSelectOptions(models, modelValue)} /> - { onEffortChange(value === CODEX_DEFAULT_VALUE ? null : value); diff --git a/src/settings/lifecycle.ts b/src/settings/lifecycle.ts index fd69df80..1cb8141d 100644 --- a/src/settings/lifecycle.ts +++ b/src/settings/lifecycle.ts @@ -1,12 +1,3 @@ -export type SettingsDataRefreshLifecycleState = - | { kind: "idle" } - | { kind: "loading"; operationToken: number } - | { kind: "completed"; failedCount: number; operationToken: number }; - -export type SettingsDataRefreshLifecycleEvent = - | { type: "started"; operationToken: number } - | { type: "completed"; failedCount: number; operationToken: number }; - export type SettingsDynamicSectionLifecycleState = | { kind: "idle"; status: "" } | { kind: "loading"; status: string; operationToken: number } @@ -19,20 +10,6 @@ export type SettingsDynamicSectionLifecycleEvent = | { type: "failed"; status: string; operationToken: number } | { type: "reset" }; -export function transitionSettingsDataRefreshLifecycle( - state: SettingsDataRefreshLifecycleState, - event: SettingsDataRefreshLifecycleEvent, -): SettingsDataRefreshLifecycleState { - switch (event.type) { - case "started": - if (isStaleSettingsDataRefreshEvent(state, event.operationToken)) return state; - return { kind: "loading", operationToken: event.operationToken }; - case "completed": - if (isStaleSettingsDataRefreshEvent(state, event.operationToken)) return state; - return { kind: "completed", failedCount: event.failedCount, operationToken: event.operationToken }; - } -} - export function createSettingsDynamicSectionLifecycle(): SettingsDynamicSectionLifecycleState { return { kind: "idle", status: "" }; } @@ -59,7 +36,3 @@ export function transitionSettingsDynamicSectionLifecycle( function isStaleSettingsDynamicSectionEvent(state: SettingsDynamicSectionLifecycleState, operationToken: number): boolean { return "operationToken" in state && state.operationToken > operationToken; } - -function isStaleSettingsDataRefreshEvent(state: SettingsDataRefreshLifecycleState, operationToken: number): boolean { - return "operationToken" in state && state.operationToken > operationToken; -} diff --git a/src/settings/setting-components.tsx b/src/settings/setting-components.tsx index 02686724..cd1d6fe6 100644 --- a/src/settings/setting-components.tsx +++ b/src/settings/setting-components.tsx @@ -1,13 +1,5 @@ import type { ComponentChild as UiNode } from "preact"; -import { - ObsidianDropdown, - type ObsidianDropdownOption, - ObsidianExtraButton, - ObsidianTextInput, - ObsidianToggle, -} from "../shared/ui/components"; - export function SettingsGroup({ className, children }: { className: string; children: UiNode }): UiNode { return
{children}
; } @@ -62,45 +54,3 @@ export function SettingRow({ ); } - -export function SettingsIconButton({ - icon, - label, - className, - onClick, -}: { - icon: string; - label: string; - className: string; - onClick: () => void; -}): UiNode { - return ; -} - -export function SelectControl({ - value, - onChange, - options, -}: { - value: string; - onChange: (value: string) => void; - options: readonly ObsidianDropdownOption[]; -}): UiNode { - return ; -} - -export function TextControl({ - value, - placeholder, - onChange, -}: { - value: string; - placeholder: string; - onChange: (value: string) => void; -}): UiNode { - return ; -} - -export function ToggleControl({ checked, onChange }: { checked: boolean; onChange: (checked: boolean) => void }): UiNode { - return ; -} diff --git a/tests/app-server/app-server-client.test.ts b/tests/app-server/app-server-client.test.ts index 3a03e069..12889fa7 100644 --- a/tests/app-server/app-server-client.test.ts +++ b/tests/app-server/app-server-client.test.ts @@ -285,6 +285,114 @@ describe("AppServerClient", () => { expect(onExit).toHaveBeenCalledOnce(); }); + it("cleans up the active transport when initialize fails", async () => { + const transports: FakeTransport[] = []; + const onExit = vi.fn(); + const client = new AppServerClient( + "/bin/codex", + "/vault", + { + onNotification: () => undefined, + onServerRequest: () => undefined, + onLog: () => undefined, + onExit, + }, + 500, + (handlers) => { + const transport = new FakeTransport(handlers); + transports.push(transport); + return transport; + }, + ); + + const connecting = client.connect(); + const firstTransport = expectTransport(transports[0]); + const initialize = latestSent(firstTransport); + if (!("id" in initialize) || typeof initialize.id !== "number") throw new Error("Expected initialize request."); + + const rejection = expect(connecting).rejects.toThrow("initialize failed"); + firstTransport.emitLine({ id: initialize.id, error: { code: -32000, message: "initialize failed" } }); + + await rejection; + expect(client.isConnected()).toBe(false); + expect(firstTransport.isRunning()).toBe(false); + + firstTransport.emitExit(1); + expect(onExit).not.toHaveBeenCalled(); + + const reconnecting = client.connect(); + const secondTransport = expectTransport(transports[1]); + const secondInitialize = latestSent(secondTransport); + if (!("id" in secondInitialize) || typeof secondInitialize.id !== "number") throw new Error("Expected initialize request."); + secondTransport.emitLine({ id: secondInitialize.id, result: { codexHome: "/tmp/codex" } satisfies Partial }); + + await expect(reconnecting).resolves.toMatchObject({ codexHome: "/tmp/codex" }); + }); + + it("cleans up the active transport when initialize times out", async () => { + vi.useFakeTimers(); + vi.stubGlobal("window", { + clearTimeout, + setTimeout, + }); + let transport!: FakeTransport; + const onExit = vi.fn(); + const client = new AppServerClient( + "/bin/codex", + "/vault", + { + onNotification: () => undefined, + onServerRequest: () => undefined, + onLog: () => undefined, + onExit, + }, + 500, + (handlers) => { + transport = new FakeTransport(handlers); + return transport; + }, + ); + + const connecting = client.connect(); + const rejection = expect(connecting).rejects.toThrow("Codex app-server request timed out: initialize"); + await vi.advanceTimersByTimeAsync(500); + + await rejection; + expect(client.isConnected()).toBe(false); + expect(transport.isRunning()).toBe(false); + + transport.emitExit(1); + expect(onExit).not.toHaveBeenCalled(); + }); + + it("rejects connect without notifying external exit handlers when transport exits during initialize", async () => { + let transport!: FakeTransport; + const onExit = vi.fn(); + const client = new AppServerClient( + "/bin/codex", + "/vault", + { + onNotification: () => undefined, + onServerRequest: () => undefined, + onLog: () => undefined, + onExit, + }, + 500, + (handlers) => { + transport = new FakeTransport(handlers); + return transport; + }, + ); + + const connecting = client.connect(); + const rejection = expect(connecting).rejects.toThrow("Codex app-server exited: 1"); + transport.emitExit(1); + + await rejection; + expect(client.isConnected()).toBe(false); + expect(onExit).not.toHaveBeenCalled(); + }); + it("ignores stale transport events after reconnecting", async () => { const transports: FakeTransport[] = []; const onExit = vi.fn(); diff --git a/tests/app-server/connection-manager.test.ts b/tests/app-server/connection-manager.test.ts index 04c85060..a17ab3b6 100644 --- a/tests/app-server/connection-manager.test.ts +++ b/tests/app-server/connection-manager.test.ts @@ -120,7 +120,7 @@ describe("ConnectionManager", () => { expect(manager.currentClient()).toBeInstanceOf(AppServerClient); }); - it("reports app-server exit during initialization", async () => { + it("rejects app-server exit during initialization without reporting a connected-client exit", async () => { let transport!: SilentTransport; const onExit = vi.fn(); const manager = new ConnectionManager( @@ -137,7 +137,7 @@ describe("ConnectionManager", () => { transport.emitExit(); await expect(connecting).rejects.toThrow("Codex app-server exited: unknown"); - expect(onExit).toHaveBeenCalledOnce(); + expect(onExit).not.toHaveBeenCalled(); expect(manager.currentClient()).toBeNull(); }); diff --git a/tests/app-server/ephemeral-structured-turn.test.ts b/tests/app-server/ephemeral-structured-turn.test.ts index e7997d20..ad1cc05c 100644 --- a/tests/app-server/ephemeral-structured-turn.test.ts +++ b/tests/app-server/ephemeral-structured-turn.test.ts @@ -226,7 +226,7 @@ function runOptions(clientFactory: EphemeralStructuredTurnClientFactory): Parame prompt: "Run.", outputSchema: { type: "object" }, timeoutMs: 10_000, - unhandledServerRequestMessage: "Structured test does not handle server requests.", + serverRequests: { kind: "reject", message: "Structured test does not handle server requests." }, exitedMessage: "Structured test app-server exited.", timedOutMessage: "Structured test timed out.", clientFactory, diff --git a/tests/app-server/tool-inventory.test.ts b/tests/app-server/tool-inventory.test.ts index 130aa5a6..35729426 100644 --- a/tests/app-server/tool-inventory.test.ts +++ b/tests/app-server/tool-inventory.test.ts @@ -76,4 +76,114 @@ describe("tool inventory", () => { ["remote-plugin", { skillCount: 0, hookCount: 0, appCount: 0, mcpServerCount: 1 }, null], ]); }); + + it("fails app inventory when app pagination repeats a cursor", async () => { + const client = toolInventoryClient({ + listApps: vi + .fn() + .mockResolvedValueOnce({ data: [{ name: "first", title: "First", readOnlyHint: false }], nextCursor: "repeat" }) + .mockResolvedValueOnce({ data: [{ name: "second", title: "Second", readOnlyHint: false }], nextCursor: "repeat" }), + }); + + const result = await readToolInventory(client, "/vault"); + + expect(result.inventory.apps).toBeNull(); + expect(result.inventory.appsError).toBe("Codex app-server returned a repeated app list cursor."); + }); + + it("fails app inventory when app pagination exceeds the page limit", async () => { + let page = 0; + const client = toolInventoryClient({ + listApps: vi.fn().mockImplementation(() => { + page += 1; + return Promise.resolve({ data: [], nextCursor: `cursor-${String(page)}` }); + }), + }); + + const result = await readToolInventory(client, "/vault"); + + expect(result.inventory.apps).toBeNull(); + expect(result.inventory.appsError).toBe("Codex app-server returned too many app list pages."); + }); + + it("limits plugin detail reads while preserving plugin order", async () => { + let activeReads = 0; + let maxActiveReads = 0; + const plugins = Array.from({ length: 10 }, (_, index) => installedPlugin(`plugin-${String(index)}`)); + const readPlugin = vi.fn(async (params: Parameters[0]) => { + activeReads += 1; + maxActiveReads = Math.max(maxActiveReads, activeReads); + await Promise.resolve(); + activeReads -= 1; + return { + plugin: { + skills: [{ name: `${params.pluginName}-skill` }], + hooks: [], + apps: [], + appTemplates: [], + mcpServers: [], + }, + }; + }); + const client = toolInventoryClient({ + listInstalledPlugins: vi.fn().mockResolvedValue({ + marketplaces: [{ name: "local-marketplace", path: "/marketplaces/local.json", plugins }], + marketplaceLoadErrors: [], + }), + readPlugin, + }); + + const result = await readToolInventory(client, "/vault"); + + expect(maxActiveReads).toBe(4); + 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); + }); }); + +function toolInventoryClient( + overrides: Partial<{ + listApps: ReturnType; + listInstalledPlugins: ReturnType; + readPlugin: ReturnType; + }> = {}, +): AppServerClient { + return { + listApps: overrides.listApps ?? vi.fn().mockResolvedValue({ data: [], nextCursor: null }), + listInstalledPlugins: + overrides.listInstalledPlugins ?? + vi.fn().mockResolvedValue({ + marketplaces: [], + marketplaceLoadErrors: [], + }), + readPlugin: + overrides.readPlugin ?? + vi.fn().mockResolvedValue({ + plugin: { skills: [], hooks: [], apps: [], appTemplates: [], mcpServers: [] }, + }), + listMcpServerStatus: vi.fn().mockResolvedValue({ data: [] }), + listSkills: vi.fn().mockResolvedValue({ data: [{ cwd: "/vault", skills: [] }] }), + } as unknown as AppServerClient; +} + +function installedPlugin(name: string): { + id: string; + name: string; + interface: { displayName: string }; + localVersion: string; + installed: boolean; + enabled: boolean; + availability: string; + source: { type: string; path: string }; +} { + return { + id: `${name}@local-marketplace`, + name, + interface: { displayName: name }, + localVersion: "1.0.0", + installed: true, + enabled: true, + availability: "AVAILABLE", + source: { type: "local", path: `/plugins/${name}` }, + }; +} diff --git a/tests/main.test.ts b/tests/main.test.ts index a78ec04c..bd03e6a8 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -572,13 +572,13 @@ describe("CodexPanelPlugin boot restored panel loading", () => { plugin.settings.codexPath = "codex"; const result = await plugin.runtime.withClient((client) => client.readEffectiveConfig("/vault") as Promise, { - unhandledServerRequestMessage: "Settings refresh does not handle server requests.", + serverRequests: { kind: "reject", message: "Settings refresh does not handle server requests." }, }); expect(result).toEqual({}); expect(runWithAppServerClient).not.toHaveBeenCalled(); expect(withShortLivedAppServerClientMock).toHaveBeenCalledWith("codex", "/vault", expect.any(Function), { - unhandledServerRequestMessage: "Settings refresh does not handle server requests.", + serverRequests: { kind: "reject", message: "Settings refresh does not handle server requests." }, }); expect(shortLivedClient.readEffectiveConfig).toHaveBeenCalledWith("/vault"); }); diff --git a/tests/settings/settings-lifecycle.test.ts b/tests/settings/settings-lifecycle.test.ts index e7b340cc..dce8305b 100644 --- a/tests/settings/settings-lifecycle.test.ts +++ b/tests/settings/settings-lifecycle.test.ts @@ -2,30 +2,9 @@ import { describe, expect, it, vi } from "vitest"; import type { AppServerClient } from "../../src/app-server/connection/client"; import { loadHookData } from "../../src/settings/app-server-data"; -import { - createSettingsDynamicSectionLifecycle, - transitionSettingsDynamicSectionLifecycle, - transitionSettingsDataRefreshLifecycle, -} from "../../src/settings/lifecycle"; +import { createSettingsDynamicSectionLifecycle, transitionSettingsDynamicSectionLifecycle } from "../../src/settings/lifecycle"; describe("settings lifecycle", () => { - it("tracks settings data refresh lifecycle", () => { - const idle = { kind: "idle" } as const; - - const loading = transitionSettingsDataRefreshLifecycle(idle, { type: "started", operationToken: 1 }); - expect(loading).toEqual({ kind: "loading", operationToken: 1 }); - expect(transitionSettingsDataRefreshLifecycle(loading, { type: "started", operationToken: 0 })).toBe(loading); - - const newerLoading = transitionSettingsDataRefreshLifecycle(loading, { type: "started", operationToken: 2 }); - expect(newerLoading).toEqual({ kind: "loading", operationToken: 2 }); - expect(transitionSettingsDataRefreshLifecycle(newerLoading, { type: "completed", failedCount: 2, operationToken: 1 })).toBe( - newerLoading, - ); - - const completed = transitionSettingsDataRefreshLifecycle(newerLoading, { type: "completed", failedCount: 2, operationToken: 2 }); - expect(completed).toEqual({ kind: "completed", failedCount: 2, operationToken: 2 }); - }); - it("tracks dynamic section lifecycle", () => { const idle = createSettingsDynamicSectionLifecycle(); expect(idle).toEqual({ kind: "idle", status: "" }); diff --git a/tests/settings/settings-tab.test.ts b/tests/settings/settings-tab.test.ts index 0038bf05..675e8c93 100644 --- a/tests/settings/settings-tab.test.ts +++ b/tests/settings/settings-tab.test.ts @@ -4,6 +4,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { CatalogHookMetadata, CatalogModel } from "../../src/app-server/protocol/catalog"; import type { AppServerObservedQueryResult } from "../../src/app-server/query/cache"; +import type { AppServerClientAccessOptions } from "../../src/app-server/connection/client-access"; import type { ThreadRecord } from "../../src/app-server/protocol/thread"; import type { ModelMetadata, ReasoningEffort } from "../../src/domain/catalog/metadata"; import { modelMetadataFromCatalogModels } from "../../src/app-server/protocol/catalog"; @@ -1060,7 +1061,7 @@ function settingsTabHost( settings, vaultPath: "/vault", clientAccess: { - withClient: (operation: (client: never) => Promise, clientOptions?: { unhandledServerRequestMessage?: string }) => + withClient: (operation: (client: never) => Promise, clientOptions?: AppServerClientAccessOptions) => withShortLivedAppServerClientMock(settings.codexPath, "/vault", operation, clientOptions) as Promise, }, saveSettings: options.saveSettings ?? vi.fn().mockResolvedValue(undefined),