diff --git a/src/app-server/query/keys.ts b/src/app-server/query/keys.ts index 396564c0..4232d0ce 100644 --- a/src/app-server/query/keys.ts +++ b/src/app-server/query/keys.ts @@ -18,13 +18,12 @@ export function cloneAppServerQueryContext(context: AppServerQueryContext): AppS return { ...context }; } +export function appServerQueryContextRawEquals(left: AppServerQueryContext, right: AppServerQueryContext): boolean { + return left.codexPath === right.codexPath && left.vaultPath === right.vaultPath; +} + export function appServerQueryContextMatches(left: AppServerQueryContext, right: AppServerQueryContext): boolean { - return ( - appServerQueryContextIsComplete(left) && - appServerQueryContextIsComplete(right) && - left.codexPath === right.codexPath && - left.vaultPath === right.vaultPath - ); + return appServerQueryContextIsComplete(left) && appServerQueryContextIsComplete(right) && appServerQueryContextRawEquals(left, right); } function appServerQueryScope(context: AppServerQueryContext): AppServerQueryScope { diff --git a/src/app-server/query/shared-queries.ts b/src/app-server/query/shared-queries.ts index 56ffb9a0..a5c86b4b 100644 --- a/src/app-server/query/shared-queries.ts +++ b/src/app-server/query/shared-queries.ts @@ -1,7 +1,12 @@ import type { ModelMetadata } from "../../domain/catalog/metadata"; import type { Thread } from "../../domain/threads/model"; import type { AppServerObservedQueryResult, AppServerQueryCache } from "./cache"; -import { appServerQueryContextMatches, cloneAppServerQueryContext, type AppServerQueryContext } from "./keys"; +import { + appServerQueryContextMatches, + appServerQueryContextRawEquals, + cloneAppServerQueryContext, + type AppServerQueryContext, +} from "./keys"; import type { SharedServerMetadata } from "./snapshots"; export interface AppServerSharedQueriesOptions { @@ -9,6 +14,17 @@ export interface AppServerSharedQueriesOptions { context: () => AppServerQueryContext; } +export class StaleAppServerSharedQueryContextError extends Error { + constructor() { + super("Codex app-server query context changed while loading shared data."); + this.name = "StaleAppServerSharedQueryContextError"; + } +} + +export function isStaleAppServerSharedQueryContextError(error: unknown): error is StaleAppServerSharedQueryContextError { + return error instanceof StaleAppServerSharedQueryContextError; +} + export class AppServerSharedQueries { private readonly contextChangeListeners = new Set<() => void>(); @@ -19,11 +35,11 @@ export class AppServerSharedQueries { } fetchActiveThreads(): Promise { - return this.options.cache.fetchActiveThreads(this.context()); + return this.runForCurrentContext((context) => this.options.cache.fetchActiveThreads(context)); } refreshActiveThreads(): Promise { - return this.options.cache.refreshActiveThreads(this.context()); + return this.runForCurrentContext((context) => this.options.cache.refreshActiveThreads(context)); } setActiveThreads(threads: readonly Thread[]): void { @@ -54,11 +70,11 @@ export class AppServerSharedQueries { } fetchAppServerMetadata(): Promise { - return this.options.cache.fetchAppServerMetadata(this.context()); + return this.runForCurrentContext((context) => this.options.cache.fetchAppServerMetadata(context)); } refreshAppServerMetadata(options: { forceSkills?: boolean } = {}): Promise { - return this.options.cache.refreshAppServerMetadata(this.context(), options); + return this.runForCurrentContext((context) => this.options.cache.refreshAppServerMetadata(context, options)); } observeAppServerMetadataResult( @@ -78,11 +94,11 @@ export class AppServerSharedQueries { } fetchModels(): Promise { - return this.options.cache.fetchModels(this.context()); + return this.runForCurrentContext((context) => this.options.cache.fetchModels(context)); } refreshModels(): Promise { - return this.options.cache.refreshModels(this.context()); + return this.runForCurrentContext((context) => this.options.cache.refreshModels(context)); } observeModelsResult( @@ -106,6 +122,15 @@ export class AppServerSharedQueries { return this.options.context(); } + private async runForCurrentContext(operation: (context: AppServerQueryContext) => Promise): Promise { + const context = cloneAppServerQueryContext(this.context()); + const result = await operation(context); + if (!appServerQueryContextRawEquals(this.context(), context)) { + throw new StaleAppServerSharedQueryContextError(); + } + return result; + } + private observeCurrentContext( observe: (context: AppServerQueryContext, listener: (value: T) => void, options: { emitCurrent?: boolean }) => () => void, listener: (value: T) => void, diff --git a/src/features/chat/app-server/actions/metadata.ts b/src/features/chat/app-server/actions/metadata.ts index fda022fd..826832c1 100644 --- a/src/features/chat/app-server/actions/metadata.ts +++ b/src/features/chat/app-server/actions/metadata.ts @@ -4,6 +4,7 @@ import { type RateLimitMetadataProbeResult, type SkillMetadataProbeResult, } from "../../../../app-server/query/metadata-probes"; +import { isStaleAppServerSharedQueryContextError } from "../../../../app-server/query/shared-queries"; import { diagnosticsWithProbe } from "../../../../domain/server/diagnostics"; import type { SharedServerMetadata } from "../../../../domain/server/metadata"; import { cloneServerDiagnostics, type ChatServerActionHost } from "./host"; @@ -67,7 +68,13 @@ async function loadAppServerMetadata(host: ChatServerMetadataActionsHost): Promi } async function refreshAppServerMetadata(host: ChatServerMetadataActionsHost): Promise { - const metadata = await host.refreshAppServerMetadata(); + let metadata: SharedServerMetadata | null; + try { + metadata = await host.refreshAppServerMetadata(); + } catch (error) { + if (isStaleAppServerSharedQueryContextError(error)) return null; + throw error; + } if (!metadata) return null; applyAppServerMetadata(host, metadata); return metadata; diff --git a/src/features/chat/application/connection/connection-controller.ts b/src/features/chat/application/connection/connection-controller.ts index 16c24c98..8b59663d 100644 --- a/src/features/chat/application/connection/connection-controller.ts +++ b/src/features/chat/application/connection/connection-controller.ts @@ -1,5 +1,6 @@ import { StaleConnectionError } from "../../../../app-server/connection/connection-manager"; import type { AppServerClient } from "../../../../app-server/connection/client"; +import { isStaleAppServerSharedQueryContextError } from "../../../../app-server/query/shared-queries"; import type { ServerInitialization } from "../../../../domain/server/initialization"; import type { ActiveConnectionWork, ConnectionWorkTracker } from "../../../../shared/lifecycle/connection-work"; import type { ChatConnectionPhase } from "../state/root-reducer"; @@ -95,6 +96,7 @@ export class ChatConnectionController { await this.host.loadSharedThreadList(); this.host.refreshTabHeader(); } catch (error) { + if (isStaleAppServerSharedQueryContextError(error)) return; this.host.addSystemMessage(error instanceof Error ? error.message : String(error)); } } @@ -140,6 +142,7 @@ export class ChatConnectionController { } catch (error) { if (this.host.connectionWork.isStale(connection)) return; if (error instanceof StaleConnectionError) return; + if (isStaleAppServerSharedQueryContextError(error)) return; const message = connectionErrorMessage(error, this.host.configuredCommand()); this.host.setStatus(STATUS_CONNECTION_FAILED, { kind: "failed", message }); this.host.addSystemMessage(message); diff --git a/src/features/chat/host/session.ts b/src/features/chat/host/session.ts index 6a1805d6..a9457cba 100644 --- a/src/features/chat/host/session.ts +++ b/src/features/chat/host/session.ts @@ -3,6 +3,8 @@ import { Notice } from "obsidian"; import type { AppServerClient } from "../../../app-server/connection/client"; import { ConnectionManager } from "../../../app-server/connection/connection-manager"; import type { AppServerObservedQueryResult } from "../../../app-server/query/cache"; +import { isStaleAppServerSharedQueryContextError } from "../../../app-server/query/shared-queries"; +import { appServerQueryContextRawEquals, type AppServerQueryContext } from "../../../app-server/query/keys"; import { renameThreadOnAppServer, threadRenameFromValue } from "../../../app-server/services/thread-rename"; import type { ModelMetadata } from "../../../domain/catalog/metadata"; @@ -71,25 +73,6 @@ import { currentModel, runtimeConfigOrDefault } from "../domain/runtime/effectiv import type { ChatSurfaceHandle } from "./surface-handle"; import type { ChatPanelEnvironment } from "./runtime"; -interface ChatPanelWarmupHost { - deferredTasks: ChatViewDeferredTasks; - opened: () => boolean; - closing: () => boolean; - connected: () => boolean; - ensureConnected: () => Promise; -} - -function scheduleChatPanelWarmup(host: ChatPanelWarmupHost): void { - const shouldWarmup = (): boolean => host.opened() && !host.connected(); - - if (!shouldWarmup()) return; - - host.deferredTasks.scheduleAppServerWarmup(() => { - if (!shouldWarmup() || host.closing()) return; - void host.ensureConnected(); - }); -} - function codexPanelDisplayTitle(activeThreadId: string | null, threads: readonly Thread[], fallbackTitle?: string | null): string { if (!activeThreadId) return "Codex"; @@ -192,10 +175,12 @@ export class ChatPanelSession implements ChatSurfaceHandle { private readonly messageScrollIntent: ChatMessageScrollIntentState = createChatMessageScrollIntentState(); private readonly localItemIds: LocalChatItemIdFactory = createLocalChatItemIdFactory(); private readonly appServerStateUnsubscribers: (() => void)[] = []; + private observedAppServerContext: AppServerQueryContext; private opened = false; private closing = false; constructor(private readonly environment: ChatPanelEnvironment) { + this.observedAppServerContext = this.currentAppServerContext(); this.deferredTasks = createChatViewDeferredTasks(() => this.viewWindow()); this.parts = this.createSessionParts(); } @@ -853,10 +838,7 @@ export class ChatPanelSession implements ChatSurfaceHandle { void goalSync.syncThreadGoal(threadId); }, }); - const loadSharedThreadList = async (): Promise => { - const threads = await environment.plugin.threadCatalog.refreshActiveThreads(); - serverThreads.applyThreadList(threads); - }; + const loadSharedThreadList = () => this.loadSharedThreadList(); const serverRequestHost = { currentClient }; const inboundController = new ChatInboundController(stateStore, { fetchActiveThreads: () => { @@ -1020,6 +1002,14 @@ export class ChatPanelSession implements ChatSurfaceHandle { } refreshSettings(): void { + const nextContext = this.currentAppServerContext(); + if (!appServerQueryContextRawEquals(this.observedAppServerContext, nextContext)) { + this.observedAppServerContext = nextContext; + this.connectionWork.invalidate(); + this.invalidateResumeWork(); + this.parts.connection.manager.resetConnection(); + this.applyCachedAppServerState(); + } this.mountOrRepairShell(); } @@ -1210,12 +1200,12 @@ export class ChatPanelSession implements ChatSurfaceHandle { } private scheduleWarmup(): void { - scheduleChatPanelWarmup({ - deferredTasks: this.deferredTasks, - opened: () => this.opened, - closing: () => this.closing, - connected: () => this.parts.connection.manager.isConnected(), - ensureConnected: () => this.parts.connection.controller.ensureConnected(), + const shouldWarmup = (): boolean => this.opened && !this.parts.connection.manager.isConnected(); + if (!shouldWarmup()) return; + + this.deferredTasks.scheduleAppServerWarmup(() => { + if (!shouldWarmup() || this.closing) return; + void this.parts.connection.controller.ensureConnected(); }); } @@ -1225,8 +1215,13 @@ export class ChatPanelSession implements ChatSurfaceHandle { } private async loadSharedThreadList(): Promise { - const threads = await this.environment.plugin.threadCatalog.refreshActiveThreads(); - this.parts.serverActions.threads.applyThreadList(threads); + try { + const threads = await this.environment.plugin.threadCatalog.refreshActiveThreads(); + this.parts.serverActions.threads.applyThreadList(threads); + } catch (error) { + if (isStaleAppServerSharedQueryContextError(error)) return; + throw error; + } } private notifyActiveThreadIdentityChanged(): void { @@ -1252,6 +1247,13 @@ export class ChatPanelSession implements ChatSurfaceHandle { return this.environment.view.viewWindow() ?? window; } + private currentAppServerContext(): AppServerQueryContext { + return { + codexPath: this.environment.plugin.settingsRef.settings.codexPath, + vaultPath: this.environment.plugin.settingsRef.vaultPath, + }; + } + private closeToolbarPanelOnOutsidePointer(event: PointerEvent): void { this.parts.toolbar.panels.closeOnOutsidePointer({ target: event.target, diff --git a/src/features/threads-view/session.ts b/src/features/threads-view/session.ts index 1dc409f2..60790e60 100644 --- a/src/features/threads-view/session.ts +++ b/src/features/threads-view/session.ts @@ -2,6 +2,7 @@ import { Notice } from "obsidian"; import type { AppServerClient } from "../../app-server/connection/client"; import type { AppServerObservedQueryResult } from "../../app-server/query/cache"; +import { isStaleAppServerSharedQueryContextError } from "../../app-server/query/shared-queries"; import { ConnectionManager, type ConnectionManagerHandlers, StaleConnectionError } from "../../app-server/connection/connection-manager"; import type { Thread } from "../../domain/threads/model"; import type { CodexPanelSettings } from "../../settings/model"; @@ -161,6 +162,7 @@ export class CodexThreadsSession { this.status = threads.length === 0 ? { kind: "empty", message: "No threads" } : { kind: "idle" }; } catch (error) { if (error instanceof StaleConnectionError) return; + if (isStaleAppServerSharedQueryContextError(error)) return; this.status = { kind: "error", message: error instanceof Error ? error.message : String(error) }; } finally { this.finishRefresh(refresh); diff --git a/src/settings/dynamic-data-controller.ts b/src/settings/dynamic-data-controller.ts index d442491d..16bbd73c 100644 --- a/src/settings/dynamic-data-controller.ts +++ b/src/settings/dynamic-data-controller.ts @@ -1,5 +1,6 @@ import type { AppServerClient } from "../app-server/connection/client"; import type { AppServerObservedQueryResult } from "../app-server/query/cache"; +import { isStaleAppServerSharedQueryContextError } from "../app-server/query/shared-queries"; import { withShortLivedAppServerClient } from "../app-server/connection/short-lived-client"; import { setHookItemEnabled, trustHookItem } from "../app-server/catalog/data"; import { restoreArchivedThread as restoreArchivedThreadOnAppServer } from "../app-server/threads/data"; @@ -153,6 +154,8 @@ export class SettingsDynamicDataController { status: `Loaded ${String(modelsResult.value.length)} model${modelsResult.value.length === 1 ? "" : "s"}.`, operationId, }); + } else if (isStaleAppServerSharedQueryContextError(modelsResult.reason)) { + return; } else { failedCount += 1; this.modelsLifecycle = transitionSettingsDynamicSectionLifecycle(this.modelsLifecycle, { diff --git a/tests/app-server/shared-queries.test.ts b/tests/app-server/shared-queries.test.ts new file mode 100644 index 00000000..9d19000f --- /dev/null +++ b/tests/app-server/shared-queries.test.ts @@ -0,0 +1,206 @@ +import { describe, expect, it, vi } from "vitest"; + +import type { AppServerObservedQueryResult, AppServerQueryCache } from "../../src/app-server/query/cache"; +import { AppServerSharedQueries, StaleAppServerSharedQueryContextError } from "../../src/app-server/query/shared-queries"; +import type { ModelMetadata } from "../../src/domain/catalog/metadata"; +import { createServerDiagnostics, diagnosticProbeOk, diagnosticsWithProbe } from "../../src/domain/server/diagnostics"; +import type { SharedServerMetadata } from "../../src/domain/server/metadata"; +import type { Thread } from "../../src/domain/threads/model"; +import { deferred } from "../support/async"; + +describe("AppServerSharedQueries", () => { + it("rejects active thread refreshes when the app-server query context changes while loading", async () => { + const context = { codexPath: "codex-a", vaultPath: "/vault" }; + const pending = deferred(); + const queries = new AppServerSharedQueries({ + cache: cacheWith({ + refreshActiveThreads: vi.fn(() => pending.promise), + }), + context: () => context, + }); + + const refresh = queries.refreshActiveThreads(); + context.codexPath = "codex-b"; + pending.resolve([thread("stale")]); + + await expect(refresh).rejects.toBeInstanceOf(StaleAppServerSharedQueryContextError); + }); + + it("rejects metadata refreshes when the app-server query context changes while loading", async () => { + const context = { codexPath: "codex-a", vaultPath: "/vault" }; + const pending = deferred(); + const queries = new AppServerSharedQueries({ + cache: cacheWith({ + refreshAppServerMetadata: vi.fn(() => pending.promise), + }), + context: () => context, + }); + + const refresh = queries.refreshAppServerMetadata(); + context.vaultPath = "/other-vault"; + pending.resolve(serverMetadata({ availableModels: [model("stale-model")] })); + + await expect(refresh).rejects.toBeInstanceOf(StaleAppServerSharedQueryContextError); + }); + + it("rejects model fetches when the app-server query context changes while loading", async () => { + const context = { codexPath: "codex-a", vaultPath: "/vault" }; + const pending = deferred(); + const queries = new AppServerSharedQueries({ + cache: cacheWith({ + fetchModels: vi.fn(() => pending.promise), + }), + context: () => context, + }); + + const fetch = queries.fetchModels(); + context.codexPath = "codex-b"; + pending.resolve([model("stale-model")]); + + await expect(fetch).rejects.toBeInstanceOf(StaleAppServerSharedQueryContextError); + }); + + it("does not notify stale active thread observers after the app-server query context changes", async () => { + const context = { codexPath: "codex-a", vaultPath: "/vault" }; + const listener = vi.fn(); + const queries = new AppServerSharedQueries({ + cache: cacheWith({ + observeActiveThreadsResult: (_queryContext, queryListener) => { + observedThreadListener = queryListener; + return () => undefined; + }, + }), + context: () => context, + }); + let observedThreadListener!: (result: AppServerObservedQueryResult) => void; + + queries.observeActiveThreadsResult(listener); + context.codexPath = "codex-b"; + observedThreadListener(observedResult([thread("stale")])); + + expect(listener).not.toHaveBeenCalled(); + }); + + it("resubscribes observers when the app-server query context changes", () => { + const context = { codexPath: "codex-a", vaultPath: "/vault" }; + const listeners = new Map) => void>(); + const queries = new AppServerSharedQueries({ + cache: cacheWith({ + observeActiveThreadsResult: (queryContext, listener) => { + listeners.set(queryContext.codexPath, listener); + return () => undefined; + }, + }), + context: () => context, + }); + const listener = vi.fn(); + queries.observeActiveThreadsResult(listener); + + listeners.get("codex-a")?.(observedResult([thread("a")])); + context.codexPath = "codex-b"; + queries.notifyContextChanged(); + listeners.get("codex-b")?.(observedResult([thread("b")])); + + expect(listener).toHaveBeenLastCalledWith(expect.objectContaining({ data: [thread("b")] })); + }); + + it("publishes metadata and model snapshots to shared query observers", () => { + const metadata = serverMetadata({ availableModels: [model("gpt-test")] }); + const metadataListener = vi.fn(); + const modelListener = vi.fn(); + const queries = new AppServerSharedQueries({ + cache: cacheWith({ + appServerMetadataSnapshot: () => metadata, + updateAppServerMetadata: () => metadata, + modelsSnapshot: () => metadata.availableModels, + observeAppServerMetadataResult: (_context, listener) => { + metadataObserver = listener; + return () => undefined; + }, + observeModelsResult: (_context, listener) => { + modelObserver = listener; + return () => undefined; + }, + }), + context: () => ({ codexPath: "codex", vaultPath: "/vault" }), + }); + let metadataObserver!: (result: AppServerObservedQueryResult) => void; + let modelObserver!: (result: AppServerObservedQueryResult) => void; + + queries.observeAppServerMetadataResult(metadataListener); + queries.observeModelsResult(modelListener); + queries.updateAppServerMetadata(() => metadata); + metadataObserver(observedResult(metadata)); + modelObserver(observedResult(metadata.availableModels)); + + expect(queries.appServerMetadataSnapshot()).toEqual(metadata); + expect(queries.modelsSnapshot()).toEqual(metadata.availableModels); + expect(metadataListener).toHaveBeenLastCalledWith(expect.objectContaining({ data: metadata })); + expect(modelListener).toHaveBeenCalledWith(expect.objectContaining({ data: metadata.availableModels })); + }); +}); + +function cacheWith(overrides: Partial): AppServerQueryCache { + return { + activeThreadsSnapshot: vi.fn(() => null), + fetchActiveThreads: vi.fn(() => Promise.resolve([])), + refreshActiveThreads: vi.fn(() => Promise.resolve([])), + setActiveThreads: vi.fn(), + updateActiveThreads: vi.fn(() => null), + observeActiveThreadsResult: vi.fn(() => () => undefined), + appServerMetadataSnapshot: vi.fn(() => null), + updateAppServerMetadata: vi.fn(() => null), + fetchAppServerMetadata: vi.fn(() => Promise.resolve(null)), + refreshAppServerMetadata: vi.fn(() => Promise.resolve(null)), + observeAppServerMetadataResult: vi.fn(() => () => undefined), + modelsSnapshot: vi.fn(() => null), + fetchModels: vi.fn(() => Promise.resolve([])), + refreshModels: vi.fn(() => Promise.resolve([])), + observeModelsResult: vi.fn(() => () => undefined), + ...overrides, + } as unknown as AppServerQueryCache; +} + +function observedResult(data: T): AppServerObservedQueryResult { + return { data, error: null } as AppServerObservedQueryResult; +} + +function thread(id: string): Thread { + return { + id, + preview: id, + createdAt: 1, + updatedAt: 1, + name: null, + archived: false, + }; +} + +function model(modelId: string): ModelMetadata { + return { + id: modelId, + model: modelId, + displayName: modelId, + description: "", + hidden: false, + supportedReasoningEfforts: [], + defaultReasoningEffort: null, + inputModalities: [], + additionalSpeedTiers: [], + serviceTiers: [], + defaultServiceTier: null, + isDefault: false, + }; +} + +function serverMetadata(overrides: Partial = {}): SharedServerMetadata { + const diagnostics = diagnosticsWithProbe(createServerDiagnostics(), diagnosticProbeOk("model/list", "0 models")); + return { + runtimeConfig: null, + availableModels: [], + availableSkills: [], + rateLimit: null, + serverDiagnostics: diagnostics, + ...overrides, + }; +} diff --git a/tests/features/chat/connection/server-actions/server-actions.test.ts b/tests/features/chat/connection/server-actions/server-actions.test.ts index 20bfb2fd..7caafc26 100644 --- a/tests/features/chat/connection/server-actions/server-actions.test.ts +++ b/tests/features/chat/connection/server-actions/server-actions.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import type { AppServerClient } from "../../../../../src/app-server/connection/client"; +import { StaleAppServerSharedQueryContextError } from "../../../../../src/app-server/query/shared-queries"; import { createServerDiagnostics, diagnosticProbeError, @@ -311,6 +312,25 @@ describe("chat server actions", () => { }); }); + it("ignores stale shared app-server metadata refreshes without applying state", async () => { + const stateStore = createChatStateStore(chatStateFixture()); + const metadata = createChatServerMetadataActions({ + stateStore, + vaultPath: "/vault", + currentClient: () => null, + ...metadataCacheHost({ current: null }), + fetchAppServerMetadata: async () => null, + refreshAppServerMetadata: async () => { + throw new StaleAppServerSharedQueryContextError(); + }, + }); + + await expect(metadata.refreshAppServerMetadata()).resolves.toBeNull(); + + expect(stateStore.getState().connection.availableModels).toEqual([]); + expect(stateStore.getState().connection.runtimeConfig).toBeNull(); + }); + it("uses metadata diagnostics as the default resource probe source", async () => { const stateStore = createChatStateStore(chatStateFixture()); const metadataCache = metadataCacheHost({ diff --git a/tests/features/chat/view-connection.test.ts b/tests/features/chat/view-connection.test.ts index c7cc6c00..3438adde 100644 --- a/tests/features/chat/view-connection.test.ts +++ b/tests/features/chat/view-connection.test.ts @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_SETTINGS } from "../../../src/settings/model"; import type { CodexChatHost } from "../../../src/features/chat/host/runtime"; import type { AppServerObservedQueryResult } from "../../../src/app-server/query/cache"; +import { StaleAppServerSharedQueryContextError } from "../../../src/app-server/query/shared-queries"; import { modelMetadataFromCatalogModels } from "../../../src/app-server/protocol/catalog"; import { createServerDiagnostics } from "../../../src/domain/server/diagnostics"; import type { Thread } from "../../../src/domain/threads/model"; @@ -189,6 +190,25 @@ describe("CodexChatView connection lifecycle", () => { }); }); + it("ignores stale shared thread refreshes after connecting", async () => { + const refreshActiveThreads = vi.fn().mockRejectedValue(new StaleAppServerSharedQueryContextError()); + connectionMock.state.client = connectedClient({ + listThreads: vi.fn(), + }); + const view = await chatView({ + host: chatHost({ refreshActiveThreads }), + }); + + await view.onOpen(); + await view.surface.connect(); + + expect(refreshActiveThreads).toHaveBeenCalledOnce(); + expect(view.surface.openPanelSnapshot()).toMatchObject({ connected: true }); + expect(notices).toEqual([]); + requiredButton(view.containerEl, '[aria-label="Show thread list"]').click(); + expect(view.containerEl.textContent).not.toContain("stale"); + }); + it("loads app-server metadata after connecting", async () => { connectionMock.state.client = connectedClient(); const view = await chatView(); @@ -199,6 +219,24 @@ describe("CodexChatView connection lifecycle", () => { expect(view.surface.openPanelSnapshot()).toMatchObject({ connected: true }); }); + it("resets the app-server connection only when settings change the app-server context", async () => { + connectionMock.state.client = connectedClient(); + const host = chatHost(); + const view = await chatView({ host }); + + await view.onOpen(); + await view.surface.connect(); + expect(view.surface.openPanelSnapshot()).toMatchObject({ connected: true }); + + host.settingsRef.settings.showToolbar = false; + view.surface.refreshSettings(); + expect(view.surface.openPanelSnapshot()).toMatchObject({ connected: true }); + + host.settingsRef.settings.codexPath = "codex-next"; + view.surface.refreshSettings(); + expect(view.surface.openPanelSnapshot()).toMatchObject({ connected: false }); + }); + it("starts an empty thread when saving a toolbar goal from a blank panel", async () => { vi.useFakeTimers(); const client = connectedClient({ diff --git a/tests/main.test.ts b/tests/main.test.ts index a75facad..b00b9d59 100644 --- a/tests/main.test.ts +++ b/tests/main.test.ts @@ -480,7 +480,7 @@ describe("CodexPanelPlugin boot restored panel loading", () => { expect(threadCatalog(plugin).activeThreadsSnapshot()).toEqual([thread("second")]); resolveFirst([thread("first")]); - await expect(first).resolves.toEqual([thread("first")]); + await expect(first).rejects.toThrow("Codex app-server query context changed while loading shared data."); expect(threadCatalog(plugin).activeThreadsSnapshot()).toEqual([thread("second")]); plugin.settings.codexPath = "codex-a"; expect(threadCatalog(plugin).activeThreadsSnapshot()).toEqual([thread("first")]); diff --git a/tests/workspace/shared-thread-catalog.test.ts b/tests/workspace/shared-thread-catalog.test.ts index e97cba04..3ea58f24 100644 --- a/tests/workspace/shared-thread-catalog.test.ts +++ b/tests/workspace/shared-thread-catalog.test.ts @@ -2,9 +2,6 @@ import { describe, expect, it, vi, type Mock } from "vitest"; import { AppServerQueryCache } from "../../src/app-server/query/cache"; import { AppServerSharedQueries } from "../../src/app-server/query/shared-queries"; -import type { ModelMetadata } from "../../src/domain/catalog/metadata"; -import { createServerDiagnostics, diagnosticProbeOk, diagnosticsWithProbe } from "../../src/domain/server/diagnostics"; -import type { SharedServerMetadata } from "../../src/domain/server/metadata"; import type { Thread } from "../../src/domain/threads/model"; import { SharedThreadCatalog } from "../../src/workspace/shared-thread-catalog"; @@ -46,78 +43,6 @@ describe("SharedThreadCatalog", () => { expect(listener).toHaveBeenLastCalledWith(expect.objectContaining({ data: [thread("thread")] })); }); - it("does not notify stale thread observers after the app-server query context changes", async () => { - const context = { codexPath: "codex-a", vaultPath: "/vault" }; - const surfaces = surfaceActions(); - const queries = new AppServerSharedQueries({ - cache: cacheWithThreads((queryContext) => { - expect(queryContext.codexPath).toBe("codex-a"); - return new Promise((resolve) => { - resolveThreads = resolve; - }); - }), - context: () => context, - }); - const catalog = new SharedThreadCatalog({ - queries, - surfaces, - }); - let resolveThreads!: (threads: Thread[]) => void; - const listener = vi.fn(); - catalog.observeActiveThreadsResult(listener); - - const fetch = catalog.refreshActiveThreads(); - await flushMicrotasks(); - context.codexPath = "codex-b"; - listener.mockClear(); - resolveThreads([thread("stale")]); - - await expect(fetch).resolves.toEqual([thread("stale")]); - expect(listener).not.toHaveBeenCalled(); - context.codexPath = "codex-a"; - expect(catalog.activeThreadsSnapshot()).toEqual([thread("stale")]); - }); - - it("resubscribes active observers when the app-server query context changes", () => { - const context = { codexPath: "codex-a", vaultPath: "/vault" }; - const queries = new AppServerSharedQueries({ - cache: new AppServerQueryCache(), - context: () => context, - }); - const catalog = new SharedThreadCatalog({ - queries, - surfaces: surfaceActions(), - }); - const listener = vi.fn(); - catalog.observeActiveThreadsResult(listener); - - catalog.setActiveThreads([thread("a")]); - context.codexPath = "codex-b"; - queries.notifyContextChanged(); - catalog.setActiveThreads([thread("b")]); - - expect(listener).toHaveBeenLastCalledWith(expect.objectContaining({ data: [thread("b")] })); - context.codexPath = "codex-a"; - queries.notifyContextChanged(); - expect(listener).toHaveBeenLastCalledWith(expect.objectContaining({ data: [thread("a")] })); - }); - - it("publishes metadata and model snapshots to shared query observers", () => { - const { queries } = catalogFixture(); - const metadata = serverMetadata({ availableModels: [model("gpt-test")] }); - const metadataListener = vi.fn(); - const modelListener = vi.fn(); - queries.observeAppServerMetadataResult(metadataListener); - queries.observeModelsResult(modelListener); - - queries.updateAppServerMetadata(() => metadata); - - expect(queries.appServerMetadataSnapshot()).toEqual(metadata); - expect(queries.modelsSnapshot()).toEqual(metadata.availableModels); - expect(metadataListener).toHaveBeenLastCalledWith(expect.objectContaining({ data: metadata })); - expect(modelListener).toHaveBeenCalledWith(expect.objectContaining({ data: metadata.availableModels })); - }); - it("applies known rename mutations to cache and surfaces", () => { const { catalog, surfaces } = catalogFixture(); const listener = vi.fn(); @@ -179,10 +104,6 @@ function cacheWithThreads( }); } -async function flushMicrotasks(): Promise { - await new Promise((resolve) => setImmediate(resolve)); -} - function surfaceActions(): MockSurfaceActions { return { refreshOpenViews: vi.fn(), @@ -203,32 +124,3 @@ function thread(id: string): Thread { archived: false, }; } - -function model(modelId: string): ModelMetadata { - return { - id: modelId, - model: modelId, - displayName: modelId, - description: "", - hidden: false, - supportedReasoningEfforts: [], - defaultReasoningEffort: null, - inputModalities: [], - additionalSpeedTiers: [], - serviceTiers: [], - defaultServiceTier: null, - isDefault: false, - }; -} - -function serverMetadata(overrides: Partial = {}): SharedServerMetadata { - const diagnostics = diagnosticsWithProbe(createServerDiagnostics(), diagnosticProbeOk("model/list", "0 models")); - return { - runtimeConfig: null, - availableModels: [], - availableSkills: [], - rateLimit: null, - serverDiagnostics: diagnostics, - ...overrides, - }; -}