diff --git a/packages/core/src/__tests__/workflow-runner.test.ts b/packages/core/src/__tests__/workflow-runner.test.ts index 76054cc..d6c74ef 100644 --- a/packages/core/src/__tests__/workflow-runner.test.ts +++ b/packages/core/src/__tests__/workflow-runner.test.ts @@ -5,7 +5,7 @@ * with a mocked DB adapter and mocked AgentRelay. */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { existsSync, mkdirSync, @@ -498,6 +498,349 @@ agents: // ── Execution ────────────────────────────────────────────────────────── + describe('Relaycast workspace provisioning resilience', () => { + // Regression for a real production failure: on 2026-09-10 a run died with + // `"status":"failed","error":"Service Unavailable","steps":0`. Workspace + // provisioning runs before step one, so a single 503 from Relaycast — which + // sheds load that way under database pressure — killed the whole run with an + // error that says nothing about the user's flow. + const okWorkspace = () => + ({ + ok: true, + status: 200, + headers: { get: () => null }, + json: async () => ({ data: { api_key: 'rk_live_test' } }), + text: async () => '', + }) as unknown as Response; + const unavailable = (retryAfter?: string) => + ({ + ok: false, + status: 503, + headers: { get: (h: string) => (h.toLowerCase() === 'retry-after' ? (retryAfter ?? null) : null) }, + text: async () => 'database_overloaded', + json: async () => ({}), + }) as unknown as Response; + + function runnerIn(prefix: string) { + const tmpDir = mkdtempSync(path.join(os.tmpdir(), prefix)); + return new WorkflowRunner({ db, cwd: tmpDir }); + } + + // This file has no global mock cleanup — the other fetch-spying tests each + // restore inside their own try/finally. Restore ONLY this block's fetch spy: + // `vi.restoreAllMocks()` is too broad here and tears down the module-level + // mocks (HarnessDriverClient.spawn and friends) that later describes rely on. + let fetchSpy: ReturnType | undefined; + const spyFetch = (impl: unknown) => { + fetchSpy = vi.spyOn(globalThis, 'fetch').mockImplementation(impl as never); + return fetchSpy; + }; + afterEach(() => { + fetchSpy?.mockRestore(); + fetchSpy = undefined; + }); + + it('retries a 503 and succeeds without failing the run', async () => { + let calls = 0; + spyFetch((async (url: string) => { + // Only /v1/workspaces is a provisioning attempt; the method also fires a + // best-effort dashboard key push that must not count. + if (!String(url).includes('/v1/workspaces')) return okWorkspace(); + calls += 1; + return calls < 3 ? unavailable() : okWorkspace(); + })); + const r = runnerIn('relayflows-provision-retry-'); + vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + await (r as any).ensureRelaycastApiKey('wf-retry'); + + expect(calls).toBe(3); + expect((r as any).relayApiKey).toBe('rk_live_test'); + }); + + it('retries a dropped connection, not just an HTTP status', async () => { + let calls = 0; + spyFetch((async (url: string) => { + if (!String(url).includes('/v1/workspaces')) return okWorkspace(); + calls += 1; + if (calls === 1) throw new Error('fetch failed'); + return okWorkspace(); + })); + const r = runnerIn('relayflows-provision-network-'); + vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + await (r as any).ensureRelaycastApiKey('wf-network'); + + expect(calls).toBe(2); + expect((r as any).relayApiKey).toBe('rk_live_test'); + }); + + it('does NOT retry a 4xx — that is the caller at fault', async () => { + let calls = 0; + spyFetch((async (url: string) => { + if (!String(url).includes('/v1/workspaces')) return okWorkspace(); + calls += 1; + return { + ok: false, + status: 401, + headers: { get: () => null }, + text: async () => 'unauthorized', + } as unknown as Response; + })); + const r = runnerIn('relayflows-provision-4xx-'); + vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + await expect((r as any).ensureRelaycastApiKey('wf-4xx')).rejects.toThrow(/401/); + expect(calls).toBe(1); + }); + + it('honours Retry-After, capped so a large value cannot stall the run', async () => { + let calls = 0; + spyFetch((async (url: string) => { + if (!String(url).includes('/v1/workspaces')) return okWorkspace(); + calls += 1; + return calls === 1 ? unavailable('3600') : okWorkspace(); + })); + const r = runnerIn('relayflows-provision-retry-after-'); + const delaySpy = vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + await (r as any).ensureRelaycastApiKey('wf-retry-after'); + + // 3600s would stall the run; the ceiling clamps it. + const waited = delaySpy.mock.calls.map(([ms]) => ms as number); + expect(Math.max(...waited)).toBeLessThanOrEqual(15_000); + expect(Math.max(...waited)).toBeGreaterThan(1_000); + }); + + it('gives up after a bounded number of attempts with an actionable message', async () => { + let calls = 0; + spyFetch((async (url: string) => { + if (!String(url).includes('/v1/workspaces')) return okWorkspace(); + calls += 1; + return unavailable(); + })); + const r = runnerIn('relayflows-provision-exhaust-'); + vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + // The message must tell the user this is upstream, not their flow. + await expect((r as any).ensureRelaycastApiKey('wf-exhaust')).rejects.toThrow( + /shedding load, not a problem with this flow/ + ); + expect(calls).toBe(4); + }); + }); + + describe('workspace provisioning transport edge cases', () => { + // Both from Codex review on #60. Each is a transient failure that would + // still have killed a run before step one despite the retry loop. + let fetchSpy3: ReturnType | undefined; + afterEach(() => { + fetchSpy3?.mockRestore(); + fetchSpy3 = undefined; + }); + + const okWorkspace3 = () => + ({ + ok: true, + status: 200, + headers: { get: () => null }, + json: async () => ({ data: { api_key: 'rk_live_test' } }), + text: async () => '', + }) as unknown as Response; + + it('retries when the connection drops while reading the response body', async () => { + let calls = 0; + fetchSpy3 = vi.spyOn(globalThis, 'fetch').mockImplementation((async (url: string) => { + if (!String(url).includes('/v1/workspaces')) return okWorkspace3(); + calls += 1; + if (calls === 1) { + // `fetch` resolves on headers; the body transfer fails afterwards. + return { + ok: true, + status: 200, + headers: { get: () => null }, + json: async () => { + throw new Error('terminated: aborted'); + }, + text: async () => '', + } as unknown as Response; + } + return okWorkspace3(); + }) as never); + const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'relayflows-body-drop-')); + const r = new WorkflowRunner({ db, cwd: tmpDir }); + vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + await (r as any).ensureRelaycastApiKey('wf-body-drop'); + + expect(calls).toBe(2); + expect((r as any).relayApiKey).toBe('rk_live_test'); + }); + + it('does not reuse a previous response Retry-After after a later connection failure', async () => { + let calls = 0; + fetchSpy3 = vi.spyOn(globalThis, 'fetch').mockImplementation((async (url: string) => { + if (!String(url).includes('/v1/workspaces')) return okWorkspace3(); + calls += 1; + if (calls === 1) { + // 503 carrying a large Retry-After, clamped to the 15s ceiling. + return { + ok: false, + status: 503, + headers: { get: (h: string) => (h.toLowerCase() === 'retry-after' ? '3600' : null) }, + text: async () => 'database_overloaded', + } as unknown as Response; + } + if (calls === 2) throw new Error('fetch failed'); + return okWorkspace3(); + }) as never); + const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'relayflows-stale-retry-after-')); + const r = new WorkflowRunner({ db, cwd: tmpDir }); + const delaySpy = vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + await (r as any).ensureRelaycastApiKey('wf-stale-retry-after'); + + const waited = delaySpy.mock.calls.map(([ms]) => ms as number); + expect(waited).toHaveLength(2); + // First wait honours the (clamped) header from that attempt's own response. + expect(waited[0]).toBe(15_000); + // The second attempt is a network rejection with NO response, so it must + // fall back to linear backoff rather than reusing the stale 503 header. + expect(waited[1]).toBe(2_000); + expect((r as any).relayApiKey).toBe('rk_live_test'); + }); + }); + + describe('provisioning retries are cancellation-aware', () => { + let fetchSpy4: ReturnType | undefined; + afterEach(() => { + fetchSpy4?.mockRestore(); + fetchSpy4 = undefined; + }); + + it('does not start another provisioning request after an abort during the delay', async () => { + let calls = 0; + fetchSpy4 = vi.spyOn(globalThis, 'fetch').mockImplementation((async (url: string) => { + if (!String(url).includes('/v1/workspaces')) { + return { ok: true, status: 200, headers: { get: () => null }, json: async () => ({}), text: async () => '' } as unknown as Response; + } + calls += 1; + return { + ok: false, + status: 503, + headers: { get: (h: string) => (h.toLowerCase() === 'retry-after' ? '3600' : null) }, + text: async () => 'database_overloaded', + } as unknown as Response; + }) as never); + const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'relayflows-abort-')); + const r = new WorkflowRunner({ db, cwd: tmpDir }); + (r as any).abortController = new AbortController(); + // Abort while the first backoff is in flight. + vi.spyOn(r as any, 'abortableDelay').mockImplementation(async () => { + (r as any).abortController.abort(); + }); + + await expect((r as any).ensureRelaycastApiKey('wf-abort')).rejects.toThrow(/aborted/i); + // One attempt made, none after the abort — without the check this would + // keep retrying for tens of seconds against a cancelled run. + expect(calls).toBe(1); + }); + }); + + describe('setup-stage attribution and broker startup retry', () => { + // The 2026-09-10 failure recorded a bare `Service Unavailable` with zero + // steps. Three calls run before step one — workspace provisioning, observer + // minting, broker startup — and any can produce that message, so the failing + // call had to be guessed. These pin that it no longer has to be. + let fetchSpy2: ReturnType | undefined; + afterEach(() => { + fetchSpy2?.mockRestore(); + fetchSpy2 = undefined; + }); + + const okWorkspace2 = () => + ({ + ok: true, + status: 200, + headers: { get: () => null }, + json: async () => ({ data: { api_key: 'rk_live_test' } }), + text: async () => '', + }) as unknown as Response; + + it('tags a workspace provisioning failure with its stage', async () => { + fetchSpy2 = vi.spyOn(globalThis, 'fetch').mockImplementation((async (url: string) => { + if (!String(url).includes('/v1/workspaces')) return okWorkspace2(); + return { + ok: false, + status: 503, + headers: { get: () => null }, + text: async () => 'database_overloaded', + } as unknown as Response; + }) as never); + const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'relayflows-stage-ws-')); + const r = new WorkflowRunner({ db, cwd: tmpDir }); + vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + await expect((r as any).ensureRelaycastApiKey('wf-stage')).rejects.toThrow( + /\[setup:workspace-provisioning\]/ + ); + }); + + it('retries a transient broker startup failure instead of failing the run', async () => { + fetchSpy2 = vi.spyOn(globalThis, 'fetch').mockImplementation((async () => okWorkspace2()) as never); + const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'relayflows-broker-retry-')); + const r = new WorkflowRunner({ db, cwd: tmpDir }); + vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + let attempts = 0; + mockHarnessDriverSpawn.mockImplementation(async () => { + attempts += 1; + if (attempts === 1) throw new Error('Service Unavailable'); + return mockRelayInstance; + }); + + await (r as any).startOrReuseSharedBroker('run-broker-retry', 'wf-broker-retry', false); + + expect(attempts).toBe(2); + }); + + it('tags an exhausted broker startup failure with its stage', async () => { + fetchSpy2 = vi.spyOn(globalThis, 'fetch').mockImplementation((async () => okWorkspace2()) as never); + const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'relayflows-broker-exhaust-')); + const r = new WorkflowRunner({ db, cwd: tmpDir }); + vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + mockHarnessDriverSpawn.mockImplementation(async () => { + throw new Error('Service Unavailable'); + }); + + // The stage tag is the whole point: this is what tells the next person + // WHICH pre-step call failed instead of leaving them to infer it. + await expect( + (r as any).startOrReuseSharedBroker('run-broker-exhaust', 'wf-broker-exhaust', false) + ).rejects.toThrow(/\[setup:broker-startup\]/); + }); + + it('does not retry a broker misconfiguration', async () => { + fetchSpy2 = vi.spyOn(globalThis, 'fetch').mockImplementation((async () => okWorkspace2()) as never); + const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'relayflows-broker-config-')); + const r = new WorkflowRunner({ db, cwd: tmpDir }); + vi.spyOn(r as any, 'delay').mockResolvedValue(undefined); + + let attempts = 0; + mockHarnessDriverSpawn.mockImplementation(async () => { + attempts += 1; + throw new Error('binary not found: agent-relay-broker'); + }); + + await expect( + (r as any).startOrReuseSharedBroker('run-broker-config', 'wf-broker-config', false) + ).rejects.toThrow(/binary not found/); + // A misconfiguration fails identically every time; retrying only delays it. + expect(attempts).toBe(1); + }); + }); + describe('Relaycast base URL consistency', () => { it('uses the default origin for workspace creation, observer minting, broker, and child env', async () => { const tmpDir = mkdtempSync(path.join(os.tmpdir(), 'relayflows-base-url-')); diff --git a/packages/core/src/runner.ts b/packages/core/src/runner.ts index 781a273..5acd942 100644 --- a/packages/core/src/runner.ts +++ b/packages/core/src/runner.ts @@ -673,6 +673,41 @@ const BROKER_OPERATION_MAX_ATTEMPTS = 3; const BROKER_OPERATION_RETRY_DELAY_MS = 1_000; const AGENT_TRANSIENT_NETWORK_MAX_ATTEMPTS = 3; const AGENT_TRANSIENT_NETWORK_RETRY_DELAY_MS = 1_000; +/** + * Workspace provisioning runs BEFORE step one, so a single transient failure + * there kills the whole run with zero steps executed and an error that says + * nothing about the user's flow ("Service Unavailable"). Relaycast sheds load + * with 503 under database pressure, which is exactly the shape worth retrying. + */ +const WORKSPACE_PROVISION_MAX_ATTEMPTS = 4; +const WORKSPACE_PROVISION_RETRY_DELAY_MS = 1_000; +const WORKSPACE_PROVISION_MAX_RETRY_AFTER_MS = 15_000; +/** + * Broker startup also registers with Relaycast, so it fails the same way and + * for the same reason as workspace provisioning — before step one, taking the + * whole run with it. + */ +const BROKER_SPAWN_MAX_ATTEMPTS = 3; +const BROKER_SPAWN_RETRY_DELAY_MS = 2_000; + +/** + * Names the setup call that failed, so a pre-step-one failure is diagnosable + * from the run record alone. + * + * A bare `Service Unavailable` in a failed run is unattributable: three + * separate calls happen before the first step (workspace provisioning, observer + * minting, broker startup) and any of them can produce it. On 2026-09-10 that + * ambiguity meant the failing call had to be GUESSED from a run record, which + * is a poor basis for choosing what to fix. + */ +export function describeSetupFailure(stage: string, error: unknown): Error { + const message = error instanceof Error ? error.message : String(error); + const tagged = new Error(`[setup:${stage}] ${message}`); + if (error instanceof Error && error.stack) tagged.stack = error.stack; + (tagged as { cause?: unknown }).cause = error; + (tagged as { setupStage?: string }).setupStage = stage; + return tagged; +} /** * The one Relayfile base URL default. @@ -2392,6 +2427,23 @@ export class WorkflowRunner { this.wireRelayClient(context.runId); } + /** + * Parse a Retry-After header. Supports both forms in RFC 9110: delay-seconds + * and an HTTP-date. Returns undefined when absent or unparseable, and clamps + * to a ceiling so a hostile or mistaken value cannot stall a run. + */ + private parseRetryAfterMs(header: string | null | undefined): number | undefined { + if (!header) return undefined; + const trimmed = header.trim(); + if (!trimmed) return undefined; + const seconds = Number(trimmed); + const ms = Number.isFinite(seconds) + ? seconds * 1000 + : Date.parse(trimmed) - Date.now(); + if (!Number.isFinite(ms) || ms <= 0) return undefined; + return Math.min(ms, WORKSPACE_PROVISION_MAX_RETRY_AFTER_MS); + } + private isRetryableProtocolError(error: unknown): boolean { const candidate = error as { retryable?: unknown; status?: unknown; message?: unknown } | undefined; if (candidate?.retryable === true) return true; @@ -2491,17 +2543,87 @@ export class WorkflowRunner { // Always create a fresh workspace — each run gets full isolation. const workspaceName = `relay-${channel}-${randomBytes(4).toString('hex')}`; const baseUrl = this.getRelaycastBaseUrl(); - const res = await fetch(`${baseUrl}/v1/workspaces`, { - method: 'POST', - headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ name: workspaceName }), - }); - if (!res.ok) { - throw new Error(`Failed to auto-create Relaycast workspace: ${res.status} ${await res.text()}`); + // Retry transient upstream failures. This call happens before any step + // runs, so giving up here fails the run with zero steps and an opaque + // message. A 5xx or a dropped connection is the service shedding load, not + // a problem with the flow; a 4xx is the caller's fault and must not retry. + // + // The BODY READ is inside the attempt on purpose. `fetch` resolves as soon + // as headers arrive, so a connection that drops mid-body rejects at + // `res.json()` — after the loop, if parsing sits outside it — and that + // transient failure would kill the run exactly like an unretried 503. + let parsed: Record | undefined; + let failure: { kind: 'network'; error: unknown } | { kind: 'http'; status: number; detail: string } | undefined; + for (let attempt = 1; attempt <= WORKSPACE_PROVISION_MAX_ATTEMPTS; attempt++) { + // Both must be cleared per attempt. Leaving a previous response in scope + // makes a later network rejection read the OLD response's `Retry-After`, + // so a stale 3600s header would stall every subsequent retry instead of + // applying the intended linear backoff. + let res: Response | undefined; + failure = undefined; + try { + res = await fetch(`${baseUrl}/v1/workspaces`, { + method: 'POST', + headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ name: workspaceName }), + ...(this.abortController ? { signal: this.abortController.signal } : {}), + }); + if (res.ok) { + parsed = (await res.json()) as Record; + } else { + failure = { kind: 'http', status: res.status, detail: await res.text() }; + } + } catch (error) { + failure = { kind: 'network', error }; + } + + if (!failure) break; + const retryable = failure.kind === 'network' || failure.status >= 500; + if (!retryable || attempt >= WORKSPACE_PROVISION_MAX_ATTEMPTS) break; + + // Honour Retry-After only from THIS attempt's response, capped so a large + // or malformed value cannot stall the run indefinitely. + const retryAfterMs = + failure.kind === 'http' ? this.parseRetryAfterMs(res?.headers?.get('retry-after')) : undefined; + const backoffMs = WORKSPACE_PROVISION_RETRY_DELAY_MS * attempt; + const reason = failure.kind === 'network' ? 'network error' : `HTTP ${failure.status}`; + this.log( + `Relaycast workspace provisioning failed (${reason}); retrying ${attempt}/${WORKSPACE_PROVISION_MAX_ATTEMPTS - 1}...` + ); + await this.abortableDelay(Math.max(retryAfterMs ?? 0, backoffMs)); + // An abort during the delay must not start another provisioning request. + this.checkAborted(); } - const body = (await res.json()) as Record; + if (failure?.kind === 'network') { + const message = + failure.error instanceof Error ? failure.error.message : String(failure.error); + throw describeSetupFailure( + 'workspace-provisioning', + new Error( + `Failed to reach Relaycast to create a workspace after ${WORKSPACE_PROVISION_MAX_ATTEMPTS} attempts: ${message}` + ) + ); + } + if (failure?.kind === 'http') { + const suffix = + failure.status >= 500 + ? ` (still failing after ${WORKSPACE_PROVISION_MAX_ATTEMPTS} attempts; Relaycast is shedding load, not a problem with this flow)` + : ''; + throw describeSetupFailure( + 'workspace-provisioning', + new Error(`Failed to auto-create Relaycast workspace: ${failure.status} ${failure.detail}${suffix}`) + ); + } + if (!parsed) { + throw describeSetupFailure( + 'workspace-provisioning', + new Error('Failed to auto-create Relaycast workspace: no response') + ); + } + + const body = parsed; const data = (body.data ?? body) as Record; const apiKey = data.api_key as string; @@ -3099,7 +3221,10 @@ export class WorkflowRunner { ? { RELAYCAST_BASE_URL: expectedBaseUrl, RELAY_BASE_URL: expectedBaseUrl } : {}), }; - this.relay = await HarnessDriverClient.spawn({ + // Broker startup registers with Relaycast, so it sheds load the same way + // workspace provisioning does — and it runs before step one, so an + // unretried blip fails the whole run with zero steps executed. + const spawnBroker = () => HarnessDriverClient.spawn({ ...this.relayOptions, cwd: brokerCwd, brokerName, @@ -3123,6 +3248,30 @@ export class WorkflowRunner { console.log(`${chalk.dim.yellow('[broker]')} ${line}`); }, }); + + let spawnError: unknown; + for (let attempt = 1; attempt <= BROKER_SPAWN_MAX_ATTEMPTS; attempt++) { + try { + this.relay = await spawnBroker(); + spawnError = undefined; + break; + } catch (error) { + spawnError = error; + // Only transient protocol/network failures are worth another go; a + // misconfiguration fails identically every time and retrying it just + // delays a real error behind a longer wait. + if (!this.isRetryableProtocolError(error) || attempt >= BROKER_SPAWN_MAX_ATTEMPTS) break; + this.log( + `Broker startup failed (${error instanceof Error ? error.message : String(error)}); retrying ${attempt}/${BROKER_SPAWN_MAX_ATTEMPTS - 1}...` + ); + await this.abortableDelay(BROKER_SPAWN_RETRY_DELAY_MS * attempt); + this.checkAborted(); + } + } + if (spawnError !== undefined) { + throw describeSetupFailure('broker-startup', spawnError); + } + lease.startedBroker = true; this.writeSharedBrokerOwner(lease); } finally { @@ -12000,6 +12149,28 @@ export class WorkflowRunner { return new Promise((resolve) => setTimeout(resolve, ms)); } + /** + * A delay that gives up as soon as the run is aborted. + * + * Retry backoff must not outlive a cancellation: with a clamped 15s + * `Retry-After` plus linear backoff, a plain `delay` would keep an aborted run + * sleeping for tens of seconds and then issue another request. + */ + private abortableDelay(ms: number): Promise { + const signal = this.abortController?.signal; + if (!signal) return this.delay(ms); + if (signal.aborted) return Promise.resolve(); + return new Promise((resolve) => { + const done = () => { + clearTimeout(timer); + signal.removeEventListener('abort', done); + resolve(); + }; + const timer = setTimeout(done, ms); + signal.addEventListener('abort', done, { once: true }); + }); + } + // ── Channel messaging ────────────────────────────────────────────────── /**