From a64e0a5f3973b922879db7d1dbb050f0524fe330 Mon Sep 17 00:00:00 2001 From: Norin Lavaee Date: Wed, 1 Jul 2026 12:03:39 -0700 Subject: [PATCH 1/6] fix: retry request-incompatible model fallbacks Treat request/context incompatibility failures as fallbackable in workflow and subagent model fallback chains so Atomic advances through configured candidates and reaches the current selected model when needed. Adds focused regression coverage for HTTP 400/413/422, request-too-large/context-window/unsupported-tool signals, and non-retryable refusal/cancellation precedence. AI-Assisted-By: GPT-5.5 --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/docs/subagents.md | 2 + packages/coding-agent/docs/workflows.md | 2 + packages/subagents/CHANGELOG.md | 4 + .../src/runs/shared/model-fallback.ts | 87 +++++------- packages/workflows/CHANGELOG.md | 4 + .../runs/shared/model-fallback-failures.ts | 38 ++++- ...odel-fallback-request-incompatible.test.ts | 130 ++++++++++++++++++ .../unit/stage-runner-fallback-resume.test.ts | 97 +++++++++++++ test/unit/subagents-model-fallback.test.ts | 47 +++++++ 10 files changed, 358 insertions(+), 54 deletions(-) create mode 100644 test/unit/model-fallback-request-incompatible.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e0bd91b86..c0c5f7f58 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,7 @@ - Fixed the `read` tool to parse colon-delimited `file:START:END` (and grep-style `file:LINE:COL`) path selectors as a line range instead of leaving the leading number glued to the path (`file:395`), which produced a bogus `ENOENT` and pushed models (e.g. Opus) to fall back to `sed` ([#1585](https://github.com/bastani-inc/atomic/issues/1585)). - Fixed GitHub Copilot models to use the live `max_output_tokens` value from the Copilot model catalog, preventing `github-copilot/claude-opus-4.8` from being capped by Atomic's stale built-in output-token limit after compaction ([#1582](https://github.com/bastani-inc/atomic/issues/1582)). - Fixed active GitHub Copilot sessions to adopt live catalog model metadata as soon as the catalog loads, so fallback models refresh their supported reasoning levels without requiring a restart. +- Fixed workflow and subagent model fallback chains so request/context incompatibility failures (HTTP 400/413/422 bad/unprocessable/payload-too-large request, unsupported tool/parameter, context-length/context-window overflow, `invalid_request`/`bad_request`/`too_large` errors) advance to the next candidate instead of stopping. This ensures that when none of the configured fallback candidates can serve the current request, Atomic falls back to the currently selected user model rather than failing outright. Refusals, content-filter/safety blocks, cancellations, and task failures still stop the chain and are never retried on another model ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). ## [0.9.4-alpha.6] - 2026-07-01 diff --git a/packages/coding-agent/docs/subagents.md b/packages/coding-agent/docs/subagents.md index 33e968c9a..22f23fc76 100644 --- a/packages/coding-agent/docs/subagents.md +++ b/packages/coding-agent/docs/subagents.md @@ -175,6 +175,8 @@ Dynamic fanout `collect.outputSchema` validates the collected result array after Agents can define ordered `fallbackModels` for retryable provider or model failures such as rate limits, quota/auth problems, unavailable models, network timeouts, or 5xx errors. Atomic tries the requested primary model first, then configured fallbacks, and finally appends the current user-selected model as the last fallback candidate when available. +A candidate that cannot serve the current request — for example an HTTP 400/413/422 bad/unprocessable/payload-too-large request, an unsupported tool or parameter, a context-length/context-window overflow, or a `too large` / `invalid_request` error — is treated as request/context incompatible and the chain advances to the next candidate rather than stopping. This means that if none of the configured candidates are applicable to the request, Atomic falls back to the currently selected user model instead of failing outright. + Fallbacks do not retry ordinary task failures, validation failures, tool failures, cancellations, or workflow-code errors. Because a fallback may send the same prompt and context to a different provider, choose models that match your cost, privacy, and data-handling requirements. Each candidate can also carry its own reasoning effort — see [Reasoning levels](#reasoning-levels). diff --git a/packages/coding-agent/docs/workflows.md b/packages/coding-agent/docs/workflows.md index f091d5c97..b3b4a16f1 100644 --- a/packages/coding-agent/docs/workflows.md +++ b/packages/coding-agent/docs/workflows.md @@ -1816,6 +1816,8 @@ For lower-level integrations, `@bastani/workflows` also exports `setupGitWorktre `fallbackModels` retries transient provider/model failures with the primary `model` first, then each fallback, then the current Atomic-selected model when available. It is for rate limits, quota/auth/provider outages, unavailable models, network timeouts, generic transport errors such as `Connection error.` / `fetch failed`, and 5xx errors — not workflow-code errors, tool failures, validation failures, or cancellations. +A candidate that is **request/context incompatible** with the current turn — for example an HTTP 400/413/422 bad/unprocessable/payload-too-large request, an unsupported tool or parameter, a context-length/context-window overflow, or a `too large` / `invalid_request` / `bad_request` error — also advances the chain to the next candidate rather than stopping. This ensures that if none of the configured candidates can serve the request, the workflow stage falls back to the currently selected user model instead of hard-failing. Refusals, content-filter/safety blocks, cancellations, and task failures still stop the chain and are never retried on another model. + When a finished stage's session is reattached for a follow-up (for example a post-completion follow-up, or after the CLI is reloaded), the stage resumes on the model the session last settled on — the one that actually worked — instead of replaying the chain from the primary. If that model fails again with a transient/retryable error, the full chain is retried from the primary. ### Reasoning levels diff --git a/packages/subagents/CHANGELOG.md b/packages/subagents/CHANGELOG.md index 901824396..553936932 100644 --- a/packages/subagents/CHANGELOG.md +++ b/packages/subagents/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed subagent model fallback so request/context incompatibility failures (HTTP 400/413/422 bad/unprocessable/payload-too-large request, unsupported tool/parameter, context-length/context-window overflow, `invalid_request`/`bad_request`/`too_large` errors) advance the chain to the next candidate instead of stopping. When none of the configured candidates can serve the request, the subagent now falls back to the current user-selected model. Refusals, content-filter/safety blocks, cancellations, and task failures still stop the chain ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). + ## [0.9.4-alpha.6] - 2026-07-01 ### Changed diff --git a/packages/subagents/src/runs/shared/model-fallback.ts b/packages/subagents/src/runs/shared/model-fallback.ts index 3b20ecb27..5502aa8f6 100644 --- a/packages/subagents/src/runs/shared/model-fallback.ts +++ b/packages/subagents/src/runs/shared/model-fallback.ts @@ -117,6 +117,7 @@ export type ModelFallbackFailureKind = | "provider_unavailable" | "network_timeout" | "model_unavailable" + | "request_incompatible" | "cancelled" | "task_failure" | "unknown"; @@ -144,6 +145,7 @@ const FALLBACKABLE_FAILURE_KINDS: ReadonlySet = new Se "provider_unavailable", "network_timeout", "model_unavailable", + "request_incompatible", ]); function asRecord(value: unknown): Record | undefined { @@ -222,6 +224,9 @@ function normalizeCode(value: string | number | undefined): string | undefined { function kindFromStatus(status: number | undefined): ModelFallbackFailureKind | undefined { switch (status) { + case 400: + case 413: case 422: + return "request_incompatible"; case 401: case 403: return "auth_on_candidate_provider"; @@ -237,6 +242,15 @@ function kindFromStatus(status: number | undefined): ModelFallbackFailureKind | } } +const REQUEST_INCOMPATIBLE_CODES: ReadonlySet = new Set([ + "invalid_request", "invalid_request_error", "bad_request", "context_length_exceeded", + "request_too_large", "too_large", "request_entity_too_large", "max_tokens", + "max_context_length", "context_window_exceeded", +]); +function requestIncompatibleKindFromCode(code: string | number | undefined): ModelFallbackFailureKind | undefined { + const normalizedCode = normalizeCode(code); + return normalizedCode !== undefined && REQUEST_INCOMPATIBLE_CODES.has(normalizedCode) ? "request_incompatible" : undefined; +} function refusalKindFromCode(code: string | number | undefined): ModelFallbackFailureKind | undefined { const normalizedCode = normalizeCode(code); if (normalizedCode === undefined) return undefined; @@ -258,6 +272,14 @@ function refusalKindFromCode(code: string | number | undefined): ModelFallbackFa } } +const CODE_KINDS_BY_KIND: ReadonlyArray]> = [ + ["auth_on_candidate_provider", new Set(["auth", "auth_required", "authentication_required", "unauthorized", "forbidden", "invalid_api_key", "missing_api_key", "invalid_key"])], + ["network_timeout", new Set(["etimedout", "econnreset", "econnrefused", "enotfound", "eai_again", "fetch_failed", "network_error", "timeout", "timeout_error", "und_err_connect_timeout"])], + ["rate_limit", new Set(["rate_limit", "rate_limit_exceeded", "too_many_requests", "quota_exceeded"])], + ["cancelled", new Set(["aborterror", "aborted", "cancelled", "canceled"])], + ["model_unavailable", new Set(["model_not_found", "model_unavailable", "model_disabled", "unknown_model"])], + ["provider_unavailable", new Set(["provider_error", "api_error", "service_unavailable", "temporarily_unavailable", "overloaded"])], +]; function kindFromCode(code: string | number | undefined): ModelFallbackFailureKind | undefined { const normalizedCode = normalizeCode(code); if (normalizedCode === undefined) return undefined; @@ -265,54 +287,19 @@ function kindFromCode(code: string | number | undefined): ModelFallbackFailureKi if (refusalKind !== undefined) return refusalKind; const httpStatusKind = kindFromStatus(integerFrom(code)); if (httpStatusKind !== undefined) return httpStatusKind; - - switch (normalizedCode) { - case "auth": - case "auth_required": - case "authentication_required": - case "unauthorized": - case "forbidden": - case "invalid_api_key": - case "missing_api_key": - case "invalid_key": - return "auth_on_candidate_provider"; - case "etimedout": - case "econnreset": - case "econnrefused": - case "enotfound": - case "eai_again": - case "fetch_failed": - case "network_error": - case "timeout": - case "timeout_error": - case "und_err_connect_timeout": - return "network_timeout"; - case "rate_limit": - case "rate_limit_exceeded": - case "too_many_requests": - case "quota_exceeded": - return "rate_limit"; - case "aborterror": - case "aborted": - case "cancelled": - case "canceled": - return "cancelled"; - case "model_not_found": - case "model_unavailable": - case "model_disabled": - case "unknown_model": - return "model_unavailable"; - case "provider_error": - case "api_error": - case "service_unavailable": - case "temporarily_unavailable": - case "overloaded": - return "provider_unavailable"; - default: - return undefined; - } -} - + const requestIncompatibleKind = requestIncompatibleKindFromCode(code); + if (requestIncompatibleKind !== undefined) return requestIncompatibleKind; + return CODE_KINDS_BY_KIND.find(([_, codes]) => codes.has(normalizedCode))?.[0]; +} + +const REQUEST_INCOMPATIBLE_FAILURE_PATTERNS: readonly RegExp[] = [ + /\bcontext[_\s-]?(?:length|window)(?:[_\s-]?exceeded)?\b/i, + /\bmax[_\s-]?(?:context|tokens?)\b/i, + /\b(?:request(?:[_\s-]?entity)?[_\s-]?too|too)[_\s-]?large\b/i, + /\b(?:unsupported|unknown|invalid)\s+(?:tool|parameter|function)\b/i, + /\b(?:tool|parameter|function)\s+(?:not\s+(?:supported|found|allowed)|unknown|invalid)\b/i, + /\b(?:invalid[_\s-]?request(?:[_\s-]?error)?|bad[_\s-]?request)\b/i, +]; const PROVIDER_REFUSAL_FAILURE_PATTERNS: readonly RegExp[] = [ /\bfinish[_\s-]?reason\b[^\n]*\bcontent[_\s-]?filter\b/i, /\bcontent[_\s-]?filter(?:ed|ing)?\b/i, @@ -335,6 +322,7 @@ function refusalKindFromMessage(message: string): ModelFallbackFailureKind | und function fallbackKindFromMessage(message: string, name: string | undefined): ModelFallbackFailureKind | undefined { const refusalKind = refusalKindFromMessage(message); if (refusalKind !== undefined) return refusalKind; + if (REQUEST_INCOMPATIBLE_FAILURE_PATTERNS.some((pattern) => pattern.test(message))) return "request_incompatible"; const nameKind = kindFromCode(name); if (nameKind !== undefined) return nameKind; if (!RETRYABLE_MODEL_FAILURE_PATTERNS.some((pattern) => pattern.test(message))) return undefined; @@ -463,8 +451,7 @@ function messageFromUnknown(value: unknown, seen: Set): string | undefi const status = statusFrom(value); if (status !== undefined) return `Model request failed with status ${status}`; const code = codeFrom(value); - if (code !== undefined) return `Model request failed with code ${String(code)}`; - return undefined; + return code !== undefined ? `Model request failed with code ${String(code)}` : undefined; } export function modelFailureMessage(error: unknown): string { const structuredMessage = messageFromUnknown(error, new Set()); diff --git a/packages/workflows/CHANGELOG.md b/packages/workflows/CHANGELOG.md index e87ec314a..d70e1a372 100644 --- a/packages/workflows/CHANGELOG.md +++ b/packages/workflows/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +### Fixed + +- Fixed workflow stage model fallback so request/context incompatibility failures (HTTP 400/413/422 bad/unprocessable/payload-too-large request, unsupported tool/parameter, context-length/context-window overflow, `invalid_request`/`bad_request`/`too_large` errors) advance the chain to the next candidate instead of stopping. When none of the configured candidates can serve the request, the stage now falls back to the current user-selected model. Refusals, content-filter/safety blocks, cancellations, and task failures still stop the chain ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). + ## [0.9.4-alpha.6] - 2026-07-01 ### Changed diff --git a/packages/workflows/src/runs/shared/model-fallback-failures.ts b/packages/workflows/src/runs/shared/model-fallback-failures.ts index e15b0e6f1..2ad1ff22a 100644 --- a/packages/workflows/src/runs/shared/model-fallback-failures.ts +++ b/packages/workflows/src/runs/shared/model-fallback-failures.ts @@ -51,6 +51,7 @@ export type ModelFallbackFailureKind = | "network_timeout" | "transport_error" | "model_unavailable" + | "request_incompatible" | "cancelled" | "task_failure" | "unknown"; @@ -79,6 +80,7 @@ const FALLBACKABLE_FAILURE_KINDS: ReadonlySet = new Se "network_timeout", "transport_error", "model_unavailable", + "request_incompatible", ]); function asRecord(value: unknown): Record | undefined { @@ -157,6 +159,9 @@ function normalizeCode(value: string | number | undefined): string | undefined { function kindFromStatus(status: number | undefined): ModelFallbackFailureKind | undefined { switch (status) { + case 400: + case 413: case 422: + return "request_incompatible"; case 401: case 403: return "auth_on_candidate_provider"; @@ -193,6 +198,15 @@ function refusalKindFromCode(code: string | number | undefined): ModelFallbackFa } } +const REQUEST_INCOMPATIBLE_CODES: ReadonlySet = new Set([ + "invalid_request", "invalid_request_error", "bad_request", "context_length_exceeded", + "request_too_large", "too_large", "request_entity_too_large", "max_tokens", + "max_context_length", "context_window_exceeded", +]); +function requestIncompatibleKindFromCode(code: string | number | undefined): ModelFallbackFailureKind | undefined { + const normalizedCode = normalizeCode(code); + return normalizedCode !== undefined && REQUEST_INCOMPATIBLE_CODES.has(normalizedCode) ? "request_incompatible" : undefined; +} function kindFromCode(code: string | number | undefined): ModelFallbackFailureKind | undefined { const normalizedCode = normalizeCode(code); if (normalizedCode === undefined) return undefined; @@ -200,6 +214,8 @@ function kindFromCode(code: string | number | undefined): ModelFallbackFailureKi if (refusalKind !== undefined) return refusalKind; const httpStatusKind = kindFromStatus(integerFrom(code)); if (httpStatusKind !== undefined) return httpStatusKind; + const requestIncompatibleKind = requestIncompatibleKindFromCode(code); + if (requestIncompatibleKind !== undefined) return requestIncompatibleKind; switch (normalizedCode) { case "auth": @@ -248,6 +264,18 @@ function kindFromCode(code: string | number | undefined): ModelFallbackFailureKi } } +const REQUEST_INCOMPATIBLE_FAILURE_PATTERNS: readonly RegExp[] = [ + /\bcontext[_\s-]?length(?:[_\s-]?exceeded)?\b/i, + /\bcontext[_\s-]?window(?:[_\s-]?exceeded)?\b/i, + /\bmax[_\s-]?context\b/i, + /\bmax[_\s-]?tokens?\b/i, + /\brequest(?:[_\s-]?entity)?[_\s-]?too[_\s-]?large\b/i, + /\btoo[_\s-]?large\b/i, + /\b(?:unsupported|unknown|invalid)\s+(?:tool|parameter|function)\b/i, + /\b(?:tool|parameter|function)\s+(?:not\s+(?:supported|found|allowed)|unknown|invalid)\b/i, + /\binvalid[_\s-]?request(?:[_\s-]?error)?\b/i, + /\bbad[_\s-]?request\b/i, +]; const PROVIDER_REFUSAL_FAILURE_PATTERNS: readonly RegExp[] = [ /\bfinish[_\s-]?reason\b[^\n]*\bcontent[_\s-]?filter\b/i, /\bcontent[_\s-]?filter(?:ed|ing)?\b/i, @@ -283,6 +311,7 @@ function fallbackKindFromMessage(message: string, name: string | undefined): Mod if (refusalKind !== undefined) return refusalKind; const transportOutageKind = transportOutageKindFromMessage(message); if (transportOutageKind !== undefined) return transportOutageKind; + if (REQUEST_INCOMPATIBLE_FAILURE_PATTERNS.some((pattern) => pattern.test(message))) return "request_incompatible"; const nameKind = kindFromCode(name); if (nameKind !== undefined) return nameKind; if (!RETRYABLE_MODEL_FAILURE_PATTERNS.some((pattern) => pattern.test(message))) return undefined; @@ -371,9 +400,6 @@ function structuredSignal( const directRefusalSignal = classifyAssistantRefusalSignal(value, source); if (directRefusalSignal !== undefined) return directRefusalSignal; - const directMessageSignal = fallbackSignalFromDirectMessage(value, source); - if (directMessageSignal !== undefined) return directMessageSignal; - const codeKind = kindFromCode(codeFrom(value)); const nameKind = kindFromCode(errorName(value)); if (codeKind === "cancelled" || nameKind === "cancelled") return makeSignal("cancelled", value, source); @@ -387,7 +413,6 @@ function structuredSignal( if (isRefusalSignal(diagnosticSignal)) return diagnosticSignal; firstNestedFallbackSignal ??= diagnosticSignal; } - const cause = causeOf(value); const causeSignal = structuredSignal(cause, nestedSeen, source) ?? fallbackSignalFromMessage(cause, source); @@ -396,6 +421,11 @@ function structuredSignal( firstNestedFallbackSignal ??= causeSignal; } + // Direct-message classification runs after nested traversal so a generic wrapper + // ("invalid request"/"400 bad request") cannot mask a non-retryable nested signal. + const directMessageSignal = fallbackSignalFromDirectMessage(value, source); + if (directMessageSignal !== undefined) return directMessageSignal; + const statusKind = kindFromStatus(statusFrom(value)); if (statusKind !== undefined) return makeSignal(statusKind, value, source); if (codeKind !== undefined) return makeSignal(codeKind, value, source); diff --git a/test/unit/model-fallback-request-incompatible.test.ts b/test/unit/model-fallback-request-incompatible.test.ts new file mode 100644 index 000000000..fea253faf --- /dev/null +++ b/test/unit/model-fallback-request-incompatible.test.ts @@ -0,0 +1,130 @@ +// @ts-nocheck +import { describe, test } from "bun:test"; +import assert from "node:assert/strict"; +import { + isRetryableModelFailure, + normalizeModelFailureSignal, +} from "../../packages/workflows/src/runs/shared/model-fallback.js"; + +describe("request/context incompatibility fallback (#1580)", () => { + test("retry classifier treats request/context incompatibility as fallbackable", () => { + // HTTP 400 / 413 / 422 indicate the candidate cannot serve this request (bad + // request body, payload too large, unsupported tools, context-window + // overflow). These must be fallbackable so the loop advances to the next + // candidate / current model. + assert.equal(normalizeModelFailureSignal({ status: 400, message: "bad request" }).kind, "request_incompatible"); + assert.equal(isRetryableModelFailure({ status: 400, message: "bad request" }), true); + assert.equal(normalizeModelFailureSignal({ statusCode: 422, message: "unprocessable" }).kind, "request_incompatible"); + assert.equal(isRetryableModelFailure({ statusCode: 422, message: "unprocessable" }), true); + assert.equal(normalizeModelFailureSignal({ code: "422", message: "unprocessable" }).kind, "request_incompatible"); + assert.equal(normalizeModelFailureSignal({ httpStatus: 400, message: "bad" }).kind, "request_incompatible"); + }); + + test("HTTP 413 payload/request-too-large is classified as fallbackable (#1580)", () => { + // HTTP 413 Payload Too Large means the request exceeds what this candidate + // can accept. It must be fallbackable so the loop advances. Covers all + // status-bearing fields plus numeric/string codes. + assert.equal(normalizeModelFailureSignal({ status: 413, message: "payload too large" }).kind, "request_incompatible"); + assert.equal(isRetryableModelFailure({ status: 413, message: "payload too large" }), true); + assert.equal(normalizeModelFailureSignal({ statusCode: 413, message: "request entity too large" }).kind, "request_incompatible"); + assert.equal(isRetryableModelFailure({ statusCode: 413, message: "request entity too large" }), true); + assert.equal(normalizeModelFailureSignal({ httpStatus: 413, message: "too large" }).kind, "request_incompatible"); + assert.equal(isRetryableModelFailure({ httpStatus: 413, message: "too large" }), true); + assert.equal(normalizeModelFailureSignal({ code: 413, message: "payload too large" }).kind, "request_incompatible"); + assert.equal(isRetryableModelFailure({ code: 413, message: "payload too large" }), true); + assert.equal(normalizeModelFailureSignal({ code: "413", message: "too large" }).kind, "request_incompatible"); + assert.equal(isRetryableModelFailure({ code: "413", message: "too large" }), true); + }); + + test("retry classifier classifies request-incompatible codes as fallbackable", () => { + const codes: readonly (string | number)[] = [ + "invalid_request_error", + "invalid_request", + "bad_request", + "context_length_exceeded", + "request_too_large", + "too_large", + "max_tokens", + ]; + for (const code of codes) { + const signal = normalizeModelFailureSignal({ code, message: "localized" }); + assert.equal(signal.kind, "request_incompatible", `code ${code}`); + assert.equal(isRetryableModelFailure({ code, message: "localized" }), true, `code ${code}`); + } + }); + + test("retry classifier classifies request-incompatible messages as fallbackable", () => { + const messages = [ + "This model's context length exceeded", + "request too large for this model", + "unsupported tool: computer-use", + "parameter not supported by this model", + "invalid_request_error: bad input", + "bad request body", + ]; + for (const message of messages) { + assert.equal(isRetryableModelFailure(new Error(message)), true, `message "${message}"`); + assert.equal(normalizeModelFailureSignal(new Error(message)).kind, "request_incompatible", `message "${message}"`); + } + }); + + test("request incompatibility does not outrank refusals or cancellations", () => { + // A 400/413/422 wrapper hiding a refusal/cancel must still stop the fallback. + assert.equal(isRetryableModelFailure({ status: 400, stopReason: "aborted", errorMessage: "aborted" }), false); + assert.equal(isRetryableModelFailure({ statusCode: 422, name: "AbortError", message: "aborted by user" }), false); + assert.equal(isRetryableModelFailure({ httpStatus: 400, message: "content_filter" }), false); + assert.equal(isRetryableModelFailure({ status: 422, diagnostics: [{ error: { message: "command failed" } }] }), false); + assert.equal(isRetryableModelFailure({ status: 413, stopReason: "aborted", errorMessage: "aborted" }), false); + }); + + test("non-retryable nested causes win over retryable wrapper messages", () => { + // A generic request-incompatible wrapper that hides a non-retryable nested + // cause/diagnostic must classify as non-retryable, not request_incompatible. + const abortCause = new Error("aborted by user"); + abortCause.name = "AbortError"; + const wrapperWithAbortCause = new Error("invalid request", { cause: abortCause }); + assert.equal(isRetryableModelFailure(wrapperWithAbortCause), false); + assert.equal(normalizeModelFailureSignal(wrapperWithAbortCause).kind, "cancelled"); + + const wrapperWithCancelCause = { message: "400 bad request", cause: { message: "request was cancelled" } }; + assert.equal(isRetryableModelFailure(wrapperWithCancelCause), false); + assert.equal(normalizeModelFailureSignal(wrapperWithCancelCause).kind, "cancelled"); + + const wrapperWithTaskFailureCause = { errorMessage: "bad request", cause: { message: "command failed: exit 1" } }; + assert.equal(isRetryableModelFailure(wrapperWithTaskFailureCause), false); + assert.equal(normalizeModelFailureSignal(wrapperWithTaskFailureCause).kind, "task_failure"); + + const wrapperWithContentFilterDiagnostic = { + message: "invalid_request_error", + diagnostics: [{ error: { finish_reason: "content_filter" } }], + }; + assert.equal(isRetryableModelFailure(wrapperWithContentFilterDiagnostic), false); + assert.equal(normalizeModelFailureSignal(wrapperWithContentFilterDiagnostic).kind, "task_failure"); + + const wrapperWithSafetyDiagnostic = { + errorMessage: "400 bad request", + diagnostics: [{ error: { message: "blocked by safety policy" } }], + }; + assert.equal(isRetryableModelFailure(wrapperWithSafetyDiagnostic), false); + assert.equal(normalizeModelFailureSignal(wrapperWithSafetyDiagnostic).kind, "task_failure"); + }); + + test("genuine request-incompatible wrappers still classify as fallbackable", () => { + // Wrappers with no non-retryable nested signal must remain request_incompatible. + assert.equal( + normalizeModelFailureSignal(new Error("invalid request")).kind, + "request_incompatible", + ); + assert.equal(isRetryableModelFailure(new Error("invalid request")), true); + + assert.equal( + normalizeModelFailureSignal({ message: "400 bad request" }).kind, + "request_incompatible", + ); + assert.equal(isRetryableModelFailure({ message: "400 bad request" }), true); + + // A retryable nested cause should not turn a retryable wrapper non-retryable. + const wrapperWithRetryableCause = { errorMessage: "bad request", cause: { message: "rate limit exceeded" } }; + assert.equal(isRetryableModelFailure(wrapperWithRetryableCause), true); + }); +}); diff --git a/test/unit/stage-runner-fallback-resume.test.ts b/test/unit/stage-runner-fallback-resume.test.ts index b43c25530..ab5e35e5c 100644 --- a/test/unit/stage-runner-fallback-resume.test.ts +++ b/test/unit/stage-runner-fallback-resume.test.ts @@ -276,3 +276,100 @@ describe("live (retained-session) follow-up resumes on the settled model (#1431 ]); }); }); +describe("request/context incompatibility advances the fallback chain to the current selected model (#1580)", () => { + const PRIMARY = { provider: "anthropic", id: "model-a" }; + const FALLBACK = { provider: "anthropic", id: "model-b" }; + const CURRENT = { provider: "openai", id: "gpt-current" }; + + function incompatibleOpts( + create: (options: StageSessionCreateOptions) => StageSessionRuntime, + createdWith: CreateRecord[], + ): StageRunnerOpts { + return { + stageId: "stage-1580", + stageName: "Reviewer", + runId: "run-1580", + stageOptions: { model: "anthropic/model-a", fallbackModels: ["anthropic/model-b"] }, + models: { + currentModel: "openai/gpt-current", + preferredProvider: "anthropic", + async listModels() { + return [ + { provider: "anthropic", id: "model-a", fullId: "anthropic/model-a" }, + { provider: "anthropic", id: "model-b", fullId: "anthropic/model-b" }, + { provider: "openai", id: "gpt-current", fullId: "openai/gpt-current" }, + ]; + }, + }, + adapters: { + agentSession: { + async create(options: StageSessionCreateOptions) { + createdWith.push({ model: options?.model, hasSessionManager: options?.sessionManager !== undefined }); + return create(options); + }, + }, + }, + }; + } + + test("a 400 request-incompatible primary advances to fallback and then the current selected model", async () => { + const createdWith: CreateRecord[] = []; + const opts = incompatibleOpts((options) => { + const model = options?.model as string | undefined; + // Primary fails with HTTP 400 (request incompatible). + if (model === "anthropic/model-a") { + return makeFakeStageSession({ model: PRIMARY, promptError: new Error("400 bad request: unsupported tool") }); + } + // Configured fallback also fails with context-length exceeded (request incompatible). + if (model === "anthropic/model-b") { + return makeFakeStageSession({ model: FALLBACK, promptError: new Error("context length exceeded for this model") }); + } + // Current selected user model succeeds. + return makeFakeStageSession({ model: CURRENT }); + }, createdWith); + + const ctx = createStageContext(opts); + await ctx.prompt("run"); + await ctx.__dispose(); + + // The chain advanced through all candidates including the appended current model. + assert.deepEqual( + createdWith.map((r) => r.model), + ["anthropic/model-a", "anthropic/model-b", "openai/gpt-current"], + "request-incompatible failures must advance through candidates to the current selected model", + ); + + const attempts = ctx.__modelFallbackMeta().modelAttempts ?? []; + assert.deepEqual( + attempts.map((a) => ({ model: a.model, success: a.success })), + [ + { model: "anthropic/model-a", success: false }, + { model: "anthropic/model-b", success: false }, + { model: "openai/gpt-current", success: true }, + ], + "the current selected user model is the final successful fallback after request-incompatible failures", + ); + }); + + test("an applicable primary still wins without exercising the fallback chain", async () => { + const createdWith: CreateRecord[] = []; + const opts = incompatibleOpts(() => { + return makeFakeStageSession({ model: PRIMARY }); + }, createdWith); + + const ctx = createStageContext(opts); + await ctx.prompt("run"); + await ctx.__dispose(); + + assert.deepEqual( + createdWith.map((r) => r.model), + ["anthropic/model-a"], + "an applicable primary does not exercise the fallback chain", + ); + const attempts = ctx.__modelFallbackMeta().modelAttempts ?? []; + assert.deepEqual( + attempts.map((a) => ({ model: a.model, success: a.success })), + [{ model: "anthropic/model-a", success: true }], + ); + }); +}); diff --git a/test/unit/subagents-model-fallback.test.ts b/test/unit/subagents-model-fallback.test.ts index f6bfce6ba..088f76861 100644 --- a/test/unit/subagents-model-fallback.test.ts +++ b/test/unit/subagents-model-fallback.test.ts @@ -216,4 +216,51 @@ describe("subagent model fallback helpers", () => { assert.equal(isRetryableModelFailure({ status: 503, message: "shell command failed" }), false); assert.equal(isRetryableModelFailure("command failed: bun test"), false); }); + + test("retry classifier treats request/context incompatibility as fallbackable (#1580)", () => { + assert.equal(normalizeModelFailureSignal({ status: 400, message: "bad request" }).kind, "request_incompatible"); + assert.equal(isRetryableModelFailure({ status: 400, message: "bad request" }), true); + assert.equal(normalizeModelFailureSignal({ statusCode: 422, message: "unprocessable" }).kind, "request_incompatible"); + assert.equal(isRetryableModelFailure({ statusCode: 422, message: "unprocessable" }), true); + assert.equal(normalizeModelFailureSignal({ code: "422", message: "unprocessable" }).kind, "request_incompatible"); + }); + + test("retry classifier treats HTTP 413 payload/request-too-large as fallbackable (#1580)", () => { + const cases: ReadonlyArray<{ label: string; failure: unknown }> = [ + { label: "status 413", failure: { status: 413, message: "payload too large" } }, + { label: "statusCode 413", failure: { statusCode: 413, message: "request entity too large" } }, + { label: "httpStatus 413", failure: { httpStatus: 413, message: "too large" } }, + { label: "code 413 numeric", failure: { code: 413, message: "payload too large" } }, + { label: "code 413 string", failure: { code: "413", message: "too large" } }, + ]; + for (const { label, failure } of cases) { + assert.equal(normalizeModelFailureSignal(failure).kind, "request_incompatible", label); + assert.equal(isRetryableModelFailure(failure), true, label); + } + }); + + test("retry classifier classifies request-incompatible codes and messages as fallbackable (#1580)", () => { + for (const code of ["invalid_request_error", "context_length_exceeded", "bad_request", "too_large", "max_tokens"]) { + assert.equal(normalizeModelFailureSignal({ code, message: "localized" }).kind, "request_incompatible", `code ${code}`); + assert.equal(isRetryableModelFailure({ code, message: "localized" }), true, `code ${code}`); + } + const messages = [ + "context length exceeded for this model", + "request too large", + "unsupported tool: foo", + "invalid_request_error", + "bad request", + ]; + for (const message of messages) { + assert.equal(isRetryableModelFailure(new Error(message)), true, `message "${message}"`); + assert.equal(normalizeModelFailureSignal(new Error(message)).kind, "request_incompatible", `message "${message}"`); + } + }); + + test("request incompatibility does not outrank refusals or cancellations (#1580)", () => { + assert.equal(isRetryableModelFailure({ status: 400, stopReason: "aborted", errorMessage: "aborted" }), false); + assert.equal(isRetryableModelFailure({ statusCode: 422, name: "AbortError", message: "aborted by user" }), false); + assert.equal(isRetryableModelFailure({ httpStatus: 400, message: "content_filter" }), false); + assert.equal(isRetryableModelFailure({ status: 422, diagnostics: [{ error: { message: "command failed" } }] }), false); + }); }); From e431b6617f88b4e4cbf3e2dd039c3bdfe661e115 Mon Sep 17 00:00:00 2001 From: Norin Lavaee Date: Wed, 1 Jul 2026 14:24:42 -0700 Subject: [PATCH 2/6] fix(subagents): add model attempt watchdog Assistant-model: GPT-5.5 --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/docs/subagents.md | 4 + packages/subagents/CHANGELOG.md | 1 + .../runs/background/async-execution-chain.ts | 12 +- .../runs/background/async-execution-single.ts | 12 +- .../runs/background/async-execution-types.ts | 2 + .../runs/background/subagent-runner-step.ts | 8 +- .../background/subagent-runner-streaming.ts | 13 ++ .../chain-execution-dynamic-step.ts | 1 + .../chain-execution-parallel-runner.ts | 1 + .../chain-execution-parallel-step.ts | 1 + .../chain-execution-sequential-step.ts | 1 + .../runs/foreground/chain-execution-types.ts | 2 + .../src/runs/foreground/chain-execution.ts | 2 + .../src/runs/foreground/execution-attempt.ts | 15 ++ .../src/runs/foreground/execution-run-sync.ts | 14 +- .../foreground/subagent-executor-async.ts | 4 + .../foreground/subagent-executor-parallel.ts | 2 + .../foreground/subagent-executor-single.ts | 3 + .../src/runs/shared/attempt-watchdog.ts | 97 ++++++++++++ .../src/runs/shared/model-candidate-filter.ts | 53 +++++++ .../src/runs/shared/parallel-utils.ts | 2 + packages/subagents/src/shared/types-config.ts | 2 + test/unit/subagents-attempt-watchdog.test.ts | 140 ++++++++++++++++++ 24 files changed, 385 insertions(+), 8 deletions(-) create mode 100644 packages/subagents/src/runs/shared/attempt-watchdog.ts create mode 100644 packages/subagents/src/runs/shared/model-candidate-filter.ts create mode 100644 test/unit/subagents-attempt-watchdog.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c0c5f7f58..31d21d574 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -13,6 +13,7 @@ - Fixed GitHub Copilot models to use the live `max_output_tokens` value from the Copilot model catalog, preventing `github-copilot/claude-opus-4.8` from being capped by Atomic's stale built-in output-token limit after compaction ([#1582](https://github.com/bastani-inc/atomic/issues/1582)). - Fixed active GitHub Copilot sessions to adopt live catalog model metadata as soon as the catalog loads, so fallback models refresh their supported reasoning levels without requiring a restart. - Fixed workflow and subagent model fallback chains so request/context incompatibility failures (HTTP 400/413/422 bad/unprocessable/payload-too-large request, unsupported tool/parameter, context-length/context-window overflow, `invalid_request`/`bad_request`/`too_large` errors) advance to the next candidate instead of stopping. This ensures that when none of the configured fallback candidates can serve the current request, Atomic falls back to the currently selected user model rather than failing outright. Refusals, content-filter/safety blocks, cancellations, and task failures still stop the chain and are never retried on another model ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). +- Fixed foreground and background subagent model attempts that produced no child activity from hanging indefinitely. Atomic now bounds each candidate with an idle watchdog and wall-clock cap, records retryable timeout attempts so fallback continues, and skips known unauthenticated providers before spawning while preserving unknown/custom providers and the current-model last resort ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). ## [0.9.4-alpha.6] - 2026-07-01 diff --git a/packages/coding-agent/docs/subagents.md b/packages/coding-agent/docs/subagents.md index 22f23fc76..4508b5584 100644 --- a/packages/coding-agent/docs/subagents.md +++ b/packages/coding-agent/docs/subagents.md @@ -177,6 +177,10 @@ Agents can define ordered `fallbackModels` for retryable provider or model failu A candidate that cannot serve the current request — for example an HTTP 400/413/422 bad/unprocessable/payload-too-large request, an unsupported tool or parameter, a context-length/context-window overflow, or a `too large` / `invalid_request` error — is treated as request/context incompatible and the chain advances to the next candidate rather than stopping. This means that if none of the configured candidates are applicable to the request, Atomic falls back to the currently selected user model instead of failing outright. +Each foreground and background model candidate is bounded by a per-attempt idle watchdog (default 5 minutes without child stdout, stderr, or JSON child events) and an absolute wall-clock cap (default 60 minutes). If either trips, Atomic terminates that child attempt, records a retryable timeout in `modelAttempts`, and continues to the next fallback candidate. The defaults can be overridden with `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` and `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS`; `ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS` controls SIGTERM-to-SIGKILL escalation. + +When registry availability shows that a known candidate provider has no configured auth, Atomic records a skipped model attempt before spawning a child. Unknown/custom providers are still attempted, and the current user-selected model appended as the final fallback is never filtered out by this pre-spawn check. + Fallbacks do not retry ordinary task failures, validation failures, tool failures, cancellations, or workflow-code errors. Because a fallback may send the same prompt and context to a different provider, choose models that match your cost, privacy, and data-handling requirements. Each candidate can also carry its own reasoning effort — see [Reasoning levels](#reasoning-levels). diff --git a/packages/subagents/CHANGELOG.md b/packages/subagents/CHANGELOG.md index 553936932..04c126fd4 100644 --- a/packages/subagents/CHANGELOG.md +++ b/packages/subagents/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed subagent model fallback so request/context incompatibility failures (HTTP 400/413/422 bad/unprocessable/payload-too-large request, unsupported tool/parameter, context-length/context-window overflow, `invalid_request`/`bad_request`/`too_large` errors) advance the chain to the next candidate instead of stopping. When none of the configured candidates can serve the request, the subagent now falls back to the current user-selected model. Refusals, content-filter/safety blocks, cancellations, and task failures still stop the chain ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). +- Fixed foreground and background subagent model attempts that produced no child activity from hanging indefinitely. Each candidate attempt now has a conservative idle watchdog and absolute wall-clock cap, records a retryable timeout failure, and advances to the next fallback candidate; known providers without configured auth are skipped before spawning while unknown/custom providers and the current-model last resort are still attempted ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). ## [0.9.4-alpha.6] - 2026-07-01 diff --git a/packages/subagents/src/runs/background/async-execution-chain.ts b/packages/subagents/src/runs/background/async-execution-chain.ts index e93ca6c02..57ca14ae0 100644 --- a/packages/subagents/src/runs/background/async-execution-chain.ts +++ b/packages/subagents/src/runs/background/async-execution-chain.ts @@ -12,6 +12,7 @@ import type { RunnerStep } from "../shared/parallel-utils.ts"; import { injectSingleOutputInstruction, resolveSingleOutputPath, validateFileOnlyOutputMode } from "../shared/single-output.ts"; import { ChainOutputValidationError, validateChainOutputBindings } from "../shared/chain-outputs.ts"; import { buildModelCandidates, resolveModelCandidate } from "../shared/model-fallback.ts"; +import { filterSpawnableModelCandidates } from "../shared/model-candidate-filter.ts"; import { NESTED_RUNS_DIR, nestedResultsPath, resolveInheritedNestedRouteFromEnv, resolveNestedParentAddressFromEnv, writeNestedEvent } from "../shared/nested-events.ts"; import { createStructuredOutputRuntime } from "../shared/structured-output.ts"; import { resolveExpectedWorktreeAgentCwd } from "../shared/worktree.ts"; @@ -57,6 +58,7 @@ export function executeAsyncChain( const resultMode = params.resultMode ?? "chain"; const chainSkills = params.chainSkills ?? []; const availableModels = params.availableModels; + const knownModelProviders = params.knownModelProviders; const fastModeScope = resolveSubagentCodexFastModeScope(workflowStageSubagentGuard); const runnerCwd = resolveChildCwd(ctx.cwd, cwd); const firstStep = chain[0]; @@ -149,9 +151,16 @@ export function executeAsyncChain( const primaryModel = resolveModelCandidate(behavior.model ?? a.model, availableModels, ctx.currentModelProvider); const model = applyThinkingSuffix(primaryModel, a.thinking); - const modelCandidates = buildModelCandidates(behavior.model ?? a.model, a.fallbackModels, availableModels, ctx.currentModelProvider, ctx.currentModel, a.fallbackThinkingLevels) + const rawModelCandidates = buildModelCandidates(behavior.model ?? a.model, a.fallbackModels, availableModels, ctx.currentModelProvider, ctx.currentModel, a.fallbackThinkingLevels) .map((candidate) => applyThinkingSuffix(candidate, a.thinking)) .filter((candidate): candidate is string => typeof candidate === "string"); + const filteredCandidates = filterSpawnableModelCandidates({ + candidates: rawModelCandidates, + availableModels, + knownModelProviders, + currentModel: applyThinkingSuffix(ctx.currentModel, a.thinking), + }); + const modelCandidates = filteredCandidates.candidates; const fastModeSettings = getSubagentCodexFastModeSettings(stepCwd); return { agent: s.agent, @@ -165,6 +174,7 @@ export function executeAsyncChain( thinking: resolveEffectiveThinking(model, a.thinking), ...resolveSubagentModelFastModeMetadata({ model, modelCandidates, cwd: stepCwd, settings: fastModeSettings, scope: fastModeScope }), modelCandidates, + modelAttempts: filteredCandidates.skippedAttempts, codexFastModeSettings: fastModeSettings, codexFastModeScope: fastModeScope, tools: a.tools, diff --git a/packages/subagents/src/runs/background/async-execution-single.ts b/packages/subagents/src/runs/background/async-execution-single.ts index 15078ea32..46dbc10ca 100644 --- a/packages/subagents/src/runs/background/async-execution-single.ts +++ b/packages/subagents/src/runs/background/async-execution-single.ts @@ -9,6 +9,7 @@ import { resolveChildCwd } from "../../shared/utils.ts"; import { applyThinkingSuffix, SUBAGENT_INTERCOM_SESSION_NAME_ENV } from "../shared/pi-args.ts"; import { injectSingleOutputInstruction, normalizeSingleOutputOverride, resolveSingleOutputPath, validateFileOnlyOutputMode } from "../shared/single-output.ts"; import { buildModelCandidates, resolveModelCandidate } from "../shared/model-fallback.ts"; +import { filterSpawnableModelCandidates } from "../shared/model-candidate-filter.ts"; import { NESTED_RUNS_DIR, nestedResultsPath, resolveInheritedNestedRouteFromEnv, resolveNestedParentAddressFromEnv, writeNestedEvent } from "../shared/nested-events.ts"; import { UNAVAILABLE_SUBAGENT_SKILL_ERROR, @@ -50,6 +51,7 @@ export function executeAsyncSingle( const runnerCwd = resolveChildCwd(ctx.cwd, cwd); const skillNames = params.skills ?? agentConfig.skills ?? []; const availableModels = params.availableModels; + const knownModelProviders = params.knownModelProviders; const { resolved: resolvedSkills, missing: missingSkills } = resolveSkillsWithFallback(skillNames, runnerCwd, ctx.cwd); if (missingSkills.includes("subagent")) return formatAsyncStartError("single", UNAVAILABLE_SUBAGENT_SKILL_ERROR); let systemPrompt = agentConfig.systemPrompt?.trim() ?? ""; @@ -84,9 +86,16 @@ export function executeAsyncSingle( resolveModelCandidate(params.modelOverride ?? agentConfig.model, availableModels, ctx.currentModelProvider), agentConfig.thinking, ); - const modelCandidates = buildModelCandidates(params.modelOverride ?? agentConfig.model, agentConfig.fallbackModels, availableModels, ctx.currentModelProvider, ctx.currentModel) + const rawModelCandidates = buildModelCandidates(params.modelOverride ?? agentConfig.model, agentConfig.fallbackModels, availableModels, ctx.currentModelProvider, ctx.currentModel) .map((candidate) => applyThinkingSuffix(candidate, agentConfig.thinking)) .filter((candidate): candidate is string => typeof candidate === "string"); + const filteredCandidates = filterSpawnableModelCandidates({ + candidates: rawModelCandidates, + availableModels, + knownModelProviders, + currentModel: applyThinkingSuffix(ctx.currentModel, agentConfig.thinking), + }); + const modelCandidates = filteredCandidates.candidates; const fastModeSettings = getSubagentCodexFastModeSettings(runnerCwd); const fastModeScope = resolveSubagentCodexFastModeScope(workflowStageSubagentGuard); let spawnResult: AsyncSpawnResult = {}; @@ -103,6 +112,7 @@ export function executeAsyncSingle( thinking: resolveEffectiveThinking(model, agentConfig.thinking), ...resolveSubagentModelFastModeMetadata({ model, modelCandidates, cwd: runnerCwd, settings: fastModeSettings, scope: fastModeScope }), modelCandidates, + modelAttempts: filteredCandidates.skippedAttempts, codexFastModeSettings: fastModeSettings, codexFastModeScope: fastModeScope, tools: agentConfig.tools, diff --git a/packages/subagents/src/runs/background/async-execution-types.ts b/packages/subagents/src/runs/background/async-execution-types.ts index c170c05ce..1f50325e3 100644 --- a/packages/subagents/src/runs/background/async-execution-types.ts +++ b/packages/subagents/src/runs/background/async-execution-types.ts @@ -26,6 +26,7 @@ export interface AsyncChainParams { agents: AgentConfig[]; ctx: AsyncExecutionContext; availableModels?: AvailableModelInfo[]; + knownModelProviders?: string[]; cwd?: string; maxOutput?: MaxOutputConfig; artifactsDir?: string; @@ -62,6 +63,7 @@ export interface AsyncSingleParams { outputMode?: "inline" | "file-only"; modelOverride?: string; availableModels?: AvailableModelInfo[]; + knownModelProviders?: string[]; maxSubagentDepth: number; workflowStageSubagentGuard?: boolean; worktreeSetupHook?: string; diff --git a/packages/subagents/src/runs/background/subagent-runner-step.ts b/packages/subagents/src/runs/background/subagent-runner-step.ts index d9d95d5db..70ec86754 100644 --- a/packages/subagents/src/runs/background/subagent-runner-step.ts +++ b/packages/subagents/src/runs/background/subagent-runner-step.ts @@ -61,14 +61,16 @@ export async function runSingleStep( } } - const candidates = step.modelCandidates && step.modelCandidates.length > 0 + const candidates = step.modelCandidates !== undefined ? step.modelCandidates : step.model ? [step.model] : [undefined]; const attemptedModels: string[] = []; - const modelAttempts: ModelAttempt[] = []; - const attemptNotes: string[] = []; + const modelAttempts: ModelAttempt[] = [...(step.modelAttempts ?? [])]; + const attemptNotes: string[] = modelAttempts + .filter((attempt) => !attempt.success && attempt.exitCode === null && attempt.error) + .map((attempt) => `[fallback] ${attempt.error}`); const pendingAttemptNotes: string[] = []; const eventsPath = path.join(path.dirname(ctx.outputFile), "events.jsonl"); let finalResult: RunPiStreamingResult | undefined; diff --git a/packages/subagents/src/runs/background/subagent-runner-streaming.ts b/packages/subagents/src/runs/background/subagent-runner-streaming.ts index a0d1e7e66..e0b1a4c10 100644 --- a/packages/subagents/src/runs/background/subagent-runner-streaming.ts +++ b/packages/subagents/src/runs/background/subagent-runner-streaming.ts @@ -12,6 +12,7 @@ import { shouldStartSubagentFinalDrain, } from "../shared/final-drain.ts"; import { modelFailureMessage } from "../shared/model-fallback.ts"; +import { createAttemptWatchdog } from "../shared/attempt-watchdog.ts"; import type { ChildEvent, ChildEventContext, RunPiStreamingResult } from "./subagent-runner-types.ts"; import { emptyUsage } from "./subagent-runner-utils.ts"; @@ -163,7 +164,16 @@ export function runPiStreaming( let finalHardKillTimer: NodeJS.Timeout | undefined; let settled = false; const clearStdioGuard = attachPostExitStdioGuard(child, { idleMs: 2000, hardMs: 8000 }); + const attemptWatchdog = createAttemptWatchdog({ + child, + isSettled: () => settled, + onTimeout(message) { + forcedTerminationSignal = true; + error ??= message; + }, + }); child.stdout.on("data", (chunk: Buffer) => { + attemptWatchdog.activity(); const text = chunk.toString(); stdoutBuf += text; const lines = stdoutBuf.split("\n"); @@ -172,6 +182,7 @@ export function runPiStreaming( }); child.stderr.on("data", (chunk: Buffer) => { + attemptWatchdog.activity(); processStderrText(chunk.toString()); }); registerInterrupt?.(() => { @@ -220,6 +231,7 @@ export function runPiStreaming( registerInterrupt?.(undefined); clearDrainTimers(); clearStdioGuard(); + attemptWatchdog.clear(); if (stdoutBuf.trim()) processStdoutLine(stdoutBuf); if (stderrBuf.trim()) appendChildLine("subagent.child.stderr", stderrBuf); outputStream.end(); @@ -246,6 +258,7 @@ export function runPiStreaming( registerInterrupt?.(undefined); clearDrainTimers(); clearStdioGuard(); + attemptWatchdog.clear(); outputStream.end(); const finalOutput = getFinalOutput(messages) || rawStdoutLines.join("\n").trim(); const spawnErrorMessage = spawnError instanceof Error ? spawnError.message : String(spawnError); diff --git a/packages/subagents/src/runs/foreground/chain-execution-dynamic-step.ts b/packages/subagents/src/runs/foreground/chain-execution-dynamic-step.ts index 315e55bf4..ba160b616 100644 --- a/packages/subagents/src/runs/foreground/chain-execution-dynamic-step.ts +++ b/packages/subagents/src/runs/foreground/chain-execution-dynamic-step.ts @@ -96,6 +96,7 @@ export async function runDynamicParallelChainStep(input: { agents: context.agents, stepIndex, availableModels: context.availableModels, + knownModelProviders: context.knownModelProviders, chainDir: context.chainDir, prev: state.prev, originalTask: context.originalTask, diff --git a/packages/subagents/src/runs/foreground/chain-execution-parallel-runner.ts b/packages/subagents/src/runs/foreground/chain-execution-parallel-runner.ts index ee2de637d..9432d0720 100644 --- a/packages/subagents/src/runs/foreground/chain-execution-parallel-runner.ts +++ b/packages/subagents/src/runs/foreground/chain-execution-parallel-runner.ts @@ -125,6 +125,7 @@ export async function runParallelChainTasks(input: ParallelChainRunInput): Promi nestedRoute: input.nestedRoute, modelOverride: effectiveModel, availableModels: input.availableModels, + knownModelProviders: input.knownModelProviders, currentModel: currentModelFullId(input.ctx.model), preferredModelProvider: input.ctx.model?.provider, skills: behavior.skills === false ? [] : behavior.skills, diff --git a/packages/subagents/src/runs/foreground/chain-execution-parallel-step.ts b/packages/subagents/src/runs/foreground/chain-execution-parallel-step.ts index 6b70c6974..1af34b7bd 100644 --- a/packages/subagents/src/runs/foreground/chain-execution-parallel-step.ts +++ b/packages/subagents/src/runs/foreground/chain-execution-parallel-step.ts @@ -78,6 +78,7 @@ export async function runStaticParallelChainStep(input: { agents: context.agents, stepIndex, availableModels: context.availableModels, + knownModelProviders: context.knownModelProviders, chainDir: context.chainDir, prev: state.prev, originalTask: context.originalTask, diff --git a/packages/subagents/src/runs/foreground/chain-execution-sequential-step.ts b/packages/subagents/src/runs/foreground/chain-execution-sequential-step.ts index 5d3005da6..05e929833 100644 --- a/packages/subagents/src/runs/foreground/chain-execution-sequential-step.ts +++ b/packages/subagents/src/runs/foreground/chain-execution-sequential-step.ts @@ -126,6 +126,7 @@ export async function runSequentialChainStep(input: { nestedRoute: context.params.nestedRoute, modelOverride: effectiveModel, availableModels: context.availableModels, + knownModelProviders: context.knownModelProviders, currentModel: currentModelFullId(context.ctx.model), preferredModelProvider: context.ctx.model?.provider, skills: behavior.skills === false ? [] : behavior.skills, diff --git a/packages/subagents/src/runs/foreground/chain-execution-types.ts b/packages/subagents/src/runs/foreground/chain-execution-types.ts index ee1c31b6b..ee6330568 100644 --- a/packages/subagents/src/runs/foreground/chain-execution-types.ts +++ b/packages/subagents/src/runs/foreground/chain-execution-types.ts @@ -109,6 +109,7 @@ export interface ParallelChainRunInput { agents: AgentConfig[]; stepIndex: number; availableModels: ModelInfo[]; + knownModelProviders: string[]; chainDir: string; prev: string; originalTask: string; @@ -179,6 +180,7 @@ export interface ChainRuntimeContext { chainSkills: string[]; chainDir: string; availableModels: ModelInfo[]; + knownModelProviders: string[]; originalTask: string; chainAgents: string[]; chainSteps: ChainStep[]; diff --git a/packages/subagents/src/runs/foreground/chain-execution.ts b/packages/subagents/src/runs/foreground/chain-execution.ts index 223dbc6a0..7beba5b26 100644 --- a/packages/subagents/src/runs/foreground/chain-execution.ts +++ b/packages/subagents/src/runs/foreground/chain-execution.ts @@ -117,6 +117,7 @@ export async function executeChain(params: ChainExecutionParams): Promise>["tuiBehaviorOverrides"]; const availableModels: ModelInfo[] = ctx.modelRegistry.getAvailable().map(toModelInfo); + const knownModelProviders = [...new Set((typeof ctx.modelRegistry.getAll === "function" ? ctx.modelRegistry.getAll() : ctx.modelRegistry.getAvailable()).map((model) => model.provider))]; const context: ChainRuntimeContext = { params, agents, @@ -140,6 +141,7 @@ export async function executeChain(params: ChainExecutionParams): Promise settled || processClosed || detached, + onTimeout(message) { + forcedTerminationSignal = true; + result.error ??= message; + progress.error = message; + }, + }); proc.stdout.on("data", (d) => { + attemptWatchdog.activity(); buf += d.toString(); const lines = buf.split("\n"); buf = lines.pop() || ""; lines.forEach(processLine); }); proc.stderr.on("data", (d) => { + attemptWatchdog.activity(); stderrBuf += d.toString(); }); proc.on("exit", () => { @@ -374,6 +387,7 @@ export async function runSingleAttempt( proc.on("close", (code, signal) => { clearFinalDrainTimers(); clearStdioGuard(); + attemptWatchdog.clear(); void jsonlWriter.close().catch(() => undefined); cleanupTempDir(tempDir); if (detached) { @@ -394,6 +408,7 @@ export async function runSingleAttempt( proc.on("error", (error) => { clearFinalDrainTimers(); clearStdioGuard(); + attemptWatchdog.clear(); void jsonlWriter.close().catch(() => undefined); cleanupTempDir(tempDir); if (!result.error) result.error = error instanceof Error ? error.message : String(error); diff --git a/packages/subagents/src/runs/foreground/execution-run-sync.ts b/packages/subagents/src/runs/foreground/execution-run-sync.ts index 49853e3cb..2fb1687e9 100644 --- a/packages/subagents/src/runs/foreground/execution-run-sync.ts +++ b/packages/subagents/src/runs/foreground/execution-run-sync.ts @@ -20,6 +20,7 @@ import { formatModelAttemptNote, isRetryableModelFailure, } from "../shared/model-fallback.ts"; +import { filterSpawnableModelCandidates } from "../shared/model-candidate-filter.ts"; import { artifactOutputByResult, emptyUsage, modelFailureSignalByResult, sumUsage } from "./execution-utils.ts"; import { runSingleAttemptWithStructuredOutputRetries } from "./execution-structured-retries.ts"; import { shouldSuppressIntermediateRetryableFailureUpdate } from "./execution-updates.ts"; @@ -65,7 +66,7 @@ export async function runSync( systemPrompt = systemPrompt ? `${systemPrompt}\n\n${skillInjection}` : skillInjection; } - const candidates = buildModelCandidates( + const rawCandidates = buildModelCandidates( options.modelOverride ?? agent.model, agent.fallbackModels, options.availableModels, @@ -73,13 +74,20 @@ export async function runSync( options.currentModel, agent.fallbackThinkingLevels, ); + const filteredCandidates = filterSpawnableModelCandidates({ + candidates: rawCandidates, + availableModels: options.availableModels, + knownModelProviders: options.knownModelProviders, + currentModel: options.currentModel, + }); + const candidates = filteredCandidates.candidates; const fastModeCwd = options.cwd ?? runtimeCwd; const fastModeSettings = getSubagentCodexFastModeSettings(fastModeCwd); const fastModeScope = resolveSubagentCodexFastModeScope(options.workflowStageSubagentGuard); const attemptedModels: string[] = []; - const modelAttempts: ModelAttempt[] = []; + const modelAttempts: ModelAttempt[] = [...filteredCandidates.skippedAttempts]; const aggregateUsage = emptyUsage(); - const attemptNotes: string[] = []; + const attemptNotes: string[] = filteredCandidates.skippedAttempts.map((attempt) => `[fallback] ${attempt.error}`); const pendingAttemptNotes: string[] = []; let totalToolCount = 0; let totalDurationMs = 0; diff --git a/packages/subagents/src/runs/foreground/subagent-executor-async.ts b/packages/subagents/src/runs/foreground/subagent-executor-async.ts index d42bc8d6b..1c28bafdb 100644 --- a/packages/subagents/src/runs/foreground/subagent-executor-async.ts +++ b/packages/subagents/src/runs/foreground/subagent-executor-async.ts @@ -76,6 +76,7 @@ export function runAsyncPath(data: ExecutionContextData, deps: ResolvedExecutorD currentModel: currentModelFullId(ctx.model), }; const availableModels: ModelInfo[] = ctx.modelRegistry.getAvailable().map(toModelInfo); + const knownModelProviders = [...new Set((typeof ctx.modelRegistry.getAll === "function" ? ctx.modelRegistry.getAll() : ctx.modelRegistry.getAvailable()).map((model) => model.provider))]; const depthPolicy = resolveSubagentDepthPolicy(ctx, deps.config.maxSubagentDepth); const currentMaxSubagentDepth = depthPolicy.maxSubagentDepth; const workflowStageSubagentGuard = depthPolicy.workflowStageSubagentGuard; @@ -110,6 +111,7 @@ export function runAsyncPath(data: ExecutionContextData, deps: ResolvedExecutorD agents, ctx: asyncCtx, availableModels, + knownModelProviders, cwd: effectiveCwd, maxOutput: params.maxOutput, artifactsDir: artifactConfig.enabled ? artifactsDir : undefined, @@ -139,6 +141,7 @@ export function runAsyncPath(data: ExecutionContextData, deps: ResolvedExecutorD agents, ctx: asyncCtx, availableModels, + knownModelProviders, cwd: effectiveCwd, maxOutput: params.maxOutput, artifactsDir: artifactConfig.enabled ? artifactsDir : undefined, @@ -181,6 +184,7 @@ export function runAsyncPath(data: ExecutionContextData, deps: ResolvedExecutorD agentConfig: a, ctx: asyncCtx, availableModels, + knownModelProviders, cwd: effectiveCwd, maxOutput: params.maxOutput, artifactsDir: artifactConfig.enabled ? artifactsDir : undefined, diff --git a/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts b/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts index ee996e6c9..7e0be3d29 100644 --- a/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts +++ b/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts @@ -108,6 +108,7 @@ export async function runParallelPath(data: ExecutionContextData, deps: Resolved const currentProvider = ctx.model?.provider; const availableModels: ModelInfo[] = ctx.modelRegistry.getAvailable().map(toModelInfo); + const knownModelProviders = [...new Set((typeof ctx.modelRegistry.getAll === "function" ? ctx.modelRegistry.getAll() : ctx.modelRegistry.getAvailable()).map((model) => model.provider))]; let taskTexts = tasks.map((t) => t.task); const skillOverrides: (string[] | false | undefined)[] = tasks.map((t) => normalizeSkillInput(t.skill), @@ -205,6 +206,7 @@ export async function runParallelPath(data: ExecutionContextData, deps: Resolved agents, ctx: asyncCtx, availableModels, + knownModelProviders, cwd: effectiveCwd, maxOutput: params.maxOutput, artifactsDir: artifactConfig.enabled ? artifactsDir : undefined, diff --git a/packages/subagents/src/runs/foreground/subagent-executor-single.ts b/packages/subagents/src/runs/foreground/subagent-executor-single.ts index 64786dc57..c36a27738 100644 --- a/packages/subagents/src/runs/foreground/subagent-executor-single.ts +++ b/packages/subagents/src/runs/foreground/subagent-executor-single.ts @@ -76,6 +76,7 @@ export async function runSinglePath(data: ExecutionContextData, deps: ResolvedEx const currentProvider = ctx.model?.provider; const availableModels: ModelInfo[] = ctx.modelRegistry.getAvailable().map(toModelInfo); + const knownModelProviders = [...new Set((typeof ctx.modelRegistry.getAll === "function" ? ctx.modelRegistry.getAll() : ctx.modelRegistry.getAvailable()).map((model) => model.provider))]; let task = params.task ?? ""; let modelOverride: string | undefined = resolveModelCandidate( (params.model as string | undefined) ?? agentConfig.model, @@ -145,6 +146,7 @@ export async function runSinglePath(data: ExecutionContextData, deps: ResolvedEx agentConfig, ctx: asyncCtx, availableModels, + knownModelProviders, cwd: effectiveCwd, maxOutput: params.maxOutput, artifactsDir: artifactConfig.enabled ? artifactsDir : undefined, @@ -246,6 +248,7 @@ export async function runSinglePath(data: ExecutionContextData, deps: ResolvedEx index: 0, modelOverride, availableModels, + knownModelProviders, preferredModelProvider: currentProvider, currentModel: currentModelFullId(ctx.model), skills: effectiveSkills, diff --git a/packages/subagents/src/runs/shared/attempt-watchdog.ts b/packages/subagents/src/runs/shared/attempt-watchdog.ts new file mode 100644 index 000000000..e343d0d7d --- /dev/null +++ b/packages/subagents/src/runs/shared/attempt-watchdog.ts @@ -0,0 +1,97 @@ +import type { ChildProcess } from "node:child_process"; +import { trySignalChild } from "../../shared/post-exit-stdio-guard.ts"; + +const DEFAULT_IDLE_MS = 5 * 60_000; +const DEFAULT_WALL_MS = 60 * 60_000; +const DEFAULT_KILL_GRACE_MS = 3_000; + +export interface AttemptTimeoutConfig { + idleMs: number; + wallMs: number; + killGraceMs: number; +} + +export interface AttemptWatchdog { + activity(): void; + clear(): void; +} + +function positiveEnvMs(name: string): number | undefined { + const raw = process.env[name]; + if (!raw) return undefined; + const value = Number(raw); + return Number.isFinite(value) && value > 0 ? Math.floor(value) : undefined; +} + +export function resolveAttemptTimeoutConfig(): AttemptTimeoutConfig { + return { + idleMs: positiveEnvMs("ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS") ?? DEFAULT_IDLE_MS, + wallMs: positiveEnvMs("ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS") ?? DEFAULT_WALL_MS, + killGraceMs: positiveEnvMs("ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS") ?? DEFAULT_KILL_GRACE_MS, + }; +} + +export function idleTimeoutMessage(idleMs: number): string { + return `Subagent model attempt timed out after ${idleMs}ms without child activity.`; +} + +export function wallTimeoutMessage(wallMs: number): string { + return `Subagent model attempt timed out after ${wallMs}ms.`; +} + +export function createAttemptWatchdog(params: { + child: ChildProcess; + config?: Partial; + onTimeout: (message: string) => void; + isSettled: () => boolean; +}): AttemptWatchdog { + const config = { ...resolveAttemptTimeoutConfig(), ...(params.config ?? {}) }; + let idleTimer: NodeJS.Timeout | undefined; + let wallTimer: NodeJS.Timeout | undefined; + let killTimer: NodeJS.Timeout | undefined; + let tripped = false; + + const clearIdle = () => { + if (!idleTimer) return; + clearTimeout(idleTimer); + idleTimer = undefined; + }; + const clearAll = () => { + clearIdle(); + if (wallTimer) { + clearTimeout(wallTimer); + wallTimer = undefined; + } + if (killTimer) { + clearTimeout(killTimer); + killTimer = undefined; + } + }; + const trip = (message: string) => { + if (tripped || params.isSettled()) return; + tripped = true; + clearIdle(); + params.onTimeout(message); + trySignalChild(params.child, "SIGTERM"); + killTimer = setTimeout(() => { + if (!params.isSettled()) trySignalChild(params.child, "SIGKILL"); + }, config.killGraceMs); + killTimer.unref?.(); + }; + const scheduleIdle = () => { + clearIdle(); + idleTimer = setTimeout(() => trip(idleTimeoutMessage(config.idleMs)), config.idleMs); + idleTimer.unref?.(); + }; + + scheduleIdle(); + wallTimer = setTimeout(() => trip(wallTimeoutMessage(config.wallMs)), config.wallMs); + wallTimer.unref?.(); + + return { + activity() { + if (!tripped && !params.isSettled()) scheduleIdle(); + }, + clear: clearAll, + }; +} diff --git a/packages/subagents/src/runs/shared/model-candidate-filter.ts b/packages/subagents/src/runs/shared/model-candidate-filter.ts new file mode 100644 index 000000000..3992b9cc9 --- /dev/null +++ b/packages/subagents/src/runs/shared/model-candidate-filter.ts @@ -0,0 +1,53 @@ +import { splitKnownThinkingSuffix } from "../../shared/model-info.ts"; +import type { ModelAttempt } from "../../shared/types.ts"; +import type { AvailableModelInfo } from "./model-fallback.ts"; + +export interface FilteredModelCandidates { + candidates: string[]; + skippedAttempts: ModelAttempt[]; +} + +function providerFromModel(model: string | undefined): string | undefined { + if (!model) return undefined; + const { baseModel } = splitKnownThinkingSuffix(model); + const slash = baseModel.indexOf("/"); + return slash > 0 ? baseModel.slice(0, slash) : undefined; +} + +export function skippedModelAttempt(model: string, reason: string): ModelAttempt { + return { + model, + success: false, + exitCode: null, + error: reason, + }; +} + +export function filterSpawnableModelCandidates(params: { + candidates: string[]; + availableModels?: AvailableModelInfo[]; + knownModelProviders?: string[]; + currentModel?: string; +}): FilteredModelCandidates { + const providersWithAvailableAuth = new Set((params.availableModels ?? []).map((model) => model.provider)); + const knownProviders = new Set(params.knownModelProviders ?? []); + const currentModel = params.currentModel; + const filtered: string[] = []; + const skippedAttempts: ModelAttempt[] = []; + for (const candidate of params.candidates) { + if (currentModel && candidate === currentModel) { + filtered.push(candidate); + continue; + } + const provider = providerFromModel(candidate); + const shouldSkip = provider !== undefined + && knownProviders.has(provider) + && !providersWithAvailableAuth.has(provider); + if (!shouldSkip) { + filtered.push(candidate); + continue; + } + skippedAttempts.push(skippedModelAttempt(candidate, `Skipped ${candidate}: provider '${provider}' has no configured API key/auth in the current session.`)); + } + return { candidates: filtered, skippedAttempts }; +} diff --git a/packages/subagents/src/runs/shared/parallel-utils.ts b/packages/subagents/src/runs/shared/parallel-utils.ts index b1875260f..a87224cff 100644 --- a/packages/subagents/src/runs/shared/parallel-utils.ts +++ b/packages/subagents/src/runs/shared/parallel-utils.ts @@ -1,4 +1,5 @@ import type { CodexFastModeResolvedSettings, CodexFastModeScope } from "@bastani/atomic"; +import type { ModelAttempt } from "../../shared/types.ts"; export interface RunnerSubagentStep { agent: string; @@ -13,6 +14,7 @@ export interface RunnerSubagentStep { fastMode?: boolean; modelFastModes?: Record; modelCandidates?: string[]; + modelAttempts?: ModelAttempt[]; codexFastModeSettings?: CodexFastModeResolvedSettings; codexFastModeScope?: CodexFastModeScope; tools?: string[]; diff --git a/packages/subagents/src/shared/types-config.ts b/packages/subagents/src/shared/types-config.ts index ecbd37077..9ff665ce5 100644 --- a/packages/subagents/src/shared/types-config.ts +++ b/packages/subagents/src/shared/types-config.ts @@ -80,6 +80,8 @@ export interface RunSyncOptions { modelOverride?: string; /** Registry models available for heuristic bare-model resolution */ availableModels?: Array<{ provider: string; id: string; fullId: string }>; + /** Providers known to the registry before auth filtering */ + knownModelProviders?: string[]; /** Current parent-session provider to prefer for ambiguous bare model ids */ preferredModelProvider?: string; /** Current parent-session model to try after configured fallback models */ diff --git a/test/unit/subagents-attempt-watchdog.test.ts b/test/unit/subagents-attempt-watchdog.test.ts new file mode 100644 index 000000000..d60261d24 --- /dev/null +++ b/test/unit/subagents-attempt-watchdog.test.ts @@ -0,0 +1,140 @@ +import { describe, test } from "bun:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { AgentConfig } from "../../packages/subagents/src/agents/agents.js"; +import { runSync } from "../../packages/subagents/src/runs/foreground/execution.js"; +import { filterSpawnableModelCandidates } from "../../packages/subagents/src/runs/shared/model-candidate-filter.js"; + +function agentConfig(): AgentConfig { + return { + name: "fake-worker", + description: "Fake worker", + source: "project", + filePath: "fake-worker.md", + systemPrompt: "Work.", + systemPromptMode: "replace", + inheritProjectContext: false, + inheritSkills: false, + model: "provider-a/stalled", + fallbackModels: ["provider-b/working"], + }; +} + +async function withFakeCli(script: string, fn: (dir: string) => Promise): Promise { + const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-watchdog-")); + const scriptPath = join(dir, "fake-pi.js"); + const previousArgv1 = process.argv[1]; + const previousIdle = process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS; + const previousWall = process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS; + const previousKill = process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS; + writeFileSync(scriptPath, script, { mode: 0o700 }); + process.argv[1] = scriptPath; + process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS = "250"; + process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = "2000"; + process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS = "20"; + try { + return await fn(dir); + } finally { + process.argv[1] = previousArgv1; + if (previousIdle === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS; + else process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS = previousIdle; + if (previousWall === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS; + else process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = previousWall; + if (previousKill === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS; + else process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS = previousKill; + rmSync(dir, { recursive: true, force: true }); + } +} + +const successEvent = (text: string) => JSON.stringify({ + type: "message_end", + message: { + role: "assistant", + content: [{ type: "text", text }], + stopReason: "stop", + usage: { input: 1, output: 1 }, + timestamp: Date.now(), + }, +}); + +describe("subagent per-attempt watchdog", () => { + test("kills a stalled model attempt and advances to the next fallback", async () => { + await withFakeCli(` + const fs = require("node:fs"); + const path = require("node:path"); + const modelIndex = process.argv.indexOf("--model"); + const model = modelIndex === -1 ? "default" : process.argv[modelIndex + 1]; + fs.appendFileSync(path.join(process.cwd(), "models.log"), model + "\\n"); + if (model === "provider-a/stalled") setInterval(() => {}, 1000); + else console.log(${JSON.stringify(successEvent("fallback ok"))}); + `, async (dir) => { + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-fallback", + availableModels: [ + { provider: "provider-a", id: "stalled", fullId: "provider-a/stalled" }, + { provider: "provider-b", id: "working", fullId: "provider-b/working" }, + ], + knownModelProviders: ["provider-a", "provider-b"], + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.error, undefined); + assert.match(result.finalOutput ?? "", /fallback ok/); + assert.deepEqual(readFileSync(join(dir, "models.log"), "utf8").trim().split("\n"), ["provider-a/stalled", "provider-b/working"]); + assert.equal(result.modelAttempts?.length, 2); + assert.equal(result.modelAttempts?.[0]?.success, false); + assert.match(result.modelAttempts?.[0]?.error ?? "", /timed out/i); + assert.equal(result.modelAttempts?.[1]?.success, true); + }); + }); + + test("resets the idle timer on child activity", async () => { + await withFakeCli(` + let ticks = 0; + const timer = setInterval(() => { + ticks += 1; + process.stderr.write("tick " + ticks + "\\n"); + if (ticks === 3) { + clearInterval(timer); + console.log(${JSON.stringify(successEvent("activity ok"))}); + } + }, 50); + `, async (dir) => { + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-activity", + modelOverride: "provider-a/stalled", + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.error, undefined); + assert.match(result.finalOutput ?? "", /activity ok/); + assert.equal(result.modelAttempts?.length, 1); + assert.equal(result.modelAttempts?.[0]?.success, true); + }); + }); + + test("pre-spawn filtering skips known keyless providers but keeps unknowns and current model", () => { + const filtered = filterSpawnableModelCandidates({ + candidates: ["provider-a/missing", "custom/model", "provider-b/ready", "provider-a/current"], + availableModels: [{ provider: "provider-b", id: "ready", fullId: "provider-b/ready" }], + knownModelProviders: ["provider-a", "provider-b"], + currentModel: "provider-a/current", + }); + + assert.deepEqual(filtered.candidates, ["custom/model", "provider-b/ready", "provider-a/current"]); + assert.deepEqual(filtered.skippedAttempts.map((attempt) => attempt.model), ["provider-a/missing"]); + assert.match(filtered.skippedAttempts[0]?.error ?? "", /no configured API key\/auth/); + + const noAvailable = filterSpawnableModelCandidates({ + candidates: ["provider-a/missing", "custom/model"], + availableModels: [], + knownModelProviders: ["provider-a"], + }); + assert.deepEqual(noAvailable.candidates, ["custom/model"]); + assert.deepEqual(noAvailable.skippedAttempts.map((attempt) => attempt.model), ["provider-a/missing"]); + }); +}); From 0818dda51a86c68533ddad21bbf8a1c7736c8e6b Mon Sep 17 00:00:00 2001 From: Norin Lavaee Date: Wed, 1 Jul 2026 14:40:41 -0700 Subject: [PATCH 3/6] fix(subagents): complete fallback watchdog edge cases Assistant-model: GPT-5.5 --- .../runs/background/subagent-runner-step.ts | 13 +- .../src/runs/foreground/execution-run-sync.ts | 6 +- .../subagent-executor-parallel-task.ts | 2 + .../foreground/subagent-executor-parallel.ts | 1 + test/unit/subagents-attempt-watchdog.test.ts | 115 +++++++++++++++++- 5 files changed, 132 insertions(+), 5 deletions(-) diff --git a/packages/subagents/src/runs/background/subagent-runner-step.ts b/packages/subagents/src/runs/background/subagent-runner-step.ts index 70ec86754..c0826e4e6 100644 --- a/packages/subagents/src/runs/background/subagent-runner-step.ts +++ b/packages/subagents/src/runs/background/subagent-runner-step.ts @@ -17,7 +17,7 @@ import { import { formatModelAttemptNote, isRetryableModelFailure } from "../shared/model-fallback.ts"; import type { ArtifactPaths, ModelAttempt } from "../../shared/types.ts"; import type { RunPiStreamingResult, SingleStepContext, SubagentStep } from "./subagent-runner-types.ts"; -import { fastModeForStepAttempt } from "./subagent-runner-utils.ts"; +import { emptyUsage, fastModeForStepAttempt } from "./subagent-runner-utils.ts"; import { runPiStreaming } from "./subagent-runner-streaming.ts"; export { outputEntryFromAsyncResult }; @@ -207,6 +207,17 @@ export async function runSingleStep( if (!tryNextModel) break; } + if (!finalResult && candidates.length === 0 && modelAttempts.length > 0) { + finalResult = { + stderr: "", + exitCode: 1, + messages: [], + usage: emptyUsage(), + error: "No spawnable subagent model candidates after pre-spawn filtering.", + finalOutput: "", + }; + } + const rawOutput = finalResult?.finalOutput ?? ""; const outputForPersistence = rawOutput; const resolvedOutput = step.outputPath && finalResult?.exitCode === 0 diff --git a/packages/subagents/src/runs/foreground/execution-run-sync.ts b/packages/subagents/src/runs/foreground/execution-run-sync.ts index 2fb1687e9..0185c0d6e 100644 --- a/packages/subagents/src/runs/foreground/execution-run-sync.ts +++ b/packages/subagents/src/runs/foreground/execution-run-sync.ts @@ -104,7 +104,7 @@ export async function runSync( } let lastResult: SingleResult | undefined; - const modelsToTry = candidates.length > 0 ? candidates : [undefined]; + const modelsToTry = candidates.length > 0 ? candidates : (rawCandidates.length === 0 ? [undefined] : []); for (let i = 0; i < modelsToTry.length; i++) { const candidate = modelsToTry[i]; if (candidate) attemptedModels.push(candidate); @@ -164,7 +164,9 @@ export async function runSync( exitCode: 1, messages: [], usage: emptyUsage(), - error: "Subagent did not produce a result.", + error: modelAttempts.length > 0 + ? "No spawnable subagent model candidates after pre-spawn filtering." + : "Subagent did not produce a result.", } satisfies SingleResult; result.usage = aggregateUsage; diff --git a/packages/subagents/src/runs/foreground/subagent-executor-parallel-task.ts b/packages/subagents/src/runs/foreground/subagent-executor-parallel-task.ts index 47f992b01..19ff5d467 100644 --- a/packages/subagents/src/runs/foreground/subagent-executor-parallel-task.ts +++ b/packages/subagents/src/runs/foreground/subagent-executor-parallel-task.ts @@ -38,6 +38,7 @@ interface ForegroundParallelRunInput { maxSubagentDepths: number[]; workflowStageSubagentGuard?: boolean; availableModels: ModelInfo[]; + knownModelProviders: string[]; modelOverrides: (string | undefined)[]; behaviors: ResolvedStepBehavior[]; firstProgressIndex: number; @@ -110,6 +111,7 @@ export async function runForegroundParallelTasks(input: ForegroundParallelRunInp nestedRoute: input.foregroundControl?.nestedRoute, modelOverride: input.modelOverrides[index], availableModels: input.availableModels, + knownModelProviders: input.knownModelProviders, preferredModelProvider: input.ctx.model?.provider, currentModel: currentModelFullId(input.ctx.model), skills: effectiveSkills === false ? [] : effectiveSkills, diff --git a/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts b/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts index 7e0be3d29..679186ed5 100644 --- a/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts +++ b/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts @@ -283,6 +283,7 @@ export async function runParallelPath(data: ExecutionContextData, deps: Resolved paramsCwd: effectiveCwd, workflowStageSubagentGuard, availableModels, + knownModelProviders, modelOverrides, behaviors, firstProgressIndex: parallelProgressPrecreated ? -1 : firstProgressIndex, diff --git a/test/unit/subagents-attempt-watchdog.test.ts b/test/unit/subagents-attempt-watchdog.test.ts index d60261d24..509ca8ec7 100644 --- a/test/unit/subagents-attempt-watchdog.test.ts +++ b/test/unit/subagents-attempt-watchdog.test.ts @@ -1,10 +1,12 @@ import { describe, test } from "bun:test"; import assert from "node:assert/strict"; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { AgentConfig } from "../../packages/subagents/src/agents/agents.js"; +import { runSingleStep } from "../../packages/subagents/src/runs/background/subagent-runner-step.js"; import { runSync } from "../../packages/subagents/src/runs/foreground/execution.js"; +import { runForegroundParallelTasks } from "../../packages/subagents/src/runs/foreground/subagent-executor-parallel-task.js"; import { filterSpawnableModelCandidates } from "../../packages/subagents/src/runs/shared/model-candidate-filter.js"; function agentConfig(): AgentConfig { @@ -97,7 +99,7 @@ describe("subagent per-attempt watchdog", () => { const timer = setInterval(() => { ticks += 1; process.stderr.write("tick " + ticks + "\\n"); - if (ticks === 3) { + if (ticks === 8) { clearInterval(timer); console.log(${JSON.stringify(successEvent("activity ok"))}); } @@ -137,4 +139,113 @@ describe("subagent per-attempt watchdog", () => { assert.deepEqual(noAvailable.candidates, ["custom/model"]); assert.deepEqual(noAvailable.skippedAttempts.map((attempt) => attempt.model), ["provider-a/missing"]); }); + + test("does not spawn a default foreground child when every configured candidate is filtered", async () => { + await withFakeCli(` + const fs = require("node:fs"); + const path = require("node:path"); + fs.writeFileSync(path.join(process.cwd(), "spawned"), "yes"); + console.log(${JSON.stringify(successEvent("should not run"))}); + `, async (dir) => { + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-all-filtered", + availableModels: [], + knownModelProviders: ["provider-a", "provider-b"], + }); + + assert.equal(result.exitCode, 1); + assert.match(result.error ?? "", /No spawnable subagent model candidates/); + assert.equal(existsSync(join(dir, "spawned")), false); + assert.deepEqual(result.modelAttempts?.map((attempt) => attempt.model), ["provider-a/stalled", "provider-b/working"]); + }); + }); + + test("background runner reports a useful error when every candidate was filtered", async () => { + const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-background-filtered-")); + try { + const result = await runSingleStep({ + agent: "fake-worker", + task: "Do work", + inheritProjectContext: false, + inheritSkills: false, + modelCandidates: [], + modelAttempts: [{ + model: "provider-a/stalled", + success: false, + exitCode: null, + error: "Skipped provider-a/stalled: provider 'provider-a' has no configured API key/auth in the current session.", + }], + }, { + previousOutput: "", + placeholder: "{previous}", + cwd: dir, + sessionEnabled: false, + id: "background-all-filtered", + flatIndex: 0, + flatStepCount: 1, + outputFile: join(dir, "output.txt"), + }); + + assert.equal(result.exitCode, 1); + assert.match(result.error ?? "", /No spawnable subagent model candidates/); + assert.match(result.output, /Skipped provider-a\/stalled/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("foreground parallel tasks pass known providers into runSync", async () => { + const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-parallel-known-")); + try { + const knownModelProviders = ["provider-a", "provider-b"]; + const captured: Array<{ knownModelProviders?: string[] }> = []; + const input: Parameters[0] = { + tasks: [{ agent: "fake-worker", task: "Do work" }], + taskTexts: ["Do work"], + agents: [agentConfig()], + ctx: { + cwd: dir, + model: { provider: "provider-b", id: "working" }, + } as Parameters[0]["ctx"], + intercomEvents: {} as Parameters[0]["intercomEvents"], + signal: new AbortController().signal, + runId: "parallel-known-providers", + sessionDirForIndex: () => undefined, + sessionFileForIndex: () => undefined, + shareEnabled: false, + artifactConfig: { enabled: false, includeInput: false, includeOutput: false, includeJsonl: false, includeMetadata: false, cleanupDays: 0 }, + artifactsDir: dir, + paramsCwd: dir, + maxSubagentDepths: [0], + availableModels: [{ provider: "provider-b", id: "working", fullId: "provider-b/working" }], + knownModelProviders, + modelOverrides: ["provider-a/stalled"], + behaviors: [{ output: false, outputMode: "inline", reads: false, progress: false, skills: false }], + firstProgressIndex: -1, + controlConfig: { enabled: false, needsAttentionAfterMs: 1, activeNoticeAfterMs: 1, failedToolAttemptsBeforeAttention: 1, notifyOn: [], notifyChannels: [] }, + concurrencyLimit: 1, + liveResults: [], + liveProgress: [], + runtime: { + async runSync(_cwd, _agents, agentName, task, options) { + captured.push({ knownModelProviders: options.knownModelProviders }); + return { + agent: agentName, + task, + exitCode: 0, + messages: [], + usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, cost: 0, turns: 0 }, + finalOutput: "ok", + }; + }, + }, + }; + + await runForegroundParallelTasks(input); + assert.deepEqual(captured, [{ knownModelProviders }]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); }); From d4378062a3a6790a2fb6eb4f195ac4a5bba9dfe6 Mon Sep 17 00:00:00 2001 From: Norin Lavaee Date: Wed, 1 Jul 2026 17:42:34 -0700 Subject: [PATCH 4/6] fix(subagents): address PR #1581 review feedback - Idle watchdog now treats an in-flight tool execution as activity so a slow, quiet tool call (long build/test run with no interim output) is not falsely killed as a stalled attempt; the wall-clock cap still bounds such attempts. Wired into both the foreground and background spawn paths. - Extract collectKnownModelProviders() helper to replace the four duplicated modelRegistry provider-derivation expressions. - Align the subagents classifier's direct-message precedence with the workflows classifier (direct message after nested cause/diagnostic traversal, before outer status/code) and add a cross-package conformance test so the two classifier copies cannot silently drift. - Remove @ts-nocheck from the request-incompatible classifier test. - Add watchdog tests: tool-active idle deferral (foreground and background), post-tool stall, wall-clock cap, SIGTERM->SIGKILL escalation. - Comment the intentional '!== undefined' empty-candidates guard in subagent-runner-step.ts; document tool-activity semantics in subagents.md; update changelogs. Refs: #1581 Assistant-model: Claude Fable 5 --- packages/coding-agent/CHANGELOG.md | 2 + packages/coding-agent/docs/subagents.md | 2 +- packages/subagents/CHANGELOG.md | 2 + .../runs/background/subagent-runner-step.ts | 4 + .../background/subagent-runner-streaming.ts | 7 + .../src/runs/foreground/chain-execution.ts | 4 +- .../src/runs/foreground/execution-attempt.ts | 3 + .../foreground/subagent-executor-async.ts | 4 +- .../foreground/subagent-executor-parallel.ts | 4 +- .../foreground/subagent-executor-single.ts | 4 +- .../src/runs/shared/attempt-watchdog.ts | 15 +- .../src/runs/shared/model-fallback.ts | 18 ++- packages/subagents/src/shared/model-info.ts | 16 ++ ...el-fallback-classifier-conformance.test.ts | 110 +++++++++++++ ...odel-fallback-request-incompatible.test.ts | 1 - test/unit/subagents-attempt-watchdog.test.ts | 144 ++++++++++++++++++ 16 files changed, 326 insertions(+), 14 deletions(-) create mode 100644 test/unit/model-fallback-classifier-conformance.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 31d21d574..be4d9bfe5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -14,6 +14,8 @@ - Fixed active GitHub Copilot sessions to adopt live catalog model metadata as soon as the catalog loads, so fallback models refresh their supported reasoning levels without requiring a restart. - Fixed workflow and subagent model fallback chains so request/context incompatibility failures (HTTP 400/413/422 bad/unprocessable/payload-too-large request, unsupported tool/parameter, context-length/context-window overflow, `invalid_request`/`bad_request`/`too_large` errors) advance to the next candidate instead of stopping. This ensures that when none of the configured fallback candidates can serve the current request, Atomic falls back to the currently selected user model rather than failing outright. Refusals, content-filter/safety blocks, cancellations, and task failures still stop the chain and are never retried on another model ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). - Fixed foreground and background subagent model attempts that produced no child activity from hanging indefinitely. Atomic now bounds each candidate with an idle watchdog and wall-clock cap, records retryable timeout attempts so fallback continues, and skips known unauthenticated providers before spawning while preserving unknown/custom providers and the current-model last resort ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). +- Fixed the subagent per-attempt idle watchdog so an in-flight tool execution counts as activity: a slow, quiet tool call (long build or test run with no interim output) is no longer killed as a stalled attempt; the wall-clock cap still bounds such attempts ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). +- Aligned the subagents and workflows model-failure classifiers' direct-message precedence and added a cross-package conformance test suite so the two classifier copies cannot silently drift ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). ## [0.9.4-alpha.6] - 2026-07-01 diff --git a/packages/coding-agent/docs/subagents.md b/packages/coding-agent/docs/subagents.md index 4508b5584..af8f2168e 100644 --- a/packages/coding-agent/docs/subagents.md +++ b/packages/coding-agent/docs/subagents.md @@ -177,7 +177,7 @@ Agents can define ordered `fallbackModels` for retryable provider or model failu A candidate that cannot serve the current request — for example an HTTP 400/413/422 bad/unprocessable/payload-too-large request, an unsupported tool or parameter, a context-length/context-window overflow, or a `too large` / `invalid_request` error — is treated as request/context incompatible and the chain advances to the next candidate rather than stopping. This means that if none of the configured candidates are applicable to the request, Atomic falls back to the currently selected user model instead of failing outright. -Each foreground and background model candidate is bounded by a per-attempt idle watchdog (default 5 minutes without child stdout, stderr, or JSON child events) and an absolute wall-clock cap (default 60 minutes). If either trips, Atomic terminates that child attempt, records a retryable timeout in `modelAttempts`, and continues to the next fallback candidate. The defaults can be overridden with `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` and `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS`; `ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS` controls SIGTERM-to-SIGKILL escalation. +Each foreground and background model candidate is bounded by a per-attempt idle watchdog (default 5 minutes without child stdout, stderr, or JSON child events) and an absolute wall-clock cap (default 60 minutes). An in-flight tool execution counts as activity, so a slow, quiet tool call (a long build or test run that streams nothing until it finishes) is not mistaken for a stalled attempt; only the wall-clock cap bounds such attempts. If either watchdog trips, Atomic terminates that child attempt, records a retryable timeout in `modelAttempts`, and continues to the next fallback candidate. The defaults can be overridden with `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` and `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS`; `ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS` controls SIGTERM-to-SIGKILL escalation. When registry availability shows that a known candidate provider has no configured auth, Atomic records a skipped model attempt before spawning a child. Unknown/custom providers are still attempted, and the current user-selected model appended as the final fallback is never filtered out by this pre-spawn check. diff --git a/packages/subagents/CHANGELOG.md b/packages/subagents/CHANGELOG.md index 04c126fd4..a8126a316 100644 --- a/packages/subagents/CHANGELOG.md +++ b/packages/subagents/CHANGELOG.md @@ -6,6 +6,8 @@ - Fixed subagent model fallback so request/context incompatibility failures (HTTP 400/413/422 bad/unprocessable/payload-too-large request, unsupported tool/parameter, context-length/context-window overflow, `invalid_request`/`bad_request`/`too_large` errors) advance the chain to the next candidate instead of stopping. When none of the configured candidates can serve the request, the subagent now falls back to the current user-selected model. Refusals, content-filter/safety blocks, cancellations, and task failures still stop the chain ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). - Fixed foreground and background subagent model attempts that produced no child activity from hanging indefinitely. Each candidate attempt now has a conservative idle watchdog and absolute wall-clock cap, records a retryable timeout failure, and advances to the next fallback candidate; known providers without configured auth are skipped before spawning while unknown/custom providers and the current-model last resort are still attempted ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). +- Fixed the per-attempt idle watchdog so an in-flight tool execution counts as activity: a slow, quiet tool call (long build or test run with no interim output) is no longer killed as a stalled attempt; the wall-clock cap still bounds such attempts ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). +- Aligned the subagents model-failure classifier's direct-message precedence with the workflows classifier (direct-message classification now runs after nested cause/diagnostic traversal and before outer status/code classification), and added a cross-package conformance test suite so the two classifier copies cannot silently drift ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). ## [0.9.4-alpha.6] - 2026-07-01 diff --git a/packages/subagents/src/runs/background/subagent-runner-step.ts b/packages/subagents/src/runs/background/subagent-runner-step.ts index c0826e4e6..b9ae96ca2 100644 --- a/packages/subagents/src/runs/background/subagent-runner-step.ts +++ b/packages/subagents/src/runs/background/subagent-runner-step.ts @@ -61,6 +61,10 @@ export async function runSingleStep( } } + // `!== undefined` is intentional: an explicitly empty array means every candidate + // was removed by pre-spawn filtering (see filterSpawnableModelCandidates) and must + // be respected — do not "simplify" this back to `.length > 0`, which would spawn a + // doomed default attempt. The empty case is surfaced as an error below. const candidates = step.modelCandidates !== undefined ? step.modelCandidates : step.model diff --git a/packages/subagents/src/runs/background/subagent-runner-streaming.ts b/packages/subagents/src/runs/background/subagent-runner-streaming.ts index e0b1a4c10..f34a8bb97 100644 --- a/packages/subagents/src/runs/background/subagent-runner-streaming.ts +++ b/packages/subagents/src/runs/background/subagent-runner-streaming.ts @@ -56,6 +56,7 @@ export function runPiStreaming( let assistantError: string | undefined; let assistantFailureSignal: unknown; let interrupted = false; + let activeToolExecutions = 0; const rawStdoutLines: string[] = []; const writeOutputLine = (line: string) => { @@ -100,6 +101,11 @@ export function runPiStreaming( appendChildEvent(event); onChildEvent?.(event); + // Track in-flight tool executions so the idle watchdog does not mistake a + // slow, quiet tool call for a stalled attempt. + if (event.type === "tool_execution_start") activeToolExecutions += 1; + else if (event.type === "tool_execution_end") activeToolExecutions = Math.max(0, activeToolExecutions - 1); + if (event.type === "tool_execution_start" && event.toolName) { const toolArgs = extractToolArgsPreview(event.args ?? {}); writeOutputLine(toolArgs ? `${event.toolName}: ${toolArgs}` : event.toolName); @@ -167,6 +173,7 @@ export function runPiStreaming( const attemptWatchdog = createAttemptWatchdog({ child, isSettled: () => settled, + isToolActive: () => activeToolExecutions > 0, onTimeout(message) { forcedTerminationSignal = true; error ??= message; diff --git a/packages/subagents/src/runs/foreground/chain-execution.ts b/packages/subagents/src/runs/foreground/chain-execution.ts index 7beba5b26..cb55006d5 100644 --- a/packages/subagents/src/runs/foreground/chain-execution.ts +++ b/packages/subagents/src/runs/foreground/chain-execution.ts @@ -2,7 +2,7 @@ * Public foreground chain execution API. */ -import { toModelInfo, type ModelInfo } from "../../shared/model-info.ts"; +import { collectKnownModelProviders, toModelInfo, type ModelInfo } from "../../shared/model-info.ts"; import { createChainDir, isDynamicParallelStep, @@ -117,7 +117,7 @@ export async function executeChain(params: ChainExecutionParams): Promise>["tuiBehaviorOverrides"]; const availableModels: ModelInfo[] = ctx.modelRegistry.getAvailable().map(toModelInfo); - const knownModelProviders = [...new Set((typeof ctx.modelRegistry.getAll === "function" ? ctx.modelRegistry.getAll() : ctx.modelRegistry.getAvailable()).map((model) => model.provider))]; + const knownModelProviders = collectKnownModelProviders(ctx.modelRegistry); const context: ChainRuntimeContext = { params, agents, diff --git a/packages/subagents/src/runs/foreground/execution-attempt.ts b/packages/subagents/src/runs/foreground/execution-attempt.ts index dfed6b9b9..fd5d0c08f 100644 --- a/packages/subagents/src/runs/foreground/execution-attempt.ts +++ b/packages/subagents/src/runs/foreground/execution-attempt.ts @@ -363,6 +363,9 @@ export async function runSingleAttempt( const attemptWatchdog = createAttemptWatchdog({ child: proc, isSettled: () => settled || processClosed || detached, + // A slow, quiet tool call (long build/test run) must not be mistaken for a + // stalled attempt: an in-flight tool execution counts as watchdog activity. + isToolActive: () => progress.currentTool !== undefined, onTimeout(message) { forcedTerminationSignal = true; result.error ??= message; diff --git a/packages/subagents/src/runs/foreground/subagent-executor-async.ts b/packages/subagents/src/runs/foreground/subagent-executor-async.ts index 1c28bafdb..4a85de5ac 100644 --- a/packages/subagents/src/runs/foreground/subagent-executor-async.ts +++ b/packages/subagents/src/runs/foreground/subagent-executor-async.ts @@ -1,7 +1,7 @@ import { randomUUID } from "node:crypto"; import { APP_NAME } from "@bastani/atomic"; import { currentModelFullId, resolveModelCandidate } from "../shared/model-fallback.ts"; -import { toModelInfo, type ModelInfo } from "../../shared/model-info.ts"; +import { collectKnownModelProviders, toModelInfo, type ModelInfo } from "../../shared/model-info.ts"; import { normalizeSkillInput } from "../../agents/skills.ts"; import { resolveSubagentIntercomTarget } from "../../intercom/intercom-bridge.ts"; import { @@ -76,7 +76,7 @@ export function runAsyncPath(data: ExecutionContextData, deps: ResolvedExecutorD currentModel: currentModelFullId(ctx.model), }; const availableModels: ModelInfo[] = ctx.modelRegistry.getAvailable().map(toModelInfo); - const knownModelProviders = [...new Set((typeof ctx.modelRegistry.getAll === "function" ? ctx.modelRegistry.getAll() : ctx.modelRegistry.getAvailable()).map((model) => model.provider))]; + const knownModelProviders = collectKnownModelProviders(ctx.modelRegistry); const depthPolicy = resolveSubagentDepthPolicy(ctx, deps.config.maxSubagentDepth); const currentMaxSubagentDepth = depthPolicy.maxSubagentDepth; const workflowStageSubagentGuard = depthPolicy.workflowStageSubagentGuard; diff --git a/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts b/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts index 679186ed5..176690bfc 100644 --- a/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts +++ b/packages/subagents/src/runs/foreground/subagent-executor-parallel.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import { APP_NAME } from "@bastani/atomic"; import { ChainClarifyComponent, type ChainClarifyResult } from "./chain-clarify.ts"; import { currentModelFullId, resolveModelCandidate } from "../shared/model-fallback.ts"; -import { toModelInfo, type ModelInfo } from "../../shared/model-info.ts"; +import { collectKnownModelProviders, toModelInfo, type ModelInfo } from "../../shared/model-info.ts"; import { discoverAvailableSkills, normalizeSkillInput } from "../../agents/skills.ts"; import { aggregateParallelOutputs } from "../shared/parallel-utils.ts"; import { recordRun } from "../shared/run-history.ts"; @@ -108,7 +108,7 @@ export async function runParallelPath(data: ExecutionContextData, deps: Resolved const currentProvider = ctx.model?.provider; const availableModels: ModelInfo[] = ctx.modelRegistry.getAvailable().map(toModelInfo); - const knownModelProviders = [...new Set((typeof ctx.modelRegistry.getAll === "function" ? ctx.modelRegistry.getAll() : ctx.modelRegistry.getAvailable()).map((model) => model.provider))]; + const knownModelProviders = collectKnownModelProviders(ctx.modelRegistry); let taskTexts = tasks.map((t) => t.task); const skillOverrides: (string[] | false | undefined)[] = tasks.map((t) => normalizeSkillInput(t.skill), diff --git a/packages/subagents/src/runs/foreground/subagent-executor-single.ts b/packages/subagents/src/runs/foreground/subagent-executor-single.ts index c36a27738..46adc9f72 100644 --- a/packages/subagents/src/runs/foreground/subagent-executor-single.ts +++ b/packages/subagents/src/runs/foreground/subagent-executor-single.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import { APP_NAME } from "@bastani/atomic"; import { ChainClarifyComponent, type ChainClarifyResult } from "./chain-clarify.ts"; import { currentModelFullId, resolveModelCandidate } from "../shared/model-fallback.ts"; -import { toModelInfo, type ModelInfo } from "../../shared/model-info.ts"; +import { collectKnownModelProviders, toModelInfo, type ModelInfo } from "../../shared/model-info.ts"; import { discoverAvailableSkills, normalizeSkillInput } from "../../agents/skills.ts"; import { recordRun } from "../shared/run-history.ts"; import { getSingleResultOutput, compactForegroundDetails } from "../../shared/utils.ts"; @@ -76,7 +76,7 @@ export async function runSinglePath(data: ExecutionContextData, deps: ResolvedEx const currentProvider = ctx.model?.provider; const availableModels: ModelInfo[] = ctx.modelRegistry.getAvailable().map(toModelInfo); - const knownModelProviders = [...new Set((typeof ctx.modelRegistry.getAll === "function" ? ctx.modelRegistry.getAll() : ctx.modelRegistry.getAvailable()).map((model) => model.provider))]; + const knownModelProviders = collectKnownModelProviders(ctx.modelRegistry); let task = params.task ?? ""; let modelOverride: string | undefined = resolveModelCandidate( (params.model as string | undefined) ?? agentConfig.model, diff --git a/packages/subagents/src/runs/shared/attempt-watchdog.ts b/packages/subagents/src/runs/shared/attempt-watchdog.ts index e343d0d7d..7c460195b 100644 --- a/packages/subagents/src/runs/shared/attempt-watchdog.ts +++ b/packages/subagents/src/runs/shared/attempt-watchdog.ts @@ -44,6 +44,11 @@ export function createAttemptWatchdog(params: { config?: Partial; onTimeout: (message: string) => void; isSettled: () => boolean; + /** Reports whether a tool call is currently executing in the child. A slow tool + * (long build, large test suite) can legitimately stay silent past the idle + * window, so an in-flight tool execution counts as activity and defers the idle + * trip. The wall-clock cap still bounds the whole attempt. */ + isToolActive?: () => boolean; }): AttemptWatchdog { const config = { ...resolveAttemptTimeoutConfig(), ...(params.config ?? {}) }; let idleTimer: NodeJS.Timeout | undefined; @@ -80,7 +85,15 @@ export function createAttemptWatchdog(params: { }; const scheduleIdle = () => { clearIdle(); - idleTimer = setTimeout(() => trip(idleTimeoutMessage(config.idleMs)), config.idleMs); + idleTimer = setTimeout(() => { + if (params.isToolActive?.()) { + // Do not kill a healthy attempt that is busy inside a slow tool call; + // re-arm the idle window and let the wall-clock cap bound the attempt. + scheduleIdle(); + return; + } + trip(idleTimeoutMessage(config.idleMs)); + }, config.idleMs); idleTimer.unref?.(); }; diff --git a/packages/subagents/src/runs/shared/model-fallback.ts b/packages/subagents/src/runs/shared/model-fallback.ts index 5502aa8f6..17d1a2f5a 100644 --- a/packages/subagents/src/runs/shared/model-fallback.ts +++ b/packages/subagents/src/runs/shared/model-fallback.ts @@ -360,15 +360,21 @@ function makeSignal( }; } -function fallbackSignalFromMessage( +function fallbackSignalForMessage( + message: string | undefined, value: unknown, source: ModelFallbackFailureSource | undefined, ): ModelFallbackFailureSignal | undefined { - const message = modelFailureMessage(value); - if (!message.trim()) return undefined; + if (message === undefined || message.trim().length === 0) return undefined; const kind = fallbackKindFromMessage(message, errorName(value)); return kind === undefined ? undefined : makeSignal(kind, value, source); } +function fallbackSignalFromMessage(value: unknown, source: ModelFallbackFailureSource | undefined): ModelFallbackFailureSignal | undefined { + return fallbackSignalForMessage(modelFailureMessage(value), value, source); +} +function fallbackSignalFromDirectMessage(value: unknown, source: ModelFallbackFailureSource | undefined): ModelFallbackFailureSignal | undefined { + return fallbackSignalForMessage(directMessageFrom(value), value, source); +} function classifyAssistantRefusalSignal( value: unknown, @@ -421,6 +427,12 @@ function structuredSignal( if (isRefusalSignal(causeSignal)) return causeSignal; firstNestedFallbackSignal ??= causeSignal; } + // Direct-message classification runs after nested traversal so a generic wrapper + // ("invalid request"/"400 bad request") cannot mask a non-retryable nested signal. + // Kept in the same position as the workflows classifier so the two copies stay + // behaviorally parallel (see test/unit/model-fallback-classifier-conformance.test.ts). + const directMessageSignal = fallbackSignalFromDirectMessage(value, source); + if (directMessageSignal !== undefined) return directMessageSignal; const statusKind = kindFromStatus(statusFrom(value)); if (statusKind !== undefined) return makeSignal(statusKind, value, source); if (codeKind !== undefined) return makeSignal(codeKind, value, source); diff --git a/packages/subagents/src/shared/model-info.ts b/packages/subagents/src/shared/model-info.ts index 9b31dde9c..721d74650 100644 --- a/packages/subagents/src/shared/model-info.ts +++ b/packages/subagents/src/shared/model-info.ts @@ -27,6 +27,22 @@ export function toModelInfo(model: RegistryModelLike): ModelInfo { }; } +interface KnownProviderRegistryLike { + getAvailable(): ReadonlyArray<{ provider: string }>; + /** Older hosts may not expose `getAll()`; callers probe at runtime and fall back + * to `getAvailable()`. */ + getAll?: () => ReadonlyArray<{ provider: string }>; +} + +/** Collect the distinct providers of every model the registry knows about — including + * providers without configured auth — falling back to `getAvailable()` when the host + * registry does not expose `getAll()`. Used to pre-filter spawn candidates whose + * provider is known but keyless. */ +export function collectKnownModelProviders(registry: KnownProviderRegistryLike): string[] { + const models = typeof registry.getAll === "function" ? registry.getAll() : registry.getAvailable(); + return [...new Set(models.map((model) => model.provider))]; +} + /** Resolve the effective thinking level from a model string (which may contain a known suffix like `:high`) * and an explicit thinking config value. Returns `undefined` when no thinking is applicable * (e.g. no model was specified, or the model has no suffix and no config was provided). */ diff --git a/test/unit/model-fallback-classifier-conformance.test.ts b/test/unit/model-fallback-classifier-conformance.test.ts new file mode 100644 index 000000000..eeef79909 --- /dev/null +++ b/test/unit/model-fallback-classifier-conformance.test.ts @@ -0,0 +1,110 @@ +import { describe, test } from "bun:test"; +import assert from "node:assert/strict"; +import { + isRetryableModelFailure as subagentsIsRetryable, + normalizeModelFailureSignal as subagentsNormalize, +} from "../../packages/subagents/src/runs/shared/model-fallback.js"; +import { + isRetryableModelFailure as workflowsIsRetryable, + normalizeModelFailureSignal as workflowsNormalize, +} from "../../packages/workflows/src/runs/shared/model-fallback.js"; + +// The subagents and workflows model-failure classifiers are maintained as parallel +// copies (packages/subagents/src/runs/shared/model-fallback.ts and +// packages/workflows/src/runs/shared/model-fallback-failures.ts). This conformance +// suite runs a shared corpus of failure fixtures through both and asserts they +// agree, so a change to one copy that silently diverges the other fails here. +// +// Known intentional difference (not covered by the shared corpus): the workflows +// classifier has an extra `transport_error` kind for bare "connection error." / +// "fetch failed." wrapper messages that the subagents classifier does not model. + +type Fixture = { label: string; failure: unknown; kind: string; retryable: boolean }; + +function abortWrappedError(): Error { + const abortCause = new Error("aborted by user"); + abortCause.name = "AbortError"; + return new Error("invalid request", { cause: abortCause }); +} + +const CONFORMANCE_FIXTURES: readonly Fixture[] = [ + // HTTP status classification. + { label: "status 400", failure: { status: 400, message: "bad request" }, kind: "request_incompatible", retryable: true }, + { label: "status 413", failure: { statusCode: 413, message: "payload too large" }, kind: "request_incompatible", retryable: true }, + { label: "status 422", failure: { httpStatus: 422, message: "unprocessable" }, kind: "request_incompatible", retryable: true }, + { label: "status 401", failure: { status: 401, message: "unauthorized" }, kind: "auth_on_candidate_provider", retryable: true }, + { label: "status 403", failure: { status: 403, message: "forbidden" }, kind: "auth_on_candidate_provider", retryable: true }, + { label: "status 404", failure: { status: 404, message: "model missing" }, kind: "model_unavailable", retryable: true }, + { label: "status 408", failure: { status: 408, message: "request timeout" }, kind: "network_timeout", retryable: true }, + { label: "status 429", failure: { status: 429, message: "slow down" }, kind: "rate_limit", retryable: true }, + { label: "status 503", failure: { status: 503, message: "service unavailable" }, kind: "provider_unavailable", retryable: true }, + // Request-incompatible provider codes (numeric, string, and named). + { label: "code 413 numeric", failure: { code: 413, message: "too big" }, kind: "request_incompatible", retryable: true }, + { label: "code 422 string", failure: { code: "422", message: "unprocessable" }, kind: "request_incompatible", retryable: true }, + { label: "code invalid_request_error", failure: { code: "invalid_request_error", message: "localized" }, kind: "request_incompatible", retryable: true }, + { label: "code bad_request", failure: { code: "bad_request", message: "localized" }, kind: "request_incompatible", retryable: true }, + { label: "code context_length_exceeded", failure: { code: "context_length_exceeded", message: "localized" }, kind: "request_incompatible", retryable: true }, + { label: "code request_too_large", failure: { code: "request_too_large", message: "localized" }, kind: "request_incompatible", retryable: true }, + { label: "code max_tokens", failure: { code: "max_tokens", message: "localized" }, kind: "request_incompatible", retryable: true }, + // Request-incompatible messages. + { label: "message context length", failure: new Error("This model's context length exceeded"), kind: "request_incompatible", retryable: true }, + { label: "message context window", failure: new Error("context window exceeded for candidate"), kind: "request_incompatible", retryable: true }, + { label: "message request too large", failure: new Error("request too large for this model"), kind: "request_incompatible", retryable: true }, + { label: "message unsupported tool", failure: new Error("unsupported tool: computer-use"), kind: "request_incompatible", retryable: true }, + { label: "message parameter not supported", failure: new Error("parameter not supported by this model"), kind: "request_incompatible", retryable: true }, + { label: "message invalid request", failure: new Error("invalid request"), kind: "request_incompatible", retryable: true }, + { label: "message bad request", failure: { message: "400 bad request" }, kind: "request_incompatible", retryable: true }, + // Refusals/cancellations must win over request-incompatible wrappers. + { label: "aborted stopReason wrapper", failure: { status: 400, stopReason: "aborted", errorMessage: "aborted" }, kind: "cancelled", retryable: false }, + { label: "AbortError name wrapper", failure: { statusCode: 422, name: "AbortError", message: "aborted by user" }, kind: "cancelled", retryable: false }, + { label: "content filter wrapper", failure: { httpStatus: 400, message: "content_filter" }, kind: "task_failure", retryable: false }, + { label: "abort cause under invalid request", failure: abortWrappedError(), kind: "cancelled", retryable: false }, + { label: "cancel cause under 400 wrapper", failure: { message: "400 bad request", cause: { message: "request was cancelled" } }, kind: "cancelled", retryable: false }, + { label: "task failure cause under bad request", failure: { errorMessage: "bad request", cause: { message: "command failed: exit 1" } }, kind: "task_failure", retryable: false }, + { label: "content filter diagnostic under invalid_request_error", failure: { message: "invalid_request_error", diagnostics: [{ error: { finish_reason: "content_filter" } }] }, kind: "task_failure", retryable: false }, + { label: "safety diagnostic under 400 wrapper", failure: { errorMessage: "400 bad request", diagnostics: [{ error: { message: "blocked by safety policy" } }] }, kind: "task_failure", retryable: false }, + { label: "task failure diagnostic under 422", failure: { status: 422, diagnostics: [{ error: { message: "command failed" } }] }, kind: "task_failure", retryable: false }, + // Retryable nested causes must not flip a retryable wrapper. + { label: "rate limit cause under bad request", failure: { errorMessage: "bad request", cause: { message: "rate limit exceeded" } }, kind: "request_incompatible", retryable: true }, + // Common retryable classifications shared by both copies. + { label: "message rate limit", failure: new Error("Rate limit exceeded, retry later"), kind: "rate_limit", retryable: true }, + { label: "message api key", failure: new Error("No API key found for provider"), kind: "auth_on_candidate_provider", retryable: true }, + { label: "message model not found", failure: new Error("model not found: foo/bar"), kind: "model_unavailable", retryable: true }, + { label: "code etimedout", failure: { code: "ETIMEDOUT", message: "socket timed out" }, kind: "network_timeout", retryable: true }, + { label: "message overloaded", failure: new Error("provider is overloaded (529)"), kind: "provider_unavailable", retryable: true }, + // Non-retryable terminal failures. + { label: "message tests failed", failure: new Error("tests failed: 3 failures"), kind: "task_failure", retryable: false }, + { label: "message interrupted", failure: new Error("interrupted by user"), kind: "cancelled", retryable: false }, + { label: "unknown structured failure", failure: { message: "something inexplicable happened" }, kind: "unknown", retryable: false }, +]; + +describe("model fallback classifier conformance (subagents vs workflows)", () => { + test("both classifiers agree on the shared failure corpus", () => { + for (const fixture of CONFORMANCE_FIXTURES) { + const subagentsSignal = subagentsNormalize(fixture.failure); + const workflowsSignal = workflowsNormalize(fixture.failure); + assert.equal(subagentsSignal.kind, fixture.kind, `subagents kind for ${fixture.label}`); + assert.equal(workflowsSignal.kind, fixture.kind, `workflows kind for ${fixture.label}`); + assert.equal(subagentsIsRetryable(fixture.failure), fixture.retryable, `subagents retryable for ${fixture.label}`); + assert.equal(workflowsIsRetryable(fixture.failure), fixture.retryable, `workflows retryable for ${fixture.label}`); + } + }); + + test("classifiers agree on retryability for wrapper permutations", () => { + const wrappers: readonly unknown[] = [ + { status: 400, cause: { name: "AbortError", message: "aborted" } }, + { status: 413, stopReason: "aborted", errorMessage: "aborted" }, + { code: "context_window_exceeded", message: "context window exceeded" }, + { code: "request_entity_too_large", message: "entity too large" }, + { message: "invalid_request_error: bad input" }, + { message: "bad request body" }, + ]; + for (const [index, wrapper] of wrappers.entries()) { + assert.equal( + subagentsIsRetryable(wrapper), + workflowsIsRetryable(wrapper), + `retryability parity for wrapper #${index}`, + ); + } + }); +}); diff --git a/test/unit/model-fallback-request-incompatible.test.ts b/test/unit/model-fallback-request-incompatible.test.ts index fea253faf..93b77ab36 100644 --- a/test/unit/model-fallback-request-incompatible.test.ts +++ b/test/unit/model-fallback-request-incompatible.test.ts @@ -1,4 +1,3 @@ -// @ts-nocheck import { describe, test } from "bun:test"; import assert from "node:assert/strict"; import { diff --git a/test/unit/subagents-attempt-watchdog.test.ts b/test/unit/subagents-attempt-watchdog.test.ts index 509ca8ec7..262502b04 100644 --- a/test/unit/subagents-attempt-watchdog.test.ts +++ b/test/unit/subagents-attempt-watchdog.test.ts @@ -61,6 +61,9 @@ const successEvent = (text: string) => JSON.stringify({ }, }); +const toolStartEvent = JSON.stringify({ type: "tool_execution_start", toolName: "bash", args: { command: "sleep" } }); +const toolEndEvent = JSON.stringify({ type: "tool_execution_end", toolName: "bash" }); + describe("subagent per-attempt watchdog", () => { test("kills a stalled model attempt and advances to the next fallback", async () => { await withFakeCli(` @@ -119,6 +122,147 @@ describe("subagent per-attempt watchdog", () => { }); }); + test("does not trip the idle watchdog while a slow tool call is active", async () => { + await withFakeCli(` + console.log(${JSON.stringify(toolStartEvent)}); + setTimeout(() => { + console.log(${JSON.stringify(toolEndEvent)}); + console.log(${JSON.stringify(successEvent("tool ok"))}); + }, 700); + `, async (dir) => { + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-tool-active", + modelOverride: "provider-a/stalled", + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.error, undefined); + assert.match(result.finalOutput ?? "", /tool ok/); + assert.equal(result.modelAttempts?.length, 1); + assert.equal(result.modelAttempts?.[0]?.success, true); + }); + }); + + test("trips the idle watchdog when the child stalls after a tool call ends", async () => { + await withFakeCli(` + const modelIndex = process.argv.indexOf("--model"); + const model = modelIndex === -1 ? "default" : process.argv[modelIndex + 1]; + if (model === "provider-a/stalled") { + console.log(${JSON.stringify(toolStartEvent)}); + console.log(${JSON.stringify(toolEndEvent)}); + setInterval(() => {}, 1000); + } else { + console.log(${JSON.stringify(successEvent("fallback ok"))}); + } + `, async (dir) => { + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-post-tool-stall", + availableModels: [ + { provider: "provider-a", id: "stalled", fullId: "provider-a/stalled" }, + { provider: "provider-b", id: "working", fullId: "provider-b/working" }, + ], + knownModelProviders: ["provider-a", "provider-b"], + }); + + assert.equal(result.exitCode, 0); + assert.match(result.finalOutput ?? "", /fallback ok/); + assert.equal(result.modelAttempts?.length, 2); + assert.equal(result.modelAttempts?.[0]?.success, false); + assert.match(result.modelAttempts?.[0]?.error ?? "", /without child activity/i); + assert.equal(result.modelAttempts?.[1]?.success, true); + }); + }); + + test("enforces the wall-clock cap even with steady child activity", async () => { + await withFakeCli(` + const modelIndex = process.argv.indexOf("--model"); + const model = modelIndex === -1 ? "default" : process.argv[modelIndex + 1]; + if (model === "provider-a/stalled") setInterval(() => process.stderr.write("tick\\n"), 50); + else console.log(${JSON.stringify(successEvent("fallback ok"))}); + `, async (dir) => { + process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = "600"; + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-wall-cap", + availableModels: [ + { provider: "provider-a", id: "stalled", fullId: "provider-a/stalled" }, + { provider: "provider-b", id: "working", fullId: "provider-b/working" }, + ], + knownModelProviders: ["provider-a", "provider-b"], + }); + + assert.equal(result.exitCode, 0); + assert.match(result.finalOutput ?? "", /fallback ok/); + assert.equal(result.modelAttempts?.length, 2); + assert.equal(result.modelAttempts?.[0]?.success, false); + assert.match(result.modelAttempts?.[0]?.error ?? "", /timed out after 600ms\./i); + assert.doesNotMatch(result.modelAttempts?.[0]?.error ?? "", /without child activity/i); + assert.equal(result.modelAttempts?.[1]?.success, true); + }); + }); + + test("escalates to SIGKILL when the child ignores SIGTERM", async () => { + await withFakeCli(` + const modelIndex = process.argv.indexOf("--model"); + const model = modelIndex === -1 ? "default" : process.argv[modelIndex + 1]; + if (model === "provider-a/stalled") { + process.on("SIGTERM", () => {}); + setInterval(() => {}, 1000); + } else { + console.log(${JSON.stringify(successEvent("fallback ok"))}); + } + `, async (dir) => { + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-sigkill-escalation", + availableModels: [ + { provider: "provider-a", id: "stalled", fullId: "provider-a/stalled" }, + { provider: "provider-b", id: "working", fullId: "provider-b/working" }, + ], + knownModelProviders: ["provider-a", "provider-b"], + }); + + assert.equal(result.exitCode, 0); + assert.match(result.finalOutput ?? "", /fallback ok/); + assert.equal(result.modelAttempts?.length, 2); + assert.equal(result.modelAttempts?.[0]?.success, false); + assert.match(result.modelAttempts?.[0]?.error ?? "", /timed out/i); + assert.equal(result.modelAttempts?.[1]?.success, true); + }); + }); + + test("background runner keeps a silent tool call alive past the idle window", async () => { + await withFakeCli(` + console.log(${JSON.stringify(toolStartEvent)}); + setTimeout(() => { + console.log(${JSON.stringify(toolEndEvent)}); + console.log(${JSON.stringify(successEvent("background tool ok"))}); + }, 700); + `, async (dir) => { + const result = await runSingleStep({ + agent: "fake-worker", + task: "Do work", + inheritProjectContext: false, + inheritSkills: false, + }, { + previousOutput: "", + placeholder: "{previous}", + cwd: dir, + sessionEnabled: false, + id: "background-tool-active", + flatIndex: 0, + flatStepCount: 1, + outputFile: join(dir, "output.txt"), + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.error, undefined); + assert.match(result.output, /background tool ok/); + }); + }); + test("pre-spawn filtering skips known keyless providers but keeps unknowns and current model", () => { const filtered = filterSpawnableModelCandidates({ candidates: ["provider-a/missing", "custom/model", "provider-b/ready", "provider-a/current"], From 776c4b067d0c999b14a51348ae52c46845b6000c Mon Sep 17 00:00:00 2001 From: Norin Lavaee Date: Thu, 2 Jul 2026 00:38:55 -0700 Subject: [PATCH 5/6] fix(subagents): spawn default attempt when no candidates configured Address PR #1581 review feedback: - Background runner now mirrors the foreground empty-candidates distinction: an empty modelCandidates array with no pre-spawn skipped attempts means no candidates were ever configured (no primary, no fallbacks, no current model), so one default-model attempt is spawned instead of silently exiting 1 with no error and no spawn. A filtered-to-empty list (which always carries skipped attempts) is still respected and surfaced as an error. - Watchdog escape hatch: ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS / ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS set to 0 (or negative) now disable the corresponding per-attempt timeout; non-numeric values remain ignored and documented as such. - Tests: background default-attempt regression, idle-disabled via env, non-numeric/negative env resolution; docs + changelogs updated. Refs: #1581 Assistant-model: Claude Fable 5 --- packages/coding-agent/CHANGELOG.md | 2 + packages/coding-agent/docs/subagents.md | 2 +- packages/subagents/CHANGELOG.md | 5 ++ .../runs/background/subagent-runner-step.ts | 12 ++- .../src/runs/shared/attempt-watchdog.ts | 25 ++++-- test/unit/subagents-attempt-watchdog.test.ts | 88 +++++++++++++++++++ 6 files changed, 124 insertions(+), 10 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index be4d9bfe5..67364033d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,6 +6,7 @@ - Added dynamic GitHub Copilot model population from the live CAPI `/models` catalog: picker-enabled, non-disabled plain chat ids are synthesized from catalog metadata (endpoints, capabilities, limits, and display names) while built-in `pi-ai` definitions still win, namespaced enterprise deployments such as `org/deployment/model` are skipped, and cached catalog metadata enables the same models on cold start. - Added catalog-driven thinking-level gating for GitHub Copilot models so dynamically synthesized entries and bundled `pi-ai` Copilot models only offer the reasoning levels advertised by CAPI's `capabilities.supports.reasoning_effort` arrays, while models without an effort array keep their existing thinking behavior. +- Added a subagent watchdog escape hatch: setting `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` or `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS` to `0` (or a negative value) now disables the corresponding per-attempt timeout entirely; non-numeric values are ignored and the defaults apply ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). ### Fixed @@ -16,6 +17,7 @@ - Fixed foreground and background subagent model attempts that produced no child activity from hanging indefinitely. Atomic now bounds each candidate with an idle watchdog and wall-clock cap, records retryable timeout attempts so fallback continues, and skips known unauthenticated providers before spawning while preserving unknown/custom providers and the current-model last resort ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). - Fixed the subagent per-attempt idle watchdog so an in-flight tool execution counts as activity: a slow, quiet tool call (long build or test run with no interim output) is no longer killed as a stalled attempt; the wall-clock cap still bounds such attempts ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). - Aligned the subagents and workflows model-failure classifiers' direct-message precedence and added a cross-package conformance test suite so the two classifier copies cannot silently drift ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). +- Fixed the background subagent runner to spawn one default-model attempt when no model candidates were ever configured (no primary model, no fallbacks, and no current model), mirroring the foreground path instead of silently exiting 1 with no error and no spawn; an explicitly empty candidate list produced by pre-spawn auth filtering is still respected and surfaced as an error ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). ## [0.9.4-alpha.6] - 2026-07-01 diff --git a/packages/coding-agent/docs/subagents.md b/packages/coding-agent/docs/subagents.md index af8f2168e..b91913faa 100644 --- a/packages/coding-agent/docs/subagents.md +++ b/packages/coding-agent/docs/subagents.md @@ -177,7 +177,7 @@ Agents can define ordered `fallbackModels` for retryable provider or model failu A candidate that cannot serve the current request — for example an HTTP 400/413/422 bad/unprocessable/payload-too-large request, an unsupported tool or parameter, a context-length/context-window overflow, or a `too large` / `invalid_request` error — is treated as request/context incompatible and the chain advances to the next candidate rather than stopping. This means that if none of the configured candidates are applicable to the request, Atomic falls back to the currently selected user model instead of failing outright. -Each foreground and background model candidate is bounded by a per-attempt idle watchdog (default 5 minutes without child stdout, stderr, or JSON child events) and an absolute wall-clock cap (default 60 minutes). An in-flight tool execution counts as activity, so a slow, quiet tool call (a long build or test run that streams nothing until it finishes) is not mistaken for a stalled attempt; only the wall-clock cap bounds such attempts. If either watchdog trips, Atomic terminates that child attempt, records a retryable timeout in `modelAttempts`, and continues to the next fallback candidate. The defaults can be overridden with `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` and `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS`; `ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS` controls SIGTERM-to-SIGKILL escalation. +Each foreground and background model candidate is bounded by a per-attempt idle watchdog (default 5 minutes without child stdout, stderr, or JSON child events) and an absolute wall-clock cap (default 60 minutes). An in-flight tool execution counts as activity, so a slow, quiet tool call (a long build or test run that streams nothing until it finishes) is not mistaken for a stalled attempt; only the wall-clock cap bounds such attempts. If either watchdog trips, Atomic terminates that child attempt, records a retryable timeout in `modelAttempts`, and continues to the next fallback candidate. The defaults can be overridden with `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` and `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS`; `ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS` controls SIGTERM-to-SIGKILL escalation. Setting the idle or wall-clock variable to `0` (or a negative value) disables that timeout entirely; non-numeric values are ignored and the default applies. When registry availability shows that a known candidate provider has no configured auth, Atomic records a skipped model attempt before spawning a child. Unknown/custom providers are still attempted, and the current user-selected model appended as the final fallback is never filtered out by this pre-spawn check. diff --git a/packages/subagents/CHANGELOG.md b/packages/subagents/CHANGELOG.md index a8126a316..09491ed81 100644 --- a/packages/subagents/CHANGELOG.md +++ b/packages/subagents/CHANGELOG.md @@ -8,6 +8,11 @@ - Fixed foreground and background subagent model attempts that produced no child activity from hanging indefinitely. Each candidate attempt now has a conservative idle watchdog and absolute wall-clock cap, records a retryable timeout failure, and advances to the next fallback candidate; known providers without configured auth are skipped before spawning while unknown/custom providers and the current-model last resort are still attempted ([#1580](https://github.com/bastani-inc/atomic/issues/1580)). - Fixed the per-attempt idle watchdog so an in-flight tool execution counts as activity: a slow, quiet tool call (long build or test run with no interim output) is no longer killed as a stalled attempt; the wall-clock cap still bounds such attempts ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). - Aligned the subagents model-failure classifier's direct-message precedence with the workflows classifier (direct-message classification now runs after nested cause/diagnostic traversal and before outer status/code classification), and added a cross-package conformance test suite so the two classifier copies cannot silently drift ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). +- Fixed the background subagent runner to spawn one default-model attempt when no model candidates were ever configured (no primary model, no fallbacks, and no current model), mirroring the foreground path instead of silently exiting 1 with no error and no spawn. An explicitly empty candidate list produced by pre-spawn auth filtering is still respected and surfaced as an error ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). + +### Added + +- Added a watchdog escape hatch: setting `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` or `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS` to `0` (or a negative value) now disables the corresponding per-attempt timeout entirely; non-numeric values are ignored and the defaults apply ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). ## [0.9.4-alpha.6] - 2026-07-01 diff --git a/packages/subagents/src/runs/background/subagent-runner-step.ts b/packages/subagents/src/runs/background/subagent-runner-step.ts index b9ae96ca2..4c456aa5f 100644 --- a/packages/subagents/src/runs/background/subagent-runner-step.ts +++ b/packages/subagents/src/runs/background/subagent-runner-step.ts @@ -64,14 +64,20 @@ export async function runSingleStep( // `!== undefined` is intentional: an explicitly empty array means every candidate // was removed by pre-spawn filtering (see filterSpawnableModelCandidates) and must // be respected — do not "simplify" this back to `.length > 0`, which would spawn a - // doomed default attempt. The empty case is surfaced as an error below. - const candidates = step.modelCandidates !== undefined + // doomed default attempt. Pre-spawn filtering always records each removal as a + // skipped attempt in step.modelAttempts, so an empty array WITHOUT skipped attempts + // means no candidates were configured at all (no primary model, no fallbacks, no + // current model); mirror the foreground path (execution-run-sync.ts `modelsToTry`) + // and run one default-model attempt instead of silently exiting with no attempt. + // The filtered-to-empty case is surfaced as an error below. + const preSkippedAttempts = step.modelAttempts ?? []; + const candidates = step.modelCandidates !== undefined && (step.modelCandidates.length > 0 || preSkippedAttempts.length > 0) ? step.modelCandidates : step.model ? [step.model] : [undefined]; const attemptedModels: string[] = []; - const modelAttempts: ModelAttempt[] = [...(step.modelAttempts ?? [])]; + const modelAttempts: ModelAttempt[] = [...preSkippedAttempts]; const attemptNotes: string[] = modelAttempts .filter((attempt) => !attempt.success && attempt.exitCode === null && attempt.error) .map((attempt) => `[fallback] ${attempt.error}`); diff --git a/packages/subagents/src/runs/shared/attempt-watchdog.ts b/packages/subagents/src/runs/shared/attempt-watchdog.ts index 7c460195b..b55f1ef04 100644 --- a/packages/subagents/src/runs/shared/attempt-watchdog.ts +++ b/packages/subagents/src/runs/shared/attempt-watchdog.ts @@ -16,17 +16,27 @@ export interface AttemptWatchdog { clear(): void; } -function positiveEnvMs(name: string): number | undefined { +/** Parse a millisecond override from the environment. Non-numeric values are + * silently ignored (the default applies). Zero or negative values are clamped + * to `0`, which callers treat as "timeout disabled". */ +function envMs(name: string): number | undefined { const raw = process.env[name]; if (!raw) return undefined; const value = Number(raw); - return Number.isFinite(value) && value > 0 ? Math.floor(value) : undefined; + if (!Number.isFinite(value)) return undefined; + return value > 0 ? Math.floor(value) : 0; +} + +function positiveEnvMs(name: string): number | undefined { + const value = envMs(name); + return value !== undefined && value > 0 ? value : undefined; } export function resolveAttemptTimeoutConfig(): AttemptTimeoutConfig { return { - idleMs: positiveEnvMs("ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS") ?? DEFAULT_IDLE_MS, - wallMs: positiveEnvMs("ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS") ?? DEFAULT_WALL_MS, + // `0` (or a negative value) disables the corresponding timeout entirely. + idleMs: envMs("ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS") ?? DEFAULT_IDLE_MS, + wallMs: envMs("ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS") ?? DEFAULT_WALL_MS, killGraceMs: positiveEnvMs("ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS") ?? DEFAULT_KILL_GRACE_MS, }; } @@ -85,6 +95,7 @@ export function createAttemptWatchdog(params: { }; const scheduleIdle = () => { clearIdle(); + if (config.idleMs <= 0) return; // idle watchdog disabled idleTimer = setTimeout(() => { if (params.isToolActive?.()) { // Do not kill a healthy attempt that is busy inside a slow tool call; @@ -98,8 +109,10 @@ export function createAttemptWatchdog(params: { }; scheduleIdle(); - wallTimer = setTimeout(() => trip(wallTimeoutMessage(config.wallMs)), config.wallMs); - wallTimer.unref?.(); + if (config.wallMs > 0) { + wallTimer = setTimeout(() => trip(wallTimeoutMessage(config.wallMs)), config.wallMs); + wallTimer.unref?.(); + } return { activity() { diff --git a/test/unit/subagents-attempt-watchdog.test.ts b/test/unit/subagents-attempt-watchdog.test.ts index 262502b04..f7f7f55e9 100644 --- a/test/unit/subagents-attempt-watchdog.test.ts +++ b/test/unit/subagents-attempt-watchdog.test.ts @@ -8,6 +8,7 @@ import { runSingleStep } from "../../packages/subagents/src/runs/background/suba import { runSync } from "../../packages/subagents/src/runs/foreground/execution.js"; import { runForegroundParallelTasks } from "../../packages/subagents/src/runs/foreground/subagent-executor-parallel-task.js"; import { filterSpawnableModelCandidates } from "../../packages/subagents/src/runs/shared/model-candidate-filter.js"; +import { resolveAttemptTimeoutConfig } from "../../packages/subagents/src/runs/shared/attempt-watchdog.js"; function agentConfig(): AgentConfig { return { @@ -305,6 +306,93 @@ describe("subagent per-attempt watchdog", () => { }); }); + test("background runner spawns one default attempt when no candidates were ever configured", async () => { + await withFakeCli(` + const fs = require("node:fs"); + const path = require("node:path"); + const modelIndex = process.argv.indexOf("--model"); + fs.writeFileSync(path.join(process.cwd(), "spawned-model"), modelIndex === -1 ? "default" : process.argv[modelIndex + 1]); + console.log(${JSON.stringify(successEvent("default ok"))}); + `, async (dir) => { + // No primary model, no fallbacks, no current model: buildModelCandidates() + // yields [] and pre-spawn filtering records no skipped attempts. The runner + // must mirror the foreground path and run one default-model attempt instead + // of silently exiting 1 with no error and no spawn. + const result = await runSingleStep({ + agent: "fake-worker", + task: "Do work", + inheritProjectContext: false, + inheritSkills: false, + modelCandidates: [], + modelAttempts: [], + }, { + previousOutput: "", + placeholder: "{previous}", + cwd: dir, + sessionEnabled: false, + id: "background-default-attempt", + flatIndex: 0, + flatStepCount: 1, + outputFile: join(dir, "output.txt"), + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.error, undefined); + assert.match(result.output, /default ok/); + assert.equal(readFileSync(join(dir, "spawned-model"), "utf8"), "default"); + assert.equal(result.modelAttempts?.length, 1); + assert.equal(result.modelAttempts?.[0]?.success, true); + }); + }); + + test("ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS=0 disables the idle watchdog", async () => { + await withFakeCli(` + const modelIndex = process.argv.indexOf("--model"); + const model = modelIndex === -1 ? "default" : process.argv[modelIndex + 1]; + if (model === "provider-a/stalled") setInterval(() => {}, 1000); + else console.log(${JSON.stringify(successEvent("fallback ok"))}); + `, async (dir) => { + process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS = "0"; + process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = "600"; + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-idle-disabled", + availableModels: [ + { provider: "provider-a", id: "stalled", fullId: "provider-a/stalled" }, + { provider: "provider-b", id: "working", fullId: "provider-b/working" }, + ], + knownModelProviders: ["provider-a", "provider-b"], + }); + + assert.equal(result.exitCode, 0); + assert.match(result.finalOutput ?? "", /fallback ok/); + assert.equal(result.modelAttempts?.length, 2); + assert.equal(result.modelAttempts?.[0]?.success, false); + // A fully silent child must outlive the (disabled) idle window and only be + // bounded by the wall-clock cap. + assert.match(result.modelAttempts?.[0]?.error ?? "", /timed out after 600ms\./i); + assert.doesNotMatch(result.modelAttempts?.[0]?.error ?? "", /without child activity/i); + assert.equal(result.modelAttempts?.[1]?.success, true); + }); + }); + + test("non-numeric watchdog overrides are ignored and defaults apply", async () => { + const previousIdle = process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS; + const previousWall = process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS; + process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS = "soon"; + process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = "-100"; + try { + const config = resolveAttemptTimeoutConfig(); + assert.equal(config.idleMs, 5 * 60_000); + assert.equal(config.wallMs, 0); + } finally { + if (previousIdle === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS; + else process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS = previousIdle; + if (previousWall === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS; + else process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = previousWall; + } + }); + test("background runner reports a useful error when every candidate was filtered", async () => { const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-background-filtered-")); try { From 723d21b6f2eecce85f129c779a8c4de307b4db09 Mon Sep 17 00:00:00 2001 From: Norin Lavaee Date: Thu, 2 Jul 2026 01:00:21 -0700 Subject: [PATCH 6/6] docs(subagents): address second-pass PR #1581 review feedback - Document the abnormal-tool-end caveat at both isToolActive callsites: if a tool never emits its end event the idle watchdog is deferred indefinitely and the wall-clock cap is the sole backstop; add a foreground test proving the wall cap terminates such attempts. - Document that ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS intentionally cannot be disabled (0/negative/non-numeric fall back to the default) in the resolver, subagents.md, and both changelogs. - Note in the classifier conformance suite that it enforces behavioral (not structural) parity over a finite corpus, that shared-module extraction is blocked by the package split, and that both copies plus a fixture must be updated together; add a fixture documenting the known benign-"too large" message false positive. - Split the watchdog test suite (shared helpers + separate pre-spawn candidate-filtering suite) to stay under the 500-line file gate. Refs: #1581 Assistant-model: Claude Fable 5 --- packages/coding-agent/CHANGELOG.md | 2 +- packages/coding-agent/docs/subagents.md | 2 +- packages/subagents/CHANGELOG.md | 2 +- .../background/subagent-runner-streaming.ts | 5 + .../src/runs/foreground/execution-attempt.ts | 4 + .../src/runs/shared/attempt-watchdog.ts | 3 + ...el-fallback-classifier-conformance.test.ts | 16 ++ .../subagents-attempt-watchdog-helpers.ts | 65 +++++ test/unit/subagents-attempt-watchdog.test.ts | 266 +++--------------- ...ubagents-model-candidate-filtering.test.ts | 181 ++++++++++++ 10 files changed, 314 insertions(+), 232 deletions(-) create mode 100644 test/unit/subagents-attempt-watchdog-helpers.ts create mode 100644 test/unit/subagents-model-candidate-filtering.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 67364033d..68e157fb8 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,7 +6,7 @@ - Added dynamic GitHub Copilot model population from the live CAPI `/models` catalog: picker-enabled, non-disabled plain chat ids are synthesized from catalog metadata (endpoints, capabilities, limits, and display names) while built-in `pi-ai` definitions still win, namespaced enterprise deployments such as `org/deployment/model` are skipped, and cached catalog metadata enables the same models on cold start. - Added catalog-driven thinking-level gating for GitHub Copilot models so dynamically synthesized entries and bundled `pi-ai` Copilot models only offer the reasoning levels advertised by CAPI's `capabilities.supports.reasoning_effort` arrays, while models without an effort array keep their existing thinking behavior. -- Added a subagent watchdog escape hatch: setting `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` or `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS` to `0` (or a negative value) now disables the corresponding per-attempt timeout entirely; non-numeric values are ignored and the defaults apply ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). +- Added a subagent watchdog escape hatch: setting `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` or `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS` to `0` (or a negative value) now disables the corresponding per-attempt timeout entirely; non-numeric values are ignored and the defaults apply. The `ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS` SIGTERM→SIGKILL grace period intentionally cannot be disabled — `0`, negative, or non-numeric values fall back to its default so escalation always stays bounded ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). ### Fixed diff --git a/packages/coding-agent/docs/subagents.md b/packages/coding-agent/docs/subagents.md index b91913faa..dade10ec5 100644 --- a/packages/coding-agent/docs/subagents.md +++ b/packages/coding-agent/docs/subagents.md @@ -177,7 +177,7 @@ Agents can define ordered `fallbackModels` for retryable provider or model failu A candidate that cannot serve the current request — for example an HTTP 400/413/422 bad/unprocessable/payload-too-large request, an unsupported tool or parameter, a context-length/context-window overflow, or a `too large` / `invalid_request` error — is treated as request/context incompatible and the chain advances to the next candidate rather than stopping. This means that if none of the configured candidates are applicable to the request, Atomic falls back to the currently selected user model instead of failing outright. -Each foreground and background model candidate is bounded by a per-attempt idle watchdog (default 5 minutes without child stdout, stderr, or JSON child events) and an absolute wall-clock cap (default 60 minutes). An in-flight tool execution counts as activity, so a slow, quiet tool call (a long build or test run that streams nothing until it finishes) is not mistaken for a stalled attempt; only the wall-clock cap bounds such attempts. If either watchdog trips, Atomic terminates that child attempt, records a retryable timeout in `modelAttempts`, and continues to the next fallback candidate. The defaults can be overridden with `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` and `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS`; `ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS` controls SIGTERM-to-SIGKILL escalation. Setting the idle or wall-clock variable to `0` (or a negative value) disables that timeout entirely; non-numeric values are ignored and the default applies. +Each foreground and background model candidate is bounded by a per-attempt idle watchdog (default 5 minutes without child stdout, stderr, or JSON child events) and an absolute wall-clock cap (default 60 minutes). An in-flight tool execution counts as activity, so a slow, quiet tool call (a long build or test run that streams nothing until it finishes) is not mistaken for a stalled attempt; only the wall-clock cap bounds such attempts. If either watchdog trips, Atomic terminates that child attempt, records a retryable timeout in `modelAttempts`, and continues to the next fallback candidate. The defaults can be overridden with `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` and `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS`; `ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS` controls SIGTERM-to-SIGKILL escalation. Setting the idle or wall-clock variable to `0` (or a negative value) disables that timeout entirely; non-numeric values are ignored and the default applies. The kill-grace period cannot be disabled — `0`, negative, or non-numeric values fall back to its default so escalation always stays bounded. When registry availability shows that a known candidate provider has no configured auth, Atomic records a skipped model attempt before spawning a child. Unknown/custom providers are still attempted, and the current user-selected model appended as the final fallback is never filtered out by this pre-spawn check. diff --git a/packages/subagents/CHANGELOG.md b/packages/subagents/CHANGELOG.md index 09491ed81..fb2c3fb6c 100644 --- a/packages/subagents/CHANGELOG.md +++ b/packages/subagents/CHANGELOG.md @@ -12,7 +12,7 @@ ### Added -- Added a watchdog escape hatch: setting `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` or `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS` to `0` (or a negative value) now disables the corresponding per-attempt timeout entirely; non-numeric values are ignored and the defaults apply ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). +- Added a watchdog escape hatch: setting `ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS` or `ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS` to `0` (or a negative value) now disables the corresponding per-attempt timeout entirely; non-numeric values are ignored and the defaults apply. The `ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS` SIGTERM→SIGKILL grace period intentionally cannot be disabled — `0`, negative, or non-numeric values fall back to its default so escalation always stays bounded ([#1581](https://github.com/bastani-inc/atomic/pull/1581)). ## [0.9.4-alpha.6] - 2026-07-01 diff --git a/packages/subagents/src/runs/background/subagent-runner-streaming.ts b/packages/subagents/src/runs/background/subagent-runner-streaming.ts index f34a8bb97..dfa8501f4 100644 --- a/packages/subagents/src/runs/background/subagent-runner-streaming.ts +++ b/packages/subagents/src/runs/background/subagent-runner-streaming.ts @@ -173,6 +173,11 @@ export function runPiStreaming( const attemptWatchdog = createAttemptWatchdog({ child, isSettled: () => settled, + // An in-flight tool execution counts as watchdog activity so a slow, quiet + // tool call is not mistaken for a stalled attempt. Caveat: the counter only + // decrements on tool_execution_end, so if a tool ends abnormally without its + // end event the idle watchdog is deferred indefinitely and the wall-clock cap + // becomes the sole backstop — do not lower the wall cap assuming idle fires. isToolActive: () => activeToolExecutions > 0, onTimeout(message) { forcedTerminationSignal = true; diff --git a/packages/subagents/src/runs/foreground/execution-attempt.ts b/packages/subagents/src/runs/foreground/execution-attempt.ts index fd5d0c08f..ff212887d 100644 --- a/packages/subagents/src/runs/foreground/execution-attempt.ts +++ b/packages/subagents/src/runs/foreground/execution-attempt.ts @@ -365,6 +365,10 @@ export async function runSingleAttempt( isSettled: () => settled || processClosed || detached, // A slow, quiet tool call (long build/test run) must not be mistaken for a // stalled attempt: an in-flight tool execution counts as watchdog activity. + // Caveat: `progress.currentTool` is cleared on tool_execution_end, so if a + // tool ends abnormally without emitting its end event the idle watchdog is + // deferred indefinitely and the wall-clock cap becomes the sole backstop for + // this attempt — do not lower the wall cap assuming idle always fires. isToolActive: () => progress.currentTool !== undefined, onTimeout(message) { forcedTerminationSignal = true; diff --git a/packages/subagents/src/runs/shared/attempt-watchdog.ts b/packages/subagents/src/runs/shared/attempt-watchdog.ts index b55f1ef04..74123d1c5 100644 --- a/packages/subagents/src/runs/shared/attempt-watchdog.ts +++ b/packages/subagents/src/runs/shared/attempt-watchdog.ts @@ -37,6 +37,9 @@ export function resolveAttemptTimeoutConfig(): AttemptTimeoutConfig { // `0` (or a negative value) disables the corresponding timeout entirely. idleMs: envMs("ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS") ?? DEFAULT_IDLE_MS, wallMs: envMs("ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS") ?? DEFAULT_WALL_MS, + // The SIGTERM→SIGKILL grace period intentionally cannot be disabled: once a + // watchdog trips, escalation must always be bounded. `0`, negative, or + // non-numeric values fall back to the default. killGraceMs: positiveEnvMs("ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS") ?? DEFAULT_KILL_GRACE_MS, }; } diff --git a/test/unit/model-fallback-classifier-conformance.test.ts b/test/unit/model-fallback-classifier-conformance.test.ts index eeef79909..ebae0190d 100644 --- a/test/unit/model-fallback-classifier-conformance.test.ts +++ b/test/unit/model-fallback-classifier-conformance.test.ts @@ -15,6 +15,14 @@ import { // suite runs a shared corpus of failure fixtures through both and asserts they // agree, so a change to one copy that silently diverges the other fails here. // +// Limitations: this enforces BEHAVIORAL parity over a finite corpus only — the two +// copies are structurally different (e.g. the subagents copy combines regex +// alternations that the workflows copy keeps separate), and a divergence on an +// input outside this corpus would pass CI silently. When adding or changing a +// classification rule, update BOTH copies and add a fixture here that exercises +// the new rule. (Extracting a single shared module is blocked by the mandated +// package split; the copies must not import each other.) +// // Known intentional difference (not covered by the shared corpus): the workflows // classifier has an extra `transport_error` kind for bare "connection error." / // "fetch failed." wrapper messages that the subagents classifier does not model. @@ -54,6 +62,14 @@ const CONFORMANCE_FIXTURES: readonly Fixture[] = [ { label: "message parameter not supported", failure: new Error("parameter not supported by this model"), kind: "request_incompatible", retryable: true }, { label: "message invalid request", failure: new Error("invalid request"), kind: "request_incompatible", retryable: true }, { label: "message bad request", failure: { message: "400 bad request" }, kind: "request_incompatible", retryable: true }, + // Known residual fragility of message-substring classification (documented on + // purpose): a benign task error that merely contains "too large" is classified + // as retryable request_incompatible by BOTH copies when no structured + // status/code and no task-failure/refusal keywords are present. Structured + // signals and refusal/cancel/task-failure patterns are checked first, which + // bounds the blast radius; if either copy tightens the bare "too large" + // pattern, update this fixture in lockstep. + { label: "benign message containing too large", failure: new Error("generated diff is too large to display inline"), kind: "request_incompatible", retryable: true }, // Refusals/cancellations must win over request-incompatible wrappers. { label: "aborted stopReason wrapper", failure: { status: 400, stopReason: "aborted", errorMessage: "aborted" }, kind: "cancelled", retryable: false }, { label: "AbortError name wrapper", failure: { statusCode: 422, name: "AbortError", message: "aborted by user" }, kind: "cancelled", retryable: false }, diff --git a/test/unit/subagents-attempt-watchdog-helpers.ts b/test/unit/subagents-attempt-watchdog-helpers.ts new file mode 100644 index 000000000..a46ebf03e --- /dev/null +++ b/test/unit/subagents-attempt-watchdog-helpers.ts @@ -0,0 +1,65 @@ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { AgentConfig } from "../../packages/subagents/src/agents/agents.js"; + +/** Shared fixtures for the subagent attempt-watchdog and model-candidate + * filtering test suites (split to satisfy the 500-line file gate). */ + +export function agentConfig(): AgentConfig { + return { + name: "fake-worker", + description: "Fake worker", + source: "project", + filePath: "fake-worker.md", + systemPrompt: "Work.", + systemPromptMode: "replace", + inheritProjectContext: false, + inheritSkills: false, + model: "provider-a/stalled", + fallbackModels: ["provider-b/working"], + }; +} + +/** Runs `fn` with process.argv[1] pointed at a fake pi CLI script and short + * watchdog timeouts (idle 250ms, wall 2000ms, kill grace 20ms); restores the + * previous argv/env afterwards. */ +export async function withFakeCli(script: string, fn: (dir: string) => Promise): Promise { + const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-watchdog-")); + const scriptPath = join(dir, "fake-pi.js"); + const previousArgv1 = process.argv[1]; + const previousIdle = process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS; + const previousWall = process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS; + const previousKill = process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS; + writeFileSync(scriptPath, script, { mode: 0o700 }); + process.argv[1] = scriptPath; + process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS = "250"; + process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = "2000"; + process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS = "20"; + try { + return await fn(dir); + } finally { + process.argv[1] = previousArgv1; + if (previousIdle === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS; + else process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS = previousIdle; + if (previousWall === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS; + else process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = previousWall; + if (previousKill === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS; + else process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS = previousKill; + rmSync(dir, { recursive: true, force: true }); + } +} + +export const successEvent = (text: string) => JSON.stringify({ + type: "message_end", + message: { + role: "assistant", + content: [{ type: "text", text }], + stopReason: "stop", + usage: { input: 1, output: 1 }, + timestamp: Date.now(), + }, +}); + +export const toolStartEvent = JSON.stringify({ type: "tool_execution_start", toolName: "bash", args: { command: "sleep" } }); +export const toolEndEvent = JSON.stringify({ type: "tool_execution_end", toolName: "bash" }); diff --git a/test/unit/subagents-attempt-watchdog.test.ts b/test/unit/subagents-attempt-watchdog.test.ts index f7f7f55e9..b6f3361aa 100644 --- a/test/unit/subagents-attempt-watchdog.test.ts +++ b/test/unit/subagents-attempt-watchdog.test.ts @@ -1,69 +1,11 @@ import { describe, test } from "bun:test"; import assert from "node:assert/strict"; -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; -import { tmpdir } from "node:os"; +import { readFileSync } from "node:fs"; import { join } from "node:path"; -import type { AgentConfig } from "../../packages/subagents/src/agents/agents.js"; import { runSingleStep } from "../../packages/subagents/src/runs/background/subagent-runner-step.js"; import { runSync } from "../../packages/subagents/src/runs/foreground/execution.js"; -import { runForegroundParallelTasks } from "../../packages/subagents/src/runs/foreground/subagent-executor-parallel-task.js"; -import { filterSpawnableModelCandidates } from "../../packages/subagents/src/runs/shared/model-candidate-filter.js"; import { resolveAttemptTimeoutConfig } from "../../packages/subagents/src/runs/shared/attempt-watchdog.js"; - -function agentConfig(): AgentConfig { - return { - name: "fake-worker", - description: "Fake worker", - source: "project", - filePath: "fake-worker.md", - systemPrompt: "Work.", - systemPromptMode: "replace", - inheritProjectContext: false, - inheritSkills: false, - model: "provider-a/stalled", - fallbackModels: ["provider-b/working"], - }; -} - -async function withFakeCli(script: string, fn: (dir: string) => Promise): Promise { - const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-watchdog-")); - const scriptPath = join(dir, "fake-pi.js"); - const previousArgv1 = process.argv[1]; - const previousIdle = process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS; - const previousWall = process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS; - const previousKill = process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS; - writeFileSync(scriptPath, script, { mode: 0o700 }); - process.argv[1] = scriptPath; - process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS = "250"; - process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = "2000"; - process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS = "20"; - try { - return await fn(dir); - } finally { - process.argv[1] = previousArgv1; - if (previousIdle === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS; - else process.env.ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS = previousIdle; - if (previousWall === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS; - else process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = previousWall; - if (previousKill === undefined) delete process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS; - else process.env.ATOMIC_SUBAGENT_ATTEMPT_KILL_GRACE_MS = previousKill; - rmSync(dir, { recursive: true, force: true }); - } -} - -const successEvent = (text: string) => JSON.stringify({ - type: "message_end", - message: { - role: "assistant", - content: [{ type: "text", text }], - stopReason: "stop", - usage: { input: 1, output: 1 }, - timestamp: Date.now(), - }, -}); - -const toolStartEvent = JSON.stringify({ type: "tool_execution_start", toolName: "bash", args: { command: "sleep" } }); -const toolEndEvent = JSON.stringify({ type: "tool_execution_end", toolName: "bash" }); +import { agentConfig, successEvent, toolEndEvent, toolStartEvent, withFakeCli } from "./subagents-attempt-watchdog-helpers.js"; describe("subagent per-attempt watchdog", () => { test("kills a stalled model attempt and advances to the next fallback", async () => { @@ -176,6 +118,41 @@ describe("subagent per-attempt watchdog", () => { }); }); + test("wall-clock cap is the backstop when a tool never emits its end event", async () => { + await withFakeCli(` + const modelIndex = process.argv.indexOf("--model"); + const model = modelIndex === -1 ? "default" : process.argv[modelIndex + 1]; + if (model === "provider-a/stalled") { + // Tool starts but its end event is never emitted (abnormal tool end): + // isToolActive stays true, so the idle watchdog is deferred indefinitely + // and the wall-clock cap must terminate the attempt. + console.log(${JSON.stringify(toolStartEvent)}); + setInterval(() => {}, 1000); + } else { + console.log(${JSON.stringify(successEvent("fallback ok"))}); + } + `, async (dir) => { + process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = "700"; + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-abnormal-tool-end", + availableModels: [ + { provider: "provider-a", id: "stalled", fullId: "provider-a/stalled" }, + { provider: "provider-b", id: "working", fullId: "provider-b/working" }, + ], + knownModelProviders: ["provider-a", "provider-b"], + }); + + assert.equal(result.exitCode, 0); + assert.match(result.finalOutput ?? "", /fallback ok/); + assert.equal(result.modelAttempts?.length, 2); + assert.equal(result.modelAttempts?.[0]?.success, false); + assert.match(result.modelAttempts?.[0]?.error ?? "", /timed out after 700ms\./i); + assert.doesNotMatch(result.modelAttempts?.[0]?.error ?? "", /without child activity/i); + assert.equal(result.modelAttempts?.[1]?.success, true); + }); + }); + test("enforces the wall-clock cap even with steady child activity", async () => { await withFakeCli(` const modelIndex = process.argv.indexOf("--model"); @@ -264,87 +241,6 @@ describe("subagent per-attempt watchdog", () => { }); }); - test("pre-spawn filtering skips known keyless providers but keeps unknowns and current model", () => { - const filtered = filterSpawnableModelCandidates({ - candidates: ["provider-a/missing", "custom/model", "provider-b/ready", "provider-a/current"], - availableModels: [{ provider: "provider-b", id: "ready", fullId: "provider-b/ready" }], - knownModelProviders: ["provider-a", "provider-b"], - currentModel: "provider-a/current", - }); - - assert.deepEqual(filtered.candidates, ["custom/model", "provider-b/ready", "provider-a/current"]); - assert.deepEqual(filtered.skippedAttempts.map((attempt) => attempt.model), ["provider-a/missing"]); - assert.match(filtered.skippedAttempts[0]?.error ?? "", /no configured API key\/auth/); - - const noAvailable = filterSpawnableModelCandidates({ - candidates: ["provider-a/missing", "custom/model"], - availableModels: [], - knownModelProviders: ["provider-a"], - }); - assert.deepEqual(noAvailable.candidates, ["custom/model"]); - assert.deepEqual(noAvailable.skippedAttempts.map((attempt) => attempt.model), ["provider-a/missing"]); - }); - - test("does not spawn a default foreground child when every configured candidate is filtered", async () => { - await withFakeCli(` - const fs = require("node:fs"); - const path = require("node:path"); - fs.writeFileSync(path.join(process.cwd(), "spawned"), "yes"); - console.log(${JSON.stringify(successEvent("should not run"))}); - `, async (dir) => { - const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { - cwd: dir, - runId: "watchdog-all-filtered", - availableModels: [], - knownModelProviders: ["provider-a", "provider-b"], - }); - - assert.equal(result.exitCode, 1); - assert.match(result.error ?? "", /No spawnable subagent model candidates/); - assert.equal(existsSync(join(dir, "spawned")), false); - assert.deepEqual(result.modelAttempts?.map((attempt) => attempt.model), ["provider-a/stalled", "provider-b/working"]); - }); - }); - - test("background runner spawns one default attempt when no candidates were ever configured", async () => { - await withFakeCli(` - const fs = require("node:fs"); - const path = require("node:path"); - const modelIndex = process.argv.indexOf("--model"); - fs.writeFileSync(path.join(process.cwd(), "spawned-model"), modelIndex === -1 ? "default" : process.argv[modelIndex + 1]); - console.log(${JSON.stringify(successEvent("default ok"))}); - `, async (dir) => { - // No primary model, no fallbacks, no current model: buildModelCandidates() - // yields [] and pre-spawn filtering records no skipped attempts. The runner - // must mirror the foreground path and run one default-model attempt instead - // of silently exiting 1 with no error and no spawn. - const result = await runSingleStep({ - agent: "fake-worker", - task: "Do work", - inheritProjectContext: false, - inheritSkills: false, - modelCandidates: [], - modelAttempts: [], - }, { - previousOutput: "", - placeholder: "{previous}", - cwd: dir, - sessionEnabled: false, - id: "background-default-attempt", - flatIndex: 0, - flatStepCount: 1, - outputFile: join(dir, "output.txt"), - }); - - assert.equal(result.exitCode, 0); - assert.equal(result.error, undefined); - assert.match(result.output, /default ok/); - assert.equal(readFileSync(join(dir, "spawned-model"), "utf8"), "default"); - assert.equal(result.modelAttempts?.length, 1); - assert.equal(result.modelAttempts?.[0]?.success, true); - }); - }); - test("ATOMIC_SUBAGENT_ATTEMPT_IDLE_TIMEOUT_MS=0 disables the idle watchdog", async () => { await withFakeCli(` const modelIndex = process.argv.indexOf("--model"); @@ -392,92 +288,4 @@ describe("subagent per-attempt watchdog", () => { else process.env.ATOMIC_SUBAGENT_ATTEMPT_TIMEOUT_MS = previousWall; } }); - - test("background runner reports a useful error when every candidate was filtered", async () => { - const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-background-filtered-")); - try { - const result = await runSingleStep({ - agent: "fake-worker", - task: "Do work", - inheritProjectContext: false, - inheritSkills: false, - modelCandidates: [], - modelAttempts: [{ - model: "provider-a/stalled", - success: false, - exitCode: null, - error: "Skipped provider-a/stalled: provider 'provider-a' has no configured API key/auth in the current session.", - }], - }, { - previousOutput: "", - placeholder: "{previous}", - cwd: dir, - sessionEnabled: false, - id: "background-all-filtered", - flatIndex: 0, - flatStepCount: 1, - outputFile: join(dir, "output.txt"), - }); - - assert.equal(result.exitCode, 1); - assert.match(result.error ?? "", /No spawnable subagent model candidates/); - assert.match(result.output, /Skipped provider-a\/stalled/); - } finally { - rmSync(dir, { recursive: true, force: true }); - } - }); - - test("foreground parallel tasks pass known providers into runSync", async () => { - const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-parallel-known-")); - try { - const knownModelProviders = ["provider-a", "provider-b"]; - const captured: Array<{ knownModelProviders?: string[] }> = []; - const input: Parameters[0] = { - tasks: [{ agent: "fake-worker", task: "Do work" }], - taskTexts: ["Do work"], - agents: [agentConfig()], - ctx: { - cwd: dir, - model: { provider: "provider-b", id: "working" }, - } as Parameters[0]["ctx"], - intercomEvents: {} as Parameters[0]["intercomEvents"], - signal: new AbortController().signal, - runId: "parallel-known-providers", - sessionDirForIndex: () => undefined, - sessionFileForIndex: () => undefined, - shareEnabled: false, - artifactConfig: { enabled: false, includeInput: false, includeOutput: false, includeJsonl: false, includeMetadata: false, cleanupDays: 0 }, - artifactsDir: dir, - paramsCwd: dir, - maxSubagentDepths: [0], - availableModels: [{ provider: "provider-b", id: "working", fullId: "provider-b/working" }], - knownModelProviders, - modelOverrides: ["provider-a/stalled"], - behaviors: [{ output: false, outputMode: "inline", reads: false, progress: false, skills: false }], - firstProgressIndex: -1, - controlConfig: { enabled: false, needsAttentionAfterMs: 1, activeNoticeAfterMs: 1, failedToolAttemptsBeforeAttention: 1, notifyOn: [], notifyChannels: [] }, - concurrencyLimit: 1, - liveResults: [], - liveProgress: [], - runtime: { - async runSync(_cwd, _agents, agentName, task, options) { - captured.push({ knownModelProviders: options.knownModelProviders }); - return { - agent: agentName, - task, - exitCode: 0, - messages: [], - usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, cost: 0, turns: 0 }, - finalOutput: "ok", - }; - }, - }, - }; - - await runForegroundParallelTasks(input); - assert.deepEqual(captured, [{ knownModelProviders }]); - } finally { - rmSync(dir, { recursive: true, force: true }); - } - }); }); diff --git a/test/unit/subagents-model-candidate-filtering.test.ts b/test/unit/subagents-model-candidate-filtering.test.ts new file mode 100644 index 000000000..330bc35b2 --- /dev/null +++ b/test/unit/subagents-model-candidate-filtering.test.ts @@ -0,0 +1,181 @@ +import { describe, test } from "bun:test"; +import assert from "node:assert/strict"; +import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { runSingleStep } from "../../packages/subagents/src/runs/background/subagent-runner-step.js"; +import { runSync } from "../../packages/subagents/src/runs/foreground/execution.js"; +import { runForegroundParallelTasks } from "../../packages/subagents/src/runs/foreground/subagent-executor-parallel-task.js"; +import { filterSpawnableModelCandidates } from "../../packages/subagents/src/runs/shared/model-candidate-filter.js"; +import { agentConfig, successEvent, withFakeCli } from "./subagents-attempt-watchdog-helpers.js"; + +describe("subagent pre-spawn model candidate filtering", () => { + test("pre-spawn filtering skips known keyless providers but keeps unknowns and current model", () => { + const filtered = filterSpawnableModelCandidates({ + candidates: ["provider-a/missing", "custom/model", "provider-b/ready", "provider-a/current"], + availableModels: [{ provider: "provider-b", id: "ready", fullId: "provider-b/ready" }], + knownModelProviders: ["provider-a", "provider-b"], + currentModel: "provider-a/current", + }); + + assert.deepEqual(filtered.candidates, ["custom/model", "provider-b/ready", "provider-a/current"]); + assert.deepEqual(filtered.skippedAttempts.map((attempt) => attempt.model), ["provider-a/missing"]); + assert.match(filtered.skippedAttempts[0]?.error ?? "", /no configured API key\/auth/); + + const noAvailable = filterSpawnableModelCandidates({ + candidates: ["provider-a/missing", "custom/model"], + availableModels: [], + knownModelProviders: ["provider-a"], + }); + assert.deepEqual(noAvailable.candidates, ["custom/model"]); + assert.deepEqual(noAvailable.skippedAttempts.map((attempt) => attempt.model), ["provider-a/missing"]); + }); + + test("does not spawn a default foreground child when every configured candidate is filtered", async () => { + await withFakeCli(` + const fs = require("node:fs"); + const path = require("node:path"); + fs.writeFileSync(path.join(process.cwd(), "spawned"), "yes"); + console.log(${JSON.stringify(successEvent("should not run"))}); + `, async (dir) => { + const result = await runSync(dir, [agentConfig()], "fake-worker", "Do work", { + cwd: dir, + runId: "watchdog-all-filtered", + availableModels: [], + knownModelProviders: ["provider-a", "provider-b"], + }); + + assert.equal(result.exitCode, 1); + assert.match(result.error ?? "", /No spawnable subagent model candidates/); + assert.equal(existsSync(join(dir, "spawned")), false); + assert.deepEqual(result.modelAttempts?.map((attempt) => attempt.model), ["provider-a/stalled", "provider-b/working"]); + }); + }); + + test("background runner spawns one default attempt when no candidates were ever configured", async () => { + await withFakeCli(` + const fs = require("node:fs"); + const path = require("node:path"); + const modelIndex = process.argv.indexOf("--model"); + fs.writeFileSync(path.join(process.cwd(), "spawned-model"), modelIndex === -1 ? "default" : process.argv[modelIndex + 1]); + console.log(${JSON.stringify(successEvent("default ok"))}); + `, async (dir) => { + // No primary model, no fallbacks, no current model: buildModelCandidates() + // yields [] and pre-spawn filtering records no skipped attempts. The runner + // must mirror the foreground path and run one default-model attempt instead + // of silently exiting 1 with no error and no spawn. + const result = await runSingleStep({ + agent: "fake-worker", + task: "Do work", + inheritProjectContext: false, + inheritSkills: false, + modelCandidates: [], + modelAttempts: [], + }, { + previousOutput: "", + placeholder: "{previous}", + cwd: dir, + sessionEnabled: false, + id: "background-default-attempt", + flatIndex: 0, + flatStepCount: 1, + outputFile: join(dir, "output.txt"), + }); + + assert.equal(result.exitCode, 0); + assert.equal(result.error, undefined); + assert.match(result.output, /default ok/); + assert.equal(readFileSync(join(dir, "spawned-model"), "utf8"), "default"); + assert.equal(result.modelAttempts?.length, 1); + assert.equal(result.modelAttempts?.[0]?.success, true); + }); + }); + + test("background runner reports a useful error when every candidate was filtered", async () => { + const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-background-filtered-")); + try { + const result = await runSingleStep({ + agent: "fake-worker", + task: "Do work", + inheritProjectContext: false, + inheritSkills: false, + modelCandidates: [], + modelAttempts: [{ + model: "provider-a/stalled", + success: false, + exitCode: null, + error: "Skipped provider-a/stalled: provider 'provider-a' has no configured API key/auth in the current session.", + }], + }, { + previousOutput: "", + placeholder: "{previous}", + cwd: dir, + sessionEnabled: false, + id: "background-all-filtered", + flatIndex: 0, + flatStepCount: 1, + outputFile: join(dir, "output.txt"), + }); + + assert.equal(result.exitCode, 1); + assert.match(result.error ?? "", /No spawnable subagent model candidates/); + assert.match(result.output, /Skipped provider-a\/stalled/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + test("foreground parallel tasks pass known providers into runSync", async () => { + const dir = mkdtempSync(join(tmpdir(), "atomic-subagent-parallel-known-")); + try { + const knownModelProviders = ["provider-a", "provider-b"]; + const captured: Array<{ knownModelProviders?: string[] }> = []; + const input: Parameters[0] = { + tasks: [{ agent: "fake-worker", task: "Do work" }], + taskTexts: ["Do work"], + agents: [agentConfig()], + ctx: { + cwd: dir, + model: { provider: "provider-b", id: "working" }, + } as Parameters[0]["ctx"], + intercomEvents: {} as Parameters[0]["intercomEvents"], + signal: new AbortController().signal, + runId: "parallel-known-providers", + sessionDirForIndex: () => undefined, + sessionFileForIndex: () => undefined, + shareEnabled: false, + artifactConfig: { enabled: false, includeInput: false, includeOutput: false, includeJsonl: false, includeMetadata: false, cleanupDays: 0 }, + artifactsDir: dir, + paramsCwd: dir, + maxSubagentDepths: [0], + availableModels: [{ provider: "provider-b", id: "working", fullId: "provider-b/working" }], + knownModelProviders, + modelOverrides: ["provider-a/stalled"], + behaviors: [{ output: false, outputMode: "inline", reads: false, progress: false, skills: false }], + firstProgressIndex: -1, + controlConfig: { enabled: false, needsAttentionAfterMs: 1, activeNoticeAfterMs: 1, failedToolAttemptsBeforeAttention: 1, notifyOn: [], notifyChannels: [] }, + concurrencyLimit: 1, + liveResults: [], + liveProgress: [], + runtime: { + async runSync(_cwd, _agents, agentName, task, options) { + captured.push({ knownModelProviders: options.knownModelProviders }); + return { + agent: agentName, + task, + exitCode: 0, + messages: [], + usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, cost: 0, turns: 0 }, + finalOutput: "ok", + }; + }, + }, + }; + + await runForegroundParallelTasks(input); + assert.deepEqual(captured, [{ knownModelProviders }]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +});