From 7d8c11d37f8d6687cc6688998fcb0d3db45e6151 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 13 Aug 2026 09:43:36 +0200 Subject: [PATCH] fix(coding-agent): share concurrent model catalog refreshes --- packages/coding-agent/CHANGELOG.md | 1 + .../interactive/components/model-selector.ts | 3 +- .../src/modes/interactive/interactive-mode.ts | 9 +-- .../interactive/model-catalog-refresh.ts | 51 ++++++++++++ .../test/model-catalog-refresh.test.ts | 81 +++++++++++++++++++ .../7153-scoped-models-refresh.test.ts | 2 +- 6 files changed, 140 insertions(+), 7 deletions(-) create mode 100644 packages/coding-agent/src/modes/interactive/model-catalog-refresh.ts create mode 100644 packages/coding-agent/test/model-catalog-refresh.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3da3c6030..a008fe693 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -17,6 +17,7 @@ ### Fixed +- Fixed opening a model selector immediately after startup cancelling and restarting the in-progress model catalog refresh. - Fixed inherited GitHub Copilot login triggering API rate limits while enabling model policies by limiting concurrent policy updates ([#6187](https://github.com/earendil-works/pi/issues/6187)). - Fixed fullscreen transcript search snapping back to the current match during manual scrolling and fragmented mouse input leaking into the search query. - Fixed inherited required LaTeX arguments starting on a new line being parsed as empty ([#7760](https://github.com/earendil-works/pi/issues/7760)). diff --git a/packages/coding-agent/src/modes/interactive/components/model-selector.ts b/packages/coding-agent/src/modes/interactive/components/model-selector.ts index 6f2668a49..1d5e58b12 100644 --- a/packages/coding-agent/src/modes/interactive/components/model-selector.ts +++ b/packages/coding-agent/src/modes/interactive/components/model-selector.ts @@ -11,6 +11,7 @@ import { } from "@earendil-works/pi-tui"; import type { ModelRuntime } from "../../../core/model-runtime.ts"; import type { SettingsManager } from "../../../core/settings-manager.ts"; +import { refreshModelCatalogs } from "../model-catalog-refresh.ts"; import { getModelSelectorSearchText } from "../model-search.ts"; import { theme } from "../theme/theme.ts"; import { DynamicBorder } from "./dynamic-border.ts"; @@ -167,7 +168,7 @@ export class ModelSelectorComponent extends Container implements Focusable { this.refreshAbortController.abort(); }, timeoutMs); try { - const result = await this.modelRuntime.refresh({ signal: this.refreshAbortController.signal }); + const result = await refreshModelCatalogs(this.modelRuntime, this.refreshAbortController.signal); if (this.closed) return; this.refreshStatusMessage = ""; if (result.aborted && timedOut) { diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index d5a1b9159..5b5b3d86a 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -152,6 +152,7 @@ import { TrustSelectorComponent } from "./components/trust-selector.ts"; import { UserMessageComponent } from "./components/user-message.ts"; import { UserMessageSelectorComponent } from "./components/user-message-selector.ts"; import { editInExternalEditor } from "./external-editor.ts"; +import { refreshModelCatalogs } from "./model-catalog-refresh.ts"; import { getModelSearchText } from "./model-search.ts"; import { getAvailableThemes, @@ -1018,8 +1019,7 @@ export class InteractiveMode { if (!process.env.PI_OFFLINE) { const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), 15_000); - void this.session.modelRuntime - .refresh({ signal: controller.signal }) + void refreshModelCatalogs(this.session.modelRuntime, controller.signal) .then(() => this.updateAvailableProviderCount()) .catch(() => {}) .finally(() => clearTimeout(timeout)); @@ -4632,7 +4632,7 @@ export class InteractiveMode { controller.abort(); }, 15_000); try { - const result = await this.session.modelRuntime.refresh({ signal: controller.signal }); + const result = await refreshModelCatalogs(this.session.modelRuntime, controller.signal); if (result.aborted && timedOut) { this.showWarning("Model refresh timed out; searching cached models."); } else if (result.errors.size > 0) { @@ -4848,8 +4848,7 @@ export class InteractiveMode { }, }, ); - void this.session.modelRuntime - .refresh({ signal: controller.signal }) + void refreshModelCatalogs(this.session.modelRuntime, controller.signal) .then((result) => { if (disposed) return; availableModels = [...this.session.modelRuntime.getAvailableSnapshot()]; diff --git a/packages/coding-agent/src/modes/interactive/model-catalog-refresh.ts b/packages/coding-agent/src/modes/interactive/model-catalog-refresh.ts new file mode 100644 index 000000000..e5915ff49 --- /dev/null +++ b/packages/coding-agent/src/modes/interactive/model-catalog-refresh.ts @@ -0,0 +1,51 @@ +import type { ModelsRefreshResult } from "@earendil-works/pi-ai"; +import type { ModelRuntime } from "../../core/model-runtime.ts"; +import { raceWithAbortSignal } from "../../utils/abort.ts"; + +type ModelCatalogRuntime = Pick; + +interface ActiveModelCatalogRefresh { + controller: AbortController; + promise: Promise; + waiters: number; +} + +class ModelCatalogRefreshCoordinator { + private readonly activeByRuntime = new WeakMap(); + + refresh(modelRuntime: ModelCatalogRuntime, signal: AbortSignal): Promise { + signal.throwIfAborted(); + let active = this.activeByRuntime.get(modelRuntime); + if (!active) { + const controller = new AbortController(); + let created!: ActiveModelCatalogRefresh; + const operation = modelRuntime.refresh({ signal: controller.signal }); + const promise = raceWithAbortSignal(operation, controller.signal).finally(() => { + if (this.activeByRuntime.get(modelRuntime) === created) { + this.activeByRuntime.delete(modelRuntime); + } + }); + created = { controller, promise, waiters: 0 }; + active = created; + this.activeByRuntime.set(modelRuntime, active); + } + + active.waiters++; + return raceWithAbortSignal(active.promise, signal).finally(() => { + active.waiters--; + if (active.waiters === 0 && this.activeByRuntime.get(modelRuntime) === active) { + active.controller.abort(); + } + }); + } +} + +const modelCatalogRefreshCoordinator = new ModelCatalogRefreshCoordinator(); + +/** Share concurrent interactive all-catalog refreshes while keeping each caller's cancellation independent. */ +export function refreshModelCatalogs( + modelRuntime: ModelCatalogRuntime, + signal: AbortSignal, +): Promise { + return modelCatalogRefreshCoordinator.refresh(modelRuntime, signal); +} diff --git a/packages/coding-agent/test/model-catalog-refresh.test.ts b/packages/coding-agent/test/model-catalog-refresh.test.ts new file mode 100644 index 000000000..0250026f9 --- /dev/null +++ b/packages/coding-agent/test/model-catalog-refresh.test.ts @@ -0,0 +1,81 @@ +import type { ModelsRefreshOptions, ModelsRefreshResult } from "@earendil-works/pi-ai"; +import { describe, expect, it, vi } from "vitest"; +import { refreshModelCatalogs } from "../src/modes/interactive/model-catalog-refresh.ts"; + +interface Deferred { + promise: Promise; + resolve(value: T): void; +} + +function createDeferred(): Deferred { + let resolvePromise!: (value: T) => void; + const promise = new Promise((resolve) => { + resolvePromise = resolve; + }); + return { promise, resolve: resolvePromise }; +} + +function successfulRefresh(): ModelsRefreshResult { + return { aborted: false, errors: new Map() }; +} + +describe("interactive model catalog refresh", () => { + it("shares one runtime refresh between concurrent callers", async () => { + const deferred = createDeferred(); + const runtime = { refresh: vi.fn((_options?: ModelsRefreshOptions) => deferred.promise) }; + const firstController = new AbortController(); + const secondController = new AbortController(); + + const first = refreshModelCatalogs(runtime, firstController.signal); + const second = refreshModelCatalogs(runtime, secondController.signal); + + expect(runtime.refresh).toHaveBeenCalledOnce(); + deferred.resolve(successfulRefresh()); + await expect(first).resolves.toEqual(successfulRefresh()); + await expect(second).resolves.toEqual(successfulRefresh()); + }); + + it("keeps the shared refresh alive when one caller stops waiting", async () => { + const deferred = createDeferred(); + let refreshSignal: AbortSignal | undefined; + const runtime = { + refresh: vi.fn((options?: ModelsRefreshOptions) => { + refreshSignal = options?.signal; + return deferred.promise; + }), + }; + const firstController = new AbortController(); + const secondController = new AbortController(); + const first = refreshModelCatalogs(runtime, firstController.signal); + const second = refreshModelCatalogs(runtime, secondController.signal); + + firstController.abort(); + await expect(first).rejects.toMatchObject({ name: "AbortError" }); + expect(refreshSignal?.aborted).toBe(false); + + deferred.resolve(successfulRefresh()); + await expect(second).resolves.toEqual(successfulRefresh()); + }); + + it("aborts an abandoned refresh and allows a later refresh to start", async () => { + const refreshSignals: AbortSignal[] = []; + const runtime = { + refresh: vi.fn((options?: ModelsRefreshOptions) => { + if (options?.signal) refreshSignals.push(options.signal); + return new Promise(() => {}); + }), + }; + const firstController = new AbortController(); + const first = refreshModelCatalogs(runtime, firstController.signal); + + firstController.abort(); + await expect(first).rejects.toMatchObject({ name: "AbortError" }); + await vi.waitFor(() => expect(refreshSignals[0]?.aborted).toBe(true)); + + const secondController = new AbortController(); + const second = refreshModelCatalogs(runtime, secondController.signal); + expect(runtime.refresh).toHaveBeenCalledTimes(2); + secondController.abort(); + await expect(second).rejects.toMatchObject({ name: "AbortError" }); + }); +}); diff --git a/packages/coding-agent/test/suite/regressions/7153-scoped-models-refresh.test.ts b/packages/coding-agent/test/suite/regressions/7153-scoped-models-refresh.test.ts index 0fb55a306..dccdc9d86 100644 --- a/packages/coding-agent/test/suite/regressions/7153-scoped-models-refresh.test.ts +++ b/packages/coding-agent/test/suite/regressions/7153-scoped-models-refresh.test.ts @@ -101,7 +101,7 @@ describe("issue #7153 scoped models refresh", () => { expect(refresh.refreshSignal).toBeDefined(); refresh.selector.handleInput("\x1b"); - expect(refresh.refreshSignal?.aborted).toBe(true); + await vi.waitFor(() => expect(refresh.refreshSignal?.aborted).toBe(true)); expect(refresh.done).toHaveBeenCalledOnce(); }); });