diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e59f80e56..9440de45e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -11,6 +11,7 @@ - Fixed repeated ambiguous truncated-response recovery being mislabeled as context overflow ([#8130](https://github.com/earendil-works/pi/issues/8130)). - Fixed duplicate fullscreen right-click paste in VS Code-based terminals on Windows ([#8186](https://github.com/earendil-works/pi/issues/8186)). - Fixed llama.cpp login guidance to direct users to `/llama` before `/model` when no local models are loaded ([#8203](https://github.com/earendil-works/pi/issues/8203)). +- Fixed hung pi.dev model catalog requests consuming the entire refresh deadline without retrying ([#8198](https://github.com/earendil-works/pi/issues/8198)). ## [0.84.2] - 2026-08-14 diff --git a/packages/coding-agent/src/core/remote-catalog-provider.ts b/packages/coding-agent/src/core/remote-catalog-provider.ts index c79b7e3bb..a12c80ba4 100644 --- a/packages/coding-agent/src/core/remote-catalog-provider.ts +++ b/packages/coding-agent/src/core/remote-catalog-provider.ts @@ -4,6 +4,7 @@ import { fetchWithRetry } from "../utils/management-http.ts"; import { getPiUserAgent } from "../utils/pi-user-agent.ts"; const DEFAULT_CATALOG_BASE_URL = "https://pi.dev"; +const REMOTE_CATALOG_ATTEMPT_TIMEOUT_MS = 4_000; export const REMOTE_CATALOG_REFRESH_INTERVAL_MS = 4 * 60 * 60 * 1000; function mergeModels(baseline: readonly Model[], dynamic: readonly Model[]): Model[] { @@ -78,14 +79,18 @@ export function withRemoteCatalog( // leave the overlay empty. const validator = stored?.models.length ? stored.etag : undefined; const url = new URL(`/api/models/providers/${encodeURIComponent(provider.id)}`, catalogBaseUrl); - const response = await fetchWithRetry(url, { - headers: { - accept: "application/json", - "User-Agent": getPiUserAgent(VERSION), - ...(validator ? { "if-none-match": validator } : {}), + const response = await fetchWithRetry( + url, + { + headers: { + accept: "application/json", + "User-Agent": getPiUserAgent(VERSION), + ...(validator ? { "if-none-match": validator } : {}), + }, + signal: context.signal, }, - signal: context.signal, - }); + { attemptTimeoutMs: REMOTE_CATALOG_ATTEMPT_TIMEOUT_MS }, + ); if (context.signal.aborted) return; const checkedAt = Date.now(); // Unchanged: dynamicModels already holds the stored overlay, so only the diff --git a/packages/coding-agent/src/utils/management-http.ts b/packages/coding-agent/src/utils/management-http.ts index bbaab9394..9a7da8144 100644 --- a/packages/coding-agent/src/utils/management-http.ts +++ b/packages/coding-agent/src/utils/management-http.ts @@ -7,8 +7,10 @@ export interface FetchRetryOptions { maxRetries?: number; /** Retry transient HTTP responses as well as transport failures. Defaults to true. */ retryOnStatus?: boolean; - /** Per-attempt timeout. A new timeout is created for every attempt. */ + /** Overall time budget shared by all attempts. */ timeoutMs?: number; + /** Per-attempt timeout. A new timeout is created for every attempt. */ + attemptTimeoutMs?: number; } /** @@ -19,8 +21,8 @@ export interface FetchRetryOptions { * agent/model operations: those can fail after the HTTP request starts and are * retried by their semantic caller instead. * - * Caller cancellation is terminal. When timeoutMs is supplied, it is the - * overall time budget shared by all attempts. + * Caller cancellation and timeoutMs are terminal. attemptTimeoutMs aborts + * only the current attempt so a hung connection can be retried. */ export async function fetchWithRetry( input: FetchInput, @@ -32,17 +34,20 @@ export async function fetchWithRetry( ? 2 : Math.max(0, Math.floor(options.maxRetries)); const retryOnStatus = options.retryOnStatus ?? true; - const parentSignal = init?.signal; + const parentSignal = init?.signal ?? undefined; const timeoutSignal = options.timeoutMs !== undefined && options.timeoutMs > 0 ? AbortSignal.timeout(options.timeoutMs) : undefined; - const signal = timeoutSignal - ? parentSignal - ? AbortSignal.any([parentSignal, timeoutSignal]) - : timeoutSignal - : parentSignal; + const attemptTimeoutMs = + options.attemptTimeoutMs !== undefined && options.attemptTimeoutMs > 0 ? options.attemptTimeoutMs : undefined; for (let attempt = 0; ; attempt++) { - signal?.throwIfAborted(); + parentSignal?.throwIfAborted(); + timeoutSignal?.throwIfAborted(); + const attemptTimeoutSignal = attemptTimeoutMs ? AbortSignal.timeout(attemptTimeoutMs) : undefined; + const signals = [parentSignal, timeoutSignal, attemptTimeoutSignal].filter( + (signal): signal is AbortSignal => signal !== undefined, + ); + const signal = signals.length > 1 ? AbortSignal.any(signals) : signals[0]; try { const response = await fetch(input, signal ? { ...init, signal } : init); @@ -55,10 +60,15 @@ export async function fetchWithRetry( // do if cancelling its body also fails. } } catch (error) { + const attemptTimedOut = + attemptTimeoutSignal?.aborted === true && !parentSignal?.aborted && !timeoutSignal?.aborted; if ( parentSignal?.aborted || timeoutSignal?.aborted || - (error instanceof Error && error.name === "AbortError" && timeoutSignal === undefined) || + (error instanceof Error && + error.name === "AbortError" && + !attemptTimedOut && + timeoutSignal === undefined) || attempt >= maxRetries ) { throw error; diff --git a/packages/coding-agent/test/management-http.test.ts b/packages/coding-agent/test/management-http.test.ts index b615bb2dc..cb0809cd4 100644 --- a/packages/coding-agent/test/management-http.test.ts +++ b/packages/coding-agent/test/management-http.test.ts @@ -30,6 +30,27 @@ describe("fetchWithRetry", () => { expect(signals[0]).toBe(signals[1]); }); + it("retries an attempt timeout", async () => { + const controllers: AbortController[] = []; + vi.spyOn(AbortSignal, "timeout").mockImplementation(() => { + const controller = new AbortController(); + controllers.push(controller); + return controller.signal; + }); + const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(async (_input, init) => { + if (fetchMock.mock.calls.length === 1) { + controllers[0].abort(); + init?.signal?.throwIfAborted(); + } + return Response.json({ ok: true }); + }); + + await fetchWithRetry("https://example.test", undefined, { attemptTimeoutMs: 4000 }); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(controllers).toHaveLength(2); + }); + it("retries transient HTTP responses and returns the successful response", async () => { const fetchMock = vi .spyOn(globalThis, "fetch")