fix(coding-agent): share concurrent model catalog refreshes

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)).

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) {

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()];

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<ModelRuntime, "refresh">;
+
+interface ActiveModelCatalogRefresh {
+	controller: AbortController;
+	promise: Promise<ModelsRefreshResult>;
+	waiters: number;
+}
+
+class ModelCatalogRefreshCoordinator {
+	private readonly activeByRuntime = new WeakMap<ModelCatalogRuntime, ActiveModelCatalogRefresh>();
+
+	refresh(modelRuntime: ModelCatalogRuntime, signal: AbortSignal): Promise<ModelsRefreshResult> {
+		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<ModelsRefreshResult> {
+	return modelCatalogRefreshCoordinator.refresh(modelRuntime, signal);
+}

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<T> {
+	promise: Promise<T>;
+	resolve(value: T): void;
+}
+
+function createDeferred<T>(): Deferred<T> {
+	let resolvePromise!: (value: T) => void;
+	const promise = new Promise<T>((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<ModelsRefreshResult>();
+		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<ModelsRefreshResult>();
+		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<ModelsRefreshResult>(() => {});
+			}),
+		};
+		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" });
+	});
+});

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();
 	});
 });