diff --git a/CHANGELOG.md b/CHANGELOG.md index c106e0d1..2250c0fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,10 @@ ### Fixed - Stop flooding startup with `[Skill conflicts]` when the same skills are installed in both `~/.agents/skills` and `~/.claude/skills`. Byte-identical copies are now skipped silently, and remaining name collisions collapse to one summary line per directory pair; `--verbose` still lists each one. +- Cancel and drain workflow children before publishing a terminal state, share host admission across concurrent workflows, accept promise arrays in `parallel()`, and distinguish concurrency notices from actual script failures. +- Clamp workflow concurrency to the host ceiling, back off on temporary 429s with an independent retry budget and adaptive admission cap, and report failed nodes in final tool results instead of silently treating partial workflows as successful. - Persist Ultracode on/off state in the current session journal so reopening that session restores its mode without changing global or project settings. - Stop appending Ultracode capability tutorials on every turn. Announce session-mode transitions after user input, reuse unchanged state, and restore necessary reminders when context is compacted or replaced. - Recognize `/exit` as an alias for `/quit`, including while the agent is running or compacting. - Recognize `search_files`, `find_files`, and `list_directory` when selecting default system-prompt file-exploration guidance. +- Report callback errors (including nested workflow errors raised before any agent starts), recognize explicit counted fan-out requests, and queue ordinary subagent batches independently of their concurrency window. diff --git a/packages/agent-core/src/agent-loop.ts b/packages/agent-core/src/agent-loop.ts index c25600d7..fa7fd4c7 100644 --- a/packages/agent-core/src/agent-loop.ts +++ b/packages/agent-core/src/agent-loop.ts @@ -5,6 +5,7 @@ import { type AssistantMessage, + type AssistantMessageEvent, type Context, EventStream, type ToolResultMessage, @@ -47,9 +48,10 @@ export function agentLoop( }, signal, streamFn, - ).then((messages) => { - stream.end(messages); - }); + ).then( + (messages) => stream.end(messages), + (error) => stream.fail(error), + ); return stream; } @@ -86,9 +88,10 @@ export function agentLoopContinue( }, signal, streamFn, - ).then((messages) => { - stream.end(messages); - }); + ).then( + (messages) => stream.end(messages), + (error) => stream.fail(error), + ); return stream; } @@ -143,8 +146,27 @@ export async function runAgentLoopContinue( return newMessages; } -function createAgentStream(): EventStream { - return new EventStream( +class AgentLoopStream extends EventStream { + private failed = false; + private failure: unknown; + fail(error: unknown): void { + this.failed = true; + this.failure = error; + this.end([]); + } + override async *[Symbol.asyncIterator](): AsyncGenerator { + yield* super[Symbol.asyncIterator](); + if (this.failed) throw this.failure; + } + override async result(): Promise { + const result = await super.result(); + if (this.failed) throw this.failure; + return result; + } +} + +function createAgentStream(): AgentLoopStream { + return new AgentLoopStream( (event: AgentEvent) => event.type === "agent_end", (event: AgentEvent) => (event.type === "agent_end" ? event.messages : []), ); @@ -338,19 +360,21 @@ async function streamAssistantResponse( tools: context.tools, }; - // Resolve API key (important for expiring tokens) - const resolvedApiKey = - (config.getApiKey ? await config.getApiKey(config.model.provider) : undefined) || config.apiKey; - - const response = await streamFunction(config.model, llmContext, { - ...config, - apiKey: resolvedApiKey, + const response = normalizeProviderStream( + async () => { + // Auth refresh and stream construction can throw before the first event. + const resolvedApiKey = + (config.getApiKey ? await config.getApiKey(config.model.provider) : undefined) || config.apiKey; + return streamFunction(config.model, llmContext, { ...config, apiKey: resolvedApiKey, signal }); + }, + config.model, signal, - }); + ); let partialMessage: AssistantMessage | null = null; let addedPartial = false; - + // Consumer/persistence errors must escape. Only the producer normalizes + // provider exceptions, so a failed message_end cannot emit a second terminal. for await (const event of response) { switch (event.type) { case "start": @@ -382,7 +406,10 @@ async function streamAssistantResponse( case "done": case "error": { - const finalMessage = await response.result(); + // The terminal event already carries the final message. Do not wait on + // result() here: a provider that closes or rejects immediately after + // emitting the terminal event may leave that promise unresolved. + const finalMessage = event.type === "done" ? event.message : event.error; if (addedPartial) { context.messages[context.messages.length - 1] = finalMessage; } else { @@ -396,16 +423,58 @@ async function streamAssistantResponse( } } } + throw new Error("Provider stream ended without a terminal event"); +} - const finalMessage = await response.result(); - if (addedPartial) { - context.messages[context.messages.length - 1] = finalMessage; - } else { - context.messages.push(finalMessage); - await emit({ type: "message_start", message: { ...finalMessage } }); +async function* normalizeProviderStream( + createStream: () => ReturnType, + model: AgentLoopConfig["model"], + signal: AbortSignal | undefined, +): AsyncGenerator { + let partial: AssistantMessage | null = null; + let terminal = false; + try { + const response = await createStream(); + for await (const event of response) { + if ("partial" in event) partial = event.partial; + terminal = event.type === "done" || event.type === "error"; + yield event; + if (terminal) return; + } + throw new Error("The model response ended before completion"); + } catch (error) { + if (terminal) throw error; + const message = interruptedResponse(partial, model, signal, error); + yield { type: "error", reason: signal?.aborted ? "aborted" : "error", error: message }; } - await emit({ type: "message_end", message: finalMessage }); - return finalMessage; +} + +function interruptedResponse( + partialMessage: AssistantMessage | null, + model: AgentLoopConfig["model"], + signal: AbortSignal | undefined, + error: unknown, +): AssistantMessage { + return { + ...(partialMessage ?? { + role: "assistant", + content: [], + api: model.api, + provider: model.provider, + model: model.id, + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + timestamp: Date.now(), + }), + stopReason: signal?.aborted ? "aborted" : "error", + errorMessage: error instanceof Error ? error.message : String(error), + }; } /** @@ -736,7 +805,7 @@ async function executePreparedToolCall( ); acceptingUpdates = false; await Promise.all(updateEvents); - return { result, isError: false }; + return { result, isError: result.isError === true }; } catch (error) { acceptingUpdates = false; await Promise.all(updateEvents); diff --git a/packages/agent-core/src/types.ts b/packages/agent-core/src/types.ts index 85947b11..a4acd33d 100644 --- a/packages/agent-core/src/types.ts +++ b/packages/agent-core/src/types.ts @@ -383,6 +383,8 @@ export interface AgentState { /** Final or partial result produced by a tool. */ export interface AgentToolResult { + /** A failed tool result may retain successful partial output and structured details. */ + isError?: boolean; /** Text or image content returned to the model. */ content: (TextContent | ImageContent)[]; /** Arbitrary structured details for logs or UI rendering. */ diff --git a/packages/agent-core/test/agent-loop.test.ts b/packages/agent-core/test/agent-loop.test.ts index 79957ef1..499e04e2 100644 --- a/packages/agent-core/test/agent-loop.test.ts +++ b/packages/agent-core/test/agent-loop.test.ts @@ -1,6 +1,7 @@ import { type AssistantMessage, type AssistantMessageEvent, + type AssistantMessageEventStream, EventStream, type Message, type Model, @@ -8,7 +9,7 @@ import { } from "@step-harness/providers"; import { Type } from "typebox"; import { describe, expect, it } from "vitest"; -import { agentLoop, agentLoopContinue } from "../src/agent-loop.ts"; +import { agentLoop, agentLoopContinue, runAgentLoop } from "../src/agent-loop.ts"; import { setDefaultStreamFn } from "../src/index.ts"; import type { AgentContext, AgentEvent, AgentLoopConfig, AgentMessage, AgentTool } from "../src/types.ts"; @@ -163,6 +164,117 @@ describe("agentLoop with AgentMessage", () => { expect(eventTypes).toContain("agent_end"); }); + it("finalizes a partial assistant when the provider stream throws", async () => { + const partial = createAssistantMessage([{ type: "text", text: "partial response" }], "pending"); + const streamFn = () => + ({ + async *[Symbol.asyncIterator]() { + yield { type: "start", partial } satisfies AssistantMessageEvent; + throw new Error("connection terminated"); + }, + result: () => new Promise(() => {}), + }) as unknown as AssistantMessageEventStream; + + const events: AgentEvent[] = []; + const stream = agentLoop( + [createUserMessage("Hello")], + { systemPrompt: "", messages: [], tools: [] }, + { model: createModel(), convertToLlm: identityConverter }, + undefined, + streamFn, + ); + for await (const event of stream) events.push(event); + + const assistantEnd = events.find((event) => event.type === "message_end" && event.message.role === "assistant"); + expect(assistantEnd).toMatchObject({ + type: "message_end", + message: { role: "assistant", stopReason: "error", errorMessage: "connection terminated" }, + }); + expect(events.at(-1)?.type).toBe("agent_end"); + }); + + it("finalizes synchronous provider construction failures", async () => { + const stream = agentLoop( + [createUserMessage("Hello")], + { systemPrompt: "", messages: [], tools: [] }, + { model: createModel(), convertToLlm: identityConverter }, + undefined, + () => { + throw new Error("provider construction failed"); + }, + ); + const events: AgentEvent[] = []; + for await (const event of stream) events.push(event); + expect( + events.filter((event) => event.type === "message_end" && event.message.role === "assistant"), + ).toMatchObject([{ message: { stopReason: "error", errorMessage: "provider construction failed" } }]); + expect(events.at(-1)?.type).toBe("agent_end"); + expect((await stream.result()).at(-1)).toMatchObject({ stopReason: "error" }); + }); + + it.each(["new", "continue"])( + "%s loop surfaces conversion failures instead of leaving consumers hanging", + async (mode) => { + const failure = new Error("conversion failed"); + const context = { systemPrompt: "", messages: [createUserMessage("Hello")], tools: [] }; + const config: AgentLoopConfig = { + model: createModel(), + convertToLlm: () => { + throw failure; + }, + }; + const streamFn = () => new MockAssistantStream(); + const stream = + mode === "new" + ? agentLoop([], context, config, undefined, streamFn) + : agentLoopContinue(context, config, undefined, streamFn); + await expect( + (async () => { + for await (const _event of stream) { + /* consume */ + } + })(), + ).rejects.toBe(failure); + await expect(stream.result()).rejects.toBe(failure); + }, + ); + + it.each(["message_update", "message_end"])( + "does not turn %s listener failures into provider errors", + async (failAt) => { + const failure = new Error("listener failed: 503"); + const partial = createAssistantMessage([{ type: "text", text: "partial" }], "pending"); + const final = createAssistantMessage([{ type: "text", text: "complete final content" }]); + const events: AgentEvent[] = []; + await expect( + runAgentLoop( + [createUserMessage("Hello")], + { systemPrompt: "", messages: [], tools: [] }, + { model: createModel(), convertToLlm: identityConverter }, + (event) => { + events.push(event); + if ( + (event.type === "message_update" && failAt === "message_update") || + (event.type === "message_end" && failAt === "message_end" && event.message.role === "assistant") + ) + throw failure; + }, + undefined, + () => { + const response = new MockAssistantStream(); + response.push({ type: "start", partial }); + response.push({ type: "text_delta", contentIndex: 0, delta: "partial", partial }); + response.push({ type: "done", reason: "stop", message: final }); + return response; + }, + ), + ).rejects.toBe(failure); + const ends = events.filter((event) => event.type === "message_end" && event.message.role === "assistant"); + expect(ends).toHaveLength(failAt === "message_end" ? 1 : 0); + if (ends.length) expect(ends[0]).toMatchObject({ message: final }); + }, + ); + it("should handle custom message types via convertToLlm", async () => { // Create a custom message type interface CustomNotification { @@ -368,6 +480,59 @@ describe("agentLoop with AgentMessage", () => { expect(toolResult?.role === "toolResult" ? toolResult.usage : undefined).toEqual(patchedToolUsage); }); + it("preserves explicit tool failure details in events and the next model context", async () => { + const details = { status: "partial_failed", completed: 12, failed: 10 }; + const tool: AgentTool = { + name: "workflow", + label: "Workflow", + description: "Run nodes", + parameters: Type.Object({}), + execute: async () => ({ isError: true, details, content: [{ type: "text", text: "10 nodes failed: 429" }] }), + }; + let calls = 0; + const stream = agentLoop( + [createUserMessage("run")], + { systemPrompt: "", messages: [], tools: [tool] }, + { model: createModel(), convertToLlm: identityConverter }, + undefined, + (_model, context) => { + const response = new MockAssistantStream(); + if (calls++ === 0) { + response.push({ + type: "done", + reason: "toolUse", + message: createAssistantMessage( + [{ type: "toolCall", id: "wf", name: "workflow", arguments: {} }], + "toolUse", + ), + }); + } else { + expect(context.messages.at(-1)).toMatchObject({ + role: "toolResult", + isError: true, + content: [{ text: "10 nodes failed: 429" }], + }); + response.push({ + type: "done", + reason: "stop", + message: createAssistantMessage([{ type: "text", text: "partial failure" }]), + }); + } + return response; + }, + ); + const events: AgentEvent[] = []; + for await (const event of stream) events.push(event); + expect(events.find((event) => event.type === "tool_execution_end")).toMatchObject({ + isError: true, + result: { details }, + }); + expect((await stream.result()).find((message) => message.role === "toolResult")).toMatchObject({ + isError: true, + details, + }); + }); + it("should not execute tool calls from a length-truncated assistant message", async () => { const toolSchema = Type.Object({ value: Type.String() }); const executed: string[] = []; diff --git a/packages/coding-agent/docs/settings.md b/packages/coding-agent/docs/settings.md index 2d10d027..8b18dfdb 100644 --- a/packages/coding-agent/docs/settings.md +++ b/packages/coding-agent/docs/settings.md @@ -137,6 +137,7 @@ For VS Code, include `--wait` so step resumes after the editor exits: |---------|------|---------|-------------| | `retry.enabled` | boolean | `true` | Enable automatic agent-level retry on transient errors | | `retry.maxRetries` | number | `3` | Maximum agent-level retry attempts | +| `retry.rateLimitMaxRetries` | number | `3` | Independent 429 retry budget per model request (0 disables, maximum 20); does not consume schema or other transient retries | | `retry.baseDelayMs` | number | `2000` | Base delay for agent-level exponential backoff (2s, 4s, 8s) | | `retry.provider.timeoutMs` | number | SDK default | Provider/SDK request timeout in milliseconds | | `retry.provider.maxRetries` | number | `0` | Provider/SDK retry attempts | @@ -146,6 +147,26 @@ When a provider requests a retry delay longer than `retry.provider.maxRetryDelay Keep `retry.provider.maxRetries` at `0` unless provider-level retries are explicitly needed. Setting it above `0` can make SDK/provider retries handle out-of-usage-limit errors before Step sees them, which may block the agent until the provider quota resets in some circumstances. +Temporary 429/concurrency errors retry the current model request, preserving completed tool calls. They honor `Retry-After` when available; otherwise delays grow from 1 second to at most 30 seconds with jitter. Cancellation interrupts the wait. Explicit billing/quota exhaustion is not retried. `retry.enabled: false` disables both session retry budgets. + +### Workflow concurrency and failures + +Explicit counted fan-out requests such as `开10个子agent` / `launch 10 subagents` activate per-turn workflow orchestration without requiring the Ultraloop keyword. A counted request is only consent when the user is asking for the work: questions about it (`开10个子agent要多少钱？`), refusals, hedging, and file/role nouns (`3 agent definition files`) are not. The ordinary `subagent` tool also accepts batches larger than its concurrent window (up to 1000 by default) and queues the excess; its default concurrency uses the same CPU-derived ceiling, but it runs in its own window and does not read `STEP_WORKFLOW_CONCURRENCY_LIMIT`. + +The workflow ceiling is `max(1, min(16, CPU count - 2))`. Model-authored `maxConcurrency` can only lower this ceiling. Set `STEP_WORKFLOW_CONCURRENCY_LIMIT=8` to impose a lower trusted provider limit (for example, a backend allowing eight simultaneous requests). The effective ceiling is the minimum of the machine and configured limits; the final result reports requested, effective, and current concurrency. + +Only the failure text the model request reported is read as a rate limit; agent output and node labels that mention "429" or "rate limit" are never treated as congestion. Temporary 429s reduce the admission cap by 25% (minimum one), once per congestion epoch. New nodes wait during backoff; running nodes keep their slots and retry their current request. Four successful responses allow a one-slot recovery when work is queued. Concurrent workflows share a process-wide host window, in addition to each run's lower limit. This conservatively covers all model overrides; it is not a cross-process/account-wide quota lock. + +A node's own retry honors the server's `Retry-After` verbatim, however large, including when the provider's own `maxRetryDelayMs` cap rejected the wait. The admission cooldown that follows a 429 is capped at 30 seconds, because that window is process-wide and its queue time is excluded from node timeouts: an unbounded server delay would otherwise stall unrelated workflows with no error and no timeout. The cap reduction above, not the cooldown length, is what protects the provider. + +`parallel()` accepts both lazy functions (`parallel(items.map(x => () => agent(String(x))))`) and eager promises (`parallel(items.map(x => agent(String(x))))`). Always await or return the operation. Invalid entries reject the batch. A script error or an early return with unfinished calls cancels running and queued nodes and waits for cleanup before publishing the terminal state; production child processes retain their shared slots until they exit. The UI shows concurrency clamping as a Notice, and script failure/cancellation separately. + +Node timeouts cover model requests, tool execution, and retries **after admission**, not time waiting in either queue. Tokens a node reports after it is cancelled or timed out (within the cleanup grace) are still charged to the budget. The deadline is armed per schema attempt, so a node retried for schema mismatches can take up to three times `agentTimeoutMs`. For experiments that report sleep durations or random choices, generate, capture and print all required values in one command, then require structured output with `agent(..., {schema})`. An empty sleep output is not evidence of the chosen duration. See [Workflow experiments](workflow-experiments.md). + +Failed nodes are retained in the journal and final `failures` list even if the script catches errors or filters `parallel()`'s null results. Final status is `partial_failed` for mixed outcomes and `failed` when all nodes fail; both are error tool results with successful output preserved. `summary.completed` counts successful/cached nodes, not attempted calls. Request retry counts (`rateLimitRetries`) are separate from schema `attempt`. + +A callback that catches an agent-node failure and rethrows a **new** error is counted twice, as one failed node and one callback failure, because the node-failure marker travels with the original error object. Rethrow the caught error itself to keep a single count. The shared admission window covers concurrent workflows. When the last one finishes, a window that was never throttled is discarded; a window that was lowered or is cooling down after a 429 is remembered for 60 seconds, so back-to-back workflows do not restart at the full ceiling straight after a 429. After that it starts fresh. + ```json { "retry": { diff --git a/packages/coding-agent/docs/workflow-experiments.md b/packages/coding-agent/docs/workflow-experiments.md new file mode 100644 index 00000000..5ba37e35 --- /dev/null +++ b/packages/coding-agent/docs/workflow-experiments.md @@ -0,0 +1,50 @@ +# Reproducible workflow experiments + +## Preserve shell variables inside JavaScript + +Shell commands are data, not JavaScript expressions. Prefer an ordinary JS string: + +```js +const command = 'd=$((RANDOM % 10 + 1)); sleep $d; echo "slept=${d}s random=$RANDOM"'; +return await parallel(Array.from({length: 10}, (_, i) => () => + agent('Run this command and report only its actual output: ' + command, {label: 'worker-' + (i + 1)}) +)); +``` + +Putting that command unchanged in a backtick JS template evaluates `${d}` in JavaScript, where `d` does not exist. Escape the dollar sign (`\${d}`) if a template is necessary. The runtime does not rewrite interpolation because `${...}` can also be an intentional JS expression. + +Errors in `parallel()` callbacks or `pipeline()` stages are included in `taskFailures`, even when they occur before any `agent()` call, including inside nested workflows. `summary.failedCallbacks` counts those errors separately from failed agent nodes; node failures propagated through the provided helpers are not counted again as callback errors. A run with callback failures cannot report `completed` merely because its agent-call count is zero. Unawaited nested workflow/iterate calls also fail the owning run and are cancelled during cleanup. + +## Random-sleep experiment + +To test a 15-node fan-out, request `maxConcurrency: 15` and leave enough execution time for model requests as well as the tools (for example `agentTimeoutMs: 180000`). A ten-core machine clamps the request to eight slots; excess nodes queue normally. Concurrent workflows share the host window. + +Prefer lazy task functions, though promises are also supported: + +```js +const ids = Array.from({ length: 15 }, (_, i) => i + 1); +return await parallel(ids.map(id => () => { + const command = `node -e 'const {randomInt}=require("node:crypto"); const seconds=randomInt(1,21); const start=Date.now(); setTimeout(()=>console.log(JSON.stringify({id:${id},sleptSeconds:seconds,randomNumber:randomInt(1,1001),elapsedMs:Date.now()-start})),seconds*1000)'`; + return agent( + `Run exactly this one foreground command (timeout_ms: 30000):\n${command}\nReturn only its JSON output. Do not guess or change any values.`, + { + label: `sleep-${id}`, + schema: { + type: "object", + properties: { + id: { type: "integer", const: id }, + sleptSeconds: { type: "integer", minimum: 1, maximum: 20 }, + randomNumber: { type: "integer", minimum: 1, maximum: 1000 }, + elapsedMs: { type: "number", minimum: 1000 }, + }, + required: ["id", "sleptSeconds", "randomNumber", "elapsedMs"], + additionalProperties: false, + }, + }, + ); +})); +``` + +Verification must compare each returned row with its child session's actual tool output. Schema validation proves shape and range, not that a model copied the observed values honestly. Check elapsed time against the selected seconds, and inspect the workflow's `summary`, `failures`, node retry counts, and final status. Aggregate results must come from those verified rows. + +For cancellation tests, start running and queued nodes and then throw a deliberate script error. The terminal snapshot must have no running/queued nodes, the child processes must be gone, and neither progress nor journal may change after the tool returns. A subsequent workflow must not overlap abandoned work. diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 2b16828d..47b38dfa 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -113,6 +113,7 @@ import { type BashExecutionMessage, type CustomMessage, convertToLlm } from "./m import { ModelRegistry } from "./model-registry.ts"; import type { ModelRuntime } from "./model-runtime.ts"; import { expandPromptTemplate, type PromptTemplate } from "./prompt-templates.ts"; +import { isRateLimitError, isTerminalQuotaError, rateLimitDelay } from "./rate-limit.ts"; import type { ResourceExtensionPaths, ResourceLoader } from "./resource-loader.ts"; import { exportSessionToJsonl } from "./session-export.ts"; import type { BranchSummaryEntry, CompactionEntry, SessionEntry, SessionManager } from "./session-manager.ts"; @@ -158,7 +159,14 @@ export type AgentSessionEvent = willRetry: boolean; errorMessage?: string; } - | { type: "auto_retry_start"; attempt: number; maxAttempts: number; delayMs: number; errorMessage: string } + | { + type: "auto_retry_start"; + attempt: number; + maxAttempts: number; + delayMs: number; + errorMessage: string; + reason?: "rate_limit"; + } | { type: "auto_retry_end"; success: boolean; attempt: number; finalError?: string } | { type: "summarization_retry_scheduled"; @@ -365,6 +373,7 @@ export class AgentSession { // Retry state private _retryAbortController: AbortController | undefined = undefined; private _retryAttempt = 0; + private _rateLimitRetryAttempt = 0; // Bash execution state private readonly _bashAbortControllers = new Set(); @@ -1176,6 +1185,7 @@ export class AgentSession { attempt: this._retryAttempt, }); this._retryAttempt = 0; + this._rateLimitRetryAttempt = 0; } } } @@ -1197,14 +1207,20 @@ export class AgentSession { private _willRetryAfterAgentEnd(event: Extract): boolean { const settings = this.settingsManager.getRetrySettings(); - if (!settings.enabled || this._retryAttempt >= settings.maxRetries) { + if (!settings.enabled) { return false; } for (let i = event.messages.length - 1; i >= 0; i--) { const message = event.messages[i]; if (message.role === "assistant") { - return this._isRetryableError(message as AssistantMessage); + const rateLimited = isRateLimitError((message as AssistantMessage).errorMessage ?? ""); + return ( + this._isRetryableError(message as AssistantMessage) && + (rateLimited + ? this._rateLimitRetryAttempt < settings.rateLimitMaxRetries + : this._retryAttempt - this._rateLimitRetryAttempt < settings.maxRetries) + ); } } return false; @@ -1681,6 +1697,7 @@ export class AgentSession { // all of them and would leave a stale count for the next prompt to read. // Cleared last so agent_settled listeners still see the final count. this._retryAttempt = 0; + this._rateLimitRetryAttempt = 0; } } @@ -1705,6 +1722,7 @@ export class AgentSession { finalError: msg.errorMessage, }); this._retryAttempt = 0; + this._rateLimitRetryAttempt = 0; } // CLM maintains context for a future request. A successful settled answer @@ -3498,9 +3516,13 @@ export class AgentSession { * Context overflow errors are NOT retryable (handled by compaction instead). */ private _isRetryableError(message: AssistantMessage): boolean { + if (isTerminalQuotaError(message.errorMessage ?? "")) return false; // Context overflow is handled by compaction, not retry. if (isContextOverflow(message, this.model?.contextWindow ?? 0)) return false; - return isRetryableAssistantError(message); + return ( + isRetryableAssistantError(message) || + (message.stopReason === "error" && isRateLimitError(message.errorMessage ?? "")) + ); } /** @@ -3544,21 +3566,26 @@ export class AgentSession { return false; } - this._retryAttempt++; - - if (this._retryAttempt > settings.maxRetries) { - // Preserve the completed attempt count so post-run handling can emit the final failure. - this._retryAttempt--; + const rateLimited = isRateLimitError(message.errorMessage ?? ""); + const maxRetries = rateLimited ? settings.rateLimitMaxRetries : settings.maxRetries; + const attempt = + (rateLimited ? this._rateLimitRetryAttempt : this._retryAttempt - this._rateLimitRetryAttempt) + 1; + if (attempt > maxRetries) { return false; } + this._retryAttempt++; + if (rateLimited) this._rateLimitRetryAttempt++; - const delayMs = settings.baseDelayMs * 2 ** (this._retryAttempt - 1); + const delayMs = rateLimited + ? rateLimitDelay(attempt, message.retryAfterMs) + : settings.baseDelayMs * 2 ** (attempt - 1); this._retryAbortController = new AbortController(); this._emit({ type: "auto_retry_start", - attempt: this._retryAttempt, - maxAttempts: settings.maxRetries, + attempt, + maxAttempts: maxRetries, + ...(rateLimited ? { reason: "rate_limit" as const } : {}), delayMs, errorMessage: message.errorMessage || "Unknown error", }); @@ -3576,6 +3603,7 @@ export class AgentSession { // Aborted during sleep - emit end event so UI can clean up const attempt = this._retryAttempt; this._retryAttempt = 0; + this._rateLimitRetryAttempt = 0; this._emit({ type: "auto_retry_end", success: false, diff --git a/packages/coding-agent/src/core/rate-limit.ts b/packages/coding-agent/src/core/rate-limit.ts new file mode 100644 index 00000000..b605ded1 --- /dev/null +++ b/packages/coding-agent/src/core/rate-limit.ts @@ -0,0 +1,19 @@ +/** Temporary throttling is distinct from exhausted billing/plan quotas. */ +export function isRateLimitError(message: string): boolean { + return /\b429\b|rate[ _-]?limit|concurrency reached|too many requests/i.test(message); +} + +export function isTerminalQuotaError(message: string): boolean { + return /GoUsageLimitError|FreeUsageLimitError|insufficient_quota|usage_limit_reached|quota exhausted|quota exceeded|billing|insufficient balance|available balance|monthly usage limit|out of budget/i.test( + message, + ); +} + +export function rateLimitDelay(attempt: number, retryAfterMs?: number): number { + if (retryAfterMs !== undefined && Number.isFinite(retryAfterMs) && retryAfterMs >= 0) return retryAfterMs; + return Math.round(Math.min(30_000, 1000 * 2 ** (attempt - 1)) * (0.75 + Math.random() * 0.25)); +} +export type ModelRequestProgress = + | { type: "started" } + | { type: "succeeded" } + | { type: "rate_limit"; attempt: number; maxRetries: number; delayMs: number; message: string }; diff --git a/packages/coding-agent/src/core/settings-manager.ts b/packages/coding-agent/src/core/settings-manager.ts index a30e2e3b..99e09c0c 100644 --- a/packages/coding-agent/src/core/settings-manager.ts +++ b/packages/coding-agent/src/core/settings-manager.ts @@ -36,6 +36,7 @@ export interface ProviderRetrySettings { export interface RetrySettings { enabled?: boolean; // default: true maxRetries?: number; // default: 3 + rateLimitMaxRetries?: number; // default: 3; independent of transient and schema retries baseDelayMs?: number; // default: 2000 (exponential backoff: 2s, 4s, 8s) provider?: ProviderRetrySettings; } @@ -925,10 +926,15 @@ export class SettingsManager { this.save(); } - getRetrySettings(): { enabled: boolean; maxRetries: number; baseDelayMs: number } { + getRetrySettings(): { enabled: boolean; maxRetries: number; rateLimitMaxRetries: number; baseDelayMs: number } { + const rateLimitMaxRetries = this.settings.retry?.rateLimitMaxRetries; return { enabled: this.getRetryEnabled(), maxRetries: this.settings.retry?.maxRetries ?? 3, + rateLimitMaxRetries: + rateLimitMaxRetries === undefined || !Number.isFinite(rateLimitMaxRetries) + ? 3 + : Math.max(0, Math.min(20, Math.floor(rateLimitMaxRetries))), baseDelayMs: this.settings.retry?.baseDelayMs ?? 2000, }; } diff --git a/packages/coding-agent/src/features/step-subagent.ts b/packages/coding-agent/src/features/step-subagent.ts index 77efd105..bc336aef 100644 --- a/packages/coding-agent/src/features/step-subagent.ts +++ b/packages/coding-agent/src/features/step-subagent.ts @@ -34,6 +34,7 @@ import { subagentListSignature, } from "./subagent/rendering.ts"; import { createSubagentRpcSession } from "./subagent/rpc-adapter.ts"; +import { clampWorkflowConcurrency } from "./workflow/budget.ts"; // Lane-notification primitives moved to ./subagent/lane-events.ts; re-exported // here to keep this module's public surface stable. @@ -69,8 +70,7 @@ export { WORKFLOW_SESSION_ID_PREFIX, } from "./subagent/helpers.ts"; -const MAX_PARALLEL_TASKS = 8; -const DEFAULT_CONCURRENCY = 4; +const MAX_PARALLEL_TASKS = 1000; export const MAX_JSON_LINE_BYTES = 2 * 1024 * 1024; const MAX_MESSAGES = 256; export const CHILD_MARKER = "STEPCODE_SUBAGENT_CHILD"; @@ -106,6 +106,7 @@ export interface StepSubagentRunResult { } export interface StepSubagentRunInput { + onModelRequest?: (event: import("../core/rate-limit.ts").ModelRequestProgress) => void; agent: StepAgentConfig; task: string; cwd: string; @@ -187,9 +188,9 @@ export interface StepSubagentExtensionOptions { configDirName?: string; /** Include the built-in general/explore/review/planner roles. */ includeBuiltinAgents?: boolean; - /** Maximum number of tasks accepted in one parallel call. */ + /** Maximum batch size accepted in one parallel call; excess tasks queue instead of running. */ maxParallelTasks?: number; - /** Number of child processes allowed to run at once. */ + /** Number of child processes allowed to run at once; tasks beyond this stay queued. */ maxConcurrency?: number; /** Optional isolated git worktree implementation. */ worktreeManager?: StepWorktreeManager; @@ -409,15 +410,34 @@ const spawnedSubagentSessions = new Set(); export async function runStepSubagentProcess(input: StepSubagentRunInput): Promise { const sessionId = input.sessionId?.trim() || `${SUBAGENT_SESSION_ID_PREFIX}${randomUUID()}`; const live = getLiveSubagentSession(sessionId); - if (live) return live.runTurn(input); + if (live) { + const result = await live.runTurn(input); + if (input.workflowAcl && !input.keepAlive) await live.exited; + return result; + } if (spawnedSubagentSessions.has(sessionId)) input.onChildRespawn?.(); spawnedSubagentSessions.add(sessionId); const session = await createSubagentRpcSession(input, sessionId); - return session.runTurn(input); + const result = await session.runTurn(input); + // A workflow node owns its child through process exit, not just agent_settled. + // This keeps its shared admission slot until cancellation has actually stopped + // the child, and lets the next workflow start without orphaned requests. + if (input.workflowAcl && !input.keepAlive) await session.exited; + return result; } -export function makeToolResult(details: StepSubagentDetails, text: string): AgentToolResult { - return { content: [{ type: "text", text }], details }; +export function makeToolResult( + details: StepSubagentDetails, + text: string, + isError = false, +): AgentToolResult { + return { + content: [{ type: "text", text }], + details, + ...(isError || details.results.some((record) => record.status === "failed" || record.status === "aborted") + ? { isError: true } + : {}), + }; } const AgentSendTargetSchema = Type.Object({ @@ -509,7 +529,7 @@ export function createStepSubagentExtension(options: StepSubagentExtensionOption maxParallelTasks: Math.max(1, Math.min(MAX_PARALLEL_TASKS, options.maxParallelTasks ?? MAX_PARALLEL_TASKS)), maxConcurrency: Math.max( 1, - Math.min(options.maxConcurrency ?? DEFAULT_CONCURRENCY, options.maxParallelTasks ?? MAX_PARALLEL_TASKS), + Math.min(clampWorkflowConcurrency(options.maxConcurrency), options.maxParallelTasks ?? MAX_PARALLEL_TASKS), ), telemetry: options.telemetry, worktreeManager: options.worktreeManager ?? { diff --git a/packages/coding-agent/src/features/subagent/execute.ts b/packages/coding-agent/src/features/subagent/execute.ts index a1b578ea..abe2934f 100644 --- a/packages/coding-agent/src/features/subagent/execute.ts +++ b/packages/coding-agent/src/features/subagent/execute.ts @@ -279,6 +279,7 @@ export async function executeSubagent( return makeToolResult( details, `Invalid parameters. Provide exactly one mode. Available agents: ${formatStepAgentCatalog(discovery.agents)}`, + true, ); } if (parallel && parallel.length > options.maxParallelTasks) { @@ -286,13 +287,14 @@ export async function executeSubagent( return makeToolResult( details, `Too many parallel tasks (${parallel.length}); maximum is ${options.maxParallelTasks}.`, + true, ); } const tasks = parallel ?? chain ?? [single!]; const mode: StepSubagentDetails["mode"] = chain ? "chain" : parallel ? "parallel" : "single"; if (!(await confirmProjectAgents(ctx, params, discovery, tasks))) { const details = buildDetails(mode, discovery, scope, []); - return makeToolResult(details, "Canceled: project-local agents not approved."); + return makeToolResult(details, "Canceled: project-local agents not approved.", true); } const records = tasks.map((task) => makeEmptyResult(task.agent, task.task)); diff --git a/packages/coding-agent/src/features/subagent/rpc-adapter.ts b/packages/coding-agent/src/features/subagent/rpc-adapter.ts index 9010d79a..72bb9304 100644 --- a/packages/coding-agent/src/features/subagent/rpc-adapter.ts +++ b/packages/coding-agent/src/features/subagent/rpc-adapter.ts @@ -13,6 +13,7 @@ import { randomUUID } from "node:crypto"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { waitForChildProcess } from "../../utils/child-process.ts"; import { CHILD_MARKER, cloneUsage, @@ -37,6 +38,7 @@ const MAX_STDERR_CHARS = 32_000; * Step-specific needs-input nag are protocol frames, not session events. */ export interface SubagentRpcLineHandlers { + onModelRequest?: StepSubagentRunInput["onModelRequest"]; /** `{"type":"response"}` command acks, correlated by `id`. */ onResponse?: (response: { id?: string; command?: string; success?: boolean; error?: string }) => void; /** `{"type":"extension_ui_request"}` dialogs; blocking ones must be answered or the child hangs. */ @@ -83,6 +85,31 @@ export function routeSubagentRpcLine(line: string, handlers: SubagentRpcLineHand handlers.onNeedsInput?.(typeof parsed.message === "string" ? parsed.message : ""); return; } + if ( + type === "auto_retry_start" && + parsed.reason === "rate_limit" && + typeof parsed.attempt === "number" && + typeof parsed.maxAttempts === "number" && + typeof parsed.delayMs === "number" && + Number.isFinite(parsed.delayMs) && + parsed.delayMs >= 0 + ) { + handlers.onModelRequest?.({ + type: "rate_limit", + attempt: parsed.attempt, + maxRetries: parsed.maxAttempts, + delayMs: parsed.delayMs, + message: String(parsed.errorMessage ?? "429 rate limited"), + }); + } + // turn_start precedes the provider request. message_start may arrive only + // after HTTP headers (or an error), too late to tag a congestion epoch. + if (type === "turn_start") handlers.onModelRequest?.({ type: "started" }); + if (isRecordValue(parsed.message) && parsed.message.role === "assistant") { + if (type === "message_end" && ["stop", "length", "toolUse"].includes(String(parsed.message.stopReason))) { + handlers.onModelRequest?.({ type: "succeeded" }); + } + } handlers.onEvent?.(line, parsed); } @@ -100,6 +127,8 @@ interface SubagentRpcTurn { /** A live `--mode rpc` child bound to one subagent session id. */ export interface StepSubagentRpcSession { + /** Resolves only once the OS child has exited (including forced shutdown). */ + readonly exited: Promise; readonly sessionId: string; /** True while the child can still accept stdin commands. */ isAlive(): boolean; @@ -205,6 +234,13 @@ export async function createSubagentRpcSession( shell: false, stdio: ["pipe", "pipe", "pipe"], }); + // `close` can be held open by inherited descendant pipes after OS exit. + // Reuse the bounded-idle stdio drain used by our other child processes. + const exitStatus = waitForChildProcess(child); + const exited = exitStatus.then( + () => undefined, + () => undefined, + ); let stdoutBuffer = ""; let turn: SubagentRpcTurn | undefined; @@ -341,6 +377,7 @@ export async function createSubagentRpcSession( }, onExtensionError: (message) => appendStderr(`extension_error: ${message}\n`), onNeedsInput: (message) => turn?.input.onNeedsInput?.(message), + onModelRequest: (event) => turn?.input.onModelRequest?.(event), onEvent: (eventLine, event) => { const active = turn; if (!active) return; @@ -388,7 +425,7 @@ export async function createSubagentRpcSession( }); void rm(tempDir, { recursive: true, force: true }); }); - child.once("close", (code) => { + const finishExitedChild = (code: number | null): void => { childExited = true; dropFromRegistry(); clearIdleWatchdog(); @@ -409,7 +446,13 @@ export async function createSubagentRpcSession( } }); void rm(tempDir, { recursive: true, force: true }); + }; + child.once("exit", () => { + childExited = true; + dropFromRegistry(); }); + child.once("close", finishExitedChild); + void exitStatus.then(finishExitedChild, () => undefined); const runTurn = (turnInput: StepSubagentRunInput): Promise => { if (turn) { @@ -484,6 +527,7 @@ export async function createSubagentRpcSession( }; const handle: StepSubagentRpcSession = { + exited, sessionId, isAlive: () => !childExited && !stdinEnded, isTurnActive: () => turn !== undefined, diff --git a/packages/coding-agent/src/features/workflow/admission.ts b/packages/coding-agent/src/features/workflow/admission.ts new file mode 100644 index 00000000..21413e80 --- /dev/null +++ b/packages/coding-agent/src/features/workflow/admission.ts @@ -0,0 +1,56 @@ +import { WorkflowSemaphore } from "./budget.ts"; + +/** A process-wide node window, shared by concurrent workflow tools/sessions. + * Conservatively includes all models: model overrides cannot escape the host + * ceiling. Each run also retains its own lower maxConcurrency window. + */ +let pool: { semaphore: WorkflowSemaphore; members: Set<{ ceiling: number }>; idleSince?: number } | undefined; + +/** + * How long a congested window is remembered after its last workflow finishes. A + * session often runs workflows back to back (one per phase); starting each at the + * full ceiling straight after a 429 would re-trigger the congestion just seen. An + * idle window that never throttled is discarded immediately. + */ +const THROTTLED_POOL_RETENTION_MS = 60_000; + +export interface WorkflowAdmissionLease { + semaphore: WorkflowSemaphore; + release(): void; +} + +export function acquireWorkflowAdmission(ceiling: number): WorkflowAdmissionLease { + if (pool && pool.members.size === 0 && Date.now() - (pool.idleSince ?? 0) >= THROTTLED_POOL_RETENTION_MS) { + pool.semaphore.dispose(); + pool = undefined; + } + pool ??= { semaphore: new WorkflowSemaphore(ceiling), members: new Set() }; + const current = pool; + current.idleSince = undefined; + const member = { ceiling }; + current.members.add(member); + current.semaphore.setCeiling(Math.min(...Array.from(current.members, (item) => item.ceiling))); + return { + semaphore: current.semaphore, + release() { + if (!current.members.delete(member)) return; + if (current.members.size > 0) { + current.semaphore.setCeiling(Math.min(...Array.from(current.members, (item) => item.ceiling))); + } else if (current.semaphore.throttled) { + current.semaphore.park(); + current.idleSince = Date.now(); + } else { + current.semaphore.dispose(); + if (pool === current) pool = undefined; + } + }, + }; +} + +/** Forget retained congestion state. Intended for tests and host shutdown. */ +export function resetWorkflowAdmission(): void { + if (pool && pool.members.size === 0) { + pool.semaphore.dispose(); + pool = undefined; + } +} diff --git a/packages/coding-agent/src/features/workflow/agent-runner.ts b/packages/coding-agent/src/features/workflow/agent-runner.ts index 7bdc602b..301f0f93 100644 --- a/packages/coding-agent/src/features/workflow/agent-runner.ts +++ b/packages/coding-agent/src/features/workflow/agent-runner.ts @@ -43,6 +43,7 @@ export function createDefaultWorkflowAgentRunner(options: DefaultWorkflowAgentRu signal: input.signal, sessionId: `${WORKFLOW_SESSION_ID_PREFIX}${input.runId}-${input.agentId}`, keepAlive: false, + onModelRequest: input.onModelRequest, workflowAcl: { baseCwd: input.cwd, ...(input.options.readOnly ? { readOnly: input.options.readOnly } : {}), diff --git a/packages/coding-agent/src/features/workflow/budget.ts b/packages/coding-agent/src/features/workflow/budget.ts index 0d728c80..cf3f68cd 100644 --- a/packages/coding-agent/src/features/workflow/budget.ts +++ b/packages/coding-agent/src/features/workflow/budget.ts @@ -2,7 +2,6 @@ import os from "node:os"; import { emptyWorkflowUsage, mergeWorkflowUsage, type WorkflowUsage, workflowUsageTokens } from "./types.ts"; const DEFAULT_MAX_CONCURRENCY = 16; -const HARD_MAX_CONCURRENCY = 32; const DEFAULT_MAX_AGENTS = 1000; /** Raised when a workflow cannot afford another completed agent call. */ @@ -75,7 +74,16 @@ export function defaultWorkflowConcurrency(cpuCount = os.cpus().length): number export function clampWorkflowConcurrency(value: number | undefined, cpuCount = os.cpus().length): number { const fallback = defaultWorkflowConcurrency(cpuCount); if (value === undefined || !Number.isFinite(value)) return fallback; - return Math.max(1, Math.min(HARD_MAX_CONCURRENCY, Math.floor(value))); + return Math.max(1, Math.min(fallback, Math.floor(value))); +} + +/** Empty optional environment values mean no configured provider override. */ +export function configuredWorkflowConcurrencyLimit( + raw = process.env.STEP_WORKFLOW_CONCURRENCY_LIMIT, +): number | undefined { + if (raw === undefined || raw.trim() === "") return undefined; + const value = Number(raw); + return Number.isFinite(value) && value > 0 ? value : undefined; } export function clampWorkflowAgentLimit(value: number | undefined): number { @@ -83,9 +91,27 @@ export function clampWorkflowAgentLimit(value: number | undefined): number { return Math.max(1, Math.min(DEFAULT_MAX_AGENTS, Math.floor(value))); } +/** + * Upper bound for an admission cooldown caused by a 429. + * + * A server's `Retry-After` can be hours, or a far-future HTTP date, and the node's + * own retry deliberately honors it verbatim. The admission window is shared by every + * workflow in the process and its queue time is excluded from node timeouts, so an + * unbounded cooldown would stall unrelated workflows indefinitely with no error and + * no timeout. The adaptive cap reduction below is what actually protects the provider; + * this bound only decides how long the window pauses before probing again. + */ +const MAX_THROTTLE_COOLDOWN_MS = 30_000; + /** FIFO semaphore used by agent() so VM scripts cannot fan out unbounded work. */ export class WorkflowSemaphore { - private readonly limit: number; + private limit: number; + private ceiling: number; + private disposed = false; + private generation = 0; + private successes = 0; + private cooldownUntil = 0; + private wakeup?: ReturnType; private activeCount = 0; private peakCount = 0; private readonly waiters: Array<{ @@ -96,7 +122,64 @@ export class WorkflowSemaphore { }> = []; constructor(limit: number) { - this.limit = clampWorkflowConcurrency(limit, limit + 2); + this.limit = Math.max(1, Math.floor(limit)); + this.ceiling = this.limit; + } + + get epoch(): number { + return this.generation; + } + + setCeiling(ceiling: number): void { + if (ceiling === this.ceiling) return; + const wasAtCeiling = this.limit === this.ceiling; + this.ceiling = ceiling; + this.limit = wasAtCeiling ? ceiling : Math.min(this.limit, ceiling); + this.generation++; + this.successes = 0; + this.drain(); + } + + throttle(epoch: number, delayMs: number): void { + if (this.disposed) return; + this.successes = 0; + if (epoch === this.generation) { + this.limit = Math.max(1, Math.floor(this.limit * 0.75)); + this.generation++; + } + const cooldownMs = Number.isFinite(delayMs) ? Math.max(0, Math.min(MAX_THROTTLE_COOLDOWN_MS, delayMs)) : 0; + this.cooldownUntil = Math.max(this.cooldownUntil, Date.now() + cooldownMs); + this.scheduleDrain(); + } + + succeeded(epoch: number): void { + if (this.disposed) return; + if (epoch !== this.generation) return; + if (++this.successes >= 4 && this.waiters.length > 0 && this.limit < this.ceiling) { + this.limit++; + this.successes = 0; + this.drain(); + } + } + + private scheduleDrain(): void { + if (this.wakeup) clearTimeout(this.wakeup); + this.wakeup = setTimeout( + () => { + this.wakeup = undefined; + this.drain(); + }, + Math.min(2_147_483_647, Math.max(0, this.cooldownUntil - Date.now())), + ); + } + + dispose(): void { + this.disposed = true; + if (this.wakeup) clearTimeout(this.wakeup); + for (const waiter of this.waiters.splice(0)) { + if (waiter.signal && waiter.onAbort) waiter.signal.removeEventListener("abort", waiter.onAbort); + waiter.reject(new Error("Workflow admission is closed")); + } } get active(): number { @@ -111,9 +194,21 @@ export class WorkflowSemaphore { return this.limit; } + /** True while congestion has lowered the cap or a cooldown is still pending. */ + get throttled(): boolean { + return this.limit < this.ceiling || Date.now() < this.cooldownUntil; + } + + /** Stop the cooldown timer while no run is attached; cap and cooldown are kept. */ + park(): void { + if (this.wakeup) clearTimeout(this.wakeup); + this.wakeup = undefined; + } + async acquire(signal?: AbortSignal): Promise<() => void> { + if (this.disposed) throw new Error("Workflow admission is closed"); if (signal?.aborted) throw new Error("Workflow operation was aborted"); - if (this.activeCount < this.limit) { + if (this.activeCount < this.limit && Date.now() >= this.cooldownUntil && this.waiters.length === 0) { this.activeCount += 1; this.peakCount = Math.max(this.peakCount, this.activeCount); return this.makeRelease(); @@ -121,6 +216,7 @@ export class WorkflowSemaphore { return new Promise<() => void>((resolve, reject) => { const waiter: (typeof this.waiters)[number] = { resolve, reject, signal }; this.waiters.push(waiter); + if (!this.wakeup && Date.now() < this.cooldownUntil) this.scheduleDrain(); if (signal) { const onAbort = (): void => { const index = this.waiters.indexOf(waiter); @@ -145,6 +241,11 @@ export class WorkflowSemaphore { } private drain(): void { + if (this.disposed) return; + if (Date.now() < this.cooldownUntil) { + this.scheduleDrain(); + return; + } while (this.activeCount < this.limit && this.waiters.length > 0) { const waiter = this.waiters.shift(); if (!waiter) return; diff --git a/packages/coding-agent/src/features/workflow/progress.ts b/packages/coding-agent/src/features/workflow/progress.ts index fbf80115..1ef6ce42 100644 --- a/packages/coding-agent/src/features/workflow/progress.ts +++ b/packages/coding-agent/src/features/workflow/progress.ts @@ -40,7 +40,11 @@ export class WorkflowProgressStore { // updates arrive while this snapshot is waiting for its disk write. const snapshot = this.current(); const serialized = `${JSON.stringify(snapshot, null, 2)}\n`; - this.writeQueue = this.writeQueue.then(() => writeWorkflowFileAtomic(this.path, serialized)); + // A failed write must not poison the queue: the next update has to be able to + // persist, or one transient disk error freezes progress.json forever. + this.writeQueue = this.writeQueue + .catch(() => undefined) + .then(() => writeWorkflowFileAtomic(this.path, serialized)); await this.writeQueue; return snapshot; } diff --git a/packages/coding-agent/src/features/workflow/rendering.ts b/packages/coding-agent/src/features/workflow/rendering.ts index 19f91bd9..6e7ef354 100644 --- a/packages/coding-agent/src/features/workflow/rendering.ts +++ b/packages/coding-agent/src/features/workflow/rendering.ts @@ -4,7 +4,7 @@ import type { ToolRenderContext, ToolRenderResultOptions } from "../../core/exte import { keyHint } from "../../render/keybinding-hints.ts"; import type { Theme } from "../../theme/theme.ts"; import type { WorkflowRequest } from "./step-workflow.ts"; -import type { WorkflowProgress, WorkflowProgressAgent, WorkflowRunResult } from "./types.ts"; +import type { WorkflowProgress, WorkflowProgressAgent, WorkflowRunResult, WorkflowTaskFailure } from "./types.ts"; const COLLAPSED_AGENTS = 8; const AGENT_ORDER: Record = { @@ -28,11 +28,17 @@ function singleLine(text: string): string { function progressSummary(progress: WorkflowProgress): string { const counts = { running: 0, queued: 0, completed: 0, failed: 0, aborted: 0, cached: 0 }; for (const agent of progress.agents) counts[agent.status] += 1; + const retrying = progress.agents.filter((agent) => agent.status === "running" && agent.retry).length; return [ progress.status.replace(/_/gu, " "), - `${counts.running} running`, + ...(progress.concurrency + ? [`cap ${progress.concurrency.cap}/${progress.concurrency.limit} (ceiling ${progress.concurrency.ceiling})`] + : []), + `${counts.running - retrying} running`, + ...(retrying ? [`${retrying} retrying`] : []), `${counts.queued} queued`, `${progress.completedAgents}/${progress.totalAgents} completed`, + ...(progress.taskFailures?.length ? [`${progress.taskFailures.length} callback errors`] : []), ...(counts.cached ? [`${counts.cached} cached`] : []), ...(counts.failed ? [`${counts.failed} failed`] : []), ...(counts.aborted ? [`${counts.aborted} aborted`] : []), @@ -40,6 +46,22 @@ function progressSummary(progress: WorkflowProgress): string { ].join(" · "); } +function concurrencyNotice(concurrency: WorkflowProgress["concurrency"]): string | undefined { + return concurrency?.requested !== undefined && concurrency.requested > concurrency.limit + ? `Notice: requested concurrency ${concurrency.requested} was limited to ${concurrency.limit} by the host ceiling (CPU count or configured provider limit); tasks will queue normally.` + : undefined; +} + +/** + * A nested failure's index refers to the child script's array, so the script is the + * only thing that tells a reader which list to inspect. + */ +function taskFailureLine(failure: WorkflowTaskFailure): string { + const stage = failure.stage !== undefined ? `.${failure.stage}` : ""; + const script = failure.script ? `${singleLine(failure.script)} ` : ""; + return `${script}${failure.primitive}[${failure.index}${stage}]: ${singleLine(failure.message)}`; +} + function visibleAgents(progress: WorkflowProgress, expanded: boolean): WorkflowProgressAgent[] { const ordered = [...progress.agents].sort((left, right) => AGENT_ORDER[left.status] - AGENT_ORDER[right.status]); // Show every running task (bounded by the runtime's concurrency limit). @@ -51,19 +73,23 @@ function visibleAgents(progress: WorkflowProgress, expanded: boolean): WorkflowP function agentSummary(agent: WorkflowProgressAgent): string { const label = singleLine(agent.label); const task = agent.task ? singleLine(agent.task) : ""; - return `${agent.status} ${label}${task && task !== label ? `: ${task}` : ""}`; + return `${agent.retry ? `retrying 429 (${agent.retry.attempt}/${agent.retry.maxRetries}, ${agent.retry.delayMs}ms)` : agent.status} ${label}${task && task !== label ? `: ${task}` : ""}${agent.error ? ` — ${singleLine(agent.error.message)}` : ""}`; } /** Text remains useful to RPC/headless consumers; details carry the complete snapshot. */ export function workflowProgressResult(progress: WorkflowProgress): AgentToolResult { const agents = visibleAgents(progress, false); const hidden = progress.agents.length - agents.length; + const notice = concurrencyNotice(progress.concurrency); return { content: [ { type: "text", text: [ progressSummary(progress), + ...(notice ? [notice] : []), + ...(progress.taskFailures ?? []).slice(0, 3).map((failure) => taskFailureLine(failure)), + ...(progress.status !== "running" && progress.message ? [singleLine(progress.message)] : []), ...(progress.currentPhase ? [`Phase: ${singleLine(progress.currentPhase)}`] : []), ...agents.map(agentSummary), ...(hidden ? [`${hidden} more agents`] : []), @@ -98,10 +124,21 @@ export function renderWorkflowResult( const bodyWidth = Math.max(1, width - 4); const rows: string[] = []; let expandable = false; + const notice = concurrencyNotice(progress?.concurrency ?? completed?.concurrency); + if (notice) rows.push(...wrapTextWithAnsi(theme.fg("muted", notice), bodyWidth)); if (progress) { const color = - progress.status === "running" ? "accent" : progress.status === "completed" ? "success" : "error"; + progress.status === "running" + ? "accent" + : progress.status === "stopping" + ? "warning" + : progress.status === "completed" + ? "success" + : "error"; rows.push(...wrapTextWithAnsi(theme.fg(color, progressSummary(progress)), bodyWidth)); + for (const failure of (progress.taskFailures ?? []).slice(0, options.expanded ? undefined : 3)) { + rows.push(...wrapTextWithAnsi(theme.fg("error", taskFailureLine(failure)), bodyWidth)); + } if (progress.currentPhase) { rows.push( ...wrapTextWithAnsi(theme.fg("muted", `Phase: ${singleLine(progress.currentPhase)}`), bodyWidth), @@ -118,14 +155,23 @@ export function renderWorkflowResult( const hidden = progress.agents.length - agents.length; if (hidden) rows.push(theme.fg("muted", `${hidden} more agents`)); expandable = progress.agents.length > 0; - if (progress.status === "running" && progress.message && progress.message !== "Workflow started") { - rows.push(truncateToWidth(theme.fg("dim", singleLine(progress.message)), bodyWidth)); + if ( + progress.message && + progress.message !== "Workflow started" && + progress.message !== "Workflow completed" + ) { + rows.push( + ...wrapTextWithAnsi( + theme.fg(progress.status === "failed" ? "error" : "muted", singleLine(progress.message)), + bodyWidth, + ), + ); } } else if (completed) { rows.push( ...wrapTextWithAnsi( theme.fg( - "success", + completed.status === "completed" ? "success" : "error", `${completed.status} · ${completed.agentCalls} agent calls · ${completed.spentTokens} tokens`, ), bodyWidth, diff --git a/packages/coding-agent/src/features/workflow/runtime.ts b/packages/coding-agent/src/features/workflow/runtime.ts index ac070d58..493e8996 100644 --- a/packages/coding-agent/src/features/workflow/runtime.ts +++ b/packages/coding-agent/src/features/workflow/runtime.ts @@ -1,12 +1,14 @@ import path from "node:path"; import type { AgentToolResult } from "@step-harness/agent-core"; import type { ExtensionContext } from "../../core/extensions/types.ts"; +import { isRateLimitError, isTerminalQuotaError } from "../../core/rate-limit.ts"; import { type StepTelemetryKnownEventName, type StepTelemetryProperties, type StepTelemetryReporter, trackStepTelemetry, } from "../../step/telemetry.ts"; +import { acquireWorkflowAdmission, type WorkflowAdmissionLease } from "./admission.ts"; import { createDefaultWorkflowAgentRunner, normalizeWorkflowAgentValue } from "./agent-runner.ts"; import { clampWorkflowAgentLimit, @@ -46,6 +48,7 @@ import type { WorkflowProgress, WorkflowProgressAgent, WorkflowRunResult, + WorkflowTaskFailure, WorkflowUsage, } from "./types.ts"; import { emptyWorkflowUsage, mergeWorkflowUsage, workflowUsageTokens } from "./types.ts"; @@ -69,8 +72,12 @@ export interface WorkflowRuntimeOptions { telemetry?: StepTelemetryReporter; budgetTotal?: number | null; maxConcurrency?: number; + /** Trusted host/provider ceiling, never a model-authored override. */ + providerConcurrencyLimit?: number; maxAgents?: number; agentTimeoutMs?: number; + /** Grace for an injected runner to settle after cancellation. Production children exit within 10s. */ + runnerCleanupTimeoutMs?: number; context?: ExtensionContext; nestedWorkflow?: (name: string, args: unknown, parent: WorkflowRuntime, depth: number) => Promise; signal?: AbortSignal; @@ -103,6 +110,7 @@ export class WorkflowRuntime { readonly name: string; readonly budget: WorkflowBudget; readonly semaphore: WorkflowSemaphore; + private readonly concurrency: NonNullable; private readonly journal: WorkflowJournal; private readonly progress: WorkflowProgressStore; private readonly onProgress?: WorkflowRuntimeOptions["onProgress"]; @@ -115,6 +123,14 @@ export class WorkflowRuntime { private readonly signal?: AbortSignal; private readonly maxAgents: number; private readonly agentTimeoutMs: number; + private readonly runnerCleanupTimeoutMs: number; + private readonly controller = new AbortController(); + private readonly pendingOperations = new Set>(); + private readonly nodeErrors = new WeakSet(); + private readonly nestedTaskFailures: WorkflowTaskFailure[] = []; + private readonly recordedFailures = new Set(); + private admission?: WorkflowAdmissionLease; + private lifecycle: "idle" | "running" | "closing" | "closed" = "idle"; private sequence = 0; private agentCount = 0; private cacheHits = 0; @@ -132,13 +148,20 @@ export class WorkflowRuntime { this.journal = options.journal; this.onProgress = options.onProgress; this.budget = new WorkflowBudget(options.budgetTotal); - this.semaphore = new WorkflowSemaphore(clampWorkflowConcurrency(options.maxConcurrency)); + const ceiling = clampWorkflowConcurrency(options.providerConcurrencyLimit); + const limit = Math.min(ceiling, clampWorkflowConcurrency(options.maxConcurrency)); + this.concurrency = { requested: options.maxConcurrency, ceiling, limit, cap: limit }; + this.semaphore = new WorkflowSemaphore(limit); this.maxAgents = clampWorkflowAgentLimit(options.maxAgents); this.agentTimeoutMs = clampAgentTimeout(options.agentTimeoutMs); + this.runnerCleanupTimeoutMs = options.runnerCleanupTimeoutMs ?? 12_000; this.runner = options.runner ?? createDefaultWorkflowAgentRunner(); this.telemetry = options.telemetry; this.context = options.context; - this.signal = options.signal; + this.signal = AbortSignal.any([ + this.controller.signal, + ...[options.signal, options.context?.signal].filter((signal): signal is AbortSignal => signal !== undefined), + ]); this.nestedWorkflow = options.nestedWorkflow; this.now = options.now ?? Date.now; this.vmExecutor = options.vmExecutor ?? runInQuickJs; @@ -149,6 +172,7 @@ export class WorkflowRuntime { runId: this.runId, name: this.name, status: "running", + concurrency: { ...this.concurrency }, startedAt, updatedAt: startedAt, agents: [], @@ -161,17 +185,25 @@ export class WorkflowRuntime { } async runScript(script: string, args: unknown, vmOptions: WorkflowVmOptions = {}): Promise { - await this.journal.initialize(script); - await this.updateProgress({ status: "running", message: "Workflow started" }); - this.track("workflow_started", { phase_count: 0 }); + if (this.lifecycle !== "idle") throw new Error("Workflow run is already started or closed"); + this.lifecycle = "running"; + this.admission ??= acquireWorkflowAdmission(this.concurrency.ceiling); try { + await this.journal.initialize(script); + await this.updateProgress({ status: "running", message: "Workflow started" }); + this.track("workflow_started", { phase_count: 0 }); if (this.signal?.aborted || this.context?.signal?.aborted) { throw new Error("Workflow operation was aborted"); } const vmResult = await this.vmExecutor(script, args, this.host(0), { ...vmOptions, + signal: this.signal, filename: vmOptions.filename ?? this.journal.paths.scriptPath, }); + if (this.pendingOperations.size > 0) + throw new Error( + "Workflow script returned with unawaited agent calls or workflow/iterate operations; await the complete operation", + ); if (this.signal?.aborted || this.context?.signal?.aborted) { // A swallowing script (parallel()/pipeline() null-on-error) can absorb the abort // thrown by semaphore.acquire()/the runner and return normally; recheck here so a @@ -187,23 +219,58 @@ export class WorkflowRuntime { throw new WorkflowBudgetExceeded(budgetTotal, this.budget.spent(), 0); } const finishedAt = this.readNow(); + const snapshot = this.progress.current(); + const failures = snapshot.agents.filter((agent) => agent.status === "failed" || agent.status === "aborted"); + const taskFailures = [...this.nestedTaskFailures, ...(vmResult.taskFailures ?? [])]; + const status = + failures.length === 0 && taskFailures.length === 0 + ? "completed" + : snapshot.completedAgents > 0 + ? "partial_failed" + : "failed"; await this.updateProgress({ - status: "completed", + status, + taskFailures, spentTokens: this.budget.spent(), - message: "Workflow completed", - }); + message: + status !== "completed" + ? `Workflow finished with ${failures.length} failed nodes and ${taskFailures.length} failed callbacks` + : "Workflow completed", + }).catch(() => undefined); this.track("workflow_finished", { - status: "completed", + status, agent_count: this.agentCount, cache_hits: this.cacheHits, spent_tokens: this.budget.spent(), }); - await Promise.all([this.journal.flush(), this.progress.flush()]); + // The computed result is authoritative; a failed final disk write must not turn a + // completed run into a thrown error. + await Promise.all([this.journal.flush(), this.progress.flush()]).catch(() => undefined); return { schemaVersion: 1, runId: this.runId, name: this.name, - status: "completed", + status, + summary: { + total: snapshot.totalAgents, + completed: snapshot.completedAgents, + failed: failures.length, + ...(taskFailures.length ? { failedCallbacks: taskFailures.length } : {}), + }, + ...(taskFailures.length ? { taskFailures } : {}), + timing: { + runElapsedMs: finishedAt - this.startedAt, + maxNodeExecutionMs: Math.max( + 0, + ...snapshot.agents.map((agent) => + agent.startedAt === undefined || agent.finishedAt === undefined + ? 0 + : agent.finishedAt - agent.startedAt, + ), + ), + }, + failures, + concurrency: this.concurrencySnapshot(), value: vmResult.value, meta: vmResult.meta, scriptPath: this.journal.paths.scriptPath, @@ -221,15 +288,35 @@ export class WorkflowRuntime { : error instanceof WorkflowBudgetExceeded ? "budget_exceeded" : "failed"; - await this.updateProgress({ status, spentTokens: this.budget.spent(), message: errorMessage(error) }); + this.lifecycle = "closing"; + try { + // A progress write failure must never replace the run's terminal error, and a + // run whose disk writes fail must still publish its terminal state. + await this.updateProgress({ + status: "stopping", + message: `${errorMessage(error)}; cancelling unfinished nodes`, + }).catch(() => undefined); + } finally { + this.controller.abort(error); + await Promise.allSettled([...this.pendingOperations]); + } + await this.updateProgress({ status, spentTokens: this.budget.spent(), message: errorMessage(error) }).catch( + () => undefined, + ); this.track("workflow_finished", { status, agent_count: this.agentCount, cache_hits: this.cacheHits, spent_tokens: this.budget.spent(), }); - await Promise.all([this.journal.flush(), this.progress.flush()]); + await Promise.all([this.journal.flush(), this.progress.flush()]).catch(() => undefined); throw error; + } finally { + this.lifecycle = "closed"; + this.controller.abort(); + this.semaphore.dispose(); + this.admission?.release(); + this.admission = undefined; } } @@ -240,11 +327,41 @@ export class WorkflowRuntime { vmOptions: WorkflowVmOptions = {}, ): Promise { if (depth > 1) throw new Error("Nested workflow depth is limited to one level"); - const result = await this.vmExecutor(script, args, this.host(depth), vmOptions); + const result = await this.vmExecutor(script, args, this.host(depth), { ...vmOptions, signal: this.signal }); + this.nestedTaskFailures.push( + ...(result.taskFailures ?? []).map((failure) => ({ + ...failure, + script: vmOptions.filename ?? "nested workflow", + })), + ); return result.value; } - async agent(prompt: string, options: WorkflowAgentOptions = {}): Promise { + agent(prompt: string, options: WorkflowAgentOptions = {}): Promise { + return this.trackOperation(() => this.runAgent(prompt, options)); + } + + private trackOperation(start: () => Promise): Promise { + if (this.lifecycle === "closed" || this.lifecycle === "closing") + return Promise.reject(new Error("Workflow scope is closed")); + this.admission ??= acquireWorkflowAdmission(this.concurrency.ceiling); + const pending = Promise.resolve().then(() => { + if (this.lifecycle === "closed" || this.lifecycle === "closing") throw new Error("Workflow scope is closed"); + return start(); + }); + this.pendingOperations.add(pending); + const settled = (): void => { + this.pendingOperations.delete(pending); + if (this.lifecycle === "idle" && this.pendingOperations.size === 0) { + this.admission?.release(); + this.admission = undefined; + } + }; + void pending.then(settled, settled); + return pending; + } + + private async runAgent(prompt: string, options: WorkflowAgentOptions): Promise { const normalizedPrompt = normalizePrompt(prompt); const normalizedOptions = this.normalizeAgentOptions(options); const seq = this.sequence; @@ -288,13 +405,26 @@ export class WorkflowRuntime { const retries = clampRetries(normalizedOptions.retries); const parentSignal = this.signal ?? this.context?.signal; let release: (() => void) | undefined; + let releaseShared: (() => void) | undefined; + const shared = this.admission!.semaphore; let startedAt: number | undefined; let lastErrors: string[] = []; let totalUsage: WorkflowUsage = emptyWorkflowUsage(); + let rateLimitRetries = 0; + let cleanupIncomplete = false; + let timedOut = false; + // Only text the runner itself reported as the failure can indicate congestion. + // Agent output, node labels and ids are never classified. + let providerFailure: string | undefined; + let schemaAttempt = 0; + let requestEpoch = this.semaphore.epoch; + let sharedEpoch = shared.epoch; try { await this.updateProgress({ agent: { ...identity, status: "queued" } }); this.validateMounts(normalizedOptions); release = await this.semaphore.acquire(parentSignal); + releaseShared = await shared.acquire(parentSignal); + if (parentSignal?.aborted) throw new Error("Workflow operation was aborted"); // Check after acquiring a slot so queued waves see completed calls' spend. // Already-running calls can still overshoot by one concurrency wave. this.requireRemainingBudget(); @@ -307,6 +437,8 @@ export class WorkflowRuntime { this.activeWriterId = agentId; } startedAt = this.readNow(); + requestEpoch = this.semaphore.epoch; + sharedEpoch = shared.epoch; await this.updateProgress({ agent: { ...identity, status: "running", startedAt }, totalAgents: this.agentCount, @@ -316,6 +448,7 @@ export class WorkflowRuntime { phase_length: normalizedOptions.phase?.length ?? 0, }); for (let attempt = 1; attempt <= retries; attempt += 1) { + schemaAttempt = attempt; if (attempt > 1) { try { this.requireRemainingBudget(); @@ -328,6 +461,7 @@ export class WorkflowRuntime { totalUsage, attempt - 1, errorMessage(error), + rateLimitRetries, ); throw error; } @@ -341,26 +475,121 @@ export class WorkflowRuntime { const relayAbort = (): void => attemptController.abort(parentSignal?.reason); if (parentSignal?.aborted) relayAbort(); else parentSignal?.addEventListener("abort", relayAbort, { once: true }); + if (attemptController.signal.aborted) + throw attemptController.signal.reason ?? new Error("Workflow operation was aborted"); let timeout: ReturnType | undefined; + let runnerPromise: Promise | undefined; + let cancelAttempt: (() => void) | undefined; + let settling: Promise | undefined; + // After cancellation the runner gets a bounded grace to settle. Returns its + // late result so the usage it reports can still be accounted. + const settleRunner = (): Promise => { + settling ??= (async () => { + if (!runnerPromise || !attemptController.signal.aborted) return undefined; + let cleanupTimer: ReturnType | undefined; + try { + const settled = await Promise.race([ + runnerPromise.then( + (value) => ({ value }), + () => ({ value: undefined }), + ), + new Promise((resolve) => { + cleanupTimer = setTimeout(() => resolve(undefined), this.runnerCleanupTimeoutMs); + }), + ]); + if (settled) return settled.value; + // An injected runner violated the cancellation contract. Keep its + // shared slot reserved until it settles; never admit replacement + // work on top of an operation that may still be using the backend. + cleanupIncomplete = true; + const retainedLease = acquireWorkflowAdmission(this.concurrency.ceiling); + const retainedRelease = releaseShared; + releaseShared = undefined; + const done = (): void => { + retainedRelease?.(); + retainedLease.release(); + }; + void runnerPromise.then(done, done); + return undefined; + } finally { + if (cleanupTimer) clearTimeout(cleanupTimer); + } + })(); + return settling; + }; try { - const timedOut = new Promise((_resolve, reject) => { + const cancelled = new Promise((_resolve, reject) => { + cancelAttempt = () => + reject(attemptController.signal.reason ?? new Error("Workflow operation was aborted")); + attemptController.signal.addEventListener("abort", cancelAttempt, { once: true }); + if (attemptController.signal.aborted) cancelAttempt(); timeout = setTimeout(() => { - attemptController.abort(); - reject(new Error(`Workflow agent ${label} timed out after ${this.agentTimeoutMs}ms`)); + timedOut = true; + attemptController.abort( + new Error( + `Workflow agent ${label} timed out after ${this.agentTimeoutMs}ms (execution only; queue time excluded)`, + ), + ); }, this.agentTimeoutMs); }); - raw = await Promise.race([ - this.runner({ - prompt: attemptPrompt, - options: normalizedOptions, - cwd: this.cwd, - signal: attemptController.signal, - runId: this.runId, - agentId, - }), - timedOut, - ]); + runnerPromise = this.runner({ + prompt: attemptPrompt, + options: normalizedOptions, + cwd: this.cwd, + signal: attemptController.signal, + runId: this.runId, + agentId, + onModelRequest: (event) => { + if (attemptController.signal.aborted) return; + if (event.type === "started") { + requestEpoch = this.semaphore.epoch; + sharedEpoch = shared.epoch; + } + if (event.type === "succeeded") { + this.semaphore.succeeded(requestEpoch); + shared.succeeded(sharedEpoch); + // The next request in this turn (a resample after leaked tool-call + // markup) has no "started" event of its own. + requestEpoch = this.semaphore.epoch; + sharedEpoch = shared.epoch; + } + if (event.type === "rate_limit") { + rateLimitRetries++; + this.semaphore.throttle(requestEpoch, event.delayMs); + shared.throttle(sharedEpoch, event.delayMs); + requestEpoch = this.semaphore.epoch; + sharedEpoch = shared.epoch; + } + void this.updateProgress({ + concurrency: this.concurrencySnapshot(), + agent: { + ...identity, + status: "running", + startedAt, + rateLimitRetries, + retry: + event.type === "rate_limit" + ? { attempt: event.attempt, maxRetries: event.maxRetries, delayMs: event.delayMs } + : undefined, + }, + }).catch(() => undefined); + }, + }); + raw = await Promise.race([runnerPromise, cancelled]); } catch (error: unknown) { + // A timed-out or aborted attempt still spent tokens. The runner reports + // them when it settles, after the race has already been decided. + const late = await settleRunner(); + if (late) { + const lateUsage = normalizeUsage(late.usage); + totalUsage = mergeWorkflowUsage(totalUsage, lateUsage); + try { + this.budget.consume(lateUsage); + } catch (budgetError: unknown) { + if (budgetError instanceof WorkflowBudgetExceeded) this.budgetError ??= budgetError; + } + } + if (!attemptController.signal.aborted) providerFailure = errorMessage(error); await this.appendFailure( seq, callHash, @@ -369,11 +598,14 @@ export class WorkflowRuntime { totalUsage, attempt, errorMessage(error), + rateLimitRetries, ); throw error; } finally { if (timeout) clearTimeout(timeout); + if (cancelAttempt) attemptController.signal.removeEventListener("abort", cancelAttempt); parentSignal?.removeEventListener("abort", relayAbort); + await settleRunner(); } const usage = normalizeUsage(raw.usage); totalUsage = mergeWorkflowUsage(totalUsage, usage); @@ -389,11 +621,13 @@ export class WorkflowRuntime { totalUsage, attempt, errorMessage(error), + rateLimitRetries, ); throw error; } if (raw.status === "failed" || raw.status === "aborted") { const failure = raw.errorMessage ?? raw.text ?? `Workflow agent ${label} ${raw.status}`; + providerFailure = raw.errorMessage; await this.appendFailure( seq, callHash, @@ -402,6 +636,7 @@ export class WorkflowRuntime { totalUsage, attempt, failure, + rateLimitRetries, ); throw new Error(failure); } @@ -419,6 +654,7 @@ export class WorkflowRuntime { totalUsage, attempt, lastErrors.join("; "), + rateLimitRetries, ); throw new WorkflowSchemaError(lastErrors, attempt); } @@ -435,11 +671,14 @@ export class WorkflowRuntime { usage: totalUsage, attempt, createdAt: this.readNow(), + rateLimitRetries, }); await this.updateProgress({ agent: { ...identity, status: "completed", + rateLimitRetries, + retry: undefined, startedAt, finishedAt: this.readNow(), usageTokens: workflowUsageTokens(totalUsage), @@ -455,29 +694,79 @@ export class WorkflowRuntime { } throw new WorkflowSchemaError(lastErrors, retries); } catch (error: unknown) { + const message = + errorMessage(error) + + (cleanupIncomplete ? "; runner did not settle after cancellation; its shared slot remains reserved" : ""); + const rateLimited = + providerFailure !== undefined && + isRateLimitError(providerFailure) && + !isTerminalQuotaError(providerFailure); + // Classify before recording anything. A failed journal or progress write must + // not replace the node's own error, and the marker has to exist before the + // bridge can ask for it, or parallel()/pipeline() reports this node failure a + // second time as a callback failure. + const propagated = error instanceof Error ? error : new Error(errorMessage(error)); + this.nodeErrors.add(propagated); + // Recording is best effort: the node already failed, and a disk error must not + // turn that into a different terminal error for the run. + if (!this.recordedFailures.has(seq)) { + await this.appendFailure( + seq, + callHash, + normalizedPrompt, + normalizedOptions, + totalUsage, + schemaAttempt, + message, + rateLimitRetries, + ); + } + if (rateLimited) { + this.semaphore.throttle(requestEpoch, 1000); + shared.throttle(sharedEpoch, 1000); + } await this.updateProgress({ + concurrency: this.concurrencySnapshot(), agent: { ...identity, status: parentSignal?.aborted ? "aborted" : "failed", + error: { + code: rateLimited + ? rateLimitRetries > 0 + ? "RATE_LIMIT_RETRIES_EXHAUSTED" + : "RATE_LIMITED" + : error instanceof WorkflowSchemaError + ? "SCHEMA_VALIDATION_FAILED" + : timedOut + ? "NODE_TIMEOUT" + : "NODE_FAILED", + message: message.slice(0, 2000), + retryable: rateLimited, + ...(rateLimited ? { retryExhausted: rateLimitRetries > 0 } : {}), + }, + rateLimitRetries, + retry: undefined, startedAt, finishedAt: this.readNow(), usageTokens: workflowUsageTokens(totalUsage), }, spentTokens: this.budget.spent(), - }); + }).catch(() => undefined); if (error instanceof WorkflowBudgetExceeded) this.track("workflow_budget_exceeded", { spent_tokens: error.spentTokens, requested_tokens: error.requestedTokens, }); - throw error; + throw propagated; } finally { + releaseShared?.(); release?.(); if (this.activeWriterId === agentId) this.activeWriterId = undefined; } } phase(title: string): void { + if (this.lifecycle === "closed") return; const normalized = title.trim().slice(0, 200); if (!normalized) return; this.phases.push({ title: normalized }); @@ -490,6 +779,7 @@ export class WorkflowRuntime { } log(message: string): void { + if (this.lifecycle === "closed") return; const normalized = message.trim().slice(0, 2_000); if (!normalized) return; void this.updateProgress({ message: normalized }).catch(() => undefined); @@ -597,20 +887,46 @@ export class WorkflowRuntime { } private async updateProgress(patch: Partial & { agent?: WorkflowProgressAgent }): Promise { - const snapshot = await this.progress.update(patch); - this.onProgress?.(snapshot); + if (this.lifecycle === "closed") return; + let snapshot: WorkflowProgress; + try { + snapshot = await this.progress.update({ ...patch, concurrency: this.concurrencySnapshot() }); + } catch { + // progress.json is a projection. The in-memory state is already updated when a + // write fails, and a disk error here must not fail a node whose work finished. + snapshot = this.progress.current(); + } + try { + this.onProgress?.(snapshot); + } catch { + // A live-update listener is a consumer of the projection. Its failure must not + // change run accounting, or one node failure is reported twice. + } + } + + private concurrencySnapshot(): NonNullable { + return { + ...this.concurrency, + cap: Math.min(this.semaphore.max, this.admission?.semaphore.max ?? this.semaphore.max), + sharedActive: this.admission?.semaphore.active ?? 0, + }; } private host(depth: number): WorkflowVmHost { return { + isNodeFailure: (error) => + error !== null && + typeof error === "object" && + (this.nodeErrors.has(error) || + (error as { __workflowNodeFailure?: boolean }).__workflowNodeFailure === true), agent: (prompt, options) => this.agent(prompt, options as WorkflowAgentOptions), phase: (title) => this.phase(title), log: (message) => this.log(message), - iterate: (options) => this.iterate(options), + iterate: (options) => this.trackOperation(() => this.iterate(options)), nestedWorkflow: (name, args) => { if (depth >= 1) return Promise.reject(new Error("Nested workflow depth is limited to one level")); if (!this.nestedWorkflow) return Promise.reject(new Error(`Nested workflow "${name}" is not available`)); - return this.nestedWorkflow(name, args, this, depth); + return this.trackOperation(() => this.nestedWorkflow!(name, args, this, depth)); }, budgetSpent: () => this.budget.spent(), budgetRemaining: () => this.budget.remaining(), @@ -713,23 +1029,32 @@ export class WorkflowRuntime { usage: WorkflowUsage, attempt: number, error: string, + rateLimitRetries = 0, ): Promise { - await this.journal.append({ - schemaVersion: 1, - seq, - callId: `${this.runId}-${seq + 1}`, - callHash, - prompt, - options: workflowJsonValue(serializableOptions(options)), - status: "failed", - usage, - attempt, - createdAt: this.readNow(), - error: error.slice(0, 2_000), - }); + // Recording a failure is best effort. The node already failed, so a journal + // write error must never replace the node's own error, change the run's + // terminal status, or skip failure classification. + await this.journal + .append({ + schemaVersion: 1, + seq, + callId: `${this.runId}-${seq + 1}`, + callHash, + prompt, + options: workflowJsonValue(serializableOptions(options)), + status: this.signal?.aborted ? "aborted" : "failed", + usage, + attempt, + rateLimitRetries, + createdAt: this.readNow(), + error: error.slice(0, 2_000), + }) + .catch(() => undefined); + this.recordedFailures.add(seq); } private track(event: StepTelemetryKnownEventName, properties: Record): void { + if (this.lifecycle === "closed") return; if (this.telemetry) trackStepTelemetry(this.telemetry, event, properties as StepTelemetryProperties); const record = { schemaVersion: 1 as const, @@ -840,8 +1165,12 @@ function errorMessage(error: unknown): string { return error instanceof Error ? error.message : String(error); } -export function workflowToolResult(value: T): AgentToolResult { - return { content: [{ type: "text", text: JSON.stringify(value, null, 2) }], details: value }; +export function workflowToolResult(value: WorkflowRunResult): AgentToolResult { + return { + content: [{ type: "text", text: JSON.stringify(value, null, 2) }], + details: value, + isError: value.status !== "completed", + }; } export { WorkflowBudgetExceeded }; diff --git a/packages/coding-agent/src/features/workflow/step-workflow.ts b/packages/coding-agent/src/features/workflow/step-workflow.ts index 3abfd8ab..e279395d 100644 --- a/packages/coding-agent/src/features/workflow/step-workflow.ts +++ b/packages/coding-agent/src/features/workflow/step-workflow.ts @@ -10,6 +10,7 @@ import type { import { resolveStepStorageRoot } from "../../step/storage-root.ts"; import type { StepTelemetryReporter } from "../../step/telemetry.ts"; import { createDefaultWorkflowAgentRunner } from "./agent-runner.ts"; +import { configuredWorkflowConcurrencyLimit } from "./budget.ts"; import { createWorkflowRunPaths, newWorkflowRunId, resolveWorkflowRoot, WorkflowJournal } from "./journal.ts"; import { formatWorkflowStatus, listSavedWorkflows, listWorkflowRuns } from "./progress.ts"; import { resolveWorkflowRegistration } from "./registration-gate.ts"; @@ -32,8 +33,22 @@ export const WorkflowParams = Type.Object({ args: Type.Optional(Type.Unknown({ description: "JSON arguments exposed as the script's args value" })), resumeFromRunId: Type.Optional(Type.String({ description: "Resume the matching cached journal prefix" })), budget: Type.Optional(Type.Integer({ minimum: 0, description: "Optional input+output token budget" })), - maxConcurrency: Type.Optional(Type.Integer({ minimum: 1, maximum: 32 })), - agentTimeoutMs: Type.Optional(Type.Integer({ minimum: 1_000, maximum: 3_600_000 })), + maxConcurrency: Type.Optional( + Type.Integer({ + minimum: 1, + maximum: 32, + description: + "Requested concurrency; can only lower the host ceiling (CPU count and any configured provider limit). Rate limiting can reduce the active cap further.", + }), + ), + agentTimeoutMs: Type.Optional( + Type.Integer({ + minimum: 1_000, + maximum: 3_600_000, + description: + "Execution deadline per schema attempt, including model requests, tools and retries; excludes queue time, and a schema retry gets a fresh deadline. Allow model overhead in addition to tool durations. Default 30 minutes.", + }), + ), }); export type WorkflowRequest = { @@ -170,7 +185,14 @@ No silent caps: log() whatever you drop — top-N truncations, skipped retries, Sizing: default medium; keep a run under ~15 agents unless the user asks for scale. Mechanics: -- parallel(tasks) IS a barrier: it awaits every task before returning, and a task that throws resolves to null in the result array — the call never rejects, so .filter(Boolean) before using the results. pipeline(items, ...stages) runs each item's stage chain concurrently with NO barrier between stages: item A can be in stage 3 while item B is still in stage 1, and a stage sees only its own item, never sibling items' earlier-stage output. Stage callbacks receive (prevResult, originalItem, index); a stage that throws drops that item to null and skips its remaining stages. Both accept at most 4096 entries. Prefer pipeline(); insert a parallel() barrier between stages only when stage N truly needs every stage N-1 result (dedup, early exit, cross-referencing). +- Explicit counted fan-out requests such as "launch 10 subagents" or "开10个子agent" also grant per-turn workflow consent, confirmed by a system-reminder. Submit all independent tasks together and let admission queue them; do not manually split 10 tasks into 8+2 waves just because the window is eight. +- When embedding shell commands in JavaScript, preserve shell substitutions as data. Prefer a normal single- or double-quoted JS string for shell code and concatenate only the JS values you need. A shell variable inside a JavaScript template literal is otherwise evaluated as a JS identifier before agent() is called. Callback failures are returned in taskFailures with the primitive/index and original error; fix those errors rather than treating a zero-call run as completed work. +- Distinguish worker counts from later stages: 15 worker agents followed by one summary agent means 15 workers, one subsequent summarizer, and 16 total calls. Later summary/verification stages are allowed unless the user explicitly caps the total agent-call budget. Prefer script code for pure sorting/sums. The shared FIFO window refills as nodes finish; do not describe it as fixed waves unless the script explicitly batches tasks. +- Use the returned timing.runElapsedMs and timing.maxNodeExecutionMs for workflow/node durations. A tool's reported sleep/elapsed time is only part of node execution and cannot establish the node's total duration or prove its timeout behavior. +- Canonical fan-out: return await parallel(items.map(item => () => agent(String(item), {label: String(item)}))); Promise arrays also work: return await parallel(items.map(item => agent(String(item)))); Always await or return the entire operation. Returning early cancels unfinished nodes and fails the run. Invalid entries reject the batch; do not pass plain strings or objects. +- Report only observed tool outputs. When a task must report a random choice or duration, capture and print it in the same command that performs the operation; do not guess a value after a command with no output. Use agent(..., {schema}) to enforce the required output fields and ranges. +- maxConcurrency can only lower the host ceiling (CPU count, optionally lowered further by a configured provider limit). The final result reports requested/effective concurrency. Temporary 429 errors retry the current model request and reduce the admission cap. Inspect status, summary and failures: partial_failed/failed means some nodes did not complete, even when the script filters null results. Report those nodes and their causes explicitly; existing files or passing tests do not prove a failed node completed its assigned work. taskFailures entries carry their script when they came from a nested workflow. +- parallel(tasks) accepts functions or promises and awaits all of them. A task rejection becomes null; invalid arguments reject the batch. Inspect final summary/failures even when filtering null results. pipeline(items, ...stages) runs each item's stage chain concurrently with NO barrier between stages: item A can be in stage 3 while item B is still in stage 1, and a stage sees only its own item, never sibling items' earlier-stage output. Stage callbacks receive (prevResult, originalItem, index); a stage that throws drops that item to null and skips its remaining stages. Both accept at most 4096 entries. Prefer pipeline(); insert a parallel() barrier between stages only when stage N truly needs every stage N-1 result (dedup, early exit, cross-referencing). - agent(prompt, {schema}) forces structured JSON output and retries schema mismatches up to 3 attempts, feeding validation errors back; failed attempts still consume budget. - Give agent() a short descriptive label; the live tool row shows running/queued counts, assigned tasks, phases, and terminal states while the workflow executes. - Budgets fail closed once spend crosses the limit; budget.total is null when unlimited, so guard adaptive waves with budget.total && budget.remaining() > estimate. A "+500k"-style token target in the user's current message becomes the run's default budget; an explicit budget parameter overrides it. @@ -185,10 +207,11 @@ Mechanics: "Never infer workflow opt-in from task shape or size, and never carry a per-turn opt-in forward from an earlier turn; a harness system-reminder confirms keyword, phrase, and session-standing opt-ins (saved-workflow and skill invocations are valid without one), and the tool being registered is an environment capability, not consent.", "When ultraloop is on for the session, the opt-in is standing: author and run a workflow for every substantive task by default, treat token cost as unconstrained, and aim for the most exhaustive correct answer. For multi-phase work, run several workflows in sequence — one per phase — so you stay in the loop between them. Lean toward adversarially verifying findings unless the work is already verified. Solo only on conversational turns or trivial mechanical edits.", "Use a short inline script for experiments and a saved script or scriptPath for repeatable work; /workflows lists saved workflows and recent runs, and /ultraloop toggles session-standing mode.", + "Report worker nodes, additional summary/verification stages, and total agent calls separately. A requested worker count does not prohibit later stages; honor a strict total agent-call budget when explicitly specified. Concurrency queues refill continuously, not in fixed waves unless the script explicitly batches.", "Default to a medium run under ~15 agents unless the user asks for scale or ultraloop is on for the session, and log() anything dropped silently: top-N truncations, skipped retries, sampling.", "Spend agents on verification: adversarial verify with perspective-diverse skeptics (majority-refute kills a finding), judge panels over single attempts, multi-modal sweeps, loop-until-dry stopping after K empty rounds, and a final completeness critic.", "Prefer pipeline() for per-item flows; it runs each item's stage chain concurrently with no barrier between stages, so a stage never sees sibling items' earlier-stage output. Stage callbacks receive (prevResult, originalItem, index), and a throwing stage drops that item to null and skips its remaining stages. Use a parallel() wave between stages only when a stage needs every previous-stage result (dedup, early exit, cross-referencing).", - "parallel(tasks) is a barrier: it awaits every task function before returning; a task that throws resolves to null in the result array, so .filter(Boolean) the results.", + "parallel(tasks) accepts functions or promises: await parallel(items.map(item => () => agent(String(item)))); task rejections become null, but invalid entries reject the batch. Always await/return the complete operation. The final summary/failures retain failed nodes even if nulls were filtered.", "Pass schema to agent() whenever a later stage consumes the result; mismatches are retried up to 3 attempts with validation errors appended, then the call fails.", "Budgets fail closed; budget.total is null when unlimited, so guard adaptive extra waves with budget.total && budget.remaining() > estimate. A \"+500k\"-style token target in the user's message sets the run's default budget; an explicit budget parameter overrides it.", "Resume with resumeFromRunId plus the identical script and args to replay the cached journal prefix; the first changed call and everything after it re-run live. The tool result's scriptPath points at the persisted script copy — edit it and re-invoke with scriptPath to iterate.", @@ -228,6 +251,7 @@ Mechanics: telemetry: options.telemetry, budgetTotal: request.budget ?? options.turnState?.budgetTotal ?? options.budgetTotal, maxConcurrency: request.maxConcurrency ?? options.maxConcurrency, + providerConcurrencyLimit: configuredWorkflowConcurrencyLimit(), maxAgents: options.maxAgents, agentTimeoutMs: request.agentTimeoutMs ?? options.agentTimeoutMs, context: { ...ctx, signal: controller.signal }, @@ -247,6 +271,7 @@ Mechanics: }); return workflowToolResult(result); } finally { + controller.abort(); activeRuns.delete(controller); if (signal) signal.removeEventListener("abort", abort); } diff --git a/packages/coding-agent/src/features/workflow/types.ts b/packages/coding-agent/src/features/workflow/types.ts index a826b4b0..038fad34 100644 --- a/packages/coding-agent/src/features/workflow/types.ts +++ b/packages/coding-agent/src/features/workflow/types.ts @@ -52,6 +52,7 @@ export interface WorkflowAgentOptions { } export interface WorkflowAgentRunInput { + onModelRequest?: (event: import("../../core/rate-limit.ts").ModelRequestProgress) => void; prompt: string; options: WorkflowAgentOptions; cwd: string; @@ -87,6 +88,9 @@ export interface WorkflowMeta { } export interface WorkflowProgressAgent { + error?: { code: string; message: string; retryable: boolean; retryExhausted?: boolean }; + rateLimitRetries?: number; + retry?: { attempt: number; maxRetries: number; delayMs: number }; id: string; label: string; /** Bounded summary of the assigned prompt, independent of an optional short label. */ @@ -99,10 +103,12 @@ export interface WorkflowProgressAgent { } export interface WorkflowProgress { + taskFailures?: WorkflowTaskFailure[]; schemaVersion: 1; runId: string; name: string; - status: "running" | "completed" | "failed" | "aborted" | "budget_exceeded"; + status: "running" | "stopping" | "completed" | "partial_failed" | "failed" | "aborted" | "budget_exceeded"; + concurrency?: { requested?: number; ceiling: number; limit: number; cap: number; sharedActive?: number }; startedAt: number; updatedAt: number; currentPhase?: string; @@ -118,7 +124,14 @@ export interface WorkflowRunResult { schemaVersion: 1; runId: string; name: string; - status: "completed" | "failed" | "aborted"; + status: "completed" | "partial_failed" | "failed" | "aborted"; + summary?: { total: number; completed: number; failed: number; failedCallbacks?: number }; + /** Callback failures can happen before agent() is reached; they are not extra agent nodes. */ + taskFailures?: WorkflowTaskFailure[]; + /** Node execution includes model/tool/retry/shutdown time; not just tool-reported duration. */ + timing?: { runElapsedMs: number; maxNodeExecutionMs: number }; + failures?: WorkflowProgressAgent[]; + concurrency?: WorkflowProgress["concurrency"]; value: unknown; meta: WorkflowMeta; /** Persisted copy of the executed script; edit and re-invoke with scriptPath to iterate. */ @@ -132,6 +145,14 @@ export interface WorkflowRunResult { stopReason?: string; } +export interface WorkflowTaskFailure { + script?: string; + primitive: "parallel" | "pipeline"; + index: number; + stage?: number; + message: string; +} + export interface IterateOptions { spec: string; maxIterations?: number; @@ -171,10 +192,12 @@ export interface WorkflowJournalEntry { callHash: string; prompt: string; options: WorkflowJsonValue; - status: "completed" | "failed" | "cached"; + status: "completed" | "failed" | "aborted" | "cached"; result?: unknown; usage: WorkflowUsage; attempt: number; + /** Request-level 429 retries across this node's schema attempts. */ + rateLimitRetries?: number; createdAt: number; error?: string; } diff --git a/packages/coding-agent/src/features/workflow/ultraloop-opt-in.ts b/packages/coding-agent/src/features/workflow/ultraloop-opt-in.ts index f7262a2b..0c1e67aa 100644 --- a/packages/coding-agent/src/features/workflow/ultraloop-opt-in.ts +++ b/packages/coding-agent/src/features/workflow/ultraloop-opt-in.ts @@ -45,6 +45,46 @@ const TRIGGER_PHRASES: readonly RegExp[] = [ /\borchestrate (?:this|it) with sub-?agents\b/iu, ]; const OPT_IN_PATTERNS: readonly RegExp[] = [KEYWORD, ...TRIGGER_PHRASES]; +// An explicit request to launch several agents is orchestration consent too. The +// phrase may sit anywhere in the message, so "麻烦开3个子agent查一下" counts while +// "先看代码" alone does not. The noun must be a fan-out target, not a file or role +// ("3 agent definition files", "创建3个agent配置文件"). +const COUNTED_AGENT_LAUNCH = + /(?:启动|开启|开|创建|派出|spawn|start|launch|run|create)\s*(\d+|[０-９]+|[一二两三四五六七八九十百千]+)\s*个?\s*(?:子\s*agents?|sub-?agents?|子代理|agents?)(?![\w.-])(?!\s*(?:definition|config|file|template|role|type|doc|配置|定义|角色|文件|模板|文档|类型|说明))/iu; +/** "一/零/半" style counts are not a fan-out request. */ +const SINGULAR_COUNT = /^(?:[一壹零〇半]|1)$/u; +/** Asking about a request (cost, feasibility, how) is not consent to perform it. */ +const META_QUESTION = + /(?:多少钱|多少成本|成本|费用|划算|可行|是否可以|是否|是不是|会不会|该不该|什么意思|怎么|如何|为什么|为啥|how much|what does|is it|should i|can i|do i|how do i|how to|why)/iu; +/** + * Refusal, hedging, or deliberation cancels a counted request. Checked across the + * whole message because a false positive here launches agents the user declined, + * which costs far more than missing an opt-in the model can still route itself. + */ +const REFUSAL = + /(?:不要|(? String.fromCharCode(digit.charCodeAt(0) - 0xfee0)); + if (/^\d+$/u.test(count) ? Number(count) <= 1 : SINGULAR_COUNT.test(count)) return undefined; + const after = prompt.slice(match.index + match[0].length); + // "Launch 10 agents?" asks about launching them; a request keeps going afterwards. + if (/^\s*[?？]/u.test(after)) return undefined; + const before = prompt.slice(0, match.index); + const clauseBefore = before.split(CLAUSE_BOUNDARY).at(-1) ?? ""; + const clauseAfter = after.split(CLAUSE_BOUNDARY)[0] ?? ""; + if (META_QUESTION.test(clauseBefore) || META_QUESTION.test(clauseAfter)) return undefined; + return match[0].trim(); +} /** * Claude Code-style per-turn token target: "+500k" or "+1.5m" anywhere in the @@ -58,7 +98,7 @@ export function detectUltraloopOptIn(prompt: string): string | undefined { const match = prompt.match(re)?.[0]; if (match !== undefined) return match; } - return undefined; + return detectCountedAgentRequest(prompt); } /** Returns the "+500k"-style token target for this turn, or undefined. */ diff --git a/packages/coding-agent/src/features/workflow/vm.ts b/packages/coding-agent/src/features/workflow/vm.ts index a8ed51d2..c5deedce 100644 --- a/packages/coding-agent/src/features/workflow/vm.ts +++ b/packages/coding-agent/src/features/workflow/vm.ts @@ -44,7 +44,7 @@ import type { QuickJSRuntime, QuickJSWASMModule, } from "quickjs-emscripten-core"; -import type { WorkflowJsonSchema, WorkflowMeta } from "./types.ts"; +import type { WorkflowJsonSchema, WorkflowMeta, WorkflowTaskFailure } from "./types.ts"; const MAX_SCRIPT_BYTES = 128 * 1024; const DEFAULT_MEMORY_LIMIT_MB = 64; @@ -52,6 +52,8 @@ const DEFAULT_TIMEOUT_MS = 120_000; /** Host primitives the guest prelude exposes as `agent`, `phase`, `log`, `iterate`, `workflow` and `budget`. */ export interface WorkflowVmHost { + /** True only for failures already recorded as agent-node failures by the owner. */ + isNodeFailure?(error: unknown): boolean; agent(prompt: string, options: Record): Promise; phase(title: string): void; log(message: string): void; @@ -63,6 +65,7 @@ export interface WorkflowVmHost { } export interface WorkflowVmOptions { + signal?: AbortSignal; filename?: string; memoryLimitMb?: number; timeoutMs?: number; @@ -72,6 +75,7 @@ export interface WorkflowVmOptions { export interface WorkflowVmResult { value: unknown; meta: WorkflowMeta; + taskFailures?: WorkflowTaskFailure[]; } /** @@ -118,6 +122,7 @@ export async function runInQuickJs( const sourceBytes = Buffer.byteLength(script, "utf8"); if (sourceBytes > MAX_SCRIPT_BYTES) throw new Error(`Workflow script exceeds ${MAX_SCRIPT_BYTES} bytes`); const quickjs = await loadQuickJsModule(); + if (options.signal?.aborted) throw new Error("Workflow operation was aborted"); const timeoutMs = clampTimeout(options.timeoutMs); const runtime = quickjs.newRuntime(); runtime.setMemoryLimit(clampMemory(options.memoryLimitMb) * 1024 * 1024); @@ -159,6 +164,8 @@ export async function runInQuickJs( const timeoutPromise = new Promise((_resolve, reject) => { rejectTimeout = reject; }); + const onAbort = (): void => rejectTimeout?.(new Error("Workflow operation was aborted")); + options.signal?.addEventListener("abort", onAbort, { once: true }); const timeoutError = (): Error => new Error(`Workflow script timed out after ${timeoutMs}ms`); const checkTimeout = (): void => { if (hostCalls.pending > 0) { @@ -210,8 +217,11 @@ export async function runInQuickJs( } const json = context.getString(settled.value); settled.value.dispose(); + if (hostCalls.pending > 0) + throw new Error("Workflow script returned with unawaited agent calls or workflow/iterate operations"); return readResult(json); } finally { + options.signal?.removeEventListener("abort", onAbort); if (timeout) clearTimeout(timeout); hostCalls.onSettled = undefined; hostCalls.refreshDeadline = undefined; @@ -253,8 +263,12 @@ function readResult(json: string): WorkflowVmResult { if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { return { value: parsed ?? null, meta: {} }; } - const record = parsed as { value?: unknown; meta?: unknown }; - return { value: record.value ?? null, meta: normalizeMeta(record.meta) }; + const record = parsed as { value?: unknown; meta?: unknown; taskFailures?: WorkflowTaskFailure[] }; + return { + value: record.value ?? null, + meta: normalizeMeta(record.meta), + ...(record.taskFailures?.length ? { taskFailures: record.taskFailures } : {}), + }; } /** @@ -267,11 +281,12 @@ function toHostError(context: QuickJSContext, handle: QuickJSHandle, override?: handle.dispose(); if (override) return override; if (dumped && typeof dumped === "object") { - const record = dumped as { name?: unknown; message?: unknown; stack?: unknown }; + const record = dumped as { name?: unknown; message?: unknown; stack?: unknown; __workflowNodeFailure?: boolean }; const message = typeof record.message === "string" ? record.message : JSON.stringify(dumped); const error = new Error(message); if (typeof record.name === "string") error.name = record.name; if (typeof record.stack === "string") error.stack = `${record.name ?? "Error"}: ${message}\n${record.stack}`; + if (record.__workflowNodeFailure === true) Object.assign(error, { __workflowNodeFailure: true }); return error; } return new Error(typeof dumped === "string" ? dumped : String(dumped)); @@ -295,7 +310,10 @@ function installHostBridge( hostCalls.live.add(deferred); hostCalls.pending += 1; void Promise.resolve() - .then(() => operation(values)) + .then(() => { + if (hostCalls.closed) throw new Error("Workflow scope is closed"); + return operation(values); + }) .then( (result) => { // The run may already have returned without awaiting this call; the @@ -309,6 +327,7 @@ function installHostBridge( if (hostCalls.closed) return; const message = error instanceof Error ? error.message : String(error); const encoded = context.newError(message); + if (host.isNodeFailure?.(error)) context.setProp(encoded, "__workflowNodeFailure", context.true); deferred.reject(encoded); encoded.dispose(); }, @@ -427,16 +446,26 @@ for (const name of [ } const __workflow_encode = (value) => JSON.stringify(value === undefined ? null : value); const __workflow_decode = (json) => (typeof json === "string" ? JSON.parse(json) : null); +const __workflow_task_failures = []; globalThis.args = __workflow_decode(__workflow_args_raw); globalThis.agent = async (prompt, options = {}) => __workflow_decode(await __workflow_agent_ref(String(prompt === undefined ? "" : prompt), __workflow_encode(options))); globalThis.iterate = async (options) => __workflow_decode(await __workflow_iterate_ref(__workflow_encode(options || {}))); globalThis.workflow = async (name, options = null) => __workflow_decode(await __workflow_nested_ref(String(name === undefined ? "" : name), __workflow_encode(options))); globalThis.parallel = async (tasks) => { - if (!Array.isArray(tasks)) throw new TypeError("parallel() requires an array of task functions"); + if (!Array.isArray(tasks)) throw new TypeError("parallel() requires an array of functions or promises"); if (tasks.length > 4096) throw new RangeError("parallel() accepts at most 4096 tasks"); - return Promise.all(tasks.map(async (task) => { - if (typeof task !== "function") throw new TypeError("parallel() entries must be functions"); - try { return await task(); } catch { return null; } + // Validate the whole batch before invoking any lazy task. Eager agent() + // promises are already scheduled; their cancellation belongs to the run scope. + for (const task of tasks) { + if (typeof task !== "function" && !(task !== null && typeof task === "object" && typeof task.then === "function")) { + throw new TypeError("parallel() entries must be functions or promises; use parallel(items.map(item => () => agent(String(item))))"); + } + } + return Promise.all(tasks.map(async (task, index) => { + try { return await (typeof task === "function" ? task() : task); } catch (error) { + if (!error || error.__workflowNodeFailure !== true) __workflow_task_failures.push({primitive: "parallel", index, message: String(error).slice(0, 2000)}); + return null; + } })); }; globalThis.pipeline = async (items, ...stages) => { @@ -447,8 +476,11 @@ globalThis.pipeline = async (items, ...stages) => { } return Promise.all(items.map(async (item, index) => { let value = item; - for (const stage of stages) { - try { value = await stage(value, item, index); } catch { return null; } + for (let stage = 0; stage < stages.length; stage++) { + try { value = await stages[stage](value, item, index); } catch (error) { + if (!error || error.__workflowNodeFailure !== true) __workflow_task_failures.push({primitive: "pipeline", index, stage, message: String(error).slice(0, 2000)}); + return null; + } } return value; })); @@ -467,6 +499,7 @@ const __workflow_main = Object.getPrototypeOf(async function() {}).constructor($ return __workflow_main().then((__workflow_value) => JSON.stringify({ value: __workflow_value === undefined ? null : __workflow_value, meta: globalThis.__workflow_meta || {}, + taskFailures: __workflow_task_failures, })); })() `; diff --git a/packages/coding-agent/src/step/system-prompt.ts b/packages/coding-agent/src/step/system-prompt.ts index 91a22789..85b1e30a 100644 --- a/packages/coding-agent/src/step/system-prompt.ts +++ b/packages/coding-agent/src/step/system-prompt.ts @@ -269,7 +269,7 @@ export function buildStepSystemPromptAppendix( ? [ "# Workflow orchestration", [ - '- workflow is opt-in: reach for it only when the user\'s current message contains the keyword "ultraloop" (or "ultracode") or an explicit request such as "use a workflow", "run a workflow", "fan out agents", or "orchestrate this with subagents"; ultraloop is on for the session (the latest on/off system-reminder in the current context reports its state; unchanged state is not repeated); the user invokes a saved workflow by name; or a skill instructs it. A system-reminder confirms per-turn and session-standing opt-ins (saved-workflow and skill invocations are valid without one); never infer opt-in from task shape, and never carry a per-turn opt-in forward.', + '- workflow is opt-in: reach for it only when the user\'s current message contains the keyword "ultraloop" (or "ultracode") or an explicit request such as "use a workflow", "run a workflow", "fan out agents", "orchestrate this with subagents", or a counted fan-out request such as "launch 10 subagents" / "开10个子agent"; ultraloop is on for the session (the latest on/off system-reminder in the current context reports its state; unchanged state is not repeated); the user invokes a saved workflow by name; or a skill instructs it. A system-reminder confirms per-turn and session-standing opt-ins (saved-workflow and skill invocations are valid without one); never infer opt-in from task shape, and never carry a per-turn opt-in forward.', "- When ultraloop is on for the session that opt-in is standing: author and run a workflow for every substantive task by default, treat token cost as unconstrained, and aim for the most exhaustive correct answer. Multi-phase work often means several workflows in sequence — one per phase — so you stay in the loop between them. Lean toward adversarially verifying findings unless the work is already verified. Solo only on conversational turns or trivial mechanical edits. When session mode is off, revert to the per-turn opt-in rule.", "- Without any signal, prefer targeted tools — search_files, read_file, or a single subagent — and never reach for workflow on single-file edits, one-shot lookups, or exploration that a few targeted queries can answer.", "- With opt-in, workflow fits broad multi-subsystem audits, mechanical migrations across many sites, multi-way parallel review with adversarial verification, research sweeps, and HoH convergence on a spec.", diff --git a/packages/coding-agent/src/utils/sleep.ts b/packages/coding-agent/src/utils/sleep.ts index 948f93c4..1c254b06 100644 --- a/packages/coding-agent/src/utils/sleep.ts +++ b/packages/coding-agent/src/utils/sleep.ts @@ -1,18 +1,32 @@ /** * Sleep helper that respects abort signal. */ -export function sleep(ms: number, signal?: AbortSignal): Promise { +export async function sleep(ms: number, signal?: AbortSignal): Promise { + // Node turns delays larger than int32 into a 1ms timer. A long Retry-After + // must wait (or be cancelled), rather than accidentally hammering the server. + let remaining = ms; + do { + const delay = Math.min(2_147_483_647, remaining); + await sleepChunk(delay, signal); + remaining -= delay; + } while (remaining > 0); +} + +function sleepChunk(ms: number, signal?: AbortSignal): Promise { return new Promise((resolve, reject) => { if (signal?.aborted) { reject(new Error("Aborted")); return; } - const timeout = setTimeout(resolve, ms); - - signal?.addEventListener("abort", () => { + const onAbort = () => { clearTimeout(timeout); reject(new Error("Aborted")); - }); + }; + const timeout = setTimeout(() => { + signal?.removeEventListener("abort", onAbort); + resolve(); + }, ms); + signal?.addEventListener("abort", onAbort, { once: true }); }); } diff --git a/packages/coding-agent/test/agent-session-retry.test.ts b/packages/coding-agent/test/agent-session-retry.test.ts index b3847df9..cdba10da 100644 --- a/packages/coding-agent/test/agent-session-retry.test.ts +++ b/packages/coding-agent/test/agent-session-retry.test.ts @@ -71,25 +71,49 @@ describe("AgentSession retry", () => { async function createSession(options?: { failCount?: number; maxRetries?: number; + rateLimitMaxRetries?: number; + errors?: string[]; + toolBeforeFailure?: boolean; delayAssistantMessageEndMs?: number; }) { const failCount = options?.failCount ?? 1; const maxRetries = options?.maxRetries ?? 3; const delayAssistantMessageEndMs = options?.delayAssistantMessageEndMs ?? 0; let callCount = 0; + let toolExecutions = 0; + const tool: AgentTool = { + name: "write_once", + label: "Write", + description: "Record a side effect", + parameters: Type.Object({}), + execute: async () => { + toolExecutions++; + return { content: [{ type: "text", text: "written" }], details: null }; + }, + }; const model = stepModel(); const agent = new Agent({ getApiKey: () => "test-key", - initialState: { model, systemPrompt: "Test", tools: [] }, + initialState: { model, systemPrompt: "Test", tools: options?.toolBeforeFailure ? [tool] : [] }, streamFn: () => { callCount++; const stream = new MockAssistantStream(); queueMicrotask(() => { - if (callCount <= failCount) { + if (options?.toolBeforeFailure && callCount === 1) { + const message = createAssistantMessage("", { + content: [{ type: "toolCall", id: "write-1", name: "write_once", arguments: {} }], + stopReason: "toolUse", + }); + stream.push({ type: "done", reason: "toolUse", message }); + return; + } + const failureIndex = callCount - (options?.toolBeforeFailure ? 1 : 0); + if (options?.errors ? failureIndex <= options.errors.length : failureIndex <= failCount) { const msg = createAssistantMessage("", { stopReason: "error", - errorMessage: "overloaded_error", + errorMessage: options?.errors?.[failureIndex - 1] ?? "overloaded_error", + retryAfterMs: 0, }); stream.push({ type: "start", partial: msg }); stream.push({ type: "error", reason: "error", error: msg }); @@ -108,7 +132,9 @@ describe("AgentSession retry", () => { const authStorage = AuthStorage.create(join(tempDir, "auth.json")); const modelRegistry = await createModelRegistry(authStorage, tempDir); await authStorage.modify("step", async () => ({ type: "api_key", key: "test-key" })); - settingsManager.applyOverrides({ retry: { enabled: true, maxRetries, baseDelayMs: 1 } }); + settingsManager.applyOverrides({ + retry: { enabled: true, maxRetries, rateLimitMaxRetries: options?.rateLimitMaxRetries, baseDelayMs: 1 }, + }); session = new AgentSession({ agent, @@ -117,6 +143,7 @@ describe("AgentSession retry", () => { cwd: tempDir, modelRuntime: getModelRuntime(modelRegistry), resourceLoader: createTestResourceLoader(), + ...(options?.toolBeforeFailure ? { baseToolsOverride: { write_once: tool } } : {}), }); if (delayAssistantMessageEndMs > 0) { @@ -130,7 +157,7 @@ describe("AgentSession retry", () => { }; } - return { session, getCallCount: () => callCount }; + return { session, getCallCount: () => callCount, getToolExecutions: () => toolExecutions }; } it("retries after a transient error and succeeds", async () => { @@ -148,6 +175,63 @@ describe("AgentSession retry", () => { expect(created.session.isRetrying).toBe(false); }); + it("uses independent rate-limit and transient retry budgets, resetting after success", async () => { + const created = await createSession({ + maxRetries: 1, + rateLimitMaxRetries: 2, + errors: ["503 overloaded_error", "429 concurrency reached, current: 9, limit: 8", "429 rate_limited"], + }); + const retries: unknown[] = []; + created.session.subscribe((event) => { + if (event.type === "auto_retry_start") retries.push(event); + }); + await created.session.prompt("Test"); + expect(created.getCallCount()).toBe(4); + expect(retries).toMatchObject([ + { attempt: 1, maxAttempts: 1 }, + { attempt: 1, maxAttempts: 2, reason: "rate_limit", delayMs: 0 }, + { attempt: 2, maxAttempts: 2, reason: "rate_limit", delayMs: 0 }, + ]); + expect(created.session.retryAttempt).toBe(0); + }); + + it("stops rate-limit retries at their own limit", async () => { + const created = await createSession({ + maxRetries: 10, + rateLimitMaxRetries: 1, + errors: ["429 rate_limited", "429 rate_limited", "429 rate_limited"], + }); + await created.session.prompt("Test"); + expect(created.getCallCount()).toBe(2); + }); + + it.each(["429 insufficient_quota", '429: {"type":"FreeUsageLimitError"}', '429: {"type":"GoUsageLimitError"}'])( + "does not retry terminal quota error %s", + async (error) => { + const created = await createSession({ errors: [error] }); + await created.session.prompt("Test"); + expect(created.getCallCount()).toBe(1); + }, + ); + + it("cancels a scheduled 429 retry without making another request", async () => { + const created = await createSession({ errors: ["429 rate_limited"] }); + created.session.subscribe((event) => { + if (event.type === "auto_retry_start") created.session.abortRetry(); + }); + await created.session.prompt("Test"); + expect(created.getCallCount()).toBe(1); + expect(created.session.retryAttempt).toBe(0); + }); + + it("retries only the failed model request after a completed side effect", async () => { + const created = await createSession({ toolBeforeFailure: true, errors: ["429 concurrency reached"] }); + await created.session.prompt("Write then summarize"); + expect(created.getCallCount()).toBe(3); + expect(created.getToolExecutions()).toBe(1); + expect(created.session.messages.filter((message) => message.role === "toolResult")).toHaveLength(1); + }); + it("exhausts max retries and emits failure", async () => { const created = await createSession({ failCount: 99, maxRetries: 2 }); const events: string[] = []; diff --git a/packages/coding-agent/test/rate-limit.test.ts b/packages/coding-agent/test/rate-limit.test.ts new file mode 100644 index 00000000..21dba30d --- /dev/null +++ b/packages/coding-agent/test/rate-limit.test.ts @@ -0,0 +1,28 @@ +import { afterEach, expect, test, vi } from "vitest"; +import { rateLimitDelay } from "../src/core/rate-limit.ts"; +import { sleep } from "../src/utils/sleep.ts"; + +afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +test("429 delay honors server hints instead of capping them to local backoff", () => { + vi.spyOn(Math, "random").mockReturnValue(0.5); + expect(rateLimitDelay(1)).toBe(875); + expect(rateLimitDelay(2)).toBe(1750); + expect(rateLimitDelay(20)).toBeLessThanOrEqual(30_000); + expect(rateLimitDelay(1, 90_000)).toBe(90_000); +}); + +test("a long Retry-After does not overflow into an immediate retry and remains cancellable", async () => { + vi.useFakeTimers(); + const controller = new AbortController(); + const waiting = sleep(3_000_000_000, controller.signal); + const rejected = expect(waiting).rejects.toThrow("Aborted"); + await vi.advanceTimersByTimeAsync(1000); + expect(vi.getTimerCount()).toBe(1); + controller.abort(); + await rejected; + expect(vi.getTimerCount()).toBe(0); +}); diff --git a/packages/coding-agent/test/step-subagent-events.test.ts b/packages/coding-agent/test/step-subagent-events.test.ts index 3595e726..0587a5e9 100644 --- a/packages/coding-agent/test/step-subagent-events.test.ts +++ b/packages/coding-agent/test/step-subagent-events.test.ts @@ -1,8 +1,8 @@ import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; -import { tmpdir } from "node:os"; +import os, { tmpdir } from "node:os"; import { join } from "node:path"; import type { AgentToolResult } from "@step-harness/agent-core"; -import { expect, test } from "vitest"; +import { expect, test, vi } from "vitest"; import type { ExtensionAPI, ExtensionContext, ToolDefinition } from "../src/core/extensions/types.ts"; import { createStepSubagentExtension, @@ -21,6 +21,31 @@ interface SentMessage { triggerTurn?: boolean; } +test("RPC forwards 429 retry and recovery signals without treating errors as successes", () => { + const progress: unknown[] = []; + const events = [ + { + type: "auto_retry_start", + reason: "rate_limit", + attempt: 2, + maxAttempts: 3, + delayMs: 2000, + errorMessage: "429 concurrency reached", + }, + { type: "message_end", message: { role: "assistant", stopReason: "error" } }, + { type: "turn_start" }, + { type: "message_end", message: { role: "assistant", stopReason: "toolUse" } }, + { type: "message_end", message: { role: "toolResult" } }, + ]; + for (const event of events) + routeSubagentRpcLine(JSON.stringify(event), { onModelRequest: (value) => progress.push(value) }); + expect(progress).toEqual([ + { type: "rate_limit", attempt: 2, maxRetries: 3, delayMs: 2000, message: "429 concurrency reached" }, + { type: "started" }, + { type: "succeeded" }, + ]); +}); + function textResult(value: string): StepSubagentRunResult { return { messages: [ @@ -202,6 +227,49 @@ test("accepts the descriptive general-purpose name as an alias for the built-in expect(result.content[0]).toMatchObject({ type: "text", text: expect.stringContaining("alias output") }); }); +test("a ten-task batch starts eight and queues two on a ten-core host", async () => { + const cpu = os.cpus()[0]!; + const spy = vi.spyOn(os, "cpus").mockReturnValue(Array(10).fill(cpu)); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + try { + const { api, tools } = createApi(); + let calls = 0; + const counts: Array<{ running: number; queued: number }> = []; + createStepSubagentExtension({ + agentDir: "/tmp/step-agent-test", + runner: async () => { + calls++; + await gate; + return textResult("ok"); + }, + })(api); + const result = tools.get("subagent")!.execute( + "ten", + { tasks: Array.from({ length: 10 }, () => ({ agent: "general", task: "test" })) } as never, + undefined, + (update) => { + const records = (update.details as { results: Array<{ status: string }> }).results; + counts.push({ + running: records.filter((r) => r.status === "running").length, + queued: records.filter((r) => r.status === "queued").length, + }); + }, + createContext("/workspace"), + ); + await waitFor(() => calls === 8); + expect(counts).toContainEqual({ running: 8, queued: 2 }); + release(); + await result; + expect(calls).toBe(10); + } finally { + release(); + spy.mockRestore(); + } +}); + test("an exact custom agent name takes precedence over a descriptive alias", () => { const custom: StepAgentConfig = { name: "general-purpose", @@ -254,6 +322,7 @@ test.each(["single", "parallel"])( ); expect(confirmations).toBe(1); expect(runs).toBe(0); + expect(result.isError).toBe(true); expect(result.content[0]).toMatchObject({ type: "text", text: "Canceled: project-local agents not approved.", diff --git a/packages/coding-agent/test/subagent-exit-drain.test.ts b/packages/coding-agent/test/subagent-exit-drain.test.ts new file mode 100644 index 00000000..2df7a5ae --- /dev/null +++ b/packages/coding-agent/test/subagent-exit-drain.test.ts @@ -0,0 +1,75 @@ +import type { ChildProcess, SpawnOptions } from "node:child_process"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { expect, test, vi } from "vitest"; +import { runStepSubagentProcess } from "../src/features/step-subagent.ts"; +import { builtinStepAgents } from "../src/features/step-subagent-agents.ts"; + +const state = vi.hoisted(() => ({ + fixture: "", + descendants: [] as number[], + child: undefined as ChildProcess | undefined, +})); +vi.mock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + spawn: (_command: string, _args: string[], options: SpawnOptions) => { + const child = actual.spawn(process.execPath, [state.fixture], options); + state.child = child; + let buffer = ""; + child.stdout?.on("data", (chunk) => { + buffer += chunk.toString(); + const lines = buffer.split("\n"); + buffer = lines.pop() ?? ""; + for (const line of lines) { + const event = JSON.parse(line); + if (event.type === "descendant_pid") state.descendants.push(event.pid); + } + }); + return child; + }, + }; +}); + +test("workflow child exit settles even when a quiet descendant holds its stdio open", async () => { + const cwd = await mkdtemp(path.join(tmpdir(), "subagent-exit-drain-")); + state.fixture = path.join(cwd, "rpc-child.cjs"); + await writeFile( + state.fixture, + ` +const {spawn} = require('node:child_process'); +process.stdin.once('data', () => { + const descendant = spawn(process.execPath, ['-e', 'setTimeout(()=>{},5000)'], {detached:true,stdio:['ignore',1,2]}); + descendant.unref(); + console.log(JSON.stringify({type:'descendant_pid',pid:descendant.pid})); + console.log(JSON.stringify({type:'message_end',message:{role:'assistant',content:[{type:'text',text:'OK'}],stopReason:'stop',timestamp:Date.now(),usage:{input:0,output:0,cacheRead:0,cacheWrite:0,cost:{total:0}}}})); + console.log(JSON.stringify({type:'agent_settled'})); + process.exit(0); +}); +`, + ); + try { + const started = Date.now(); + const result = await runStepSubagentProcess({ + agent: { ...builtinStepAgents.find((agent) => agent.name === "general")!, systemPrompt: "" }, + task: "test", + cwd, + workflowAcl: { baseCwd: cwd }, + keepAlive: false, + }); + expect(result.stopReason).toBe("stop"); + expect(Date.now() - started).toBeLessThan(2000); + expect(state.descendants).toHaveLength(1); + expect(state.child?.exitCode).toBe(0); + } finally { + for (const pid of state.descendants.splice(0)) { + try { + process.kill(pid); + } catch {} + } + state.child?.kill(); + await rm(cwd, { recursive: true, force: true }); + } +}); diff --git a/packages/coding-agent/test/workflow-extension.test.ts b/packages/coding-agent/test/workflow-extension.test.ts index 66ee8b72..2cf36205 100644 --- a/packages/coding-agent/test/workflow-extension.test.ts +++ b/packages/coding-agent/test/workflow-extension.test.ts @@ -5,6 +5,7 @@ import type { AgentToolResult } from "@step-harness/agent-core"; import { afterEach, expect, test, vi } from "vitest"; import type { ExtensionAPI, ExtensionCommandContext, ToolDefinition } from "../src/core/extensions/types.ts"; import { registerWorkflowChildAcl, WORKFLOW_ACL_ENV } from "../src/features/workflow/acl-extension.ts"; +import { workflowProgressResult } from "../src/features/workflow/rendering.ts"; import { createStepWorkflowExtension, resolveWorkflowScript, @@ -15,6 +16,58 @@ import type { StepTelemetryReporter } from "../src/step/telemetry.ts"; const cleanups: string[] = []; +test("concurrency clamping is a notice, independent of workflow failure state", () => { + const progress: WorkflowProgress = { + schemaVersion: 1, + runId: "notice", + name: "notice", + status: "running", + startedAt: 0, + updatedAt: 0, + agents: [], + completedAgents: 0, + totalAgents: 15, + spentTokens: 0, + concurrency: { requested: 15, ceiling: 8, limit: 8, cap: 8 }, + }; + const output = workflowProgressResult(progress); + expect(output.details.status).toBe("running"); + expect(output.isError).not.toBe(true); + expect(output.content).toMatchObject([ + { text: expect.stringContaining("Notice: requested concurrency 15 was limited to 8") }, + ]); + const failure = workflowProgressResult({ ...progress, status: "failed", message: "parallel() invalid entry" }); + expect(failure.content).toMatchObject([{ text: expect.stringContaining("parallel() invalid entry") }]); +}); + +test("nested callback failures name their script in the text channels", () => { + const progress: WorkflowProgress = { + schemaVersion: 1, + runId: "nested", + name: "nested", + status: "partial_failed", + startedAt: 0, + updatedAt: 0, + agents: [], + completedAgents: 1, + totalAgents: 1, + spentTokens: 0, + taskFailures: [ + { + script: "child.js", + primitive: "parallel", + index: 1, + message: "ReferenceError: d is not defined", + }, + ], + }; + const output = workflowProgressResult(progress); + expect(output.details?.status).toBe("partial_failed"); + expect(output.content).toMatchObject([ + { text: expect.stringContaining("child.js parallel[1]: ReferenceError: d is not defined") }, + ]); +}); + afterEach(async () => { vi.unstubAllEnvs(); await Promise.all(cleanups.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))); diff --git a/packages/coding-agent/test/workflow-runtime.test.ts b/packages/coding-agent/test/workflow-runtime.test.ts index da55614f..60f37b00 100644 --- a/packages/coding-agent/test/workflow-runtime.test.ts +++ b/packages/coding-agent/test/workflow-runtime.test.ts @@ -1,15 +1,22 @@ import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises"; -import { tmpdir } from "node:os"; +import os, { tmpdir } from "node:os"; import path from "node:path"; -import { afterEach, expect, test } from "vitest"; -import { WorkflowBudgetExceeded } from "../src/features/workflow/budget.ts"; +import { afterEach, beforeEach, expect, test, vi } from "vitest"; +import { acquireWorkflowAdmission, resetWorkflowAdmission } from "../src/features/workflow/admission.ts"; +import { + clampWorkflowConcurrency, + configuredWorkflowConcurrencyLimit, + WorkflowBudgetExceeded, + WorkflowSemaphore, +} from "../src/features/workflow/budget.ts"; import { createWorkflowRunPaths, readJsonLines, WorkflowJournal, type WorkflowRunPaths, } from "../src/features/workflow/journal.ts"; -import { WorkflowRuntime } from "../src/features/workflow/runtime.ts"; +import { WorkflowProgressStore } from "../src/features/workflow/progress.ts"; +import { WorkflowRuntime, workflowToolResult } from "../src/features/workflow/runtime.ts"; import { checkWorkflowPathAccess, checkWorkflowToolCall } from "../src/features/workflow/tool-profile.ts"; import type { IterateResult, @@ -17,13 +24,22 @@ import type { WorkflowAgentRunInput, WorkflowAgentRunner, WorkflowProgress, + WorkflowProgressAgent, WorkflowTelemetryRecord, } from "../src/features/workflow/types.ts"; const cleanups: string[] = []; let runSequence = 0; +beforeEach(() => { + const cpu = os.cpus()[0]!; + vi.spyOn(os, "cpus").mockReturnValue(Array(10).fill(cpu)); +}); + afterEach(async () => { + vi.useRealTimers(); + vi.restoreAllMocks(); + resetWorkflowAdmission(); await Promise.all(cleanups.splice(0).map((directory) => rm(directory, { recursive: true, force: true }))); }); @@ -40,9 +56,12 @@ function runtimeAt( resumeFrom?: WorkflowRunPaths; budgetTotal?: number; maxConcurrency?: number; + providerConcurrencyLimit?: number; + runnerCleanupTimeoutMs?: number; maxAgents?: number; agentTimeoutMs?: number; vmExecutor?: ConstructorParameters[0]["vmExecutor"]; + nestedWorkflow?: ConstructorParameters[0]["nestedWorkflow"]; } = {}, ): { runtime: WorkflowRuntime; journal: WorkflowJournal; paths: WorkflowRunPaths } { runSequence += 1; @@ -56,9 +75,12 @@ function runtimeAt( runner, budgetTotal: options.budgetTotal, maxConcurrency: options.maxConcurrency, + providerConcurrencyLimit: options.providerConcurrencyLimit, + runnerCleanupTimeoutMs: options.runnerCleanupTimeoutMs, maxAgents: options.maxAgents, agentTimeoutMs: options.agentTimeoutMs, vmExecutor: options.vmExecutor, + nestedWorkflow: options.nestedWorkflow, }); return { runtime, journal, paths }; } @@ -71,6 +93,564 @@ function result(value: unknown, tokens = 1) { }; } +function initialProgress(runId: string): WorkflowProgress { + return { + schemaVersion: 1, + runId, + name: "test", + status: "running", + startedAt: 0, + updatedAt: 0, + agents: [], + completedAgents: 0, + totalAgents: 0, + spentTokens: 0, + }; +} + +test("explicit concurrency cannot exceed the machine ceiling", () => { + expect(clampWorkflowConcurrency(22, 10)).toBe(8); + expect(clampWorkflowConcurrency(22, 32)).toBe(16); + expect(clampWorkflowConcurrency(2, 10)).toBe(2); +}); + +test("optional provider concurrency ignores blank and invalid environment values", () => { + for (const raw of ["", " ", "abc", "Infinity", "-1", "0"]) + expect(configuredWorkflowConcurrencyLimit(raw)).toBeUndefined(); + expect(configuredWorkflowConcurrencyLimit(" 8 ")).toBe(8); +}); + +test("a long server Retry-After cannot freeze the shared admission window", async () => { + // A node's own retry honors Retry-After verbatim, but the admission window is + // process-wide, so an hour-long (or far-future-date) cooldown must not stall + // unrelated workflows with no timeout and no error. + vi.useFakeTimers(); + const semaphore = new WorkflowSemaphore(4); + try { + semaphore.throttle(semaphore.epoch, 3_600_000); + let admitted = false; + const acquired = semaphore.acquire().then((release) => { + admitted = true; + return release; + }); + await vi.advanceTimersByTimeAsync(29_000); + expect(admitted).toBe(false); + await vi.advanceTimersByTimeAsync(2_000); + const release = await acquired; + expect(admitted).toBe(true); + release(); + } finally { + semaphore.dispose(); + vi.useRealTimers(); + } +}); + +test("an invalid throttle delay neither blocks admission nor schedules a far-future timer", async () => { + vi.useFakeTimers(); + const semaphore = new WorkflowSemaphore(2); + try { + for (const delay of [Number.NaN, Number.POSITIVE_INFINITY, -1]) semaphore.throttle(semaphore.epoch, delay); + const release = await semaphore.acquire(); + release(); + } finally { + semaphore.dispose(); + vi.useRealTimers(); + } +}); + +test("nested workflow callback failures propagate to the owning run", async () => { + const child = 'return await parallel([() => agent("ok"), () => { throw new ReferenceError("missingRef"); }]);'; + const { runtime } = runtimeAt(await workspace(), async () => result("ok"), { + nestedWorkflow: (_name, args, parent, depth) => + parent.runNestedScript(child, args, depth + 1, { filename: "child.js" }), + }); + const output = await runtime.runScript('return await workflow("child");', null); + expect(output).toMatchObject({ + status: "partial_failed", + summary: { total: 1, completed: 1, failed: 0, failedCallbacks: 1 }, + taskFailures: [{ primitive: "parallel", index: 1, script: "child.js", message: "ReferenceError: missingRef" }], + }); +}); + +test.each([false, true])("agent failures are not double counted as callback failures (nested=%s)", async (nested) => { + const { runtime } = runtimeAt( + await workspace(), + async () => ({ status: "failed", errorMessage: "provider failed" }), + { + nestedWorkflow: (_name, args, parent, depth) => + parent.runNestedScript('return await agent("bad");', args, depth + 1), + }, + ); + const output = await runtime.runScript( + nested ? 'return await parallel([() => workflow("child")]);' : 'return await parallel([() => agent("bad")]);', + null, + ); + expect(output.status).toBe("failed"); + expect(output.summary).toEqual({ total: 1, completed: 0, failed: 1 }); + expect(output.taskFailures).toBeUndefined(); +}); + +test("unawaited nested workflows fail rather than returning a successful zero-call run", async () => { + const { runtime, paths } = runtimeAt(await workspace(), async () => result("ok"), { + nestedWorkflow: (_name, args, parent, depth) => + parent.runNestedScript('return await agent("child");', args, depth + 1), + }); + await expect(runtime.runScript('void workflow("child"); return "done";', null)).rejects.toThrow("unawaited"); + const progress = JSON.parse(await readFile(paths.progressPath, "utf8")); + expect(progress.status).toBe("failed"); + expect(progress.agents.every((agent: { status: string }) => !["running", "queued"].includes(agent.status))).toBe( + true, + ); +}); + +test("a failing terminal progress write never replaces the run's terminal error", async () => { + const cwd = await workspace(); + const paths = createWorkflowRunPaths(cwd, "progress_write_failure"); + const journal = new WorkflowJournal(paths); + // A disk that only fails on terminal writes: the run's real error must survive. + class TerminalFailingProgress extends WorkflowProgressStore { + override async update( + patch: Partial & { agent?: WorkflowProgressAgent }, + ): Promise { + if (patch.status !== undefined && patch.status !== "running") throw new Error("progress disk full"); + return super.update(patch); + } + } + const runtime = new WorkflowRuntime({ + cwd, + runId: "progress_write_failure", + name: "test", + journal, + progress: new TerminalFailingProgress(paths.progressPath, initialProgress("progress_write_failure")), + runner: async () => { + throw new Error("provider exploded"); + }, + }); + await expect(runtime.runScript('return await agent("boom");', null)).rejects.toThrow("provider exploded"); + const entries = await readJsonLines<{ status?: string; error?: string }>(journal.paths.journalPath); + expect(entries.filter((entry) => entry.status === "failed")).toMatchObject([ + { status: "failed", error: expect.stringContaining("provider exploded") }, + ]); +}); + +test("a failed progress write while recording a node keeps the failure classified once", async () => { + const cwd = await workspace(); + const paths = createWorkflowRunPaths(cwd, "node_record_progress_failure"); + // State mutates before the write, so the agent is already failed when the write + // rejects. The run must still report that node once, not also as a callback error. + class LateFailingProgress extends WorkflowProgressStore { + failWrites = false; + override async update( + patch: Partial & { agent?: WorkflowProgressAgent }, + ): Promise { + const snapshot = await super.update(patch); + if (this.failWrites) throw new Error("progress disk full"); + return snapshot; + } + } + const progress = new LateFailingProgress(paths.progressPath, initialProgress("node_record_progress_failure")); + const runtime = new WorkflowRuntime({ + cwd, + runId: "node_record_progress_failure", + name: "test", + journal: new WorkflowJournal(paths), + progress, + runner: async () => { + progress.failWrites = true; + return { status: "failed", errorMessage: "provider failed" }; + }, + }); + const output = await runtime.runScript('return await parallel([() => agent("bad")]);', null); + expect(output).toMatchObject({ status: "failed", summary: { total: 1, completed: 0, failed: 1 } }); + expect(output.taskFailures).toBeUndefined(); + expect(output.failures?.[0]?.error?.message).toContain("provider failed"); +}); + +test("a failed journal append while recording a node keeps the failure classified once", async () => { + const cwd = await workspace(); + const paths = createWorkflowRunPaths(cwd, "node_record_journal_failure"); + await mkdir(paths.runDir, { recursive: true }); + // A directory at the journal path makes every append fail while the script copy + // and progress writes still work. + await mkdir(paths.journalPath, { recursive: true }); + const runtime = new WorkflowRuntime({ + cwd, + runId: "node_record_journal_failure", + name: "test", + journal: new WorkflowJournal(paths), + runner: async () => ({ status: "failed", errorMessage: "provider failed" }), + }); + const output = await runtime.runScript('return await parallel([() => agent("bad")]);', null); + expect(output).toMatchObject({ status: "failed", summary: { total: 1, completed: 0, failed: 1 } }); + expect(output.taskFailures).toBeUndefined(); + expect(output.failures?.[0]?.error?.message).toContain("provider failed"); +}); + +test("a runner that throws while the journal is unwritable still reports the runner error", async () => { + const cwd = await workspace(); + const paths = createWorkflowRunPaths(cwd, "node_throw_journal_failure"); + await mkdir(paths.runDir, { recursive: true }); + await mkdir(paths.journalPath, { recursive: true }); + const runtime = new WorkflowRuntime({ + cwd, + runId: "node_throw_journal_failure", + name: "test", + journal: new WorkflowJournal(paths), + runner: async () => { + throw new Error("provider exploded"); + }, + }); + // The journal error must not replace the runner's error, in either channel. + await expect(runtime.runScript('return await agent("boom");', null)).rejects.toThrow("provider exploded"); + const progress = JSON.parse(await readFile(paths.progressPath, "utf8")); + expect(progress.agents[0].error.message).toContain("provider exploded"); + expect(progress.agents[0].error.message).not.toContain("EISDIR"); +}); + +test("a throwing progress listener cannot double count a node failure", async () => { + const { runtime } = runtimeAt( + await workspace(), + async () => ({ status: "failed", errorMessage: "provider failed" }), + {}, + ); + // Only the update that records the node failure throws, which is the path where a + // listener error used to escape and turn one failure into two. + (runtime as unknown as { onProgress?: (progress: WorkflowProgress) => void }).onProgress = (progress) => { + if (progress.agents.some((agent) => agent.status === "failed")) throw new Error("listener exploded"); + }; + const output = await runtime.runScript('return await parallel([() => agent("bad")]);', null); + expect(output).toMatchObject({ status: "failed", summary: { total: 1, completed: 0, failed: 1 } }); + expect(output.taskFailures).toBeUndefined(); +}); + +test("a node label that looks like a rate limit does not throttle the pool", async () => { + const { runtime } = runtimeAt( + await workspace(), + (input) => + new Promise((resolve) => { + input.signal?.addEventListener("abort", () => resolve({ status: "aborted" }), { once: true }); + }), + { maxConcurrency: 4, agentTimeoutMs: 1_000 }, + ); + const output = await runtime.runScript('return await parallel([() => agent("a", {label: "fix 429"})]);', null); + expect(output.failures?.[0]?.error).toMatchObject({ code: "NODE_TIMEOUT", retryable: false }); + // A synthesized timeout is not congestion, so the shared window keeps its cap. + expect(runtime.semaphore.max).toBe(4); +}); + +test("failed agent output that mentions rate limits is not provider congestion", async () => { + const { runtime } = runtimeAt( + await workspace(), + async () => ({ status: "failed", text: "I could not finish: the 429 rate limit docs were unreachable" }), + { maxConcurrency: 4 }, + ); + const output = await runtime.runScript('return await parallel([() => agent("document the 429 handler")]);', null); + expect(output.failures?.[0]?.error).toMatchObject({ code: "NODE_FAILED", retryable: false }); + expect(runtime.semaphore.max).toBe(4); +}); + +test("a tool timeout in agent output is not reported as the node's own timeout", async () => { + const { runtime } = runtimeAt(await workspace(), async () => ({ + status: "failed", + errorMessage: "bash command timed out after 30s", + })); + const output = await runtime.runScript('return await parallel([() => agent("run it")]);', null); + expect(output.failures?.[0]?.error).toMatchObject({ code: "NODE_FAILED" }); +}); + +test("usage reported by a runner after cancellation is still charged to the budget", async () => { + const controller = new AbortController(); + const cwd = await workspace(); + const paths = createWorkflowRunPaths(cwd, "late_usage"); + const runtime = new WorkflowRuntime({ + cwd, + runId: "late_usage", + name: "test", + journal: new WorkflowJournal(paths), + signal: controller.signal, + runner: (input) => + new Promise((resolve) => { + input.signal?.addEventListener( + "abort", + () => setTimeout(() => resolve({ ...result("partial", 7), status: "aborted" }), 5), + { once: true }, + ); + queueMicrotask(() => controller.abort()); + }), + }); + await expect(runtime.runScript('return await agent("spend then cancel");', null)).rejects.toThrow(/aborted/i); + expect(runtime.budget.spent()).toBe(7); + const entries = await readJsonLines<{ status?: string; usage?: { input: number } }>(paths.journalPath); + expect(entries.at(-1)).toMatchObject({ status: "aborted", usage: { input: 7 } }); +}); + +test("a resampled request in the same turn is tagged with the current congestion epoch", async () => { + let finish!: () => void; + const gate = new Promise((resolve) => { + finish = resolve; + }); + let capAfterRecovery = 0; + let runtime!: WorkflowRuntime; + const built = runtimeAt( + await workspace(), + async (input) => { + if (input.prompt !== "d") { + await gate; + return result(input.prompt); + } + input.onModelRequest?.({ type: "started" }); + // Wait until the window is full and "e" is queued behind it. + await vi.waitFor(() => expect(runtime.semaphore.active).toBe(4)); + runtime.semaphore.throttle(runtime.semaphore.epoch, 0); + expect(runtime.semaphore.max).toBe(3); + // The first success belongs to the pre-throttle epoch and is ignored. The + // resample that follows has no "started" event and must count again. + for (let i = 0; i < 5; i++) input.onModelRequest?.({ type: "succeeded" }); + capAfterRecovery = runtime.semaphore.max; + finish(); + return result("d"); + }, + { maxConcurrency: 4 }, + ); + runtime = built.runtime; + // "d" is the probe; "e" is queued behind the full window, so demand exists. + const output = await runtime.runScript( + 'return await parallel([() => agent("a"), () => agent("b"), () => agent("c"), () => agent("d"), () => agent("e")]);', + null, + ); + expect(output.failures).toEqual([]); + expect(output.status).toBe("completed"); + expect(capAfterRecovery).toBe(4); +}); + +test("progress write failures never fail a finished node and recovery persists", async () => { + const cwd = await workspace(); + const paths = createWorkflowRunPaths(cwd, "progress_disk_blip"); + let calls = 0; + const runtime = new WorkflowRuntime({ + cwd, + runId: "progress_disk_blip", + name: "test", + journal: new WorkflowJournal(paths), + runner: async () => { + calls++; + if (calls === 1) { + // Renaming onto a directory fails, so every write while this exists errors. + await rm(paths.progressPath, { force: true }); + await mkdir(paths.progressPath); + } else { + await rm(paths.progressPath, { recursive: true, force: true }); + } + return result(`call ${calls}`); + }, + }); + const output = await runtime.runScript('return [await agent("one"), await agent("two")];', null); + expect(output).toMatchObject({ status: "completed", value: ["call 1", "call 2"], summary: { completed: 2 } }); + const entries = await readJsonLines<{ status?: string }>(paths.journalPath); + expect(entries.map((entry) => entry.status)).toEqual(["completed", "completed"]); + expect(JSON.parse(await readFile(paths.progressPath, "utf8"))).toMatchObject({ + status: "completed", + completedAgents: 2, + }); +}); + +test("a throttled admission window is remembered briefly after the last workflow finishes", async () => { + vi.useFakeTimers(); + const first = acquireWorkflowAdmission(4); + first.semaphore.throttle(first.semaphore.epoch, 20_000); + expect(first.semaphore.max).toBe(3); + first.release(); + // No timer may keep the process alive while nothing is attached. + expect(vi.getTimerCount()).toBe(0); + const second = acquireWorkflowAdmission(4); + expect(second.semaphore.max).toBe(3); + // The retained cooldown still gates admission and re-arms its wakeup for waiters. + let admitted = false; + const acquired = second.semaphore.acquire().then((release) => { + admitted = true; + return release; + }); + await vi.advanceTimersByTimeAsync(19_000); + expect(admitted).toBe(false); + await vi.advanceTimersByTimeAsync(1_500); + (await acquired)(); + expect(admitted).toBe(true); + second.release(); + // After the retention window the window starts fresh. + await vi.advanceTimersByTimeAsync(61_000); + const third = acquireWorkflowAdmission(4); + expect(third.semaphore.max).toBe(4); + third.release(); +}); + +test("an untouched admission window is discarded as soon as it is idle", () => { + const first = acquireWorkflowAdmission(4); + const semaphore = first.semaphore; + first.release(); + const second = acquireWorkflowAdmission(4); + expect(second.semaphore).not.toBe(semaphore); + second.release(); +}); + +test("a transient progress write failure does not poison later writes", async () => { + const cwd = await workspace(); + const paths = createWorkflowRunPaths(cwd, "progress_recovery"); + const store = new WorkflowProgressStore(paths.progressPath, initialProgress("progress_recovery")); + await store.update({ status: "running" }); + // Point the store at a path that cannot be written (a directory sits there), then + // move it back: the second write must still land. + const blocked = path.join(cwd, "blocked"); + await mkdir(path.join(blocked, "progress.json"), { recursive: true }); + const target = store as unknown as { path: string }; + target.path = path.join(blocked, "progress.json"); + await expect(store.update({ message: "first failure" })).rejects.toThrow(/EISDIR|ENOTDIR/); + target.path = paths.progressPath; + await store.update({ status: "completed", message: "recovered" }); + expect(JSON.parse(await readFile(paths.progressPath, "utf8"))).toMatchObject({ + status: "completed", + message: "recovered", + }); +}); + +test("shell interpolation errors before agent() cannot become a successful zero-call workflow", async () => { + let calls = 0; + const { runtime, paths } = runtimeAt(await workspace(), async (input) => { + calls++; + return result(input.prompt); + }); + const output = await runtime.runScript( + // biome-ignore lint/suspicious/noTemplateCurlyInString: Execute the user's broken JS template verbatim in the guest VM. + 'const tasks = Array.from({length: 10}, () => () => agent(`sleep $d; echo "slept=${d}s"`)); return await parallel(tasks);', + null, + ); + expect(calls).toBe(0); + expect(output).toMatchObject({ status: "failed", agentCalls: 0, summary: { failedCallbacks: 10 } }); + expect(output.taskFailures).toHaveLength(10); + expect( + output.taskFailures?.every( + (failure) => failure.message.includes("ReferenceError") && failure.message.includes("d"), + ), + ).toBe(true); + expect(workflowToolResult(output).isError).toBe(true); + expect(workflowToolResult(output).content).toMatchObject([{ text: expect.stringContaining("ReferenceError") }]); + const progress = JSON.parse(await readFile(paths.progressPath, "utf8")); + expect(progress.status).toBe("failed"); + expect(progress.taskFailures).toHaveLength(10); +}); + +test("ordinary JS strings preserve shell substitutions and dispatch all ten agents", async () => { + const prompts: string[] = []; + const { runtime } = runtimeAt(await workspace(), async (input) => { + prompts.push(input.prompt); + return result("ok"); + }); + const output = await runtime.runScript( + // biome-ignore lint/suspicious/noTemplateCurlyInString: Shell substitutions must survive JavaScript string construction. + "const command = 'sleep $d; echo \"slept=${d}s\"'; return await parallel(Array.from({length: 10}, () => () => agent(command)));", + null, + ); + expect(output).toMatchObject({ + status: "completed", + agentCalls: 10, + summary: { total: 10, completed: 10, failed: 0 }, + }); + // biome-ignore lint/suspicious/noTemplateCurlyInString: Assert the exact literal shell command. + expect(prompts).toEqual(Array(10).fill('sleep $d; echo "slept=${d}s"')); +}); + +test("throttling one wave reduces cap once and queued work resumes after releases", async () => { + const semaphore = new WorkflowSemaphore(8); + const releases = await Promise.all(Array.from({ length: 8 }, () => semaphore.acquire())); + let admitted = 0; + const queued = Array.from({ length: 3 }, () => + semaphore.acquire().then((release) => { + admitted++; + return release; + }), + ); + const epoch = semaphore.epoch; + semaphore.throttle(epoch, 0); + semaphore.throttle(epoch, 0); + expect(semaphore.max).toBe(6); + for (const release of releases.slice(0, 2)) release(); + await Promise.resolve(); + expect(admitted).toBe(0); + for (const release of releases.slice(2)) release(); + for (const release of await Promise.all(queued)) release(); + expect(admitted).toBe(3); + semaphore.dispose(); +}); + +test("cooldown blocks new admissions, cancellation removes waiters, and successes recover cap", async () => { + vi.useFakeTimers(); + const semaphore = new WorkflowSemaphore(4); + semaphore.throttle(semaphore.epoch, 1000); + const controller = new AbortController(); + const cancelled = semaphore.acquire(controller.signal); + const cancelledCheck = expect(cancelled).rejects.toThrow("aborted"); + controller.abort(); + await cancelledCheck; + let admitted = 0; + const pending = Array.from({ length: 4 }, () => + semaphore.acquire().then((release) => { + admitted++; + return release; + }), + ); + await vi.advanceTimersByTimeAsync(999); + expect(admitted).toBe(0); + await vi.advanceTimersByTimeAsync(1); + expect(admitted).toBe(3); + for (let i = 0; i < 4; i++) semaphore.succeeded(semaphore.epoch); + expect(semaphore.max).toBe(4); + for (const release of await Promise.all(pending)) release(); + semaphore.dispose(); + expect(vi.getTimerCount()).toBe(0); +}); + +test("parallel failures survive null filtering in final tool output and disk progress", async () => { + const { runtime, paths } = runtimeAt(await workspace(), async (input) => + input.prompt === "bad" + ? { status: "failed", errorMessage: "429 concurrency reached, current: 9, limit: 8" } + : result("ok"), + ); + const output = await runtime.runScript( + 'return (await parallel([() => agent("ok"), () => agent("bad")])).filter(Boolean);', + null, + ); + expect(output).toMatchObject({ + status: "partial_failed", + value: ["ok"], + summary: { total: 2, completed: 1, failed: 1 }, + failures: [{ error: { code: "RATE_LIMITED", retryable: true } }], + }); + expect(workflowToolResult(output).isError).toBe(true); + expect(workflowToolResult(output).content).toMatchObject([ + { text: expect.stringContaining("429 concurrency reached") }, + ]); + const progress = JSON.parse(await readFile(paths.progressPath, "utf8")); + expect(progress.status).toBe("partial_failed"); + expect(progress.agents[1].error.message).toContain("429"); +}); + +test("a recovered 429 adjusts cap without restarting the node or reporting it failed", async () => { + let calls = 0; + const { runtime } = runtimeAt( + await workspace(), + async (input) => { + calls++; + input.onModelRequest?.({ type: "rate_limit", attempt: 1, maxRetries: 3, delayMs: 0, message: "429" }); + input.onModelRequest?.({ type: "started" }); + input.onModelRequest?.({ type: "succeeded" }); + return result("ok"); + }, + { maxConcurrency: 4 }, + ); + const output = await runtime.runScript('return await agent("test");', null); + expect(calls).toBe(1); + expect(output).toMatchObject({ status: "completed", failures: [], concurrency: { limit: 4, cap: 3 } }); +}); + test("agent retries invalid schema output and strips unknown fields", async () => { const cwd = await workspace(); const prompts: string[] = []; @@ -337,16 +917,222 @@ test("parallel calls cannot race past the workflow agent limit", async () => { test("agent timeout aborts the attempt and releases its semaphore slot", async () => { const cwd = await workspace(); - const { runtime, journal } = runtimeAt(cwd, async () => new Promise(() => {}), { - agentTimeoutMs: 1_000, - maxConcurrency: 1, - }); + const { runtime, journal } = runtimeAt( + cwd, + async (input) => + new Promise((resolve) => { + input.signal?.addEventListener("abort", () => resolve({ status: "aborted" }), { once: true }); + }), + { + agentTimeoutMs: 1_000, + maxConcurrency: 1, + }, + ); await journal.initialize("agent-timeout"); await expect(runtime.agent("never settles")).rejects.toThrow("timed out after 1000ms"); expect(runtime.semaphore.active).toBe(0); }); +test("script failure cancels running and queued nodes, awaits cleanup, and seals terminal progress", async () => { + let signalStarted!: () => void; + const started = new Promise((resolve) => { + signalStarted = resolve; + }); + const calls: string[] = []; + let cleaned = false; + const { runtime, paths } = runtimeAt( + await workspace(), + async (input) => { + calls.push(input.prompt); + signalStarted(); + await new Promise((resolve) => + input.signal?.addEventListener( + "abort", + () => { + setTimeout(() => { + cleaned = true; + resolve(); + }, 10); + }, + { once: true }, + ), + ); + return { status: "aborted" }; + }, + { + maxConcurrency: 1, + vmExecutor: async (_script, _args, host) => { + void host.agent("running", {}); + void host.agent("queued", {}); + await started; + throw new Error("script exploded"); + }, + }, + ); + await expect(runtime.runScript("bad script", null)).rejects.toThrow("script exploded"); + expect(cleaned).toBe(true); + expect(calls).toEqual(["running"]); + const before = await readFile(paths.progressPath, "utf8"); + const progress = JSON.parse(before); + expect(progress).toMatchObject({ status: "failed", message: "script exploded" }); + expect(progress.agents.every((a: { status: string }) => a.status === "aborted")).toBe(true); + const journal = await readJsonLines<{ status: string }>(paths.journalPath); + expect(journal).toHaveLength(2); + expect(journal.every((entry) => entry.status === "aborted")).toBe(true); + await expect(runtime.agent("late")).rejects.toThrow("scope is closed"); + runtime.log("late update"); + expect(await readFile(paths.progressPath, "utf8")).toBe(before); +}); + +test("returning with detached agent calls fails and cancels them instead of claiming completion", async () => { + let aborted = false; + let signalStarted!: () => void; + const started = new Promise((resolve) => { + signalStarted = resolve; + }); + const { runtime } = runtimeAt( + await workspace(), + async (input) => { + signalStarted(); + await new Promise((resolve) => + input.signal?.addEventListener( + "abort", + () => { + aborted = true; + resolve(); + }, + { once: true }, + ), + ); + return { status: "aborted" }; + }, + { + vmExecutor: async (_script, _args, host) => { + void host.agent("detached", {}); + await started; + return { value: null, meta: {} }; + }, + }, + ); + await expect(runtime.runScript("missing await", null)).rejects.toThrow("unawaited agent calls"); + expect(aborted).toBe(true); +}); + +test("abort during admission progress never calls the runner", async () => { + const cwd = await workspace(); + const controller = new AbortController(); + let calls = 0; + const paths = createWorkflowRunPaths(cwd, "abort-before-runner"); + const runtime = new WorkflowRuntime({ + cwd, + runId: "abort-before-runner", + name: "abort", + journal: new WorkflowJournal(paths), + signal: controller.signal, + onProgress: (progress) => { + if (progress.agents.some((agent) => agent.status === "running")) controller.abort(); + }, + runner: async () => { + calls++; + return result("unexpected"); + }, + }); + await expect(runtime.runScript('return await agent("test");', null)).rejects.toThrow(/aborted/i); + expect(calls).toBe(0); +}); + +test("uncooperative runners retain admission until settlement, then the pool can be recreated", async () => { + let started!: () => void; + const ready = new Promise((resolve) => { + started = resolve; + }); + let finish!: () => void; + const gate = new Promise((resolve) => { + finish = resolve; + }); + const first = runtimeAt( + await workspace(), + async () => { + started(); + await gate; + return result("late"); + }, + { + maxConcurrency: 1, + providerConcurrencyLimit: 1, + runnerCleanupTimeoutMs: 10, + vmExecutor: async (_script, _args, host) => { + void host.agent("uncooperative", {}); + await ready; + throw new Error("stop this run"); + }, + }, + ); + try { + await expect(first.runtime.runScript("stop", null)).rejects.toThrow("stop this run"); + const progress = JSON.parse(await readFile(first.paths.progressPath, "utf8")); + expect(progress.agents[0].error.message).toContain("shared slot remains reserved"); + let nextCalls = 0; + const next = runtimeAt( + await workspace(), + async () => { + nextCalls++; + return result("ok"); + }, + { maxConcurrency: 1, providerConcurrencyLimit: 1 }, + ); + const run = next.runtime.runScript('return await agent("next");', null); + await vi.waitFor(() => expect(next.runtime.semaphore.active).toBe(1)); + expect(nextCalls).toBe(0); + finish(); + expect((await run).status).toBe("completed"); + expect(nextCalls).toBe(1); + const recreated = acquireWorkflowAdmission(2); + expect(recreated.semaphore.active).toBe(0); + expect(recreated.semaphore.max).toBe(2); + recreated.release(); + } finally { + finish(); + } +}); + +test("two workflows share one host window and both drain their queues", async () => { + let active = 0; + let peak = 0; + let startedCount = 0; + let signalSaturated!: () => void; + const saturated = new Promise((resolve) => { + signalSaturated = resolve; + }); + let release!: () => void; + const gate = new Promise((resolve) => { + release = resolve; + }); + const runner: WorkflowAgentRunner = async (input) => { + active++; + startedCount++; + peak = Math.max(peak, active); + if (active === 2) signalSaturated(); + await gate; + active--; + return result(input.prompt); + }; + const first = runtimeAt(await workspace(), runner, { maxConcurrency: 2, providerConcurrencyLimit: 2 }); + const second = runtimeAt(await workspace(), runner, { maxConcurrency: 2, providerConcurrencyLimit: 2 }); + const script = 'return await parallel([agent("a"), agent("b"), agent("c")]);'; + const runs = [first.runtime.runScript(script, null), second.runtime.runScript(script, null)]; + await saturated; + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(startedCount).toBe(2); + release(); + const results = await Promise.all(runs); + expect(results.map((run) => run.status)).toEqual(["completed", "completed"]); + expect(peak).toBe(2); + expect(startedCount).toBe(6); + expect(active).toBe(0); +}); + test("named tool profiles resolve and writable agents remain single-writer", async () => { const cwd = await workspace(); const options: WorkflowAgentOptions[] = []; diff --git a/packages/coding-agent/test/workflow-ultraloop-opt-in.test.ts b/packages/coding-agent/test/workflow-ultraloop-opt-in.test.ts index 2b0374c9..3b9e3362 100644 --- a/packages/coding-agent/test/workflow-ultraloop-opt-in.test.ts +++ b/packages/coding-agent/test/workflow-ultraloop-opt-in.test.ts @@ -13,6 +13,87 @@ afterEach(() => { vi.unstubAllEnvs(); }); +test("explicit counted fan-out requests grant workflow consent without the ultraloop keyword", () => { + for (const prompt of [ + "开10个子agent 随机sleep1-10s", + "请启动十个子代理", + "launch 10 subagents", + "帮我创建 2 个子 agent", + // The request may sit anywhere in the message. + "麻烦开3个子agent查一下", + "先看代码，然后开10个子agent", + "帮我开10个agent", + "启动10个agent", + "Can you spawn 5 subagents to review this PR?", + "I'd like you to launch 10 subagents for the audit", + "run 10 subagents in parallel", + ]) { + expect(detectUltraloopOptIn(prompt), prompt).toBeDefined(); + } + for (const prompt of [ + "不要开10个子agent", + "为什么开10个子agent", + "开1个子agent", + "启动一个子代理", + "开零个子agent", + "开半个子agent", + "开负1个子agent", + "开２０个子agent要多少钱？", + ]) { + expect(detectUltraloopOptIn(prompt), prompt).toBeUndefined(); + } +}); + +test("asking about a counted fan-out is not consent to run one", () => { + for (const prompt of [ + "开10个子agent要多少钱？", + "开10个子agent可行吗？", + "开10个子agent是不是太多了？", + "开10个子agent，还是先用subagent工具吧", + "Launch 10 agents? That seems excessive.", + "launch 10 agents?", + "怎么开10个子agent", + "how do i launch 10 subagents", + "开10个子agent怎么随机sleep", + ]) { + expect(detectUltraloopOptIn(prompt), prompt).toBeUndefined(); + } +}); + +test("refusals, hedging, and deliberation are never consent", () => { + for (const prompt of [ + "Please don't launch 10 subagents", + "do not spawn 5 agents for this", + "never run 3 agents here", + "no need to spawn 5 agents", + "Rather than launch 10 agents, just read the file", + "I might launch 10 subagents later, for now read X", + "不要开10个子agent", + "先别开10个子agent", + "开10个子agent，还是先用subagent工具吧", + ]) { + expect(detectUltraloopOptIn(prompt), prompt).toBeUndefined(); + } + // "特别" contains 别; a real request that uses it is not a refusal. + expect(detectUltraloopOptIn("开10个子agent，特别是review")).toBeDefined(); +}); + +test("a counted agent noun that is not a fan-out target is not consent", () => { + for (const prompt of [ + "create 3 agent definition files", + "创建3个agent配置文件", + "start 4 agentic tests", + "run 3 agents.md files through the linter", + "start 3 agent-based tests", + ]) { + expect(detectUltraloopOptIn(prompt), prompt).toBeUndefined(); + } + // The same shape with a real fan-out target still opts in. + for (const prompt of ["开10个子agent查一下", "launch 10 subagents for the audit", "开10个agent 随机sleep"]) { + expect(detectUltraloopOptIn(prompt), prompt).toBeDefined(); + } +}); + type Command = Parameters[1]; interface Recorded { diff --git a/packages/coding-agent/test/workflow-vm.test.ts b/packages/coding-agent/test/workflow-vm.test.ts index 22b1f77a..6c6b5e72 100644 --- a/packages/coding-agent/test/workflow-vm.test.ts +++ b/packages/coding-agent/test/workflow-vm.test.ts @@ -111,6 +111,34 @@ test("parallel() starts every task before its barrier", async () => { expect(probe.peak()).toBe(3); }); +test("parallel accepts eager promises and mixed lazy tasks without losing results", async () => { + const probe = concurrencyProbe(); + const result = await runInQuickJs( + 'return await parallel([agent("a"), () => agent("b"), agent("c")]);', + null, + probe.host, + ); + expect(result.value).toEqual(["a", "b", "c"]); + expect(probe.peak()).toBe(3); +}); + +test("parallel validates the entire batch before invoking lazy tasks", async () => { + const { host, calls } = createHost(); + await expect(runInQuickJs('return parallel([() => agent("must-not-start"), 42]);', null, host)).rejects.toThrow( + "functions or promises", + ); + expect(calls.agents).toEqual([]); +}); + +test("cancellation interrupts a guest waiting forever without host calls", async () => { + const { host } = createHost(); + const controller = new AbortController(); + const run = runInQuickJs("await new Promise(() => {});", null, host, { signal: controller.signal }); + const rejected = expect(run).rejects.toThrow("aborted"); + controller.abort(); + await rejected; +}); + test("pipeline() runs item chains concurrently and passes (prev, item, index) to stages", async () => { const probe = concurrencyProbe(); const result = await runInQuickJs( @@ -144,6 +172,10 @@ test("a throwing stage or task resolves to null instead of rejecting the wave", host, ); expect(result.value).toEqual({ piped: [11, null, 31], waved: ["ok", null, "also-ok"] }); + expect(result.taskFailures).toEqual([ + { primitive: "pipeline", index: 1, stage: 0, message: "Error: boom" }, + { primitive: "parallel", index: 1, message: "Error: boom" }, + ]); }); test("parallel() and pipeline() reject batches above the 4096-entry cap", async () => { @@ -312,26 +344,27 @@ test("keeps a fan-out branch whose host call outlasts the timeout budget", async test.each([ ["a single call", `agent("a"); return "early";`], ["a parallel wave", `parallel([() => agent("a"), () => agent("b")]); return "early";`], -])("discards %s the script never awaited", async (_label, script) => { +])("rejects and releases %s the script never awaited", async (_label, script) => { const { host } = createHost({ agent: async (prompt) => { await new Promise((resolve) => setTimeout(resolve, 50)); return prompt; }, }); - // The run returns while those calls are still in flight. Their deferred + // The run fails while those calls are still in flight. Their deferred // promises have to be released before the context goes away, or QuickJS aborts // the shared WebAssembly instance; the settlements that land afterwards must // leave the disposed context alone rather than raise an unhandled rejection. - const result = await runInQuickJs(script, null, host, { timeoutMs: 5_000 }); - expect(result.value).toBe("early"); + await expect(runInQuickJs(script, null, host, { timeoutMs: 5_000 })).rejects.toThrow("unawaited"); // Give the abandoned calls time to settle: a use-after-free would surface here. await new Promise((resolve) => setTimeout(resolve, 150)); }); test("keeps working after a run that abandoned host calls", async () => { const { host } = createHost(); - await runInQuickJs(`agent("a"); return "early";`, null, host, { timeoutMs: 5_000 }); + await expect(runInQuickJs(`agent("a"); return "early";`, null, host, { timeoutMs: 5_000 })).rejects.toThrow( + "unawaited", + ); await new Promise((resolve) => setTimeout(resolve, 50)); // The WebAssembly module is cached process-wide, so a botched teardown would // poison every later run rather than just the one that caused it. diff --git a/packages/providers/src/api/anthropic-messages.ts b/packages/providers/src/api/anthropic-messages.ts index 20f61fde..171ac4fa 100644 --- a/packages/providers/src/api/anthropic-messages.ts +++ b/packages/providers/src/api/anthropic-messages.ts @@ -33,7 +33,7 @@ import { AssistantMessageEventStream } from "../utils/event-stream.ts"; import { headersToRecord } from "../utils/headers.ts"; import { parseJsonWithRepair, parseStreamingJson } from "../utils/json-parse.ts"; import { getPiUserAgent } from "../utils/pi-user-agent.ts"; -import { retryProviderRequest } from "../utils/provider-retry.ts"; +import { providerRetryAfterMs, retryProviderRequest } from "../utils/provider-retry.ts"; import { sanitizeSurrogates } from "../utils/sanitize-unicode.ts"; import { getJsonSchemaToolParameters, resolveJsonSchemaStrictSampling } from "./constrained-sampling.ts"; @@ -733,6 +733,7 @@ export const stream: StreamFunction<"anthropic-messages", AnthropicOptions> = ( } output.stopReason = options?.signal?.aborted ? "aborted" : "error"; output.errorMessage = error instanceof Error ? error.message : JSON.stringify(error); + output.retryAfterMs = providerRetryAfterMs(error); stream.push({ type: "error", reason: output.stopReason, error: output }); stream.end(); } diff --git a/packages/providers/src/api/openai-completions.ts b/packages/providers/src/api/openai-completions.ts index d9b23849..a7ec5b5a 100644 --- a/packages/providers/src/api/openai-completions.ts +++ b/packages/providers/src/api/openai-completions.ts @@ -41,7 +41,7 @@ import { shortHash } from "../utils/hash.ts"; import { headersToRecord } from "../utils/headers.ts"; import { parseStreamingJson } from "../utils/json-parse.ts"; import { getPiUserAgent } from "../utils/pi-user-agent.ts"; -import { retryProviderRequest } from "../utils/provider-retry.ts"; +import { providerRetryAfterMs, retryProviderRequest } from "../utils/provider-retry.ts"; import { sanitizeSurrogates } from "../utils/sanitize-unicode.ts"; import { appendGrammarToolInputJsonDelta, @@ -696,6 +696,7 @@ export const stream: StreamFunction<"openai-completions", OpenAICompletionsOptio } output.stopReason = options?.signal?.aborted ? "aborted" : "error"; output.errorMessage = formatProviderError(normalizeProviderError(error)); + output.retryAfterMs = providerRetryAfterMs(error); // Some providers via OpenRouter give additional information in this field. // normalizeProviderError already stringifies the parsed body (error.error) // into errorMessage, so only append the raw metadata when it is not already diff --git a/packages/providers/src/api/openai-responses.ts b/packages/providers/src/api/openai-responses.ts index b6d05a72..18ca021b 100644 --- a/packages/providers/src/api/openai-responses.ts +++ b/packages/providers/src/api/openai-responses.ts @@ -19,7 +19,7 @@ import { formatProviderError, normalizeProviderError } from "../utils/error-body import { AssistantMessageEventStream } from "../utils/event-stream.ts"; import { headersToRecord } from "../utils/headers.ts"; import { getPiUserAgent } from "../utils/pi-user-agent.ts"; -import { retryProviderRequest } from "../utils/provider-retry.ts"; +import { providerRetryAfterMs, retryProviderRequest } from "../utils/provider-retry.ts"; import { createGrammarToolInputProperties } from "./constrained-sampling.ts"; import { buildCopilotDynamicHeaders, hasCopilotVisionInput } from "./github-copilot-headers.ts"; import { clampOpenAIPromptCacheKey } from "./openai-prompt-cache.ts"; @@ -175,6 +175,7 @@ export const stream: StreamFunction<"openai-responses", OpenAIResponsesOptions> } output.stopReason = options?.signal?.aborted ? "aborted" : "error"; output.errorMessage = formatOpenAIResponsesError(error); + output.retryAfterMs = providerRetryAfterMs(error); stream.push({ type: "error", reason: output.stopReason, error: output }); stream.end(); } diff --git a/packages/providers/src/types.ts b/packages/providers/src/types.ts index e0bc30ec..37b75028 100644 --- a/packages/providers/src/types.ts +++ b/packages/providers/src/types.ts @@ -374,6 +374,8 @@ export interface AssistantMessage { stopReason: StopReason; deferred?: DeferredHandle; errorMessage?: string; + /** Server-requested backoff retained for session-level retries. */ + retryAfterMs?: number; rawStopReason?: string; /** * Provider indication of whether the model explicitly ended its turn. diff --git a/packages/providers/src/utils/event-stream.ts b/packages/providers/src/utils/event-stream.ts index e5eef921..7d33537c 100644 --- a/packages/providers/src/utils/event-stream.ts +++ b/packages/providers/src/utils/event-stream.ts @@ -47,7 +47,7 @@ export class EventStream implements AsyncIterable { } } - async *[Symbol.asyncIterator](): AsyncIterator { + async *[Symbol.asyncIterator](): AsyncGenerator { while (true) { if (this.queue.length > 0) { yield this.queue.shift()!; diff --git a/packages/providers/src/utils/provider-retry.ts b/packages/providers/src/utils/provider-retry.ts index e7b0db57..6270e482 100644 --- a/packages/providers/src/utils/provider-retry.ts +++ b/packages/providers/src/utils/provider-retry.ts @@ -11,6 +11,40 @@ interface ProviderError extends Error { headers: Headers | undefined; } +/** Thrown instead of sleeping when the server asks for a delay above the cap. */ +class ProviderRetryDelayError extends Error { + /** The server's requested delay, kept only when it came from a 429. */ + readonly retryAfterMs: number | undefined; + + constructor(message: string, retryAfterMs: number | undefined) { + super(message); + this.name = "ProviderRetryDelayError"; + this.retryAfterMs = retryAfterMs; + } +} + +/** Preserve Retry-After when provider retries are disabled or exhausted. */ +export function providerRetryAfterMs(error: unknown, now = Date.now()): number | undefined { + if (error instanceof ProviderRetryDelayError) { + return error.retryAfterMs !== undefined && Number.isFinite(error.retryAfterMs) ? error.retryAfterMs : undefined; + } + if (!isProviderError(error) || error.status !== 429) return undefined; + const retryAfterMs = error.headers?.get("retry-after-ms"); + const header = error.headers?.get("retry-after"); + // An empty or non-numeric retry-after-ms is not a delay; fall back to retry-after + // rather than reading it as zero. + const milliseconds = + retryAfterMs !== null && retryAfterMs !== undefined && retryAfterMs.trim() !== "" ? Number(retryAfterMs) : NaN; + const value = Number.isFinite(milliseconds) + ? milliseconds + : header + ? /^\d+(\.\d+)?$/.test(header) + ? Number(header) * 1000 + : Date.parse(header) - now + : NaN; + return Number.isFinite(value) ? Math.max(0, value) : undefined; +} + function isProviderError(error: unknown): error is ProviderError { if (!(error instanceof Error) || !("status" in error) || !("headers" in error)) return false; return ( @@ -37,12 +71,13 @@ function isRetryableProviderError(error: ProviderError): boolean { function validateServerRetryDelayMs( delayMs: number, maxRetryDelayMs: number | undefined, - providerErrorMessage: string, + error: ProviderError, ): number { const maxDelayMs = maxRetryDelayMs ?? DEFAULT_MAX_RETRY_DELAY_MS; if (maxDelayMs > 0 && delayMs > maxDelayMs) { - throw new Error( - `Server requested ${Math.ceil(delayMs / 1000)}s retry delay (max: ${Math.ceil(maxDelayMs / 1000)}s). ${providerErrorMessage}`, + throw new ProviderRetryDelayError( + `Server requested ${Math.ceil(delayMs / 1000)}s retry delay (max: ${Math.ceil(maxDelayMs / 1000)}s). ${error.message}`, + error.status === 429 ? delayMs : undefined, ); } return delayMs; @@ -52,14 +87,14 @@ function getRetryDelayMs(error: ProviderError, retryIndex: number, maxRetryDelay const retryAfterMs = error.headers?.get("retry-after-ms"); if (retryAfterMs) { const value = Number.parseFloat(retryAfterMs); - if (!Number.isNaN(value)) return validateServerRetryDelayMs(value, maxRetryDelayMs, error.message); + if (!Number.isNaN(value)) return validateServerRetryDelayMs(value, maxRetryDelayMs, error); } const retryAfter = error.headers?.get("retry-after"); if (retryAfter) { const seconds = Number.parseFloat(retryAfter); const delayMs = Number.isNaN(seconds) ? Date.parse(retryAfter) - Date.now() : seconds * 1000; - return validateServerRetryDelayMs(delayMs, maxRetryDelayMs, error.message); + return validateServerRetryDelayMs(delayMs, maxRetryDelayMs, error); } const exponentialDelay = Math.min(0.5 * 2 ** retryIndex, 8) * 1000; diff --git a/packages/providers/src/utils/retry.ts b/packages/providers/src/utils/retry.ts index b17bf091..396f4b77 100644 --- a/packages/providers/src/utils/retry.ts +++ b/packages/providers/src/utils/retry.ts @@ -73,6 +73,10 @@ const RETRYABLE_PROVIDER_ERROR_PATTERN = buildProviderErrorPattern([ "ended without", "stream ended before message_stop", "stream ended before a terminal response event", + // The agent loop finalizes a partial assistant message with this text when a + // provider stream closes before its terminal event (agent-core + // normalizeProviderStream). A truncated stream is transient and retryable. + "ended before completion", "http2 request did not get a response", // Provider-requested retry delay cap failures should flow through the outer diff --git a/packages/providers/test/openai-completions-retry.test.ts b/packages/providers/test/openai-completions-retry.test.ts index 0c48b502..916cc2de 100644 --- a/packages/providers/test/openai-completions-retry.test.ts +++ b/packages/providers/test/openai-completions-retry.test.ts @@ -89,6 +89,18 @@ describe("openai-completions provider retries", () => { expect(mockState.requestOptions).toEqual([expect.objectContaining({ maxRetries: 0 })]); }); + it("preserves 429 Retry-After for the session retry owner", async () => { + mockState.requestErrors = [ + Object.assign(new Error("429 concurrency reached"), { + status: 429, + headers: new Headers({ "retry-after": "2" }), + }), + ]; + const result = await consume(); + expect(result).toMatchObject({ stopReason: "error", retryAfterMs: 2000 }); + expect(mockState.requestOptions).toHaveLength(1); + }); + it("honors provider retries while keeping SDK retries disabled", async () => { vi.useFakeTimers(); mockState.requestErrors = [ diff --git a/packages/providers/test/provider-retry.test.ts b/packages/providers/test/provider-retry.test.ts index 0c5313ec..ac915ab1 100644 --- a/packages/providers/test/provider-retry.test.ts +++ b/packages/providers/test/provider-retry.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { retryProviderRequest } from "../src/utils/provider-retry.ts"; +import { providerRetryAfterMs, retryProviderRequest } from "../src/utils/provider-retry.ts"; function providerError(status: number | undefined, headers?: Record): Error { return Object.assign(new Error(`Provider error: ${status}`), { @@ -9,6 +9,25 @@ function providerError(status: number | undefined, headers?: Record { + it("parses retry hints without accepting invalid numbers or leaking headers", () => { + expect(providerRetryAfterMs(providerError(429, { "retry-after-ms": "250", "retry-after": "10" }))).toBe(250); + expect(providerRetryAfterMs(providerError(429, { "retry-after": "Thu, 01 Jan 1970 00:00:10 GMT" }), 1000)).toBe( + 9000, + ); + expect(providerRetryAfterMs(providerError(429, { "retry-after": "invalid" }))).toBeUndefined(); + expect(providerRetryAfterMs(providerError(429, { "retry-after-ms": "Infinity" }))).toBeUndefined(); + // Zero and past values are an immediate retry, never a negative or undefined wait. + expect(providerRetryAfterMs(providerError(429, { "retry-after": "0" }))).toBe(0); + expect(providerRetryAfterMs(providerError(429, { "retry-after": "-10" }))).toBe(0); + expect(providerRetryAfterMs(providerError(429, { "retry-after-ms": "-50" }))).toBe(0); + expect(providerRetryAfterMs(providerError(429, { "retry-after": "Thu, 01 Jan 1970 00:00:00 GMT" }), 5000)).toBe( + 0, + ); + // An empty or non-numeric retry-after-ms must fall back to retry-after, not read as zero. + expect(providerRetryAfterMs(providerError(429, { "retry-after-ms": "", "retry-after": "7" }))).toBe(7000); + expect(providerRetryAfterMs(providerError(429, { "retry-after-ms": " ", "retry-after": "7" }))).toBe(7000); + expect(providerRetryAfterMs(providerError(429, { "retry-after-ms": "soon", "retry-after": "7" }))).toBe(7000); + }); afterEach(() => { vi.useRealTimers(); }); @@ -46,6 +65,19 @@ describe("provider request retries", () => { expect(request).toHaveBeenCalledTimes(1); }); + it("keeps the server's 429 delay when the cap rejects it, but not for other statuses", async () => { + const capped = async (status: number): Promise => { + const request = vi + .fn<() => Promise>() + .mockRejectedValue(providerError(status, { "retry-after": "120" })); + return retryProviderRequest(request, { maxRetries: 1, maxRetryDelayMs: 1000 }).catch( + (error: unknown) => error, + ); + }; + expect(providerRetryAfterMs(await capped(429))).toBe(120_000); + expect(providerRetryAfterMs(await capped(503))).toBeUndefined(); + }); + it("allows disabling the provider-requested retry delay cap", async () => { vi.useFakeTimers(); const request = vi diff --git a/packages/providers/test/retry.test.ts b/packages/providers/test/retry.test.ts index 0d8875e0..da21f26f 100644 --- a/packages/providers/test/retry.test.ts +++ b/packages/providers/test/retry.test.ts @@ -68,6 +68,20 @@ describe("provider retry classification", () => { ).toBe(true); }); + it("retries the agent loop's truncated-stream finalization", () => { + // agent-core finalizes a partial assistant message with this text when a + // provider stream closes before its terminal event. A truncated stream is + // transient, so the session retry owner must treat it as retryable. + expect( + isRetryableAssistantError( + fauxAssistantMessage("", { + stopReason: "error", + errorMessage: "The model response ended before completion", + }), + ), + ).toBe(true); + }); + it("keeps provider limit errors non-retryable", () => { expect( isRetryableAssistantError(