fix(config): cache failed !command resolutions, bound dynamic fetch timeout

resolveCommandConfig used execSync with a 10s timeout and only cached non-empty successful results, so empty/failing/timeout !command values in models.yml re-ran on every API key lookup and blocked the event loop repeatedly. It now probes the cache with has(), stores empty stdout and caught failures as undefined, and returns the cached undefined on later lookups. Extension-registered fetchDynamicModels is now wrapped with a 15s Promise.race timeout so a stuck extension cannot hang model refresh indefinitely. Verified by Bun transpiler syntax check of packages/coding-agent/src/config/model-registry.ts and the model-registry test suites.

Closes #4237

packages/coding-agent/CHANGELOG.md

@@ -2,6 +2,11 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Cache failed `!command` config resolutions and time out extension dynamic model fetches after 15s ([#4237](https://github.com/can1357/oh-my-pi/issues/4237))
+
+
 ## [16.3.0] - 2026-07-02
 
 ### Added

packages/coding-agent/src/config/model-registry.ts

@@ -48,6 +48,13 @@ const STARTUP_MODEL_CACHE_PROVIDER_IDS: readonly string[] = [
 // packages/ai/src/registry/lm-studio.ts, and packages/ai/src/registry/vllm.ts.
 const LOCAL_PROVIDER_PLACEHOLDERS = new Set<string>(["llama-cpp-local", "lm-studio-local", "vllm-local"]);
 
+/**
+ * Hard bound for extension-provided fetchDynamicModels to prevent indefinite hangs
+ * during runtime provider discovery. Uses a cancellable manual timer (not AbortSignal.timeout)
+ * so a successful fast path does not leave an armed timeout signal for concurrent GC.
+ */
+const RUNTIME_DYNAMIC_MODEL_FETCH_TIMEOUT_MS = 15_000;
+
 import type { ApiKeyResolver, FetchImpl } from "@oh-my-pi/pi-ai";
 import { registerOAuthProvider, unregisterOAuthProviders } from "@oh-my-pi/pi-ai/oauth";
 import type { OAuthCredentials, OAuthLoginCallbacks } from "@oh-my-pi/pi-ai/oauth/types";
@@ -81,6 +88,24 @@ function isDiscoveryBearerApiKey(apiKey: string | undefined | null): apiKey is s
 	return isAuthenticated(apiKey) && !LOCAL_PROVIDER_PLACEHOLDERS.has(apiKey);
 }
 
+/**
+ * Wraps an extension-provided fetchDynamicModels call with a hard timeout.
+ * Uses a cancellable manual timer (not AbortSignal.timeout) so that a fast
+ * successful path does not leave an armed timeout signal for concurrent GC.
+ * The inner fetcher does not receive a signal (extension contract has none).
+ */
+async function withRuntimeDynamicModelsTimeout<T>(timeoutMs: number, run: () => Promise<T>): Promise<T> {
+	const { promise: timeoutPromise, reject: timeoutReject } = Promise.withResolvers<never>();
+	const timer = setTimeout(() => {
+		timeoutReject(new Error(`fetchDynamicModels timed out after ${timeoutMs}ms`));
+	}, timeoutMs);
+	try {
+		return await Promise.race([run(), timeoutPromise]);
+	} finally {
+		clearTimeout(timer);
+	}
+}
+
 /** Provider override config (baseUrl, headers, apiKey, compat, transport) without custom models */
 interface ProviderOverride {
 	baseUrl?: string;
@@ -239,22 +264,25 @@ interface CustomModelsResult {
 	found: boolean;
 }
 
-const commandValueCache = new Map<string, string>();
+const commandValueCache = new Map<string, string | undefined>();
 
 function isCommandConfigValue(valueConfig: string | undefined): valueConfig is string {
 	return valueConfig?.startsWith("!") === true;
 }
 
 function resolveCommandConfig(command: string): string | undefined {
-	const cached = commandValueCache.get(command);
-	if (cached !== undefined) return cached;
+	if (commandValueCache.has(command)) return commandValueCache.get(command);
 	try {
 		const stdout = execSync(command, { encoding: "utf8", timeout: 10_000, windowsHide: true });
 		const trimmed = stdout.trim();
-		if (trimmed.length === 0) return undefined;
+		if (trimmed.length === 0) {
+			commandValueCache.set(command, undefined);
+			return undefined;
+		}
 		commandValueCache.set(command, trimmed);
 		return trimmed;
 	} catch {
+		commandValueCache.set(command, undefined);
 		return undefined;
 	}
 }
@@ -2134,7 +2162,9 @@ export class ModelRegistry {
 				fetchDynamicModels: async () => {
 					const apiKey = await this.#peekApiKeyForProvider(providerName);
 					const resolvedKey = isAuthenticated(apiKey) ? apiKey : undefined;
-					const modelDefs = await fetcher(resolvedKey);
+					const modelDefs = await withRuntimeDynamicModelsTimeout(RUNTIME_DYNAMIC_MODEL_FETCH_TIMEOUT_MS, () =>
+						fetcher(resolvedKey),
+					);
 					const results: Model<Api>[] = [];
 					for (const modelDef of modelDefs) {
 						const overlay = buildCustomModelOverlay(

packages/coding-agent/test/model-registry-command-values.test.ts

@@ -2,6 +2,8 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test";
 import * as fs from "node:fs";
 import * as os from "node:os";
 import * as path from "node:path";
+import type { Api, Model } from "@oh-my-pi/pi-ai/types";
+import { buildModel } from "@oh-my-pi/pi-catalog/build";
 import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
 import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
 import { removeSyncWithRetries, Snowflake } from "@oh-my-pi/pi-utils";
@@ -84,4 +86,50 @@ describe("ModelRegistry command-resolved models.yml values", () => {
 		expect(model?.headers?.["X-Model-Key"]).toBe("cmd-model-header");
 		expect(model?.headers?.Authorization).toBe("Bearer cmd-api-key");
 	});
+
+	test("resolveCommandConfig caches failed executions so they do not retry", async () => {
+		const counterFile = path.join(tempDir, "counter.txt");
+		fs.writeFileSync(counterFile, "0");
+
+		// Command increments a counter and then fails (exit 1).
+		const trackingCommand = `node -e "const fs=require('fs'); fs.writeFileSync('${counterFile.replace(/\\/g, "/")}', String(Number(fs.readFileSync('${counterFile.replace(/\\/g, "/")}', 'utf8')) + 1)); process.exit(1);"`;
+
+		fs.writeFileSync(
+			modelsPath,
+			JSON.stringify({
+				providers: {
+					"custom-proxy": {
+						baseUrl: "https://custom-proxy.example.com/v1",
+						api: "openai-completions",
+						apiKey: `!${trackingCommand}`,
+					},
+				},
+			}),
+		);
+
+		// Init triggers the first command resolution.
+		const registry = new ModelRegistry(authStorage, modelsPath);
+
+		const dummyModel: Model<Api> = buildModel({
+			id: "foo",
+			name: "foo",
+			api: "openai-completions",
+			provider: "custom-proxy",
+			baseUrl: "a",
+			reasoning: false,
+			input: ["text"],
+			cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
+			contextWindow: 4096,
+			maxTokens: 1024,
+		});
+
+		// Trigger the fallback resolver which also calls resolveConfigValue.
+		await registry.getApiKey(dummyModel);
+
+		// Another call to ensure it hits cache multiple times.
+		await registry.getApiKey(dummyModel);
+
+		// The command should have only run once.
+		expect(fs.readFileSync(counterFile, "utf8")).toBe("1");
+	});
 });