diff --git a/src/features/chat/chat-session-controller.ts b/src/features/chat/chat-session-controller.ts index 5b709dba..9668170c 100644 --- a/src/features/chat/chat-session-controller.ts +++ b/src/features/chat/chat-session-controller.ts @@ -18,6 +18,10 @@ export interface ChatSessionControllerHost { forceMessagesToBottom: () => void; } +export interface RefreshCapabilityDiagnosticsOptions { + cachedSessionMetadata?: boolean; +} + export class ChatSessionController { constructor(private readonly host: ChatSessionControllerHost) {} @@ -33,9 +37,7 @@ export class ChatSessionController { const client = this.host.currentClient(); if (!client) return; this.host.state.effectiveConfig = await client.readEffectiveConfig(this.host.vaultPath); - await this.refreshModels(); - await this.refreshSkills(); - await this.refreshRateLimits(); + await Promise.all([this.refreshModels(), this.refreshSkills(), this.refreshRateLimits()]); } async startThread(): Promise> | null> { @@ -64,8 +66,10 @@ export class ChatSessionController { try { const response = await client.listModels(false); this.host.state.availableModels = response.data; - } catch { + this.host.state.appServerDiagnostics.probes["model/list"] = capabilityProbeOk("model/list", `${String(response.data.length)} models`); + } catch (error) { this.host.state.availableModels = []; + this.host.state.appServerDiagnostics.probes["model/list"] = capabilityProbeError("model/list", error); } } @@ -75,8 +79,11 @@ export class ChatSessionController { try { const response = await client.listSkills(this.host.vaultPath, forceReload); this.host.state.availableSkills = response.data.flatMap((entry) => entry.skills).filter((skill) => skill.enabled); - } catch { + const count = response.data.reduce((total, entry) => total + entry.skills.length, 0); + this.host.state.appServerDiagnostics.probes["skills/list"] = capabilityProbeOk("skills/list", `${String(count)} skills`); + } catch (error) { this.host.state.availableSkills = []; + this.host.state.appServerDiagnostics.probes["skills/list"] = capabilityProbeError("skills/list", error); } } @@ -88,29 +95,45 @@ export class ChatSessionController { const rateLimitsByLimitId = response.rateLimitsByLimitId; const codexRateLimit = rateLimitsByLimitId && Object.hasOwn(rateLimitsByLimitId, "codex") ? rateLimitsByLimitId["codex"] : undefined; this.host.state.rateLimit = codexRateLimit ?? response.rateLimits; - } catch { + this.host.state.appServerDiagnostics.probes["account/rateLimits/read"] = capabilityProbeOk( + "account/rateLimits/read", + response.rateLimitsByLimitId ? `${String(Object.keys(response.rateLimitsByLimitId).length)} limits` : "available", + ); + } catch (error) { this.host.state.rateLimit = null; + this.host.state.appServerDiagnostics.probes["account/rateLimits/read"] = capabilityProbeError("account/rateLimits/read", error); } } - async refreshCapabilityDiagnostics(): Promise { + async refreshCapabilityDiagnostics(options: RefreshCapabilityDiagnosticsOptions = {}): Promise { const client = this.host.currentClient(); if (!client) return; - await Promise.all([ - this.probeCapability( - "model/list", - () => client.listModels(false), - (response) => `${String(response.data.length)} models`, - ), - this.probeCapability( - "skills/list", - () => client.listSkills(this.host.vaultPath), - (response) => { - const count = response.data.reduce((total, entry) => total + entry.skills.length, 0); - return `${String(count)} skills`; - }, - ), + const probes: Promise[] = []; + if (!options.cachedSessionMetadata) { + probes.push( + this.probeCapability( + "model/list", + () => client.listModels(false), + (response) => `${String(response.data.length)} models`, + ), + this.probeCapability( + "skills/list", + () => client.listSkills(this.host.vaultPath), + (response) => { + const count = response.data.reduce((total, entry) => total + entry.skills.length, 0); + return `${String(count)} skills`; + }, + ), + this.probeCapability( + "account/rateLimits/read", + () => client.readAccountRateLimits(), + (response) => (response.rateLimitsByLimitId ? `${String(Object.keys(response.rateLimitsByLimitId).length)} limits` : "available"), + ), + ); + } + + probes.push( this.probeCapability( "hooks/list", () => client.listHooks(this.host.vaultPath), @@ -119,11 +142,6 @@ export class ChatSessionController { return `${String(count)} hooks`; }, ), - this.probeCapability( - "account/rateLimits/read", - () => client.readAccountRateLimits(), - (response) => (response.rateLimitsByLimitId ? `${String(Object.keys(response.rateLimitsByLimitId).length)} limits` : "available"), - ), this.probeCapability( "mcpServerStatus/list", () => client.listMcpServerStatus(), @@ -152,7 +170,9 @@ export class ChatSessionController { .filter(Boolean) .join(", ") || "no optional capabilities", ), - ]); + ); + + await Promise.all(probes); } recordMcpStartupStatus(name: string, startupStatus: "starting" | "ready" | "failed" | "cancelled", message: string | null): void { diff --git a/src/features/chat/view.ts b/src/features/chat/view.ts index f33e7d77..d0e01011 100644 --- a/src/features/chat/view.ts +++ b/src/features/chat/view.ts @@ -95,7 +95,11 @@ export class CodexChatView extends ItemView { private configSlotEl: HTMLElement | null = null; private messagesSlotEl: HTMLElement | null = null; private composerSlotEl: HTMLElement | null = null; + private scheduledConnectionTimer: number | null = null; private scheduledRenderTimer: number | null = null; + private scheduledDiagnosticsTimer: number | null = null; + private connectingPromise: Promise | null = null; + private connectionGeneration = 0; private toolbarSignature: string | null = null; private forceScrollMessagesToBottomOnNextRender = false; @@ -245,14 +249,18 @@ export class CodexChatView extends ItemView { this.closeToolbarPanelOnOutsidePointer(event); }); this.render(); - await this.ensureConnected(); + this.scheduleDeferredConnection(); } override async onClose(): Promise { + this.connectionGeneration += 1; + this.connectingPromise = null; + this.clearDeferredConnection(); if (this.scheduledRenderTimer !== null) { this.containerEl.win.clearTimeout(this.scheduledRenderTimer); this.scheduledRenderTimer = null; } + this.clearDeferredDiagnostics(); this.connection.disconnect(); this.client = null; } @@ -261,29 +269,58 @@ export class CodexChatView extends ItemView { this.composerController.setDraft(text, { focus: true, renderIfDetached: true }); } + async connect(): Promise { + this.clearDeferredConnection(); + await this.ensureConnected(); + } + private async ensureConnected(): Promise { if (this.connection.isConnected()) { this.client = this.connection.currentClient(); return; } + if (this.connectingPromise) return this.connectingPromise; + const generation = this.connectionGeneration; + const promise = this.initializeConnection(generation); + this.connectingPromise = promise; + try { + await promise; + } finally { + if (this.connectingPromise === promise) { + this.connectingPromise = null; + } + } + } + + private async initializeConnection(generation: number): Promise { this.setStatus("Starting Codex app-server..."); try { this.state.initializeResponse = await this.connection.connect(); + if (this.isStaleConnectionGeneration(generation)) return; this.client = this.connection.currentClient(); if (!this.client) throw new Error("Codex app-server connection did not initialize."); await this.session.refreshSessionMetadata(); - await this.session.refreshCapabilityDiagnostics(); + if (this.isStaleConnectionGeneration(generation)) return; await this.session.refreshThreadList(); + if (this.isStaleConnectionGeneration(generation)) return; + this.scheduleDeferredDiagnostics(); this.refreshTabHeader(); this.setStatus("Connected."); } catch (error) { + if (this.isStaleConnectionGeneration(generation)) return; if (error instanceof StaleConnectionError) return; this.setStatus("Connection failed."); this.addSystemMessage(error instanceof Error ? error.message : String(error)); new Notice("Codex app-server connection failed."); } - this.scheduleRender(); + if (!this.isStaleConnectionGeneration(generation)) { + this.scheduleRender(); + } + } + + private isStaleConnectionGeneration(generation: number): boolean { + return generation !== this.connectionGeneration; } async startNewThread(): Promise { @@ -321,10 +358,11 @@ export class CodexChatView extends ItemView { } private async refreshDiagnostics(): Promise { - const alreadyConnected = this.connection.isConnected(); + this.clearDeferredDiagnostics(); await this.ensureConnected(); if (!this.client) return; - if (alreadyConnected) await this.session.refreshCapabilityDiagnostics(); + this.clearDeferredDiagnostics(); + await this.session.refreshCapabilityDiagnostics(); this.render(); } @@ -916,6 +954,10 @@ export class CodexChatView extends ItemView { private async reconnectFromToolbar(): Promise { const threadId = this.state.activeThreadId; this.state.openDetails.delete("status-panel"); + this.connectionGeneration += 1; + this.connectingPromise = null; + this.clearDeferredConnection(); + this.clearDeferredDiagnostics(); this.connection.reconnect(); this.client = null; this.state.busy = false; @@ -1013,6 +1055,40 @@ export class CodexChatView extends ItemView { }, 50); } + private scheduleDeferredConnection(): void { + if (this.scheduledConnectionTimer !== null || this.connection.isConnected()) return; + this.scheduledConnectionTimer = this.containerEl.win.setTimeout(() => { + this.scheduledConnectionTimer = null; + void this.ensureConnected(); + }, 1_500); + } + + private clearDeferredConnection(): void { + if (this.scheduledConnectionTimer === null) return; + this.containerEl.win.clearTimeout(this.scheduledConnectionTimer); + this.scheduledConnectionTimer = null; + } + + private scheduleDeferredDiagnostics(): void { + if (this.scheduledDiagnosticsTimer !== null) return; + this.scheduledDiagnosticsTimer = this.containerEl.win.setTimeout(() => { + this.scheduledDiagnosticsTimer = null; + void this.refreshDeferredDiagnostics(); + }, 1_000); + } + + private clearDeferredDiagnostics(): void { + if (this.scheduledDiagnosticsTimer === null) return; + this.containerEl.win.clearTimeout(this.scheduledDiagnosticsTimer); + this.scheduledDiagnosticsTimer = null; + } + + private async refreshDeferredDiagnostics(): Promise { + if (!this.connection.isConnected()) return; + await this.session.refreshCapabilityDiagnostics({ cachedSessionMetadata: true }); + this.render(); + } + private renderShell(root: HTMLElement): void { root.empty(); root.addClass("codex-panel"); diff --git a/src/main.ts b/src/main.ts index 3b59f346..58a2b616 100644 --- a/src/main.ts +++ b/src/main.ts @@ -54,7 +54,9 @@ export default class CodexPanelPlugin extends Plugin { active: true, reveal: true, }); - return leaf.view as CodexChatView; + const view = leaf.view as CodexChatView; + await view.connect(); + return view; } async activateNewView(): Promise { @@ -63,7 +65,9 @@ export default class CodexPanelPlugin extends Plugin { await leaf.setViewState({ type: VIEW_TYPE_CODEX_PANEL, active: true }); await this.app.workspace.revealLeaf(leaf); - return leaf.view as CodexChatView; + const view = leaf.view as CodexChatView; + await view.connect(); + return view; } async openThreadInNewView(threadId: string): Promise { diff --git a/tests/features/chat/chat-session-controller.test.ts b/tests/features/chat/chat-session-controller.test.ts new file mode 100644 index 00000000..77e671f1 --- /dev/null +++ b/tests/features/chat/chat-session-controller.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it, vi } from "vitest"; + +import type { AppServerClient } from "../../../src/app-server/client"; +import { ChatSessionController } from "../../../src/features/chat/chat-session-controller"; +import { createChatState } from "../../../src/features/chat/chat-state"; +import type { Model } from "../../../src/generated/app-server/v2/Model"; +import type { RateLimitSnapshot } from "../../../src/generated/app-server/v2/RateLimitSnapshot"; +import type { SkillMetadata } from "../../../src/generated/app-server/v2/SkillMetadata"; + +describe("ChatSessionController", () => { + it("reuses cached session metadata for deferred diagnostics", async () => { + const state = createChatState(); + + const listModels = vi.fn().mockResolvedValue({ data: [{ model: "gpt-5.1" } as Model] }); + const listSkills = vi.fn().mockResolvedValue({ data: [{ skills: [{ name: "writer", enabled: true } as SkillMetadata] }] }); + const readAccountRateLimits = vi.fn().mockResolvedValue({ rateLimits: {} as RateLimitSnapshot }); + const listHooks = vi.fn().mockResolvedValue({ data: [{ cwd: "/vault", hooks: [] }] }); + const client = { + readEffectiveConfig: vi.fn().mockResolvedValue({}), + listModels, + listSkills, + readAccountRateLimits, + listHooks, + listMcpServerStatus: vi.fn().mockResolvedValue({ data: [] }), + listCollaborationModes: vi.fn().mockResolvedValue({ data: [] }), + readModelProviderCapabilities: vi.fn().mockResolvedValue({}), + } as unknown as AppServerClient; + + const controller = new ChatSessionController({ + state, + vaultPath: "/vault", + currentClient: () => client, + runtimeSnapshot: () => ({}) as never, + forceMessagesToBottom: () => undefined, + }); + + await controller.refreshSessionMetadata(); + listModels.mockClear(); + listSkills.mockClear(); + readAccountRateLimits.mockClear(); + + await controller.refreshCapabilityDiagnostics({ cachedSessionMetadata: true }); + + expect(listModels).not.toHaveBeenCalled(); + expect(listSkills).not.toHaveBeenCalled(); + expect(readAccountRateLimits).not.toHaveBeenCalled(); + expect(listHooks).toHaveBeenCalledWith("/vault"); + expect(state.appServerDiagnostics.probes["model/list"]).toMatchObject({ + status: "ok", + summary: "1 models", + }); + expect(state.appServerDiagnostics.probes["skills/list"]).toMatchObject({ + status: "ok", + summary: "1 skills", + }); + expect(state.appServerDiagnostics.probes["account/rateLimits/read"]).toMatchObject({ + status: "ok", + summary: "available", + }); + }); +}); diff --git a/tests/features/chat/view-connection.test.ts b/tests/features/chat/view-connection.test.ts new file mode 100644 index 00000000..2328cbc8 --- /dev/null +++ b/tests/features/chat/view-connection.test.ts @@ -0,0 +1,152 @@ +// @vitest-environment jsdom + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { DEFAULT_SETTINGS } from "../../../src/settings/model"; +import { notices } from "../../mocks/obsidian"; +import { installObsidianDomShims } from "./ui/dom-test-helpers"; + +const connectionMock = vi.hoisted(() => { + const state = { + client: null as Record | null, + connectCalls: 0, + connected: false, + }; + + return { + state, + reset(): void { + state.client = null; + state.connectCalls = 0; + state.connected = false; + }, + }; +}); + +vi.mock("../../../src/app-server/connection-manager", () => { + class StaleConnectionError extends Error {} + + class ConnectionManager { + connect(): Promise { + connectionMock.state.connectCalls += 1; + connectionMock.state.connected = true; + return Promise.resolve({ + userAgent: "codex-test", + codexHome: "/tmp/codex", + platformFamily: "unix", + platformOs: "macos", + }); + } + + currentClient(): unknown { + return connectionMock.state.connected ? connectionMock.state.client : null; + } + + isConnected(): boolean { + return connectionMock.state.connected; + } + + reconnect(): void { + this.disconnect(); + } + + disconnect(): void { + connectionMock.state.connected = false; + } + } + + return { ConnectionManager, StaleConnectionError }; +}); + +installObsidianDomShims(); + +describe("CodexChatView connection lifecycle", () => { + beforeEach(() => { + vi.useRealTimers(); + notices.length = 0; + connectionMock.reset(); + }); + + it("shares post-initialize metadata loading across concurrent connect calls", async () => { + const client = connectedClient(); + connectionMock.state.client = client; + const view = await chatView(); + + await Promise.all([view.connect(), view.connect()]); + + expect(connectionMock.state.connectCalls).toBe(1); + expect(client.readEffectiveConfig).toHaveBeenCalledTimes(1); + expect(client.listModels).toHaveBeenCalledTimes(1); + expect(client.listSkills).toHaveBeenCalledTimes(1); + expect(client.readAccountRateLimits).toHaveBeenCalledTimes(1); + expect(client.listThreads).toHaveBeenCalledTimes(1); + }); + + it("ignores stale connection work after the view closes", async () => { + let resolveConfig!: (value: unknown) => void; + const client = connectedClient({ + readEffectiveConfig: vi.fn( + () => + new Promise((resolve) => { + resolveConfig = resolve; + }), + ), + }); + connectionMock.state.client = client; + const view = await chatView(); + + const connecting = view.connect(); + await Promise.resolve(); + await view.onClose(); + resolveConfig({}); + await connecting; + + expect(notices).toEqual([]); + expect(client.listThreads).not.toHaveBeenCalled(); + }); +}); + +function connectedClient(overrides: Partial> = {}): ReturnType { + return { + ...baseClient(), + ...overrides, + }; +} + +function baseClient() { + return { + readEffectiveConfig: vi.fn().mockResolvedValue({}), + listModels: vi.fn().mockResolvedValue({ data: [] }), + listSkills: vi.fn().mockResolvedValue({ data: [] }), + readAccountRateLimits: vi.fn().mockResolvedValue({ rateLimits: null }), + listThreads: vi.fn().mockResolvedValue({ data: [] }), + }; +} + +async function chatView() { + const { CodexChatView } = await import("../../../src/features/chat/view"); + const containerEl = document.createElement("div"); + containerEl.createDiv(); + containerEl.createDiv(); + return new CodexChatView( + { + app: { + vault: { + on: vi.fn(() => ({})), + }, + }, + containerEl, + } as never, + { + settings: { + ...DEFAULT_SETTINGS, + codexPath: "codex", + sendShortcut: "enter", + }, + vaultPath: "/vault", + openThreadInNewView: vi.fn(), + openTurnDiff: vi.fn(), + refreshOpenThreadLists: vi.fn(), + }, + ); +} diff --git a/tests/mocks/obsidian.ts b/tests/mocks/obsidian.ts index 1d15b495..4a56f127 100644 --- a/tests/mocks/obsidian.ts +++ b/tests/mocks/obsidian.ts @@ -44,6 +44,29 @@ export class Notice { } } +export class ItemView { + readonly app: App; + readonly containerEl: HTMLElement; + + constructor(readonly leaf: { app?: App; containerEl?: HTMLElement }) { + ensureElementHelpers(); + this.app = leaf.app ?? {}; + this.containerEl = leaf.containerEl ?? document.createElement("div"); + } + + registerDomEvent(element: Document, type: K, callback: (event: DocumentEventMap[K]) => void): void { + element.addEventListener(type, callback); + } + + registerEvent(_eventRef: unknown): void { + // Test mock placeholder. + } +} + +export class MarkdownView { + file: TFile | null = null; +} + export class PluginSettingTab { containerEl: HTMLElement;