diff --git a/src/lib/__tests__/wizard-tools.test.ts b/src/lib/__tests__/wizard-tools.test.ts index 523f9f37..c7f77e3c 100644 --- a/src/lib/__tests__/wizard-tools.test.ts +++ b/src/lib/__tests__/wizard-tools.test.ts @@ -1134,6 +1134,51 @@ describe('downloadWithRetry', () => { expect(attempts).toBe(2); }); + it('retries a body read that aborts mid-stream', async () => { + let attempts = 0; + + const bytes = await __test.downloadWithRetry(url, { + fetchImpl: (() => { + attempts += 1; + const failBody = attempts < 3; + return Promise.resolve({ + ok: true, + status: 200, + statusText: 'OK', + arrayBuffer: () => + failBody + ? Promise.reject( + new Error('The operation was aborted due to timeout'), + ) + : Promise.resolve(new ArrayBuffer(3)), + }); + }) as any, + sleepImpl: noSleep, + }); + + expect(attempts).toBe(3); + expect(bytes).toHaveLength(3); + }); + + it('keeps the url in the message when every body read aborts', async () => { + await expect( + __test.downloadWithRetry(url, { + fetchImpl: (() => + Promise.resolve({ + ok: true, + status: 200, + statusText: 'OK', + arrayBuffer: () => + Promise.reject( + new Error('The operation was aborted due to timeout'), + ), + })) as any, + sleepImpl: noSleep, + maxAttempts: 3, + }), + ).rejects.toThrow(/example\.com\/skill\.zip failed/); + }); + it('reports every attempt when all retries fail', async () => { await expect( __test.downloadWithRetry(url, { @@ -1172,6 +1217,32 @@ describe('fetchSkillMenu', () => { expect(result).toEqual(menu); }); + it('retries a body read that aborts mid-stream', async () => { + let attempts = 0; + + const result = await fetchSkillMenu('http://localhost:8765', { + fetchImpl: (() => { + attempts += 1; + const failBody = attempts < 3; + return Promise.resolve({ + ok: true, + status: 200, + statusText: 'OK', + json: () => + failBody + ? Promise.reject( + new Error('The operation was aborted due to timeout'), + ) + : Promise.resolve(menu), + }); + }) as any, + sleepImpl: noSleep, + }); + + expect(attempts).toBe(3); + expect(result).toEqual(menu); + }); + it('returns null after exhausting retries', async () => { let attempts = 0; diff --git a/src/lib/agent/agent-prompt-loader.ts b/src/lib/agent/agent-prompt-loader.ts index 576dcc2b..209f4d6c 100644 --- a/src/lib/agent/agent-prompt-loader.ts +++ b/src/lib/agent/agent-prompt-loader.ts @@ -406,8 +406,7 @@ export function parseAgentPrompt( } async function fetchText(url: string): Promise { - const res = await fetchWithRetry(url); - return res.text(); + return fetchWithRetry(url, (res) => res.text()); } /** diff --git a/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts b/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts index 6ada1369..04174a62 100644 --- a/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts +++ b/src/lib/agent/runner/sequence/orchestrator/orchestrator-runner.ts @@ -969,16 +969,22 @@ export async function runOrchestrator( if (result.kind === 'ok') { skillPaths.push(path.join(result.path, 'SKILL.md')); } else { + const detail = + result.kind === 'download-failed' ? `: ${result.message}` : ''; logToFile( - `[orchestrator] skill install failed type=${task.type} skill=${variantId} ${result.kind}`, + `[orchestrator] skill install failed type=${task.type} skill=${variantId} ${result.kind}${detail}`, ); // A task without its instructions must fail here, not run blind: // run 91cf40eb's report task started after two EACCES install // failures (unwritable external-volume cache) and died silently. // The executor catches this, captures the exception, and fails the // task through the normal outcome check. + // + // The message stays constant across skills and tasks — the variant and + // task ids live in the log line above. Interpolating them here split + // one defect across dozens of error-tracking issues. throw new Error( - `Skill "${variantId}" for task "${task.type}" could not be installed (${result.kind}). ` + + `A required skill could not be installed (${result.kind}). ` + 'If this is a permissions error, check that the project directory is writable.', ); } diff --git a/src/lib/fetch-retry.ts b/src/lib/fetch-retry.ts index fa5d9673..c97278f2 100644 --- a/src/lib/fetch-retry.ts +++ b/src/lib/fetch-retry.ts @@ -21,11 +21,21 @@ export interface RetryOpts { backoffMs?: number; } -/** Fetch a URL, retrying transient failures (network error or non-ok HTTP) with backoff. */ -export async function fetchWithRetry( +/** + * Fetch a URL, read its body with `read`, and retry transient failures with + * backoff. Transient failures are a network error, a non-ok HTTP status, or a + * body read that aborts mid-stream. + * + * The body read runs inside the retry loop and under the same per-attempt + * timeout as the request. A slow body read that hits the timeout therefore + * retries like any other transient failure, instead of throwing once with the + * URL stripped from its message. + */ +export async function fetchWithRetry( url: string, + read: (resp: Response) => Promise, opts: RetryOpts = {}, -): Promise { +): Promise { const { fetchImpl = fetch, sleepImpl = sleep, @@ -41,7 +51,7 @@ export async function fetchWithRetry( signal: AbortSignal.timeout(timeoutMs), }); if (!resp.ok) throw new Error(`HTTP ${resp.status} ${resp.statusText}`); - return resp; + return await read(resp); } catch (err: any) { failures.push(`attempt ${attempt}: ${err.message}`); if (attempt < maxAttempts) { diff --git a/src/lib/wizard-tools/tools.ts b/src/lib/wizard-tools/tools.ts index 25b54bc0..84f08593 100644 --- a/src/lib/wizard-tools/tools.ts +++ b/src/lib/wizard-tools/tools.ts @@ -111,8 +111,11 @@ export async function fetchSkillMenu( const menuUrl = `${skillsBaseUrl}/skill-menu.json`; try { logToFile(`fetchSkillMenu: fetching from ${menuUrl}`); - const resp = await fetchWithRetry(menuUrl, opts); - const data = (await resp.json()) as SkillMenu; + const data = (await fetchWithRetry( + menuUrl, + (resp) => resp.json(), + opts, + )) as SkillMenu; for (const [category, entries] of Object.entries(data.categories)) { data.categories[category] = entries.flatMap(expandBundleEntry); } @@ -179,13 +182,23 @@ function extractBundle( return written; } +/** + * Per-attempt budget for a skill download. Skill bundles are multi-megabyte, so + * a slow connection needs longer to stream the body than the default menu/prompt + * budget allows. + */ +const DOWNLOAD_TIMEOUT_MS = 120000; + /** Download a URL to a buffer, retrying transient failures with backoff. */ async function downloadWithRetry( url: string, opts: RetryOpts = {}, ): Promise { - const resp = await fetchWithRetry(url, opts); - return new Uint8Array(await resp.arrayBuffer()); + return fetchWithRetry( + url, + async (resp) => new Uint8Array(await resp.arrayBuffer()), + { timeoutMs: DOWNLOAD_TIMEOUT_MS, ...opts }, + ); } /** How to place a skill and what triages it — `triage` is stated by every caller so none inherits a silent default. */