fix(subagents): retain live turns and align runtime guards
- commit
- 3e8568caf50618825e2a224e8522c94e758ebcb6
- author
- brice · · 2026-08-13 02:35 UTC
- parent
- 2ae56a3
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
25 files · +542 -296
M README.md +4 -4
| @@ -237,7 +237,7 @@ Print-mode stats contain `provider`, `model`, `prompt_tokens`, `reasoning_tokens | |||
| 237 | 237 | | `--no-tools` | Disable all tools. | | |
| 238 | 238 | | `--no-lsp` | Disable the built-in LSP/linter tool for this run. | | |
| 239 | 239 | | `--tools <csv>` | Only enable the listed tools. Include `lsp` to select it explicitly. An explicit list must include `web_search` to retain or opt into web search for that invocation; explicitly including it overrides the persisted web-search setting for that invocation. | | |
| 240 | - | | `--max-steps <n>` | Cap agent loop iterations (default 50). | | |
| 240 | + | | `--max-steps <n>` | Cap agent loop iterations (default: unlimited). | | |
| 241 | 241 | | `-e`, `--ext <path>` | Load an extension from `<path>` for this run (repeatable; wins against installed extensions of the same name). | | |
| 242 | 242 | | `--no-ext` | Skip extension discovery for this run. `--ext` still works on top, so `--no-ext --ext ./x` runs only `x`. | | |
| 243 | 243 | | `--no-skill` | Disable all skills, including built-ins. No `skill` tool is registered and the system prompt has no skill manifest. | | |
| @@ -293,7 +293,7 @@ Single-part names resolve only to a matching local directory or `.zut` archive. | |||
| 293 | 293 | ||
| 294 | 294 | Two ways to drive zut from another program: | |
| 295 | 295 | ||
| 296 | - | - **Go in-process**: import `github.com/bnema/zut/packages/agent/sdk`. One `Runtime` per project; `Prompt(ctx, text, images)` returns a channel of `Event`. Small example in `examples/sdk/`. | |
| 296 | + | - **Go in-process**: import `github.com/bnema/zut/packages/agent/sdk`. One `Runtime` per project; `Prompt(ctx, text, images)` returns a channel of `Event`. `sdk.Config.MaxSteps` is unlimited when zero; embedders preserving the former SDK default should set it to `50` explicitly. Small example in `examples/sdk/`. | |
| 297 | 297 | - **Any language, out-of-process**: spawn `zut rpc` as a subprocess and exchange newline-delimited JSON over its stdin/stdout. Wire format and event schema in [docs/rpc.md](docs/rpc.md). Reference clients live under `examples/rpc/`. | |
| 298 | 298 | ||
| 299 | 299 | Both interfaces share the same event schema, so transcripts captured by one can be replayed through the other. | |
| @@ -425,7 +425,7 @@ Background subagents that run alongside your main session. Each one is a separat | |||
| 425 | 425 | ||
| 426 | 426 | **`/session export` does NOT bundle subagents.** A `.zutsession` is just the main chat transcript; per-agent state (session file, unix-socket inbox) is machine-local and doesn't round-trip through a JSONL file. To share what an agent said, copy it out of the transcript view manually. | |
| 427 | 427 | ||
| 428 | - | **Subagent Orchestrator.** With `/settings` → **Subagent Orchestrator** on (or `/orchestrator`), the interactive primary agent enters strict orchestrator mode and gets whichever of the canonical `subagent_spawn` and `subagent_status` tools are permitted by launch-time policy. It must delegate all implementation, debugging/testing, and code-review work to an appropriately named profile, or to a clearly described general worker when no profile fits. It must not write or edit code, make direct implementation tool calls, inspect or review code, or apply worker patches itself. It may decompose requests, select and spawn workers, check status, coordinate follow-ups, and synthesize results. Give workers self-contained tasks: shared-worktree workers edit the same working directory and need coordination, while `isolation:"worktree"` is for parallel coding in separate trees; a worktree patch must be integrated by a worker rather than applied by the primary agent. Child workers cannot recursively spawn more sub-agents in v1. The `subagent_status` tool accepts an optional `agent_id`: omit it to list visible workers, or pass a full id / unique prefix to query one without waiting for completion. It returns bounded JSON metadata with the worker id, lifecycle state (`starting`, `running`, `completed`, `failed`, `cancelled`, or `detached`), underlying process/turn state, timestamps, a bounded first-line task summary, required state/target/unmet metadata when applicable, and terminal-result metadata when available; it omits full prompts, transcripts, result output, provider settings, and filesystem paths. Treat the bounded task summary as session data. The spawn tool always returns the child id and initial state immediately. Set `required:true` when the parent or workflow depends on the result: the worker remains asynchronous, its outcome is persisted and delivered through a host completion update, and failure, timeout, or cancellation remains recoverable until a retry succeeds or the user explicitly removes the terminal worker. An outcome not durably observed before host restart becomes indeterminate and requires the user to inspect side effects before explicitly resuming, restarting, or removing the worker. Completion is a durable `turn.result` with a `subagent://<id>/result` reference. The tool accepts named profiles, `max_turns`, and shared/worktree isolation. Delegated turns are unlimited by default unless a positive manager timeout is configured; a global `subagents.max_steps` budget (128 by default) bounds model/tool loops for every profile. Omit `max_turns` to use the supervisor policy; the default ceiling is 3 and `subagents.max_turns` changes it. Global concurrency limits apply equally to slash commands, tool calls, and batches. Completion is host-event-driven for every worker through `[auto-subagents update]`, with required state also supplied as high-priority `[required-subagents update]` context. Never use `bash sleep`, `watch`, `tail -f`, polling loops, repeated `subagent_status`, or dashboard/metadata/event-log/file checks solely to wait; work on unrelated independent tasks or end/yield your turn. Legitimate waits inside user-requested commands, provider flows, extensions, or tests remain allowed. Off by default; toggle with `/orchestrator` or from `/settings`. When off, the permitted canonical subagent tools and profile manifest remain available for user-requested delegation or when an active skill workflow requires it; the primary otherwise performs work itself without the strict orchestrator contract. | |
| 428 | + | **Subagent Orchestrator.** With `/settings` → **Subagent Orchestrator** on (or `/orchestrator`), the interactive primary agent enters strict orchestrator mode and gets whichever of the canonical `subagent_spawn` and `subagent_status` tools are permitted by launch-time policy. It must delegate all implementation, debugging/testing, and code-review work to an appropriately named profile, or to a clearly described general worker when no profile fits. It must not write or edit code, make direct implementation tool calls, inspect or review code, or apply worker patches itself. It may decompose requests, select and spawn workers, check status, coordinate follow-ups, and synthesize results. Give workers self-contained tasks: shared-worktree workers edit the same working directory and need coordination, while `isolation:"worktree"` is for parallel coding in separate trees; a worktree patch must be integrated by a worker rather than applied by the primary agent. Child workers cannot recursively spawn more sub-agents in v1. The `subagent_status` tool accepts an optional `agent_id`: omit it to list visible workers, or pass a full id / unique prefix to query one without waiting for completion. It returns bounded JSON metadata with the worker id, lifecycle state (`starting`, `running`, `completed`, `failed`, `cancelled`, or `detached`), underlying process/turn state, timestamps, a bounded first-line task summary, required state/target/unmet metadata when applicable, and terminal-result metadata when available; it omits full prompts, transcripts, result output, provider settings, and filesystem paths. Treat the bounded task summary as session data. The spawn tool always returns the child id and initial state immediately. Set `required:true` when the parent or workflow depends on the result: the worker remains asynchronous, its outcome is persisted and delivered through a host completion update, and failure, timeout, or cancellation remains recoverable until a retry succeeds or the user explicitly removes the terminal worker. An outcome not durably observed before host restart becomes indeterminate and requires the user to inspect side effects before explicitly resuming, restarting, or removing the worker. Completion is a durable `turn.result` with a `subagent://<id>/result` reference. The tool accepts named profiles and shared/worktree isolation; deadline, turn, and step budgets are manager-owned and are not exposed in its schema. Delegated turns, run turns, and model/tool loops are unlimited by default unless positive `subagents.default_timeout`, `subagents.max_turns`, or `subagents.max_steps` values are configured. Global concurrency limits apply equally to slash commands, tool calls, and batches. Completion is host-event-driven for every worker through `[auto-subagents update]`, with required state also supplied as high-priority `[required-subagents update]` context. Never use `bash sleep`, `watch`, `tail -f`, polling loops, repeated `subagent_status`, or dashboard/metadata/event-log/file checks solely to wait; work on unrelated independent tasks or end/yield your turn. Legitimate waits inside user-requested commands, provider flows, extensions, or tests remain allowed. Off by default; toggle with `/orchestrator` or from `/settings`. When off, the permitted canonical subagent tools and profile manifest remain available for user-requested delegation or when an active skill workflow requires it; the primary otherwise performs work itself without the strict orchestrator contract. | |
| 429 | 429 | ||
| 430 | 430 | ### Named subagent profiles | |
| 431 | 431 | ||
| @@ -502,7 +502,7 @@ When a turn fails because of a recoverable provider error — expired token (`40 | |||
| 502 | 502 | ||
| 503 | 503 | The picker is the same vertical list / fuzzy filter UI as `/model`, but it only shows models from providers you're currently logged in to (env vars, `auth.json`, Kimi CLI fallback, ollama). The failed model is excluded. Press `↑`/`↓` to choose, `enter` to retry the **same prompt** on the new model, `esc` to dismiss. | |
| 504 | 504 | ||
| 505 | - | Before the actual provider request fires, the OpenAI / Anthropic / Kimi / DeepSeek / Google / OpenAI-Codex clients also do up to two silent retries with short backoff (250ms, 750ms) on `502`/`503`/`504` and connection-reset / EOF-before-headers errors. Most edge-proxy blips disappear without you ever seeing the rescue picker. | |
| 505 | + | Before the actual provider request fires, the shared HTTP streaming clients used by OpenAI / Anthropic / Kimi / DeepSeek / Google / OpenAI-Codex also do up to four silent retries with short backoff (250ms, then 750ms) on transient HTTP failures and connection-reset / EOF-before-headers errors. Once a stream is open, five reconnect attempts and a five-minute idle timeout independently guard stalled or interrupted streams. Most edge-proxy blips disappear without you ever seeing the rescue picker. | |
| 506 | 506 | ||
| 507 | 507 | A rescue retry always **drops launch-time `--api-key` and `--base-url`** before rebuilding the agent. Those overrides are usually the reason the rescue triggered (bad key, typo'd base URL, corporate gateway only valid for the originally-picked provider), so the retry re-resolves credentials from env vars / `auth.json` / provider defaults instead. Use `/model` if you want overrides to stick. | |
| 508 | 508 | ||
| @@ -237,7 +237,7 @@ Print-mode stats contain `provider`, `model`, `prompt_tokens`, `reasoning_tokens | |||||
| 237 | | `--no-tools` | Disable all tools. | | 237 | | `--no-tools` | Disable all tools. | | ||
| 238 | | `--no-lsp` | Disable the built-in LSP/linter tool for this run. | | 238 | | `--no-lsp` | Disable the built-in LSP/linter tool for this run. | | ||
| 239 | | `--tools <csv>` | Only enable the listed tools. Include `lsp` to select it explicitly. An explicit list must include `web_search` to retain or opt into web search for that invocation; explicitly including it overrides the persisted web-search setting for that invocation. | | 239 | | `--tools <csv>` | Only enable the listed tools. Include `lsp` to select it explicitly. An explicit list must include `web_search` to retain or opt into web search for that invocation; explicitly including it overrides the persisted web-search setting for that invocation. | | ||
| 240 | - | | `--max-steps <n>` | Cap agent loop iterations (default 50). | | 240 | + | | `--max-steps <n>` | Cap agent loop iterations (default: unlimited). | |
| 241 | | `-e`, `--ext <path>` | Load an extension from `<path>` for this run (repeatable; wins against installed extensions of the same name). | | 241 | | `-e`, `--ext <path>` | Load an extension from `<path>` for this run (repeatable; wins against installed extensions of the same name). | | ||
| 242 | | `--no-ext` | Skip extension discovery for this run. `--ext` still works on top, so `--no-ext --ext ./x` runs only `x`. | | 242 | | `--no-ext` | Skip extension discovery for this run. `--ext` still works on top, so `--no-ext --ext ./x` runs only `x`. | | ||
| 243 | | `--no-skill` | Disable all skills, including built-ins. No `skill` tool is registered and the system prompt has no skill manifest. | | 243 | | `--no-skill` | Disable all skills, including built-ins. No `skill` tool is registered and the system prompt has no skill manifest. | | ||
| @@ -293,7 +293,7 @@ Single-part names resolve only to a matching local directory or `.zut` archive. | |||||
| 293 | 293 | ||||
| 294 | Two ways to drive zut from another program: | 294 | Two ways to drive zut from another program: | ||
| 295 | 295 | ||||
| 296 | - | - **Go in-process**: import `github.com/bnema/zut/packages/agent/sdk`. One `Runtime` per project; `Prompt(ctx, text, images)` returns a channel of `Event`. Small example in `examples/sdk/`. | 296 | + | - **Go in-process**: import `github.com/bnema/zut/packages/agent/sdk`. One `Runtime` per project; `Prompt(ctx, text, images)` returns a channel of `Event`. `sdk.Config.MaxSteps` is unlimited when zero; embedders preserving the former SDK default should set it to `50` explicitly. Small example in `examples/sdk/`. |
| 297 | - **Any language, out-of-process**: spawn `zut rpc` as a subprocess and exchange newline-delimited JSON over its stdin/stdout. Wire format and event schema in [docs/rpc.md](docs/rpc.md). Reference clients live under `examples/rpc/`. | 297 | - **Any language, out-of-process**: spawn `zut rpc` as a subprocess and exchange newline-delimited JSON over its stdin/stdout. Wire format and event schema in [docs/rpc.md](docs/rpc.md). Reference clients live under `examples/rpc/`. | ||
| 298 | 298 | ||||
| 299 | Both interfaces share the same event schema, so transcripts captured by one can be replayed through the other. | 299 | Both interfaces share the same event schema, so transcripts captured by one can be replayed through the other. | ||
| @@ -425,7 +425,7 @@ Background subagents that run alongside your main session. Each one is a separat | |||||
| 425 | 425 | ||||
| 426 | **`/session export` does NOT bundle subagents.** A `.zutsession` is just the main chat transcript; per-agent state (session file, unix-socket inbox) is machine-local and doesn't round-trip through a JSONL file. To share what an agent said, copy it out of the transcript view manually. | 426 | **`/session export` does NOT bundle subagents.** A `.zutsession` is just the main chat transcript; per-agent state (session file, unix-socket inbox) is machine-local and doesn't round-trip through a JSONL file. To share what an agent said, copy it out of the transcript view manually. | ||
| 427 | 427 | ||||
| 428 | - | **Subagent Orchestrator.** With `/settings` → **Subagent Orchestrator** on (or `/orchestrator`), the interactive primary agent enters strict orchestrator mode and gets whichever of the canonical `subagent_spawn` and `subagent_status` tools are permitted by launch-time policy. It must delegate all implementation, debugging/testing, and code-review work to an appropriately named profile, or to a clearly described general worker when no profile fits. It must not write or edit code, make direct implementation tool calls, inspect or review code, or apply worker patches itself. It may decompose requests, select and spawn workers, check status, coordinate follow-ups, and synthesize results. Give workers self-contained tasks: shared-worktree workers edit the same working directory and need coordination, while `isolation:"worktree"` is for parallel coding in separate trees; a worktree patch must be integrated by a worker rather than applied by the primary agent. Child workers cannot recursively spawn more sub-agents in v1. The `subagent_status` tool accepts an optional `agent_id`: omit it to list visible workers, or pass a full id / unique prefix to query one without waiting for completion. It returns bounded JSON metadata with the worker id, lifecycle state (`starting`, `running`, `completed`, `failed`, `cancelled`, or `detached`), underlying process/turn state, timestamps, a bounded first-line task summary, required state/target/unmet metadata when applicable, and terminal-result metadata when available; it omits full prompts, transcripts, result output, provider settings, and filesystem paths. Treat the bounded task summary as session data. The spawn tool always returns the child id and initial state immediately. Set `required:true` when the parent or workflow depends on the result: the worker remains asynchronous, its outcome is persisted and delivered through a host completion update, and failure, timeout, or cancellation remains recoverable until a retry succeeds or the user explicitly removes the terminal worker. An outcome not durably observed before host restart becomes indeterminate and requires the user to inspect side effects before explicitly resuming, restarting, or removing the worker. Completion is a durable `turn.result` with a `subagent://<id>/result` reference. The tool accepts named profiles, `max_turns`, and shared/worktree isolation. Delegated turns are unlimited by default unless a positive manager timeout is configured; a global `subagents.max_steps` budget (128 by default) bounds model/tool loops for every profile. Omit `max_turns` to use the supervisor policy; the default ceiling is 3 and `subagents.max_turns` changes it. Global concurrency limits apply equally to slash commands, tool calls, and batches. Completion is host-event-driven for every worker through `[auto-subagents update]`, with required state also supplied as high-priority `[required-subagents update]` context. Never use `bash sleep`, `watch`, `tail -f`, polling loops, repeated `subagent_status`, or dashboard/metadata/event-log/file checks solely to wait; work on unrelated independent tasks or end/yield your turn. Legitimate waits inside user-requested commands, provider flows, extensions, or tests remain allowed. Off by default; toggle with `/orchestrator` or from `/settings`. When off, the permitted canonical subagent tools and profile manifest remain available for user-requested delegation or when an active skill workflow requires it; the primary otherwise performs work itself without the strict orchestrator contract. | 428 | + | **Subagent Orchestrator.** With `/settings` → **Subagent Orchestrator** on (or `/orchestrator`), the interactive primary agent enters strict orchestrator mode and gets whichever of the canonical `subagent_spawn` and `subagent_status` tools are permitted by launch-time policy. It must delegate all implementation, debugging/testing, and code-review work to an appropriately named profile, or to a clearly described general worker when no profile fits. It must not write or edit code, make direct implementation tool calls, inspect or review code, or apply worker patches itself. It may decompose requests, select and spawn workers, check status, coordinate follow-ups, and synthesize results. Give workers self-contained tasks: shared-worktree workers edit the same working directory and need coordination, while `isolation:"worktree"` is for parallel coding in separate trees; a worktree patch must be integrated by a worker rather than applied by the primary agent. Child workers cannot recursively spawn more sub-agents in v1. The `subagent_status` tool accepts an optional `agent_id`: omit it to list visible workers, or pass a full id / unique prefix to query one without waiting for completion. It returns bounded JSON metadata with the worker id, lifecycle state (`starting`, `running`, `completed`, `failed`, `cancelled`, or `detached`), underlying process/turn state, timestamps, a bounded first-line task summary, required state/target/unmet metadata when applicable, and terminal-result metadata when available; it omits full prompts, transcripts, result output, provider settings, and filesystem paths. Treat the bounded task summary as session data. The spawn tool always returns the child id and initial state immediately. Set `required:true` when the parent or workflow depends on the result: the worker remains asynchronous, its outcome is persisted and delivered through a host completion update, and failure, timeout, or cancellation remains recoverable until a retry succeeds or the user explicitly removes the terminal worker. An outcome not durably observed before host restart becomes indeterminate and requires the user to inspect side effects before explicitly resuming, restarting, or removing the worker. Completion is a durable `turn.result` with a `subagent://<id>/result` reference. The tool accepts named profiles and shared/worktree isolation; deadline, turn, and step budgets are manager-owned and are not exposed in its schema. Delegated turns, run turns, and model/tool loops are unlimited by default unless positive `subagents.default_timeout`, `subagents.max_turns`, or `subagents.max_steps` values are configured. Global concurrency limits apply equally to slash commands, tool calls, and batches. Completion is host-event-driven for every worker through `[auto-subagents update]`, with required state also supplied as high-priority `[required-subagents update]` context. Never use `bash sleep`, `watch`, `tail -f`, polling loops, repeated `subagent_status`, or dashboard/metadata/event-log/file checks solely to wait; work on unrelated independent tasks or end/yield your turn. Legitimate waits inside user-requested commands, provider flows, extensions, or tests remain allowed. Off by default; toggle with `/orchestrator` or from `/settings`. When off, the permitted canonical subagent tools and profile manifest remain available for user-requested delegation or when an active skill workflow requires it; the primary otherwise performs work itself without the strict orchestrator contract. |
| 429 | 429 | ||||
| 430 | ### Named subagent profiles | 430 | ### Named subagent profiles | ||
| 431 | 431 | ||||
| @@ -502,7 +502,7 @@ When a turn fails because of a recoverable provider error — expired token (`40 | |||||
| 502 | 502 | ||||
| 503 | The picker is the same vertical list / fuzzy filter UI as `/model`, but it only shows models from providers you're currently logged in to (env vars, `auth.json`, Kimi CLI fallback, ollama). The failed model is excluded. Press `↑`/`↓` to choose, `enter` to retry the **same prompt** on the new model, `esc` to dismiss. | 503 | The picker is the same vertical list / fuzzy filter UI as `/model`, but it only shows models from providers you're currently logged in to (env vars, `auth.json`, Kimi CLI fallback, ollama). The failed model is excluded. Press `↑`/`↓` to choose, `enter` to retry the **same prompt** on the new model, `esc` to dismiss. | ||
| 504 | 504 | ||||
| 505 | - | Before the actual provider request fires, the OpenAI / Anthropic / Kimi / DeepSeek / Google / OpenAI-Codex clients also do up to two silent retries with short backoff (250ms, 750ms) on `502`/`503`/`504` and connection-reset / EOF-before-headers errors. Most edge-proxy blips disappear without you ever seeing the rescue picker. | 505 | + | Before the actual provider request fires, the shared HTTP streaming clients used by OpenAI / Anthropic / Kimi / DeepSeek / Google / OpenAI-Codex also do up to four silent retries with short backoff (250ms, then 750ms) on transient HTTP failures and connection-reset / EOF-before-headers errors. Once a stream is open, five reconnect attempts and a five-minute idle timeout independently guard stalled or interrupted streams. Most edge-proxy blips disappear without you ever seeing the rescue picker. |
| 506 | 506 | ||||
| 507 | A rescue retry always **drops launch-time `--api-key` and `--base-url`** before rebuilding the agent. Those overrides are usually the reason the rescue triggered (bad key, typo'd base URL, corporate gateway only valid for the originally-picked provider), so the retry re-resolves credentials from env vars / `auth.json` / provider defaults instead. Use `/model` if you want overrides to stick. | 507 | A rescue retry always **drops launch-time `--api-key` and `--base-url`** before rebuilding the agent. Those overrides are usually the reason the rescue triggered (bad key, typo'd base URL, corporate gateway only valid for the originally-picked provider), so the retry re-resolves credentials from env vars / `auth.json` / provider defaults instead. Use `/model` if you want overrides to stick. | ||
| 508 | 508 | ||||
M docs/subagents.md +2 -2
| @@ -137,7 +137,7 @@ Shared mode preserves the historical host working directory, so workers there mu | |||
| 137 | 137 | ||
| 138 | 138 | Every child retains narrowly scoped internal process control and durable task/result metadata. These details coordinate startup, cancellation, and recovery; they are not a user-facing progress model. Diagnostic progress comes only from paired trace boundaries and their projection into open operations, terminal facts, or an explicitly insufficient observation. | |
| 139 | 139 | ||
| 140 | - | During a trace-observed open operation, interactive mode shows compact rows beside the input area: a themed spinner, the named profile (or agent ID), the most specific open operation, and its elapsed duration. A recent safe observation is shown alongside it when available, for example `assistant streaming 1s ago`; normal mode never displays message text, reasoning text, tool arguments, or tool output. Rows default below the input and can move above it through **running subagent position** in `/settings` → **tui settings**. On short terminals, omitted operations collapse into one count summary so the editor remains visible. A process lifecycle, heartbeat, or generic `working` label never creates a row; use `/subagents` for the current factual operation or last observed fact. | |
| 140 | + | During a trace-observed open operation, interactive mode shows compact rows beside the input area: a themed spinner, the named profile (or agent ID), the most specific open operation, and its elapsed duration. The trace keeps the active turn boundary even after older raw stream fragments leave its bounded in-memory window, so a long-running worker remains visible and its elapsed timer stays anchored to the delegated turn. A recent safe observation is shown alongside it when available, for example `assistant streaming 1s ago`; normal mode never displays message text, reasoning text, tool arguments, or tool output. Rows default below the input and can move above it through **running subagent position** in `/settings` → **tui settings**. On short terminals, omitted operations collapse into one count summary so the editor remains visible. A process lifecycle, heartbeat, or generic `working` label never creates a row; use `/subagents` for the current factual operation or last observed fact. | |
| 141 | 141 | ||
| 142 | 142 | `/subagents` opens on workers from the current host session. Press **Tab** to switch to the all-sessions history, use **Up/Down** to select a worker, and press **Right** or **l** to expand a bounded live snapshot of its user messages, assistant output, and tool activity (**Left** or **h** collapses it). The dashboard derives its row budget from the terminal height, keeping the selected worker visible on short terminals. Press **Enter** for the complete transcript. `/subagents wait <id>` observes the worker's current delegated turn, not its long-lived daemon process. Canceling that wait only removes the observer; it never stops the worker. | |
| 143 | 143 | ||
| @@ -178,4 +178,4 @@ Use `subagent_stop` with an `agent_id` to request termination of a stuck worker | |||
| 178 | 178 | ||
| 179 | 179 | ## Resource policy | |
| 180 | 180 | ||
| 181 | - | The persisted `subagents` config object supports `max_concurrent`, `max_concurrent_per_parent`, `queue_timeout`, `startup_timeout`, `default_timeout`, `max_turns`, `max_steps`, output caps, allowed tools/roots, heartbeat and idle timeouts. A worker fails with an actionable queue-timeout error when no execution slot becomes available before `queue_timeout` (5 minutes by default). Once admitted, it must report `agent_ready` before `startup_timeout` (1 minute by default) or fails with guidance to inspect worker output and retry. Missing or non-positive `default_timeout` leaves delegated turns unlimited. `max_steps` defaults to 128 and bounds provider/model/tool-loop iterations for every delegated turn, independent of profile type. Exhausting it produces a failed result with error code `step_limit`; partial output and session history remain available for an explicit resume. `max_turns` bounds one active worker run; lifetime and current-run turn counters remain persisted and visible in worker status. An explicit `subagent_resume` starts a new run with fresh turn and step budgets without replaying the original task or losing session context. Previously saved `max_total_spawned` values are accepted but ignored. A missing or non-positive configured `max_turns` uses the default ceiling of 3. The `max_turns` field in `subagent_spawn` is optional: omit it to use the policy ceiling, or provide a value from 1 through the configured maximum. Concurrency limits apply to slash commands, `subagent_spawn`, and batch operations. A child cannot create descendants in v1. Per-worker deadline mode and step limit are retained across reload/resume. Packaged `zut run` agents keep their declared capability ceiling by disabling subagent delegation; profiles are not a substitute for that permission boundary. A primary whose launch-time policy disables delegation still receives the strict orchestrator contract, but does not receive actionable profile metadata or a replacement implementation path. | |
| 181 | + | The persisted `subagents` config object supports `max_concurrent`, `max_concurrent_per_parent`, `queue_timeout`, `startup_timeout`, `default_timeout`, `max_turns`, `max_steps`, output caps, allowed tools/roots, heartbeat and idle timeouts. Duration strings use Go duration syntax and malformed values fail configuration loading with the field name. A worker fails with an actionable queue-timeout error when no execution slot becomes available before `queue_timeout` (5 minutes by default). Once admitted, it must report `agent_ready` before `startup_timeout` (1 minute by default) or fails with guidance to inspect worker output and retry. Missing or non-positive `default_timeout`, `max_turns`, and `max_steps` leave delegated turns, active-run turns, and provider/model/tool-loop iterations unlimited. Positive values opt into those manager-owned budgets; they are not fields in the model-facing `subagent_spawn` schema, and stale calls that still send them are rejected explicitly. Exhausting `max_steps` produces a failed result with error code `step_limit`; partial output and session history remain available for an explicit resume. `max_turns` bounds one active worker run; lifetime and current-run turn counters remain persisted and visible in worker status. An explicit `subagent_resume` starts a new run with fresh turn and step budgets without replaying the original task or losing session context. Previously saved `max_total_spawned` values are accepted but ignored. Concurrency limits apply to slash commands, `subagent_spawn`, and batch operations. A child cannot create descendants in v1. Per-worker deadline mode and explicitly configured limits are retained across reload/resume. The shared HTTP streaming providers retry transient request-open failures four times; after opening, a stream may reconnect five times and is canceled after five minutes without an event. Packaged `zut run` agents keep their declared capability ceiling by disabling subagent delegation; profiles are not a substitute for that permission boundary. A primary whose launch-time policy disables delegation still receives the strict orchestrator contract, but does not receive actionable profile metadata or a replacement implementation path. | |
| @@ -137,7 +137,7 @@ Shared mode preserves the historical host working directory, so workers there mu | |||||
| 137 | 137 | ||||
| 138 | Every child retains narrowly scoped internal process control and durable task/result metadata. These details coordinate startup, cancellation, and recovery; they are not a user-facing progress model. Diagnostic progress comes only from paired trace boundaries and their projection into open operations, terminal facts, or an explicitly insufficient observation. | 138 | Every child retains narrowly scoped internal process control and durable task/result metadata. These details coordinate startup, cancellation, and recovery; they are not a user-facing progress model. Diagnostic progress comes only from paired trace boundaries and their projection into open operations, terminal facts, or an explicitly insufficient observation. | ||
| 139 | 139 | ||||
| 140 | - | During a trace-observed open operation, interactive mode shows compact rows beside the input area: a themed spinner, the named profile (or agent ID), the most specific open operation, and its elapsed duration. A recent safe observation is shown alongside it when available, for example `assistant streaming 1s ago`; normal mode never displays message text, reasoning text, tool arguments, or tool output. Rows default below the input and can move above it through **running subagent position** in `/settings` → **tui settings**. On short terminals, omitted operations collapse into one count summary so the editor remains visible. A process lifecycle, heartbeat, or generic `working` label never creates a row; use `/subagents` for the current factual operation or last observed fact. | 140 | + | During a trace-observed open operation, interactive mode shows compact rows beside the input area: a themed spinner, the named profile (or agent ID), the most specific open operation, and its elapsed duration. The trace keeps the active turn boundary even after older raw stream fragments leave its bounded in-memory window, so a long-running worker remains visible and its elapsed timer stays anchored to the delegated turn. A recent safe observation is shown alongside it when available, for example `assistant streaming 1s ago`; normal mode never displays message text, reasoning text, tool arguments, or tool output. Rows default below the input and can move above it through **running subagent position** in `/settings` → **tui settings**. On short terminals, omitted operations collapse into one count summary so the editor remains visible. A process lifecycle, heartbeat, or generic `working` label never creates a row; use `/subagents` for the current factual operation or last observed fact. |
| 141 | 141 | ||||
| 142 | `/subagents` opens on workers from the current host session. Press **Tab** to switch to the all-sessions history, use **Up/Down** to select a worker, and press **Right** or **l** to expand a bounded live snapshot of its user messages, assistant output, and tool activity (**Left** or **h** collapses it). The dashboard derives its row budget from the terminal height, keeping the selected worker visible on short terminals. Press **Enter** for the complete transcript. `/subagents wait <id>` observes the worker's current delegated turn, not its long-lived daemon process. Canceling that wait only removes the observer; it never stops the worker. | 142 | `/subagents` opens on workers from the current host session. Press **Tab** to switch to the all-sessions history, use **Up/Down** to select a worker, and press **Right** or **l** to expand a bounded live snapshot of its user messages, assistant output, and tool activity (**Left** or **h** collapses it). The dashboard derives its row budget from the terminal height, keeping the selected worker visible on short terminals. Press **Enter** for the complete transcript. `/subagents wait <id>` observes the worker's current delegated turn, not its long-lived daemon process. Canceling that wait only removes the observer; it never stops the worker. | ||
| 143 | 143 | ||||
| @@ -178,4 +178,4 @@ Use `subagent_stop` with an `agent_id` to request termination of a stuck worker | |||||
| 178 | 178 | ||||
| 179 | ## Resource policy | 179 | ## Resource policy | ||
| 180 | 180 | ||||
| 181 | - | The persisted `subagents` config object supports `max_concurrent`, `max_concurrent_per_parent`, `queue_timeout`, `startup_timeout`, `default_timeout`, `max_turns`, `max_steps`, output caps, allowed tools/roots, heartbeat and idle timeouts. A worker fails with an actionable queue-timeout error when no execution slot becomes available before `queue_timeout` (5 minutes by default). Once admitted, it must report `agent_ready` before `startup_timeout` (1 minute by default) or fails with guidance to inspect worker output and retry. Missing or non-positive `default_timeout` leaves delegated turns unlimited. `max_steps` defaults to 128 and bounds provider/model/tool-loop iterations for every delegated turn, independent of profile type. Exhausting it produces a failed result with error code `step_limit`; partial output and session history remain available for an explicit resume. `max_turns` bounds one active worker run; lifetime and current-run turn counters remain persisted and visible in worker status. An explicit `subagent_resume` starts a new run with fresh turn and step budgets without replaying the original task or losing session context. Previously saved `max_total_spawned` values are accepted but ignored. A missing or non-positive configured `max_turns` uses the default ceiling of 3. The `max_turns` field in `subagent_spawn` is optional: omit it to use the policy ceiling, or provide a value from 1 through the configured maximum. Concurrency limits apply to slash commands, `subagent_spawn`, and batch operations. A child cannot create descendants in v1. Per-worker deadline mode and step limit are retained across reload/resume. Packaged `zut run` agents keep their declared capability ceiling by disabling subagent delegation; profiles are not a substitute for that permission boundary. A primary whose launch-time policy disables delegation still receives the strict orchestrator contract, but does not receive actionable profile metadata or a replacement implementation path. | 181 | + | The persisted `subagents` config object supports `max_concurrent`, `max_concurrent_per_parent`, `queue_timeout`, `startup_timeout`, `default_timeout`, `max_turns`, `max_steps`, output caps, allowed tools/roots, heartbeat and idle timeouts. Duration strings use Go duration syntax and malformed values fail configuration loading with the field name. A worker fails with an actionable queue-timeout error when no execution slot becomes available before `queue_timeout` (5 minutes by default). Once admitted, it must report `agent_ready` before `startup_timeout` (1 minute by default) or fails with guidance to inspect worker output and retry. Missing or non-positive `default_timeout`, `max_turns`, and `max_steps` leave delegated turns, active-run turns, and provider/model/tool-loop iterations unlimited. Positive values opt into those manager-owned budgets; they are not fields in the model-facing `subagent_spawn` schema, and stale calls that still send them are rejected explicitly. Exhausting `max_steps` produces a failed result with error code `step_limit`; partial output and session history remain available for an explicit resume. `max_turns` bounds one active worker run; lifetime and current-run turn counters remain persisted and visible in worker status. An explicit `subagent_resume` starts a new run with fresh turn and step budgets without replaying the original task or losing session context. Previously saved `max_total_spawned` values are accepted but ignored. Concurrency limits apply to slash commands, `subagent_spawn`, and batch operations. A child cannot create descendants in v1. Per-worker deadline mode and explicitly configured limits are retained across reload/resume. The shared HTTP streaming providers retry transient request-open failures four times; after opening, a stream may reconnect five times and is canceled after five minutes without an event. Packaged `zut run` agents keep their declared capability ceiling by disabling subagent delegation; profiles are not a substitute for that permission boundary. A primary whose launch-time policy disables delegation still receives the strict orchestrator contract, but does not receive actionable profile metadata or a replacement implementation path. |
M packages/agent/config.go +28 -0
| @@ -322,9 +322,37 @@ func LoadConfig() (Config, error) { | |||
| 322 | 322 | if err := json.Unmarshal(b, &c); err != nil { | |
| 323 | 323 | return c, fmt.Errorf("parse config: %w", err) | |
| 324 | 324 | } | |
| 325 | + | if err := validateSubagentDurations(c.Subagents); err != nil { | |
| 326 | + | return c, err | |
| 327 | + | } | |
| 325 | 328 | return c, nil | |
| 326 | 329 | } | |
| 327 | 330 | ||
| 331 | + | func validateSubagentDurations(cfg SubagentsConfig) error { | |
| 332 | + | values := []struct { | |
| 333 | + | name string | |
| 334 | + | value string | |
| 335 | + | }{ | |
| 336 | + | {"queue_timeout", cfg.QueueTimeout}, | |
| 337 | + | {"startup_timeout", cfg.StartupTimeout}, | |
| 338 | + | {"default_timeout", cfg.DefaultTimeout}, | |
| 339 | + | {"heartbeat_interval", cfg.HeartbeatInterval}, | |
| 340 | + | {"idle_timeout", cfg.IdleTimeout}, | |
| 341 | + | {"reconnect_timeout", cfg.ReconnectTimeout}, | |
| 342 | + | {"cancel_grace_period", cfg.CancelGracePeriod}, | |
| 343 | + | } | |
| 344 | + | for _, item := range values { | |
| 345 | + | value := strings.TrimSpace(item.value) | |
| 346 | + | if value == "" { | |
| 347 | + | continue | |
| 348 | + | } | |
| 349 | + | if _, err := time.ParseDuration(value); err != nil { | |
| 350 | + | return fmt.Errorf("parse config: subagents.%s: %w", item.name, err) | |
| 351 | + | } | |
| 352 | + | } | |
| 353 | + | return nil | |
| 354 | + | } | |
| 355 | + | ||
| 328 | 356 | // SaveConfig writes the config file, creating parent dirs. | |
| 329 | 357 | func SaveConfig(c Config) error { | |
| 330 | 358 | if err := os.MkdirAll(ZutHome(), 0o755); err != nil { | |
| @@ -322,9 +322,37 @@ func LoadConfig() (Config, error) { | |||||
| 322 | if err := json.Unmarshal(b, &c); err != nil { | 322 | if err := json.Unmarshal(b, &c); err != nil { | ||
| 323 | return c, fmt.Errorf("parse config: %w", err) | 323 | return c, fmt.Errorf("parse config: %w", err) | ||
| 324 | } | 324 | } | ||
| 325 | + | if err := validateSubagentDurations(c.Subagents); err != nil { | |||
| 326 | + | return c, err | |||
| 327 | + | } | |||
| 325 | return c, nil | 328 | return c, nil | ||
| 326 | } | 329 | } | ||
| 327 | 330 | ||||
| 331 | + | func validateSubagentDurations(cfg SubagentsConfig) error { | |||
| 332 | + | values := []struct { | |||
| 333 | + | name string | |||
| 334 | + | value string | |||
| 335 | + | }{ | |||
| 336 | + | {"queue_timeout", cfg.QueueTimeout}, | |||
| 337 | + | {"startup_timeout", cfg.StartupTimeout}, | |||
| 338 | + | {"default_timeout", cfg.DefaultTimeout}, | |||
| 339 | + | {"heartbeat_interval", cfg.HeartbeatInterval}, | |||
| 340 | + | {"idle_timeout", cfg.IdleTimeout}, | |||
| 341 | + | {"reconnect_timeout", cfg.ReconnectTimeout}, | |||
| 342 | + | {"cancel_grace_period", cfg.CancelGracePeriod}, | |||
| 343 | + | } | |||
| 344 | + | for _, item := range values { | |||
| 345 | + | value := strings.TrimSpace(item.value) | |||
| 346 | + | if value == "" { | |||
| 347 | + | continue | |||
| 348 | + | } | |||
| 349 | + | if _, err := time.ParseDuration(value); err != nil { | |||
| 350 | + | return fmt.Errorf("parse config: subagents.%s: %w", item.name, err) | |||
| 351 | + | } | |||
| 352 | + | } | |||
| 353 | + | return nil | |||
| 354 | + | } | |||
| 355 | + | ||||
| 328 | // SaveConfig writes the config file, creating parent dirs. | 356 | // SaveConfig writes the config file, creating parent dirs. | ||
| 329 | func SaveConfig(c Config) error { | 357 | func SaveConfig(c Config) error { | ||
| 330 | if err := os.MkdirAll(ZutHome(), 0o755); err != nil { | 358 | if err := os.MkdirAll(ZutHome(), 0o755); err != nil { | ||
M packages/agent/sdk/sdk.go +1 -4
| @@ -67,7 +67,7 @@ type Config struct { | |||
| 67 | 67 | Temperature *float32 | |
| 68 | 68 | ||
| 69 | 69 | // MaxSteps caps the agent loop iterations per Prompt call. | |
| 70 | - | // 0 uses the default (50). | |
| 70 | + | // 0 leaves the loop unlimited. | |
| 71 | 71 | MaxSteps int | |
| 72 | 72 | ||
| 73 | 73 | // APIKey overrides the credential lookup chain. | |
| @@ -134,9 +134,6 @@ func New(cfg Config) (*Runtime, error) { | |||
| 134 | 134 | WebSearchPolicy: webSearchPolicy, | |
| 135 | 135 | NoSess: true, // SDK callers manage persistence themselves | |
| 136 | 136 | } | |
| 137 | - | if args.MaxSteps == 0 { | |
| 138 | - | args.MaxSteps = 50 | |
| 139 | - | } | |
| 140 | 137 | r, err := agent.Resolve(args, true) | |
| 141 | 138 | if err != nil { | |
| 142 | 139 | return nil, err | |
| @@ -67,7 +67,7 @@ type Config struct { | |||||
| 67 | Temperature *float32 | 67 | Temperature *float32 | ||
| 68 | 68 | ||||
| 69 | // MaxSteps caps the agent loop iterations per Prompt call. | 69 | // MaxSteps caps the agent loop iterations per Prompt call. | ||
| 70 | - | // 0 uses the default (50). | 70 | + | // 0 leaves the loop unlimited. |
| 71 | MaxSteps int | 71 | MaxSteps int | ||
| 72 | 72 | ||||
| 73 | // APIKey overrides the credential lookup chain. | 73 | // APIKey overrides the credential lookup chain. | ||
| @@ -134,9 +134,6 @@ func New(cfg Config) (*Runtime, error) { | |||||
| 134 | WebSearchPolicy: webSearchPolicy, | 134 | WebSearchPolicy: webSearchPolicy, | ||
| 135 | NoSess: true, // SDK callers manage persistence themselves | 135 | NoSess: true, // SDK callers manage persistence themselves | ||
| 136 | } | 136 | } | ||
| 137 | - | if args.MaxSteps == 0 { | |||
| 138 | - | args.MaxSteps = 50 | |||
| 139 | - | } | |||
| 140 | r, err := agent.Resolve(args, true) | 137 | r, err := agent.Resolve(args, true) | ||
| 141 | if err != nil { | 138 | if err != nil { | ||
| 142 | return nil, err | 139 | return nil, err | ||
A packages/agent/sdk/sdk_defaults_test.go +19 -0
| @@ -0,0 +1,19 @@ | |||
| 1 | + | package sdk | |
| 2 | + | ||
| 3 | + | import "testing" | |
| 4 | + | ||
| 5 | + | func TestRuntimeLeavesAgentStepsUnlimitedByDefault(t *testing.T) { | |
| 6 | + | runtime, err := New(Config{ | |
| 7 | + | Provider: "openai", | |
| 8 | + | Model: "gpt-5", | |
| 9 | + | APIKey: "synthetic-test-key", | |
| 10 | + | NoTools: true, | |
| 11 | + | }) | |
| 12 | + | if err != nil { | |
| 13 | + | t.Fatal(err) | |
| 14 | + | } | |
| 15 | + | t.Cleanup(func() { _ = runtime.Close() }) | |
| 16 | + | if runtime.agent.MaxSteps != 0 { | |
| 17 | + | t.Fatalf("max steps = %d, want unlimited", runtime.agent.MaxSteps) | |
| 18 | + | } | |
| 19 | + | } | |
| @@ -0,0 +1,19 @@ | |||||
| 1 | + | package sdk | |||
| 2 | + | ||||
| 3 | + | import "testing" | |||
| 4 | + | ||||
| 5 | + | func TestRuntimeLeavesAgentStepsUnlimitedByDefault(t *testing.T) { | |||
| 6 | + | runtime, err := New(Config{ | |||
| 7 | + | Provider: "openai", | |||
| 8 | + | Model: "gpt-5", | |||
| 9 | + | APIKey: "synthetic-test-key", | |||
| 10 | + | NoTools: true, | |||
| 11 | + | }) | |||
| 12 | + | if err != nil { | |||
| 13 | + | t.Fatal(err) | |||
| 14 | + | } | |||
| 15 | + | t.Cleanup(func() { _ = runtime.Close() }) | |||
| 16 | + | if runtime.agent.MaxSteps != 0 { | |||
| 17 | + | t.Fatalf("max steps = %d, want unlimited", runtime.agent.MaxSteps) | |||
| 18 | + | } | |||
| 19 | + | } | |||
M packages/agent/subagent_policy_test.go +12 -0
| @@ -3,6 +3,7 @@ package agent | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "os" | |
| 6 | + | "strings" | |
| 6 | 7 | "testing" | |
| 7 | 8 | "time" | |
| 8 | 9 | ||
| @@ -16,6 +17,17 @@ func TestSubagentPolicyParsesStartupTimeout(t *testing.T) { | |||
| 16 | 17 | } | |
| 17 | 18 | } | |
| 18 | 19 | ||
| 20 | + | func TestLoadConfigRejectsMalformedSubagentDuration(t *testing.T) { | |
| 21 | + | t.Setenv("ZUT_HOME", t.TempDir()) | |
| 22 | + | if err := os.WriteFile(ConfigPath(), []byte(`{"subagents":{"default_timeout":"20"}}`), 0o600); err != nil { | |
| 23 | + | t.Fatal(err) | |
| 24 | + | } | |
| 25 | + | _, err := LoadConfig() | |
| 26 | + | if err == nil || !strings.Contains(err.Error(), "subagents.default_timeout") { | |
| 27 | + | t.Fatalf("LoadConfig error = %v, want field-qualified duration error", err) | |
| 28 | + | } | |
| 29 | + | } | |
| 30 | + | ||
| 19 | 31 | func TestSubagentPolicyIgnoresLegacyMaxTotalSpawned(t *testing.T) { | |
| 20 | 32 | t.Setenv("ZUT_HOME", t.TempDir()) | |
| 21 | 33 | if err := os.WriteFile(ConfigPath(), []byte(`{"subagents":{"max_concurrent":1,"max_total_spawned":1}}`), 0o600); err != nil { | |
| @@ -3,6 +3,7 @@ package agent | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "os" | 5 | "os" | ||
| 6 | + | "strings" | |||
| 6 | "testing" | 7 | "testing" | ||
| 7 | "time" | 8 | "time" | ||
| 8 | 9 | ||||
| @@ -16,6 +17,17 @@ func TestSubagentPolicyParsesStartupTimeout(t *testing.T) { | |||||
| 16 | } | 17 | } | ||
| 17 | } | 18 | } | ||
| 18 | 19 | ||||
| 20 | + | func TestLoadConfigRejectsMalformedSubagentDuration(t *testing.T) { | |||
| 21 | + | t.Setenv("ZUT_HOME", t.TempDir()) | |||
| 22 | + | if err := os.WriteFile(ConfigPath(), []byte(`{"subagents":{"default_timeout":"20"}}`), 0o600); err != nil { | |||
| 23 | + | t.Fatal(err) | |||
| 24 | + | } | |||
| 25 | + | _, err := LoadConfig() | |||
| 26 | + | if err == nil || !strings.Contains(err.Error(), "subagents.default_timeout") { | |||
| 27 | + | t.Fatalf("LoadConfig error = %v, want field-qualified duration error", err) | |||
| 28 | + | } | |||
| 29 | + | } | |||
| 30 | + | ||||
| 19 | func TestSubagentPolicyIgnoresLegacyMaxTotalSpawned(t *testing.T) { | 31 | func TestSubagentPolicyIgnoresLegacyMaxTotalSpawned(t *testing.T) { | ||
| 20 | t.Setenv("ZUT_HOME", t.TempDir()) | 32 | t.Setenv("ZUT_HOME", t.TempDir()) | ||
| 21 | if err := os.WriteFile(ConfigPath(), []byte(`{"subagents":{"max_concurrent":1,"max_total_spawned":1}}`), 0o600); err != nil { | 33 | if err := os.WriteFile(ConfigPath(), []byte(`{"subagents":{"max_concurrent":1,"max_total_spawned":1}}`), 0o600); err != nil { | ||
M packages/agent/subagent_requirements.go +11 -4
| @@ -3,6 +3,7 @@ package agent | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "encoding/json" | |
| 6 | + | "errors" | |
| 6 | 7 | "fmt" | |
| 7 | 8 | "strings" | |
| 8 | 9 | ||
| @@ -65,10 +66,14 @@ func (rt *subagentRuntime) WireRequiredWorkerGate(parent *core.Agent) { | |||
| 65 | 66 | if !allowed { | |
| 66 | 67 | return false, reason, replacement | |
| 67 | 68 | } | |
| 68 | - | rt.acknowledgeSatisfiedRequirements() | |
| 69 | + | if err := rt.acknowledgeSatisfiedRequirements(); err != nil { | |
| 70 | + | return false, err.Error(), "" | |
| 71 | + | } | |
| 69 | 72 | return true, reason, replacement | |
| 70 | 73 | } | |
| 71 | - | rt.acknowledgeSatisfiedRequirements() | |
| 74 | + | if err := rt.acknowledgeSatisfiedRequirements(); err != nil { | |
| 75 | + | return false, err.Error(), "" | |
| 76 | + | } | |
| 72 | 77 | return true, "", "" | |
| 73 | 78 | } | |
| 74 | 79 | } | |
| @@ -105,12 +110,14 @@ func (rt *subagentRuntime) waitRequiredWorkerReady(ctx context.Context) error { | |||
| 105 | 110 | } | |
| 106 | 111 | } | |
| 107 | 112 | ||
| 108 | - | func (rt *subagentRuntime) acknowledgeSatisfiedRequirements() { | |
| 113 | + | func (rt *subagentRuntime) acknowledgeSatisfiedRequirements() error { | |
| 114 | + | var result error | |
| 109 | 115 | for _, snapshot := range rt.supervisor.RequiredSnapshots() { | |
| 110 | 116 | if snapshot.Requirement.State == subagents.RequirementSatisfied && !snapshot.Requirement.Notified { | |
| 111 | - | _ = rt.supervisor.MarkRequirementNotified(snapshot.ID) | |
| 117 | + | result = errors.Join(result, rt.supervisor.MarkRequirementNotified(snapshot.ID)) | |
| 112 | 118 | } | |
| 113 | 119 | } | |
| 120 | + | return result | |
| 114 | 121 | } | |
| 115 | 122 | ||
| 116 | 123 | const requiredWorkerContextRuneBudget = 6000 | |
| @@ -3,6 +3,7 @@ package agent | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "encoding/json" | 5 | "encoding/json" | ||
| 6 | + | "errors" | |||
| 6 | "fmt" | 7 | "fmt" | ||
| 7 | "strings" | 8 | "strings" | ||
| 8 | 9 | ||||
| @@ -65,10 +66,14 @@ func (rt *subagentRuntime) WireRequiredWorkerGate(parent *core.Agent) { | |||||
| 65 | if !allowed { | 66 | if !allowed { | ||
| 66 | return false, reason, replacement | 67 | return false, reason, replacement | ||
| 67 | } | 68 | } | ||
| 68 | - | rt.acknowledgeSatisfiedRequirements() | 69 | + | if err := rt.acknowledgeSatisfiedRequirements(); err != nil { |
| 70 | + | return false, err.Error(), "" | |||
| 71 | + | } | |||
| 69 | return true, reason, replacement | 72 | return true, reason, replacement | ||
| 70 | } | 73 | } | ||
| 71 | - | rt.acknowledgeSatisfiedRequirements() | 74 | + | if err := rt.acknowledgeSatisfiedRequirements(); err != nil { |
| 75 | + | return false, err.Error(), "" | |||
| 76 | + | } | |||
| 72 | return true, "", "" | 77 | return true, "", "" | ||
| 73 | } | 78 | } | ||
| 74 | } | 79 | } | ||
| @@ -105,12 +110,14 @@ func (rt *subagentRuntime) waitRequiredWorkerReady(ctx context.Context) error { | |||||
| 105 | } | 110 | } | ||
| 106 | } | 111 | } | ||
| 107 | 112 | ||||
| 108 | - | func (rt *subagentRuntime) acknowledgeSatisfiedRequirements() { | 113 | + | func (rt *subagentRuntime) acknowledgeSatisfiedRequirements() error { |
| 114 | + | var result error | |||
| 109 | for _, snapshot := range rt.supervisor.RequiredSnapshots() { | 115 | for _, snapshot := range rt.supervisor.RequiredSnapshots() { | ||
| 110 | if snapshot.Requirement.State == subagents.RequirementSatisfied && !snapshot.Requirement.Notified { | 116 | if snapshot.Requirement.State == subagents.RequirementSatisfied && !snapshot.Requirement.Notified { | ||
| 111 | - | _ = rt.supervisor.MarkRequirementNotified(snapshot.ID) | 117 | + | result = errors.Join(result, rt.supervisor.MarkRequirementNotified(snapshot.ID)) |
| 112 | } | 118 | } | ||
| 113 | } | 119 | } | ||
| 120 | + | return result | |||
| 114 | } | 121 | } | ||
| 115 | 122 | ||||
| 116 | const requiredWorkerContextRuneBudget = 6000 | 123 | const requiredWorkerContextRuneBudget = 6000 | ||
M packages/agent/subagent_worker.go +0 -5
| @@ -251,11 +251,6 @@ func runSubagentWorkerMode(ctx context.Context, args Args, version string) (runE | |||
| 251 | 251 | mu.Unlock() | |
| 252 | 252 | ||
| 253 | 253 | turnStarted := false | |
| 254 | - | // Match Codex's Responses stream policy: five minutes without an | |
| 255 | - | // event marks only the current stream as lost, and up to five retries | |
| 256 | - | // reconnect the sampling request without ending the delegated turn. | |
| 257 | - | ag.StreamIdleTimeout = 5 * time.Minute | |
| 258 | - | ag.MaxRetries = 5 | |
| 259 | 254 | sink := func(ev core.AgentEvent) { | |
| 260 | 255 | data := subagentEventData(ev) | |
| 261 | 256 | if ev.Type() == "user_message" && !turnStarted { | |
| @@ -251,11 +251,6 @@ func runSubagentWorkerMode(ctx context.Context, args Args, version string) (runE | |||||
| 251 | mu.Unlock() | 251 | mu.Unlock() | ||
| 252 | 252 | ||||
| 253 | turnStarted := false | 253 | turnStarted := false | ||
| 254 | - | // Match Codex's Responses stream policy: five minutes without an | |||
| 255 | - | // event marks only the current stream as lost, and up to five retries | |||
| 256 | - | // reconnect the sampling request without ending the delegated turn. | |||
| 257 | - | ag.StreamIdleTimeout = 5 * time.Minute | |||
| 258 | - | ag.MaxRetries = 5 | |||
| 259 | sink := func(ev core.AgentEvent) { | 254 | sink := func(ev core.AgentEvent) { | ||
| 260 | data := subagentEventData(ev) | 255 | data := subagentEventData(ev) | ||
| 261 | if ev.Type() == "user_message" && !turnStarted { | 256 | if ev.Type() == "user_message" && !turnStarted { | ||
M packages/agent/subagents/model.go +12 -9
| @@ -107,9 +107,9 @@ const ( | |||
| 107 | 107 | ||
| 108 | 108 | // SubagentPolicy contains manager-owned safety and resource limits. normalize | |
| 109 | 109 | // replaces zero MaxOutputBytes, MaxOutputLines, QueueTimeout, and StartupTimeout | |
| 110 | - | // values with positive manager defaults. A zero DefaultTimeout leaves delegated | |
| 111 | - | // turns unlimited; queue, startup, stream-idle, and idle-process limits remain | |
| 112 | - | // independently bounded. | |
| 110 | + | // values with positive manager defaults. Zero DefaultTimeout, MaxTurns, and | |
| 111 | + | // MaxSteps leave delegated turns and model loops unlimited; queue, startup, | |
| 112 | + | // stream-idle, and idle-process limits remain independently bounded. | |
| 113 | 113 | type SubagentPolicy struct { | |
| 114 | 114 | MaxConcurrent int | |
| 115 | 115 | MaxConcurrentPerParent int | |
| @@ -129,6 +129,15 @@ type SubagentPolicy struct { | |||
| 129 | 129 | } | |
| 130 | 130 | ||
| 131 | 131 | func (p *SubagentPolicy) normalize() { | |
| 132 | + | if p.DefaultTimeout < 0 { | |
| 133 | + | p.DefaultTimeout = 0 | |
| 134 | + | } | |
| 135 | + | if p.MaxTurns < 0 { | |
| 136 | + | p.MaxTurns = 0 | |
| 137 | + | } | |
| 138 | + | if p.MaxSteps < 0 { | |
| 139 | + | p.MaxSteps = 0 | |
| 140 | + | } | |
| 132 | 141 | if p.MaxConcurrent <= 0 { | |
| 133 | 142 | p.MaxConcurrent = 8 | |
| 134 | 143 | } | |
| @@ -141,12 +150,6 @@ func (p *SubagentPolicy) normalize() { | |||
| 141 | 150 | if p.StartupTimeout <= 0 { | |
| 142 | 151 | p.StartupTimeout = time.Minute | |
| 143 | 152 | } | |
| 144 | - | if p.MaxTurns <= 0 { | |
| 145 | - | p.MaxTurns = 3 | |
| 146 | - | } | |
| 147 | - | if p.MaxSteps <= 0 { | |
| 148 | - | p.MaxSteps = 128 | |
| 149 | - | } | |
| 150 | 153 | if p.MaxOutputBytes <= 0 { | |
| 151 | 154 | p.MaxOutputBytes = 500_000 | |
| 152 | 155 | } | |
| @@ -107,9 +107,9 @@ const ( | |||||
| 107 | 107 | ||||
| 108 | // SubagentPolicy contains manager-owned safety and resource limits. normalize | 108 | // SubagentPolicy contains manager-owned safety and resource limits. normalize | ||
| 109 | // replaces zero MaxOutputBytes, MaxOutputLines, QueueTimeout, and StartupTimeout | 109 | // replaces zero MaxOutputBytes, MaxOutputLines, QueueTimeout, and StartupTimeout | ||
| 110 | - | // values with positive manager defaults. A zero DefaultTimeout leaves delegated | 110 | + | // values with positive manager defaults. Zero DefaultTimeout, MaxTurns, and |
| 111 | - | // turns unlimited; queue, startup, stream-idle, and idle-process limits remain | 111 | + | // MaxSteps leave delegated turns and model loops unlimited; queue, startup, |
| 112 | - | // independently bounded. | 112 | + | // stream-idle, and idle-process limits remain independently bounded. |
| 113 | type SubagentPolicy struct { | 113 | type SubagentPolicy struct { | ||
| 114 | MaxConcurrent int | 114 | MaxConcurrent int | ||
| 115 | MaxConcurrentPerParent int | 115 | MaxConcurrentPerParent int | ||
| @@ -129,6 +129,15 @@ type SubagentPolicy struct { | |||||
| 129 | } | 129 | } | ||
| 130 | 130 | ||||
| 131 | func (p *SubagentPolicy) normalize() { | 131 | func (p *SubagentPolicy) normalize() { | ||
| 132 | + | if p.DefaultTimeout < 0 { | |||
| 133 | + | p.DefaultTimeout = 0 | |||
| 134 | + | } | |||
| 135 | + | if p.MaxTurns < 0 { | |||
| 136 | + | p.MaxTurns = 0 | |||
| 137 | + | } | |||
| 138 | + | if p.MaxSteps < 0 { | |||
| 139 | + | p.MaxSteps = 0 | |||
| 140 | + | } | |||
| 132 | if p.MaxConcurrent <= 0 { | 141 | if p.MaxConcurrent <= 0 { | ||
| 133 | p.MaxConcurrent = 8 | 142 | p.MaxConcurrent = 8 | ||
| 134 | } | 143 | } | ||
| @@ -141,12 +150,6 @@ func (p *SubagentPolicy) normalize() { | |||||
| 141 | if p.StartupTimeout <= 0 { | 150 | if p.StartupTimeout <= 0 { | ||
| 142 | p.StartupTimeout = time.Minute | 151 | p.StartupTimeout = time.Minute | ||
| 143 | } | 152 | } | ||
| 144 | - | if p.MaxTurns <= 0 { | |||
| 145 | - | p.MaxTurns = 3 | |||
| 146 | - | } | |||
| 147 | - | if p.MaxSteps <= 0 { | |||
| 148 | - | p.MaxSteps = 128 | |||
| 149 | - | } | |||
| 150 | if p.MaxOutputBytes <= 0 { | 153 | if p.MaxOutputBytes <= 0 { | ||
| 151 | p.MaxOutputBytes = 500_000 | 154 | p.MaxOutputBytes = 500_000 | ||
| 152 | } | 155 | } | ||
M packages/agent/subagents/projection.go +103 -60
| @@ -159,75 +159,100 @@ func (v AgentTraceView) Summary() string { | |||
| 159 | 159 | // Unknown worker protocol events remain factual observations and are never | |
| 160 | 160 | // guessed into operations. | |
| 161 | 161 | func ProjectTrace(events []TraceEvent) map[string]AgentTraceView { | |
| 162 | - | views := make(map[string]AgentTraceView) | |
| 163 | - | open := make(map[string]map[string]Operation) | |
| 162 | + | projection := newTraceProjection() | |
| 164 | 163 | for _, event := range events { | |
| 165 | - | if event.AgentID == "" { | |
| 166 | - | continue | |
| 167 | - | } | |
| 168 | - | view := views[event.AgentID] | |
| 169 | - | view.AgentID = event.AgentID | |
| 170 | - | view.LastEvent = event | |
| 171 | - | key, starts, ends, terminal := traceBoundary(event) | |
| 172 | - | var endedOperation Operation | |
| 173 | - | var endedOperationFound bool | |
| 174 | - | if ends { | |
| 175 | - | endedOperation, endedOperationFound = open[event.AgentID][key] | |
| 164 | + | projection.apply(event) | |
| 165 | + | } | |
| 166 | + | return projection.snapshot() | |
| 167 | + | } | |
| 168 | + | ||
| 169 | + | // traceProjection incrementally derives user-facing facts from the trace. It | |
| 170 | + | // deliberately stores only compact facts and currently open operations, so a | |
| 171 | + | // TraceWriter can evict old raw events without losing the start boundary of a | |
| 172 | + | // long-running delegated turn. | |
| 173 | + | type traceProjection struct { | |
| 174 | + | views map[string]AgentTraceView | |
| 175 | + | open map[string]map[string]Operation | |
| 176 | + | } | |
| 177 | + | ||
| 178 | + | func newTraceProjection() *traceProjection { | |
| 179 | + | return &traceProjection{ | |
| 180 | + | views: make(map[string]AgentTraceView), | |
| 181 | + | open: make(map[string]map[string]Operation), | |
| 182 | + | } | |
| 183 | + | } | |
| 184 | + | ||
| 185 | + | func (p *traceProjection) apply(event TraceEvent) { | |
| 186 | + | if event.AgentID == "" { | |
| 187 | + | return | |
| 188 | + | } | |
| 189 | + | view := p.views[event.AgentID] | |
| 190 | + | view.AgentID = event.AgentID | |
| 191 | + | view.LastEvent = event | |
| 192 | + | key, starts, ends, terminal := traceBoundary(event) | |
| 193 | + | var endedOperation Operation | |
| 194 | + | var endedOperationFound bool | |
| 195 | + | if ends { | |
| 196 | + | endedOperation, endedOperationFound = p.open[event.AgentID][key] | |
| 197 | + | } | |
| 198 | + | if starts { | |
| 199 | + | if event.Type == "provider.request.started" { | |
| 200 | + | view.LastRequest = requestFact(event, "open") | |
| 176 | 201 | } | |
| 177 | - | if starts { | |
| 178 | - | if event.Type == "provider.request.started" { | |
| 179 | - | view.LastRequest = requestFact(event, "open") | |
| 180 | - | } | |
| 181 | - | if open[event.AgentID] == nil { | |
| 182 | - | open[event.AgentID] = make(map[string]Operation) | |
| 183 | - | } | |
| 184 | - | callID, _ := event.Data["call_id"].(string) | |
| 185 | - | name, _ := event.Data["name"].(string) | |
| 186 | - | provider, _ := event.Data["provider"].(string) | |
| 187 | - | model, _ := event.Data["model"].(string) | |
| 188 | - | attempt := traceInt(event.Data["attempt"]) | |
| 189 | - | maxAttempts := traceInt(event.Data["max_attempts"]) | |
| 190 | - | open[event.AgentID][key] = Operation{Type: event.Type, AgentID: event.AgentID, TurnID: event.TurnID, CallID: callID, Name: name, Provider: provider, Model: model, Attempt: attempt, MaxAttempts: maxAttempts, StartedAt: event.Timestamp} | |
| 202 | + | if p.open[event.AgentID] == nil { | |
| 203 | + | p.open[event.AgentID] = make(map[string]Operation) | |
| 191 | 204 | } | |
| 192 | - | if ends { | |
| 193 | - | if event.Type == "provider.request.finished" || event.Type == "provider.request.failed" || event.Type == "provider.request.cancelled" || event.Type == "provider.request.retry.scheduled" { | |
| 194 | - | if active, found := open[event.AgentID][key]; found { | |
| 195 | - | outcome := strings.TrimPrefix(event.Type, "provider.request.") | |
| 196 | - | request := requestFact(event, outcome) | |
| 197 | - | request.Provider, request.Model = active.Provider, active.Model | |
| 198 | - | request.Attempt, request.MaxAttempts = active.Attempt, active.MaxAttempts | |
| 199 | - | request.StartedAt = active.StartedAt | |
| 200 | - | view.LastRequest = request | |
| 201 | - | } | |
| 205 | + | callID, _ := event.Data["call_id"].(string) | |
| 206 | + | name, _ := event.Data["name"].(string) | |
| 207 | + | provider, _ := event.Data["provider"].(string) | |
| 208 | + | model, _ := event.Data["model"].(string) | |
| 209 | + | attempt := traceInt(event.Data["attempt"]) | |
| 210 | + | maxAttempts := traceInt(event.Data["max_attempts"]) | |
| 211 | + | p.open[event.AgentID][key] = Operation{Type: event.Type, AgentID: event.AgentID, TurnID: event.TurnID, CallID: callID, Name: name, Provider: provider, Model: model, Attempt: attempt, MaxAttempts: maxAttempts, StartedAt: event.Timestamp} | |
| 212 | + | } | |
| 213 | + | if ends { | |
| 214 | + | if event.Type == "provider.request.finished" || event.Type == "provider.request.failed" || event.Type == "provider.request.cancelled" || event.Type == "provider.request.retry.scheduled" { | |
| 215 | + | if active, found := p.open[event.AgentID][key]; found { | |
| 216 | + | outcome := strings.TrimPrefix(event.Type, "provider.request.") | |
| 217 | + | request := requestFact(event, outcome) | |
| 218 | + | request.Provider, request.Model = active.Provider, active.Model | |
| 219 | + | request.Attempt, request.MaxAttempts = active.Attempt, active.MaxAttempts | |
| 220 | + | request.StartedAt = active.StartedAt | |
| 221 | + | view.LastRequest = request | |
| 202 | 222 | } | |
| 203 | - | delete(open[event.AgentID], key) | |
| 204 | 223 | } | |
| 205 | - | if observation := traceObservation(event); observation != nil { | |
| 206 | - | if observation.Type == "tool.finished" && observation.Name == "" { | |
| 207 | - | // tool_result carries only the call ID. Recover the safe tool name | |
| 208 | - | // from its matching open execution boundary; an unscoped generic | |
| 209 | - | // "tool finished" fact is too low-value for the live indicator. | |
| 210 | - | if endedOperationFound && endedOperation.Type == "tool.started" && endedOperation.Name != "" { | |
| 211 | - | observation.Name = endedOperation.Name | |
| 212 | - | } else { | |
| 213 | - | observation = nil | |
| 214 | - | } | |
| 215 | - | } | |
| 216 | - | if observation != nil { | |
| 217 | - | view.LastObservation = observation | |
| 224 | + | delete(p.open[event.AgentID], key) | |
| 225 | + | } | |
| 226 | + | if observation := traceObservation(event); observation != nil { | |
| 227 | + | if observation.Type == "tool.finished" && observation.Name == "" { | |
| 228 | + | // tool_result carries only the call ID. Recover the safe tool name | |
| 229 | + | // from its matching open execution boundary; an unscoped generic | |
| 230 | + | // "tool finished" fact is too low-value for the live indicator. | |
| 231 | + | if endedOperationFound && endedOperation.Type == "tool.started" && endedOperation.Name != "" { | |
| 232 | + | observation.Name = endedOperation.Name | |
| 233 | + | } else { | |
| 234 | + | observation = nil | |
| 218 | 235 | } | |
| 219 | 236 | } | |
| 220 | - | if result := traceResultFact(event, view.Result); result != nil { | |
| 221 | - | view.Result = result | |
| 222 | - | } | |
| 223 | - | if terminal != "" { | |
| 224 | - | view.Terminal = terminal | |
| 225 | - | delete(open, event.AgentID) | |
| 237 | + | if observation != nil { | |
| 238 | + | view.LastObservation = observation | |
| 226 | 239 | } | |
| 227 | - | views[event.AgentID] = view | |
| 228 | 240 | } | |
| 229 | - | for agentID, operations := range open { | |
| 230 | - | view := views[agentID] | |
| 241 | + | if result := traceResultFact(event, view.Result); result != nil { | |
| 242 | + | view.Result = result | |
| 243 | + | } | |
| 244 | + | if terminal != "" { | |
| 245 | + | view.Terminal = terminal | |
| 246 | + | delete(p.open, event.AgentID) | |
| 247 | + | } | |
| 248 | + | p.views[event.AgentID] = view | |
| 249 | + | } | |
| 250 | + | ||
| 251 | + | func (p *traceProjection) snapshot() map[string]AgentTraceView { | |
| 252 | + | views := make(map[string]AgentTraceView, len(p.views)) | |
| 253 | + | for agentID, stored := range p.views { | |
| 254 | + | view := cloneAgentTraceView(stored) | |
| 255 | + | operations := p.open[agentID] | |
| 231 | 256 | for _, operation := range operations { | |
| 232 | 257 | view.OpenOperations = append(view.OpenOperations, operation) | |
| 233 | 258 | } | |
| @@ -256,6 +281,24 @@ func ProjectTrace(events []TraceEvent) map[string]AgentTraceView { | |||
| 256 | 281 | return views | |
| 257 | 282 | } | |
| 258 | 283 | ||
| 284 | + | func cloneAgentTraceView(view AgentTraceView) AgentTraceView { | |
| 285 | + | view.OpenOperations = nil | |
| 286 | + | view.PrimaryOperation = nil | |
| 287 | + | if view.LastObservation != nil { | |
| 288 | + | observation := *view.LastObservation | |
| 289 | + | view.LastObservation = &observation | |
| 290 | + | } | |
| 291 | + | if view.Result != nil { | |
| 292 | + | result := *view.Result | |
| 293 | + | view.Result = &result | |
| 294 | + | } | |
| 295 | + | if view.LastRequest != nil { | |
| 296 | + | request := *view.LastRequest | |
| 297 | + | view.LastRequest = &request | |
| 298 | + | } | |
| 299 | + | return view | |
| 300 | + | } | |
| 301 | + | ||
| 259 | 302 | // TurnStartedAt returns the factual start of the open delegated turn owning | |
| 260 | 303 | // operation. It is stable across nested provider requests and tool calls. | |
| 261 | 304 | func (v AgentTraceView) TurnStartedAt(operation Operation) time.Time { | |
| @@ -159,75 +159,100 @@ func (v AgentTraceView) Summary() string { | |||||
| 159 | // Unknown worker protocol events remain factual observations and are never | 159 | // Unknown worker protocol events remain factual observations and are never | ||
| 160 | // guessed into operations. | 160 | // guessed into operations. | ||
| 161 | func ProjectTrace(events []TraceEvent) map[string]AgentTraceView { | 161 | func ProjectTrace(events []TraceEvent) map[string]AgentTraceView { | ||
| 162 | - | views := make(map[string]AgentTraceView) | 162 | + | projection := newTraceProjection() |
| 163 | - | open := make(map[string]map[string]Operation) | |||
| 164 | for _, event := range events { | 163 | for _, event := range events { | ||
| 165 | - | if event.AgentID == "" { | 164 | + | projection.apply(event) |
| 166 | - | continue | 165 | + | } |
| 167 | - | } | 166 | + | return projection.snapshot() |
| 168 | - | view := views[event.AgentID] | 167 | + | } |
| 169 | - | view.AgentID = event.AgentID | 168 | + | |
| 170 | - | view.LastEvent = event | 169 | + | // traceProjection incrementally derives user-facing facts from the trace. It |
| 171 | - | key, starts, ends, terminal := traceBoundary(event) | 170 | + | // deliberately stores only compact facts and currently open operations, so a |
| 172 | - | var endedOperation Operation | 171 | + | // TraceWriter can evict old raw events without losing the start boundary of a |
| 173 | - | var endedOperationFound bool | 172 | + | // long-running delegated turn. |
| 174 | - | if ends { | 173 | + | type traceProjection struct { |
| 175 | - | endedOperation, endedOperationFound = open[event.AgentID][key] | 174 | + | views map[string]AgentTraceView |
| 175 | + | open map[string]map[string]Operation | |||
| 176 | + | } | |||
| 177 | + | ||||
| 178 | + | func newTraceProjection() *traceProjection { | |||
| 179 | + | return &traceProjection{ | |||
| 180 | + | views: make(map[string]AgentTraceView), | |||
| 181 | + | open: make(map[string]map[string]Operation), | |||
| 182 | + | } | |||
| 183 | + | } | |||
| 184 | + | ||||
| 185 | + | func (p *traceProjection) apply(event TraceEvent) { | |||
| 186 | + | if event.AgentID == "" { | |||
| 187 | + | return | |||
| 188 | + | } | |||
| 189 | + | view := p.views[event.AgentID] | |||
| 190 | + | view.AgentID = event.AgentID | |||
| 191 | + | view.LastEvent = event | |||
| 192 | + | key, starts, ends, terminal := traceBoundary(event) | |||
| 193 | + | var endedOperation Operation | |||
| 194 | + | var endedOperationFound bool | |||
| 195 | + | if ends { | |||
| 196 | + | endedOperation, endedOperationFound = p.open[event.AgentID][key] | |||
| 197 | + | } | |||
| 198 | + | if starts { | |||
| 199 | + | if event.Type == "provider.request.started" { | |||
| 200 | + | view.LastRequest = requestFact(event, "open") | |||
| 176 | } | 201 | } | ||
| 177 | - | if starts { | 202 | + | if p.open[event.AgentID] == nil { |
| 178 | - | if event.Type == "provider.request.started" { | 203 | + | p.open[event.AgentID] = make(map[string]Operation) |
| 179 | - | view.LastRequest = requestFact(event, "open") | |||
| 180 | - | } | |||
| 181 | - | if open[event.AgentID] == nil { | |||
| 182 | - | open[event.AgentID] = make(map[string]Operation) | |||
| 183 | - | } | |||
| 184 | - | callID, _ := event.Data["call_id"].(string) | |||
| 185 | - | name, _ := event.Data["name"].(string) | |||
| 186 | - | provider, _ := event.Data["provider"].(string) | |||
| 187 | - | model, _ := event.Data["model"].(string) | |||
| 188 | - | attempt := traceInt(event.Data["attempt"]) | |||
| 189 | - | maxAttempts := traceInt(event.Data["max_attempts"]) | |||
| 190 | - | open[event.AgentID][key] = Operation{Type: event.Type, AgentID: event.AgentID, TurnID: event.TurnID, CallID: callID, Name: name, Provider: provider, Model: model, Attempt: attempt, MaxAttempts: maxAttempts, StartedAt: event.Timestamp} | |||
| 191 | } | 204 | } | ||
| 192 | - | if ends { | 205 | + | callID, _ := event.Data["call_id"].(string) |
| 193 | - | if event.Type == "provider.request.finished" || event.Type == "provider.request.failed" || event.Type == "provider.request.cancelled" || event.Type == "provider.request.retry.scheduled" { | 206 | + | name, _ := event.Data["name"].(string) |
| 194 | - | if active, found := open[event.AgentID][key]; found { | 207 | + | provider, _ := event.Data["provider"].(string) |
| 195 | - | outcome := strings.TrimPrefix(event.Type, "provider.request.") | 208 | + | model, _ := event.Data["model"].(string) |
| 196 | - | request := requestFact(event, outcome) | 209 | + | attempt := traceInt(event.Data["attempt"]) |
| 197 | - | request.Provider, request.Model = active.Provider, active.Model | 210 | + | maxAttempts := traceInt(event.Data["max_attempts"]) |
| 198 | - | request.Attempt, request.MaxAttempts = active.Attempt, active.MaxAttempts | 211 | + | p.open[event.AgentID][key] = Operation{Type: event.Type, AgentID: event.AgentID, TurnID: event.TurnID, CallID: callID, Name: name, Provider: provider, Model: model, Attempt: attempt, MaxAttempts: maxAttempts, StartedAt: event.Timestamp} |
| 199 | - | request.StartedAt = active.StartedAt | 212 | + | } |
| 200 | - | view.LastRequest = request | 213 | + | if ends { |
| 201 | - | } | 214 | + | if event.Type == "provider.request.finished" || event.Type == "provider.request.failed" || event.Type == "provider.request.cancelled" || event.Type == "provider.request.retry.scheduled" { |
| 215 | + | if active, found := p.open[event.AgentID][key]; found { | |||
| 216 | + | outcome := strings.TrimPrefix(event.Type, "provider.request.") | |||
| 217 | + | request := requestFact(event, outcome) | |||
| 218 | + | request.Provider, request.Model = active.Provider, active.Model | |||
| 219 | + | request.Attempt, request.MaxAttempts = active.Attempt, active.MaxAttempts | |||
| 220 | + | request.StartedAt = active.StartedAt | |||
| 221 | + | view.LastRequest = request | |||
| 202 | } | 222 | } | ||
| 203 | - | delete(open[event.AgentID], key) | |||
| 204 | } | 223 | } | ||
| 205 | - | if observation := traceObservation(event); observation != nil { | 224 | + | delete(p.open[event.AgentID], key) |
| 206 | - | if observation.Type == "tool.finished" && observation.Name == "" { | 225 | + | } |
| 207 | - | // tool_result carries only the call ID. Recover the safe tool name | 226 | + | if observation := traceObservation(event); observation != nil { |
| 208 | - | // from its matching open execution boundary; an unscoped generic | 227 | + | if observation.Type == "tool.finished" && observation.Name == "" { |
| 209 | - | // "tool finished" fact is too low-value for the live indicator. | 228 | + | // tool_result carries only the call ID. Recover the safe tool name |
| 210 | - | if endedOperationFound && endedOperation.Type == "tool.started" && endedOperation.Name != "" { | 229 | + | // from its matching open execution boundary; an unscoped generic |
| 211 | - | observation.Name = endedOperation.Name | 230 | + | // "tool finished" fact is too low-value for the live indicator. |
| 212 | - | } else { | 231 | + | if endedOperationFound && endedOperation.Type == "tool.started" && endedOperation.Name != "" { |
| 213 | - | observation = nil | 232 | + | observation.Name = endedOperation.Name |
| 214 | - | } | 233 | + | } else { |
| 215 | - | } | 234 | + | observation = nil |
| 216 | - | if observation != nil { | |||
| 217 | - | view.LastObservation = observation | |||
| 218 | } | 235 | } | ||
| 219 | } | 236 | } | ||
| 220 | - | if result := traceResultFact(event, view.Result); result != nil { | 237 | + | if observation != nil { |
| 221 | - | view.Result = result | 238 | + | view.LastObservation = observation |
| 222 | - | } | |||
| 223 | - | if terminal != "" { | |||
| 224 | - | view.Terminal = terminal | |||
| 225 | - | delete(open, event.AgentID) | |||
| 226 | } | 239 | } | ||
| 227 | - | views[event.AgentID] = view | |||
| 228 | } | 240 | } | ||
| 229 | - | for agentID, operations := range open { | 241 | + | if result := traceResultFact(event, view.Result); result != nil { |
| 230 | - | view := views[agentID] | 242 | + | view.Result = result |
| 243 | + | } | |||
| 244 | + | if terminal != "" { | |||
| 245 | + | view.Terminal = terminal | |||
| 246 | + | delete(p.open, event.AgentID) | |||
| 247 | + | } | |||
| 248 | + | p.views[event.AgentID] = view | |||
| 249 | + | } | |||
| 250 | + | ||||
| 251 | + | func (p *traceProjection) snapshot() map[string]AgentTraceView { | |||
| 252 | + | views := make(map[string]AgentTraceView, len(p.views)) | |||
| 253 | + | for agentID, stored := range p.views { | |||
| 254 | + | view := cloneAgentTraceView(stored) | |||
| 255 | + | operations := p.open[agentID] | |||
| 231 | for _, operation := range operations { | 256 | for _, operation := range operations { | ||
| 232 | view.OpenOperations = append(view.OpenOperations, operation) | 257 | view.OpenOperations = append(view.OpenOperations, operation) | ||
| 233 | } | 258 | } | ||
| @@ -256,6 +281,24 @@ func ProjectTrace(events []TraceEvent) map[string]AgentTraceView { | |||||
| 256 | return views | 281 | return views | ||
| 257 | } | 282 | } | ||
| 258 | 283 | ||||
| 284 | + | func cloneAgentTraceView(view AgentTraceView) AgentTraceView { | |||
| 285 | + | view.OpenOperations = nil | |||
| 286 | + | view.PrimaryOperation = nil | |||
| 287 | + | if view.LastObservation != nil { | |||
| 288 | + | observation := *view.LastObservation | |||
| 289 | + | view.LastObservation = &observation | |||
| 290 | + | } | |||
| 291 | + | if view.Result != nil { | |||
| 292 | + | result := *view.Result | |||
| 293 | + | view.Result = &result | |||
| 294 | + | } | |||
| 295 | + | if view.LastRequest != nil { | |||
| 296 | + | request := *view.LastRequest | |||
| 297 | + | view.LastRequest = &request | |||
| 298 | + | } | |||
| 299 | + | return view | |||
| 300 | + | } | |||
| 301 | + | ||||
| 259 | // TurnStartedAt returns the factual start of the open delegated turn owning | 302 | // TurnStartedAt returns the factual start of the open delegated turn owning | ||
| 260 | // operation. It is stable across nested provider requests and tool calls. | 303 | // operation. It is stable across nested provider requests and tool calls. | ||
| 261 | func (v AgentTraceView) TurnStartedAt(operation Operation) time.Time { | 304 | func (v AgentTraceView) TurnStartedAt(operation Operation) time.Time { | ||
M packages/agent/subagents/requirement.go +33 -14
| @@ -27,12 +27,13 @@ const ( | |||
| 27 | 27 | // delegated message turn rather than a process attempt because resumable | |
| 28 | 28 | // workers may execute several turns in one process. | |
| 29 | 29 | type RequirementSnapshot struct { | |
| 30 | - | Required bool `json:"required"` | |
| 31 | - | State RequirementState `json:"state,omitempty"` | |
| 32 | - | TargetTurn int `json:"target_turn,omitempty"` | |
| 33 | - | UpdatedAt time.Time `json:"updated_at,omitempty"` | |
| 34 | - | ErrorCode string `json:"error_code,omitempty"` | |
| 35 | - | Notified bool `json:"notified,omitempty"` | |
| 30 | + | Required bool `json:"required"` | |
| 31 | + | State RequirementState `json:"state,omitempty"` | |
| 32 | + | TargetTurn int `json:"target_turn,omitempty"` | |
| 33 | + | ResultTurnID string `json:"result_turn_id,omitempty"` | |
| 34 | + | UpdatedAt time.Time `json:"updated_at,omitempty"` | |
| 35 | + | ErrorCode string `json:"error_code,omitempty"` | |
| 36 | + | Notified bool `json:"notified,omitempty"` | |
| 36 | 37 | } | |
| 37 | 38 | ||
| 38 | 39 | // Unmet reports whether this requirement still prevents parent completion. | |
| @@ -102,14 +103,14 @@ func (a *Agent) restoreRequirement(previous RequirementSnapshot) { | |||
| 102 | 103 | a.lifecycleMu.Unlock() | |
| 103 | 104 | } | |
| 104 | 105 | ||
| 105 | - | func (a *Agent) markRequirementNotified() bool { | |
| 106 | + | func (a *Agent) markRequirementNotified() (bool, error) { | |
| 106 | 107 | if a == nil { | |
| 107 | - | return false | |
| 108 | + | return false, nil | |
| 108 | 109 | } | |
| 109 | 110 | a.lifecycleMu.Lock() | |
| 110 | 111 | if !a.requirement.Required || a.requirement.Notified { | |
| 111 | 112 | a.lifecycleMu.Unlock() | |
| 112 | - | return false | |
| 113 | + | return false, nil | |
| 113 | 114 | } | |
| 114 | 115 | previous := a.requirement | |
| 115 | 116 | a.requirement.Notified = true | |
| @@ -124,7 +125,7 @@ func (a *Agent) markRequirementNotified() bool { | |||
| 124 | 125 | } | |
| 125 | 126 | a.lifecycleMu.Unlock() | |
| 126 | 127 | a.recordPersistenceError(err) | |
| 127 | - | return false | |
| 128 | + | return false, err | |
| 128 | 129 | } | |
| 129 | 130 | } | |
| 130 | 131 | a.lifecycleMu.Lock() | |
| @@ -132,7 +133,7 @@ func (a *Agent) markRequirementNotified() bool { | |||
| 132 | 133 | a.signalRequirementLocked() | |
| 133 | 134 | } | |
| 134 | 135 | a.lifecycleMu.Unlock() | |
| 135 | - | return true | |
| 136 | + | return true, nil | |
| 136 | 137 | } | |
| 137 | 138 | ||
| 138 | 139 | func (a *Agent) resolveRequirement(step int, result *TurnResult, errMsg string, force bool) RequirementSnapshot { | |
| @@ -182,6 +183,9 @@ func (a *Agent) resolveRequirement(step int, result *TurnResult, errMsg string, | |||
| 182 | 183 | state, errorCode := classifyRequirementOutcome(result, errMsg) | |
| 183 | 184 | a.requirement.State = state | |
| 184 | 185 | a.requirement.ErrorCode = errorCode | |
| 186 | + | if result != nil { | |
| 187 | + | a.requirement.ResultTurnID = strings.TrimSpace(result.TurnID) | |
| 188 | + | } | |
| 185 | 189 | a.requirement.Notified = false | |
| 186 | 190 | a.requirement.UpdatedAt = time.Now().UTC() | |
| 187 | 191 | resolved := a.requirement | |
| @@ -311,11 +315,26 @@ func (f *Supervisor) MarkRequirementNotified(id string) error { | |||
| 311 | 315 | if a == nil { | |
| 312 | 316 | return fmt.Errorf("subagents: no such agent %q", id) | |
| 313 | 317 | } | |
| 314 | - | if strings.TrimSpace(a.Snapshot().ResultRef) == "" { | |
| 318 | + | snapshot := a.Snapshot() | |
| 319 | + | if strings.TrimSpace(snapshot.ResultRef) == "" { | |
| 315 | 320 | return errors.New("subagents: required result is not durably available") | |
| 316 | 321 | } | |
| 317 | - | if a.markRequirementNotified() { | |
| 318 | - | a.recordTrace(TraceEvent{Type: "result.delivered", TurnID: a.CurrentTurnID(), Data: map[string]any{"ref": ResultRef(a.ID)}}) | |
| 322 | + | requirement := snapshot.Requirement | |
| 323 | + | expectedTurnID := strings.TrimSpace(requirement.ResultTurnID) | |
| 324 | + | if expectedTurnID == "" && requirement.TargetTurn > 0 { | |
| 325 | + | expectedTurnID = fmt.Sprintf("turn-%d", requirement.TargetTurn) | |
| 326 | + | } | |
| 327 | + | result := snapshot.Result | |
| 328 | + | if result == nil || strings.TrimSpace(result.TurnID) != expectedTurnID { | |
| 329 | + | return fmt.Errorf("subagents: durable required result does not match %s", expectedTurnID) | |
| 330 | + | } | |
| 331 | + | marked, err := a.markRequirementNotified() | |
| 332 | + | if err != nil { | |
| 333 | + | a.recordTrace(TraceEvent{Type: "result.delivery.failed", TurnID: expectedTurnID, Data: map[string]any{"ref": ResultRef(a.ID)}}) | |
| 334 | + | return fmt.Errorf("subagents: persist required result delivery: %w", err) | |
| 335 | + | } | |
| 336 | + | if marked { | |
| 337 | + | a.recordTrace(TraceEvent{Type: "result.delivered", TurnID: expectedTurnID, Data: map[string]any{"ref": ResultRef(a.ID)}}) | |
| 319 | 338 | } | |
| 320 | 339 | return nil | |
| 321 | 340 | } | |
| @@ -27,12 +27,13 @@ const ( | |||||
| 27 | // delegated message turn rather than a process attempt because resumable | 27 | // delegated message turn rather than a process attempt because resumable | ||
| 28 | // workers may execute several turns in one process. | 28 | // workers may execute several turns in one process. | ||
| 29 | type RequirementSnapshot struct { | 29 | type RequirementSnapshot struct { | ||
| 30 | - | Required bool `json:"required"` | 30 | + | Required bool `json:"required"` |
| 31 | - | State RequirementState `json:"state,omitempty"` | 31 | + | State RequirementState `json:"state,omitempty"` |
| 32 | - | TargetTurn int `json:"target_turn,omitempty"` | 32 | + | TargetTurn int `json:"target_turn,omitempty"` |
| 33 | - | UpdatedAt time.Time `json:"updated_at,omitempty"` | 33 | + | ResultTurnID string `json:"result_turn_id,omitempty"` |
| 34 | - | ErrorCode string `json:"error_code,omitempty"` | 34 | + | UpdatedAt time.Time `json:"updated_at,omitempty"` |
| 35 | - | Notified bool `json:"notified,omitempty"` | 35 | + | ErrorCode string `json:"error_code,omitempty"` |
| 36 | + | Notified bool `json:"notified,omitempty"` | |||
| 36 | } | 37 | } | ||
| 37 | 38 | ||||
| 38 | // Unmet reports whether this requirement still prevents parent completion. | 39 | // Unmet reports whether this requirement still prevents parent completion. | ||
| @@ -102,14 +103,14 @@ func (a *Agent) restoreRequirement(previous RequirementSnapshot) { | |||||
| 102 | a.lifecycleMu.Unlock() | 103 | a.lifecycleMu.Unlock() | ||
| 103 | } | 104 | } | ||
| 104 | 105 | ||||
| 105 | - | func (a *Agent) markRequirementNotified() bool { | 106 | + | func (a *Agent) markRequirementNotified() (bool, error) { |
| 106 | if a == nil { | 107 | if a == nil { | ||
| 107 | - | return false | 108 | + | return false, nil |
| 108 | } | 109 | } | ||
| 109 | a.lifecycleMu.Lock() | 110 | a.lifecycleMu.Lock() | ||
| 110 | if !a.requirement.Required || a.requirement.Notified { | 111 | if !a.requirement.Required || a.requirement.Notified { | ||
| 111 | a.lifecycleMu.Unlock() | 112 | a.lifecycleMu.Unlock() | ||
| 112 | - | return false | 113 | + | return false, nil |
| 113 | } | 114 | } | ||
| 114 | previous := a.requirement | 115 | previous := a.requirement | ||
| 115 | a.requirement.Notified = true | 116 | a.requirement.Notified = true | ||
| @@ -124,7 +125,7 @@ func (a *Agent) markRequirementNotified() bool { | |||||
| 124 | } | 125 | } | ||
| 125 | a.lifecycleMu.Unlock() | 126 | a.lifecycleMu.Unlock() | ||
| 126 | a.recordPersistenceError(err) | 127 | a.recordPersistenceError(err) | ||
| 127 | - | return false | 128 | + | return false, err |
| 128 | } | 129 | } | ||
| 129 | } | 130 | } | ||
| 130 | a.lifecycleMu.Lock() | 131 | a.lifecycleMu.Lock() | ||
| @@ -132,7 +133,7 @@ func (a *Agent) markRequirementNotified() bool { | |||||
| 132 | a.signalRequirementLocked() | 133 | a.signalRequirementLocked() | ||
| 133 | } | 134 | } | ||
| 134 | a.lifecycleMu.Unlock() | 135 | a.lifecycleMu.Unlock() | ||
| 135 | - | return true | 136 | + | return true, nil |
| 136 | } | 137 | } | ||
| 137 | 138 | ||||
| 138 | func (a *Agent) resolveRequirement(step int, result *TurnResult, errMsg string, force bool) RequirementSnapshot { | 139 | func (a *Agent) resolveRequirement(step int, result *TurnResult, errMsg string, force bool) RequirementSnapshot { | ||
| @@ -182,6 +183,9 @@ func (a *Agent) resolveRequirement(step int, result *TurnResult, errMsg string, | |||||
| 182 | state, errorCode := classifyRequirementOutcome(result, errMsg) | 183 | state, errorCode := classifyRequirementOutcome(result, errMsg) | ||
| 183 | a.requirement.State = state | 184 | a.requirement.State = state | ||
| 184 | a.requirement.ErrorCode = errorCode | 185 | a.requirement.ErrorCode = errorCode | ||
| 186 | + | if result != nil { | |||
| 187 | + | a.requirement.ResultTurnID = strings.TrimSpace(result.TurnID) | |||
| 188 | + | } | |||
| 185 | a.requirement.Notified = false | 189 | a.requirement.Notified = false | ||
| 186 | a.requirement.UpdatedAt = time.Now().UTC() | 190 | a.requirement.UpdatedAt = time.Now().UTC() | ||
| 187 | resolved := a.requirement | 191 | resolved := a.requirement | ||
| @@ -311,11 +315,26 @@ func (f *Supervisor) MarkRequirementNotified(id string) error { | |||||
| 311 | if a == nil { | 315 | if a == nil { | ||
| 312 | return fmt.Errorf("subagents: no such agent %q", id) | 316 | return fmt.Errorf("subagents: no such agent %q", id) | ||
| 313 | } | 317 | } | ||
| 314 | - | if strings.TrimSpace(a.Snapshot().ResultRef) == "" { | 318 | + | snapshot := a.Snapshot() |
| 319 | + | if strings.TrimSpace(snapshot.ResultRef) == "" { | |||
| 315 | return errors.New("subagents: required result is not durably available") | 320 | return errors.New("subagents: required result is not durably available") | ||
| 316 | } | 321 | } | ||
| 317 | - | if a.markRequirementNotified() { | 322 | + | requirement := snapshot.Requirement |
| 318 | - | a.recordTrace(TraceEvent{Type: "result.delivered", TurnID: a.CurrentTurnID(), Data: map[string]any{"ref": ResultRef(a.ID)}}) | 323 | + | expectedTurnID := strings.TrimSpace(requirement.ResultTurnID) |
| 324 | + | if expectedTurnID == "" && requirement.TargetTurn > 0 { | |||
| 325 | + | expectedTurnID = fmt.Sprintf("turn-%d", requirement.TargetTurn) | |||
| 326 | + | } | |||
| 327 | + | result := snapshot.Result | |||
| 328 | + | if result == nil || strings.TrimSpace(result.TurnID) != expectedTurnID { | |||
| 329 | + | return fmt.Errorf("subagents: durable required result does not match %s", expectedTurnID) | |||
| 330 | + | } | |||
| 331 | + | marked, err := a.markRequirementNotified() | |||
| 332 | + | if err != nil { | |||
| 333 | + | a.recordTrace(TraceEvent{Type: "result.delivery.failed", TurnID: expectedTurnID, Data: map[string]any{"ref": ResultRef(a.ID)}}) | |||
| 334 | + | return fmt.Errorf("subagents: persist required result delivery: %w", err) | |||
| 335 | + | } | |||
| 336 | + | if marked { | |||
| 337 | + | a.recordTrace(TraceEvent{Type: "result.delivered", TurnID: expectedTurnID, Data: map[string]any{"ref": ResultRef(a.ID)}}) | |||
| 319 | } | 338 | } | ||
| 320 | return nil | 339 | return nil | ||
| 321 | } | 340 | } | ||
M packages/agent/subagents/requirement_test.go +31 -1
| @@ -5,6 +5,7 @@ import ( | |||
| 5 | 5 | "errors" | |
| 6 | 6 | "path/filepath" | |
| 7 | 7 | "runtime" | |
| 8 | + | "strings" | |
| 8 | 9 | "testing" | |
| 9 | 10 | "time" | |
| 10 | 11 | ) | |
| @@ -221,7 +222,7 @@ func TestRequiredOutcomePersistsAcrossReload(t *testing.T) { | |||
| 221 | 222 | func TestRequirementNotificationRecordsDeliveryExactlyOnce(t *testing.T) { | |
| 222 | 223 | trace := NewMemoryTraceWriter() | |
| 223 | 224 | t.Cleanup(func() { _ = trace.Close() }) | |
| 224 | - | a := &Agent{ID: "agent-1", trace: trace, resultRef: ResultRef("agent-1"), requirement: RequirementSnapshot{Required: true, State: RequirementSatisfied, TargetTurn: 1}} | |
| 225 | + | a := &Agent{ID: "agent-1", trace: trace, result: &TurnResult{AgentID: "agent-1", TurnID: "turn-1", Status: ResultSucceeded}, resultRef: ResultRef("agent-1"), requirement: RequirementSnapshot{Required: true, State: RequirementSatisfied, TargetTurn: 1, ResultTurnID: "turn-1"}} | |
| 225 | 226 | s := &Supervisor{agents: map[string]*Agent{a.ID: a}} | |
| 226 | 227 | if err := s.MarkRequirementNotified(a.ID); err != nil { | |
| 227 | 228 | t.Fatal(err) | |
| @@ -243,6 +244,35 @@ func TestRequirementNotificationRecordsDeliveryExactlyOnce(t *testing.T) { | |||
| 243 | 244 | } | |
| 244 | 245 | } | |
| 245 | 246 | ||
| 247 | + | func TestRequirementNotificationPersistenceFailureIsReported(t *testing.T) { | |
| 248 | + | trace := NewMemoryTraceWriter() | |
| 249 | + | t.Cleanup(func() { _ = trace.Close() }) | |
| 250 | + | a := &Agent{ | |
| 251 | + | ID: "agent-1", | |
| 252 | + | trace: trace, | |
| 253 | + | result: &TurnResult{AgentID: "agent-1", TurnID: "turn-1", Status: ResultSucceeded}, | |
| 254 | + | resultRef: ResultRef("agent-1"), | |
| 255 | + | requirement: RequirementSnapshot{ | |
| 256 | + | Required: true, State: RequirementSatisfied, TargetTurn: 1, ResultTurnID: "turn-1", | |
| 257 | + | }, | |
| 258 | + | persistFn: func(*Agent) error { return errors.New("disk full") }, | |
| 259 | + | } | |
| 260 | + | s := &Supervisor{agents: map[string]*Agent{a.ID: a}} | |
| 261 | + | if err := s.MarkRequirementNotified(a.ID); err == nil || !strings.Contains(err.Error(), "disk full") { | |
| 262 | + | t.Fatalf("MarkRequirementNotified error = %v, want persistence failure", err) | |
| 263 | + | } | |
| 264 | + | if a.Snapshot().Requirement.Notified { | |
| 265 | + | t.Fatal("requirement remained notified after persistence failure") | |
| 266 | + | } | |
| 267 | + | if err := trace.Flush(); err != nil { | |
| 268 | + | t.Fatal(err) | |
| 269 | + | } | |
| 270 | + | view := trace.Views()[a.ID] | |
| 271 | + | if view.Result == nil || !view.Result.Failed { | |
| 272 | + | t.Fatalf("delivery failure trace = %#v", view.Result) | |
| 273 | + | } | |
| 274 | + | } | |
| 275 | + | ||
| 246 | 276 | func TestRequirementNotificationRejectsMissingDurableResult(t *testing.T) { | |
| 247 | 277 | trace := NewMemoryTraceWriter() | |
| 248 | 278 | t.Cleanup(func() { _ = trace.Close() }) | |
| @@ -5,6 +5,7 @@ import ( | |||||
| 5 | "errors" | 5 | "errors" | ||
| 6 | "path/filepath" | 6 | "path/filepath" | ||
| 7 | "runtime" | 7 | "runtime" | ||
| 8 | + | "strings" | |||
| 8 | "testing" | 9 | "testing" | ||
| 9 | "time" | 10 | "time" | ||
| 10 | ) | 11 | ) | ||
| @@ -221,7 +222,7 @@ func TestRequiredOutcomePersistsAcrossReload(t *testing.T) { | |||||
| 221 | func TestRequirementNotificationRecordsDeliveryExactlyOnce(t *testing.T) { | 222 | func TestRequirementNotificationRecordsDeliveryExactlyOnce(t *testing.T) { | ||
| 222 | trace := NewMemoryTraceWriter() | 223 | trace := NewMemoryTraceWriter() | ||
| 223 | t.Cleanup(func() { _ = trace.Close() }) | 224 | t.Cleanup(func() { _ = trace.Close() }) | ||
| 224 | - | a := &Agent{ID: "agent-1", trace: trace, resultRef: ResultRef("agent-1"), requirement: RequirementSnapshot{Required: true, State: RequirementSatisfied, TargetTurn: 1}} | 225 | + | a := &Agent{ID: "agent-1", trace: trace, result: &TurnResult{AgentID: "agent-1", TurnID: "turn-1", Status: ResultSucceeded}, resultRef: ResultRef("agent-1"), requirement: RequirementSnapshot{Required: true, State: RequirementSatisfied, TargetTurn: 1, ResultTurnID: "turn-1"}} |
| 225 | s := &Supervisor{agents: map[string]*Agent{a.ID: a}} | 226 | s := &Supervisor{agents: map[string]*Agent{a.ID: a}} | ||
| 226 | if err := s.MarkRequirementNotified(a.ID); err != nil { | 227 | if err := s.MarkRequirementNotified(a.ID); err != nil { | ||
| 227 | t.Fatal(err) | 228 | t.Fatal(err) | ||
| @@ -243,6 +244,35 @@ func TestRequirementNotificationRecordsDeliveryExactlyOnce(t *testing.T) { | |||||
| 243 | } | 244 | } | ||
| 244 | } | 245 | } | ||
| 245 | 246 | ||||
| 247 | + | func TestRequirementNotificationPersistenceFailureIsReported(t *testing.T) { | |||
| 248 | + | trace := NewMemoryTraceWriter() | |||
| 249 | + | t.Cleanup(func() { _ = trace.Close() }) | |||
| 250 | + | a := &Agent{ | |||
| 251 | + | ID: "agent-1", | |||
| 252 | + | trace: trace, | |||
| 253 | + | result: &TurnResult{AgentID: "agent-1", TurnID: "turn-1", Status: ResultSucceeded}, | |||
| 254 | + | resultRef: ResultRef("agent-1"), | |||
| 255 | + | requirement: RequirementSnapshot{ | |||
| 256 | + | Required: true, State: RequirementSatisfied, TargetTurn: 1, ResultTurnID: "turn-1", | |||
| 257 | + | }, | |||
| 258 | + | persistFn: func(*Agent) error { return errors.New("disk full") }, | |||
| 259 | + | } | |||
| 260 | + | s := &Supervisor{agents: map[string]*Agent{a.ID: a}} | |||
| 261 | + | if err := s.MarkRequirementNotified(a.ID); err == nil || !strings.Contains(err.Error(), "disk full") { | |||
| 262 | + | t.Fatalf("MarkRequirementNotified error = %v, want persistence failure", err) | |||
| 263 | + | } | |||
| 264 | + | if a.Snapshot().Requirement.Notified { | |||
| 265 | + | t.Fatal("requirement remained notified after persistence failure") | |||
| 266 | + | } | |||
| 267 | + | if err := trace.Flush(); err != nil { | |||
| 268 | + | t.Fatal(err) | |||
| 269 | + | } | |||
| 270 | + | view := trace.Views()[a.ID] | |||
| 271 | + | if view.Result == nil || !view.Result.Failed { | |||
| 272 | + | t.Fatalf("delivery failure trace = %#v", view.Result) | |||
| 273 | + | } | |||
| 274 | + | } | |||
| 275 | + | ||||
| 246 | func TestRequirementNotificationRejectsMissingDurableResult(t *testing.T) { | 276 | func TestRequirementNotificationRejectsMissingDurableResult(t *testing.T) { | ||
| 247 | trace := NewMemoryTraceWriter() | 277 | trace := NewMemoryTraceWriter() | ||
| 248 | t.Cleanup(func() { _ = trace.Close() }) | 278 | t.Cleanup(func() { _ = trace.Close() }) | ||
M packages/agent/subagents/runner.go +1 -0
| @@ -782,6 +782,7 @@ func updateAgentFromEvent(a *Agent, ev Event) error { | |||
| 782 | 782 | a.lifecycleMu.Lock() | |
| 783 | 783 | a.resultRef = ResultRef(a.ID) | |
| 784 | 784 | a.lifecycleMu.Unlock() | |
| 785 | + | a.recordTrace(TraceEvent{Type: "result.available", TurnID: result.TurnID, Data: map[string]any{"ref": ResultRef(a.ID)}}) | |
| 785 | 786 | } | |
| 786 | 787 | } | |
| 787 | 788 | a.setResult(result) | |
| @@ -782,6 +782,7 @@ func updateAgentFromEvent(a *Agent, ev Event) error { | |||||
| 782 | a.lifecycleMu.Lock() | 782 | a.lifecycleMu.Lock() | ||
| 783 | a.resultRef = ResultRef(a.ID) | 783 | a.resultRef = ResultRef(a.ID) | ||
| 784 | a.lifecycleMu.Unlock() | 784 | a.lifecycleMu.Unlock() | ||
| 785 | + | a.recordTrace(TraceEvent{Type: "result.available", TurnID: result.TurnID, Data: map[string]any{"ref": ResultRef(a.ID)}}) | |||
| 785 | } | 786 | } | ||
| 786 | } | 787 | } | ||
| 787 | a.setResult(result) | 788 | a.setResult(result) | ||
M packages/agent/subagents/runner_test.go +19 -0
| @@ -783,6 +783,25 @@ func TestUpdateAgentFromEventSurfacesTurnResultPersistenceFailure(t *testing.T) | |||
| 783 | 783 | } | |
| 784 | 784 | } | |
| 785 | 785 | ||
| 786 | + | func TestUpdateAgentFromEventRecordsDurableResultAvailability(t *testing.T) { | |
| 787 | + | trace := NewMemoryTraceWriter() | |
| 788 | + | t.Cleanup(func() { _ = trace.Close() }) | |
| 789 | + | a := &Agent{ID: "agent-1", stateDir: t.TempDir(), trace: trace} | |
| 790 | + | if err := updateAgentFromEvent(a, Event{Type: EventTurnResult, TurnID: "turn-1", Data: map[string]any{ | |
| 791 | + | "status": "succeeded", | |
| 792 | + | "output": "review complete", | |
| 793 | + | }}); err != nil { | |
| 794 | + | t.Fatal(err) | |
| 795 | + | } | |
| 796 | + | if err := trace.Flush(); err != nil { | |
| 797 | + | t.Fatal(err) | |
| 798 | + | } | |
| 799 | + | view := trace.Views()[a.ID] | |
| 800 | + | if view.Result == nil || !view.Result.Available || view.Result.Ref != ResultRef(a.ID) { | |
| 801 | + | t.Fatalf("live result trace = %#v, want durable availability", view.Result) | |
| 802 | + | } | |
| 803 | + | } | |
| 804 | + | ||
| 786 | 805 | func TestRestoreResumePromptPreservesCommandIdentity(t *testing.T) { | |
| 787 | 806 | a := &Agent{} | |
| 788 | 807 | a.setResumePrompt("old prompt", time.Now()) | |
| @@ -783,6 +783,25 @@ func TestUpdateAgentFromEventSurfacesTurnResultPersistenceFailure(t *testing.T) | |||||
| 783 | } | 783 | } | ||
| 784 | } | 784 | } | ||
| 785 | 785 | ||||
| 786 | + | func TestUpdateAgentFromEventRecordsDurableResultAvailability(t *testing.T) { | |||
| 787 | + | trace := NewMemoryTraceWriter() | |||
| 788 | + | t.Cleanup(func() { _ = trace.Close() }) | |||
| 789 | + | a := &Agent{ID: "agent-1", stateDir: t.TempDir(), trace: trace} | |||
| 790 | + | if err := updateAgentFromEvent(a, Event{Type: EventTurnResult, TurnID: "turn-1", Data: map[string]any{ | |||
| 791 | + | "status": "succeeded", | |||
| 792 | + | "output": "review complete", | |||
| 793 | + | }}); err != nil { | |||
| 794 | + | t.Fatal(err) | |||
| 795 | + | } | |||
| 796 | + | if err := trace.Flush(); err != nil { | |||
| 797 | + | t.Fatal(err) | |||
| 798 | + | } | |||
| 799 | + | view := trace.Views()[a.ID] | |||
| 800 | + | if view.Result == nil || !view.Result.Available || view.Result.Ref != ResultRef(a.ID) { | |||
| 801 | + | t.Fatalf("live result trace = %#v, want durable availability", view.Result) | |||
| 802 | + | } | |||
| 803 | + | } | |||
| 804 | + | ||||
| 786 | func TestRestoreResumePromptPreservesCommandIdentity(t *testing.T) { | 805 | func TestRestoreResumePromptPreservesCommandIdentity(t *testing.T) { | ||
| 787 | a := &Agent{} | 806 | a := &Agent{} | ||
| 788 | a.setResumePrompt("old prompt", time.Now()) | 807 | a.setResumePrompt("old prompt", time.Now()) | ||
M packages/agent/subagents/scheduler.go +1 -1
| @@ -63,7 +63,7 @@ func (f *Supervisor) validateSpawnRequest(req SpawnRequest) error { | |||
| 63 | 63 | ||
| 64 | 64 | func (f *Supervisor) validateSpawnOptions(req SpawnRequest) error { | |
| 65 | 65 | p := f.cfg.Policy | |
| 66 | - | if req.MaxTurns < 0 || req.MaxTurns > p.MaxTurns { | |
| 66 | + | if req.MaxTurns < 0 || (p.MaxTurns > 0 && req.MaxTurns > p.MaxTurns) { | |
| 67 | 67 | return fmt.Errorf("subagents: max_turns must be 0 through %d (zero uses the policy default)", p.MaxTurns) | |
| 68 | 68 | } | |
| 69 | 69 | if req.Timeout < 0 { | |
| @@ -63,7 +63,7 @@ func (f *Supervisor) validateSpawnRequest(req SpawnRequest) error { | |||||
| 63 | 63 | ||||
| 64 | func (f *Supervisor) validateSpawnOptions(req SpawnRequest) error { | 64 | func (f *Supervisor) validateSpawnOptions(req SpawnRequest) error { | ||
| 65 | p := f.cfg.Policy | 65 | p := f.cfg.Policy | ||
| 66 | - | if req.MaxTurns < 0 || req.MaxTurns > p.MaxTurns { | 66 | + | if req.MaxTurns < 0 || (p.MaxTurns > 0 && req.MaxTurns > p.MaxTurns) { |
| 67 | return fmt.Errorf("subagents: max_turns must be 0 through %d (zero uses the policy default)", p.MaxTurns) | 67 | return fmt.Errorf("subagents: max_turns must be 0 through %d (zero uses the policy default)", p.MaxTurns) | ||
| 68 | } | 68 | } | ||
| 69 | if req.Timeout < 0 { | 69 | if req.Timeout < 0 { | ||
M packages/agent/subagents/supervisor.go +1 -1
| @@ -643,7 +643,7 @@ func (f *Supervisor) TraceViews() map[string]AgentTraceView { | |||
| 643 | 643 | if f == nil || f.trace == nil { | |
| 644 | 644 | return map[string]AgentTraceView{} | |
| 645 | 645 | } | |
| 646 | - | return ProjectTrace(f.trace.Events()) | |
| 646 | + | return f.trace.Views() | |
| 647 | 647 | } | |
| 648 | 648 | ||
| 649 | 649 | // Close flushes and closes the optional trace bundle. It does not stop agents; | |
| @@ -643,7 +643,7 @@ func (f *Supervisor) TraceViews() map[string]AgentTraceView { | |||||
| 643 | if f == nil || f.trace == nil { | 643 | if f == nil || f.trace == nil { | ||
| 644 | return map[string]AgentTraceView{} | 644 | return map[string]AgentTraceView{} | ||
| 645 | } | 645 | } | ||
| 646 | - | return ProjectTrace(f.trace.Events()) | 646 | + | return f.trace.Views() |
| 647 | } | 647 | } | ||
| 648 | 648 | ||||
| 649 | // Close flushes and closes the optional trace bundle. It does not stop agents; | 649 | // Close flushes and closes the optional trace bundle. It does not stop agents; | ||
M packages/agent/subagents/supervisor_test.go +16 -4
| @@ -86,20 +86,32 @@ func TestSpawnWithoutTimeoutUsesUnlimitedTurns(t *testing.T) { | |||
| 86 | 86 | a.Wait() | |
| 87 | 87 | } | |
| 88 | 88 | ||
| 89 | - | func TestSpawnUsesDefaultModelStepBudget(t *testing.T) { | |
| 89 | + | func TestSpawnLeavesModelAndTurnBudgetsUnlimitedByDefault(t *testing.T) { | |
| 90 | 90 | f := newTestSupervisor(t, func(*Agent) Runner { | |
| 91 | 91 | return RunnerFunc(func(context.Context, Sink) error { return nil }) | |
| 92 | 92 | }) | |
| 93 | - | a, err := f.Spawn(context.Background(), "bounded model loop") | |
| 93 | + | a, err := f.Spawn(context.Background(), "unlimited model loop") | |
| 94 | 94 | if err != nil { | |
| 95 | 95 | t.Fatal(err) | |
| 96 | 96 | } | |
| 97 | - | if a.MaxSteps != 128 { | |
| 98 | - | t.Fatalf("max steps = %d, want 128", a.MaxSteps) | |
| 97 | + | if a.MaxSteps != 0 || a.MaxTurns != 0 { | |
| 98 | + | t.Fatalf("model/turn budgets = %d/%d, want unlimited", a.MaxSteps, a.MaxTurns) | |
| 99 | 99 | } | |
| 100 | 100 | a.Wait() | |
| 101 | 101 | } | |
| 102 | 102 | ||
| 103 | + | func TestSubagentPolicyCanonicalizesNegativeUnlimitedBudgets(t *testing.T) { | |
| 104 | + | policy := SubagentPolicy{ | |
| 105 | + | DefaultTimeout: -time.Second, | |
| 106 | + | MaxTurns: -1, | |
| 107 | + | MaxSteps: -1, | |
| 108 | + | } | |
| 109 | + | policy.normalize() | |
| 110 | + | if policy.DefaultTimeout != 0 || policy.MaxTurns != 0 || policy.MaxSteps != 0 { | |
| 111 | + | t.Fatalf("normalized budgets = %s/%d/%d, want unlimited zeros", policy.DefaultTimeout, policy.MaxTurns, policy.MaxSteps) | |
| 112 | + | } | |
| 113 | + | } | |
| 114 | + | ||
| 103 | 115 | func TestSpawnRunsAndCompletes(t *testing.T) { | |
| 104 | 116 | ran := make(chan string, 1) | |
| 105 | 117 | f := newTestSupervisor(t, func(a *Agent) Runner { | |
| @@ -86,20 +86,32 @@ func TestSpawnWithoutTimeoutUsesUnlimitedTurns(t *testing.T) { | |||||
| 86 | a.Wait() | 86 | a.Wait() | ||
| 87 | } | 87 | } | ||
| 88 | 88 | ||||
| 89 | - | func TestSpawnUsesDefaultModelStepBudget(t *testing.T) { | 89 | + | func TestSpawnLeavesModelAndTurnBudgetsUnlimitedByDefault(t *testing.T) { |
| 90 | f := newTestSupervisor(t, func(*Agent) Runner { | 90 | f := newTestSupervisor(t, func(*Agent) Runner { | ||
| 91 | return RunnerFunc(func(context.Context, Sink) error { return nil }) | 91 | return RunnerFunc(func(context.Context, Sink) error { return nil }) | ||
| 92 | }) | 92 | }) | ||
| 93 | - | a, err := f.Spawn(context.Background(), "bounded model loop") | 93 | + | a, err := f.Spawn(context.Background(), "unlimited model loop") |
| 94 | if err != nil { | 94 | if err != nil { | ||
| 95 | t.Fatal(err) | 95 | t.Fatal(err) | ||
| 96 | } | 96 | } | ||
| 97 | - | if a.MaxSteps != 128 { | 97 | + | if a.MaxSteps != 0 || a.MaxTurns != 0 { |
| 98 | - | t.Fatalf("max steps = %d, want 128", a.MaxSteps) | 98 | + | t.Fatalf("model/turn budgets = %d/%d, want unlimited", a.MaxSteps, a.MaxTurns) |
| 99 | } | 99 | } | ||
| 100 | a.Wait() | 100 | a.Wait() | ||
| 101 | } | 101 | } | ||
| 102 | 102 | ||||
| 103 | + | func TestSubagentPolicyCanonicalizesNegativeUnlimitedBudgets(t *testing.T) { | |||
| 104 | + | policy := SubagentPolicy{ | |||
| 105 | + | DefaultTimeout: -time.Second, | |||
| 106 | + | MaxTurns: -1, | |||
| 107 | + | MaxSteps: -1, | |||
| 108 | + | } | |||
| 109 | + | policy.normalize() | |||
| 110 | + | if policy.DefaultTimeout != 0 || policy.MaxTurns != 0 || policy.MaxSteps != 0 { | |||
| 111 | + | t.Fatalf("normalized budgets = %s/%d/%d, want unlimited zeros", policy.DefaultTimeout, policy.MaxTurns, policy.MaxSteps) | |||
| 112 | + | } | |||
| 113 | + | } | |||
| 114 | + | ||||
| 103 | func TestSpawnRunsAndCompletes(t *testing.T) { | 115 | func TestSpawnRunsAndCompletes(t *testing.T) { | ||
| 104 | ran := make(chan string, 1) | 116 | ran := make(chan string, 1) | ||
| 105 | f := newTestSupervisor(t, func(a *Agent) Runner { | 117 | f := newTestSupervisor(t, func(a *Agent) Runner { | ||
M packages/agent/subagents/trace.go +23 -0
| @@ -92,6 +92,7 @@ type TraceWriter struct { | |||
| 92 | 92 | sequence uint64 | |
| 93 | 93 | dropped uint64 | |
| 94 | 94 | events []TraceEvent | |
| 95 | + | projection *traceProjection | |
| 95 | 96 | ||
| 96 | 97 | errorMu sync.Mutex | |
| 97 | 98 | err error | |
| @@ -252,6 +253,13 @@ func (w *TraceWriter) Record(event TraceEvent) { | |||
| 252 | 253 | } | |
| 253 | 254 | w.sequence++ | |
| 254 | 255 | record.event.Seq = w.sequence | |
| 256 | + | if w.projection == nil { | |
| 257 | + | w.projection = newTraceProjection() | |
| 258 | + | } | |
| 259 | + | // Derive live state at ingestion rather than after asynchronous disk I/O. | |
| 260 | + | // This preserves operation boundaries even when the bounded write queue | |
| 261 | + | // drops an old raw event under sustained streaming pressure. | |
| 262 | + | w.projection.apply(record.event) | |
| 255 | 263 | if len(w.pending) >= traceQueueLimit { | |
| 256 | 264 | for index, pending := range w.pending { | |
| 257 | 265 | if pending.barrier == nil { | |
| @@ -375,6 +383,21 @@ func (w *TraceWriter) appendEvent(event TraceEvent) { | |||
| 375 | 383 | } | |
| 376 | 384 | } | |
| 377 | 385 | ||
| 386 | + | // Views returns the incrementally derived trace projection. Unlike replaying | |
| 387 | + | // Events, it retains open operation boundaries after old raw events are evicted | |
| 388 | + | // from the bounded in-memory window. | |
| 389 | + | func (w *TraceWriter) Views() map[string]AgentTraceView { | |
| 390 | + | if w == nil { | |
| 391 | + | return map[string]AgentTraceView{} | |
| 392 | + | } | |
| 393 | + | w.mu.Lock() | |
| 394 | + | defer w.mu.Unlock() | |
| 395 | + | if w.projection == nil { | |
| 396 | + | return map[string]AgentTraceView{} | |
| 397 | + | } | |
| 398 | + | return w.projection.snapshot() | |
| 399 | + | } | |
| 400 | + | ||
| 378 | 401 | // Events returns a stable copy of events already written by this writer. It | |
| 379 | 402 | // never waits for queued filesystem work, keeping status and UI reads cheap. | |
| 380 | 403 | func (w *TraceWriter) Events() []TraceEvent { | |
| @@ -92,6 +92,7 @@ type TraceWriter struct { | |||||
| 92 | sequence uint64 | 92 | sequence uint64 | ||
| 93 | dropped uint64 | 93 | dropped uint64 | ||
| 94 | events []TraceEvent | 94 | events []TraceEvent | ||
| 95 | + | projection *traceProjection | |||
| 95 | 96 | ||||
| 96 | errorMu sync.Mutex | 97 | errorMu sync.Mutex | ||
| 97 | err error | 98 | err error | ||
| @@ -252,6 +253,13 @@ func (w *TraceWriter) Record(event TraceEvent) { | |||||
| 252 | } | 253 | } | ||
| 253 | w.sequence++ | 254 | w.sequence++ | ||
| 254 | record.event.Seq = w.sequence | 255 | record.event.Seq = w.sequence | ||
| 256 | + | if w.projection == nil { | |||
| 257 | + | w.projection = newTraceProjection() | |||
| 258 | + | } | |||
| 259 | + | // Derive live state at ingestion rather than after asynchronous disk I/O. | |||
| 260 | + | // This preserves operation boundaries even when the bounded write queue | |||
| 261 | + | // drops an old raw event under sustained streaming pressure. | |||
| 262 | + | w.projection.apply(record.event) | |||
| 255 | if len(w.pending) >= traceQueueLimit { | 263 | if len(w.pending) >= traceQueueLimit { | ||
| 256 | for index, pending := range w.pending { | 264 | for index, pending := range w.pending { | ||
| 257 | if pending.barrier == nil { | 265 | if pending.barrier == nil { | ||
| @@ -375,6 +383,21 @@ func (w *TraceWriter) appendEvent(event TraceEvent) { | |||||
| 375 | } | 383 | } | ||
| 376 | } | 384 | } | ||
| 377 | 385 | ||||
| 386 | + | // Views returns the incrementally derived trace projection. Unlike replaying | |||
| 387 | + | // Events, it retains open operation boundaries after old raw events are evicted | |||
| 388 | + | // from the bounded in-memory window. | |||
| 389 | + | func (w *TraceWriter) Views() map[string]AgentTraceView { | |||
| 390 | + | if w == nil { | |||
| 391 | + | return map[string]AgentTraceView{} | |||
| 392 | + | } | |||
| 393 | + | w.mu.Lock() | |||
| 394 | + | defer w.mu.Unlock() | |||
| 395 | + | if w.projection == nil { | |||
| 396 | + | return map[string]AgentTraceView{} | |||
| 397 | + | } | |||
| 398 | + | return w.projection.snapshot() | |||
| 399 | + | } | |||
| 400 | + | ||||
| 378 | // Events returns a stable copy of events already written by this writer. It | 401 | // Events returns a stable copy of events already written by this writer. It | ||
| 379 | // never waits for queued filesystem work, keeping status and UI reads cheap. | 402 | // never waits for queued filesystem work, keeping status and UI reads cheap. | ||
| 380 | func (w *TraceWriter) Events() []TraceEvent { | 403 | func (w *TraceWriter) Events() []TraceEvent { | ||
M packages/agent/subagents/trace_test.go +88 -0
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | "strings" | |
| 10 | 10 | "sync" | |
| 11 | 11 | "testing" | |
| 12 | + | "time" | |
| 12 | 13 | ) | |
| 13 | 14 | ||
| 14 | 15 | func TestTraceContextZeroValueIsNoOp(t *testing.T) { | |
| @@ -100,6 +101,93 @@ func TestMemoryTraceWriterRetainsOrderedEventsWithoutBundle(t *testing.T) { | |||
| 100 | 101 | } | |
| 101 | 102 | } | |
| 102 | 103 | ||
| 104 | + | func TestSupervisorTraceViewsRetainOpenTurnAcrossEventEviction(t *testing.T) { | |
| 105 | + | writer := NewMemoryTraceWriter() | |
| 106 | + | t.Cleanup(func() { _ = writer.Close() }) | |
| 107 | + | supervisor := &Supervisor{trace: writer} | |
| 108 | + | started := time.Date(2026, time.August, 13, 1, 52, 0, 0, time.UTC) | |
| 109 | + | ||
| 110 | + | writer.Record(TraceEvent{ | |
| 111 | + | Timestamp: started, | |
| 112 | + | Type: "turn.started", | |
| 113 | + | AgentID: "agent-1", | |
| 114 | + | TurnID: "turn-1", | |
| 115 | + | }) | |
| 116 | + | for index := 0; index < memoryTraceEventLimit; index++ { | |
| 117 | + | writer.Record(TraceEvent{ | |
| 118 | + | Timestamp: started.Add(time.Duration(index+1) * time.Millisecond), | |
| 119 | + | Type: "assistant.stream.observed", | |
| 120 | + | AgentID: "agent-1", | |
| 121 | + | TurnID: "turn-1", | |
| 122 | + | }) | |
| 123 | + | if (index+1)%traceQueueInitialCapacity == 0 { | |
| 124 | + | if err := writer.Flush(); err != nil { | |
| 125 | + | t.Fatal(err) | |
| 126 | + | } | |
| 127 | + | } | |
| 128 | + | } | |
| 129 | + | requestStarted := started.Add(time.Minute) | |
| 130 | + | writer.Record(TraceEvent{ | |
| 131 | + | Timestamp: requestStarted, | |
| 132 | + | Type: "provider.request.started", | |
| 133 | + | AgentID: "agent-1", | |
| 134 | + | TurnID: "turn-1", | |
| 135 | + | }) | |
| 136 | + | writer.Record(TraceEvent{ | |
| 137 | + | Timestamp: requestStarted.Add(time.Second), | |
| 138 | + | Type: "provider.request.finished", | |
| 139 | + | AgentID: "agent-1", | |
| 140 | + | TurnID: "turn-1", | |
| 141 | + | }) | |
| 142 | + | if err := writer.Flush(); err != nil { | |
| 143 | + | t.Fatal(err) | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | view := supervisor.TraceViews()["agent-1"] | |
| 147 | + | if view.PrimaryOperation == nil || view.PrimaryOperation.Type != "turn.started" { | |
| 148 | + | t.Fatalf("primary operation after request = %#v, want open delegated turn", view.PrimaryOperation) | |
| 149 | + | } | |
| 150 | + | if !view.PrimaryOperation.StartedAt.Equal(started) { | |
| 151 | + | t.Fatalf("delegated turn started at %v, want %v", view.PrimaryOperation.StartedAt, started) | |
| 152 | + | } | |
| 153 | + | ||
| 154 | + | nextRequestStarted := requestStarted.Add(2 * time.Second) | |
| 155 | + | writer.Record(TraceEvent{ | |
| 156 | + | Timestamp: nextRequestStarted, | |
| 157 | + | Type: "provider.request.started", | |
| 158 | + | AgentID: "agent-1", | |
| 159 | + | TurnID: "turn-1", | |
| 160 | + | }) | |
| 161 | + | if err := writer.Flush(); err != nil { | |
| 162 | + | t.Fatal(err) | |
| 163 | + | } | |
| 164 | + | view = supervisor.TraceViews()["agent-1"] | |
| 165 | + | if view.PrimaryOperation == nil || view.PrimaryOperation.Type != "provider.request.started" { | |
| 166 | + | t.Fatalf("primary operation during next request = %#v, want provider request", view.PrimaryOperation) | |
| 167 | + | } | |
| 168 | + | if got := view.TurnStartedAt(*view.PrimaryOperation); !got.Equal(started) { | |
| 169 | + | t.Fatalf("elapsed timer anchor = %v, want delegated turn start %v", got, started) | |
| 170 | + | } | |
| 171 | + | } | |
| 172 | + | ||
| 173 | + | func TestTraceViewsObserveIngestedBoundariesBeforeAsyncWrite(t *testing.T) { | |
| 174 | + | writer := &TraceWriter{ | |
| 175 | + | pending: make([]traceRecord, 0, traceQueueInitialCapacity), | |
| 176 | + | } | |
| 177 | + | writer.cond = sync.NewCond(&writer.mu) | |
| 178 | + | writer.Record(TraceEvent{ | |
| 179 | + | Timestamp: time.Date(2026, time.August, 13, 2, 0, 0, 0, time.UTC), | |
| 180 | + | Type: "turn.started", | |
| 181 | + | AgentID: "agent-1", | |
| 182 | + | TurnID: "turn-1", | |
| 183 | + | }) | |
| 184 | + | ||
| 185 | + | view := writer.Views()["agent-1"] | |
| 186 | + | if view.PrimaryOperation == nil || view.PrimaryOperation.Type != "turn.started" { | |
| 187 | + | t.Fatalf("primary operation before async write = %#v, want delegated turn", view.PrimaryOperation) | |
| 188 | + | } | |
| 189 | + | } | |
| 190 | + | ||
| 103 | 191 | func TestTraceWriterNormalRedactsSensitiveValues(t *testing.T) { | |
| 104 | 192 | writer, err := NewTraceWriter(filepath.Join(t.TempDir(), "trace"), TraceModeNormal) | |
| 105 | 193 | if err != nil { | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | "strings" | 9 | "strings" | ||
| 10 | "sync" | 10 | "sync" | ||
| 11 | "testing" | 11 | "testing" | ||
| 12 | + | "time" | |||
| 12 | ) | 13 | ) | ||
| 13 | 14 | ||||
| 14 | func TestTraceContextZeroValueIsNoOp(t *testing.T) { | 15 | func TestTraceContextZeroValueIsNoOp(t *testing.T) { | ||
| @@ -100,6 +101,93 @@ func TestMemoryTraceWriterRetainsOrderedEventsWithoutBundle(t *testing.T) { | |||||
| 100 | } | 101 | } | ||
| 101 | } | 102 | } | ||
| 102 | 103 | ||||
| 104 | + | func TestSupervisorTraceViewsRetainOpenTurnAcrossEventEviction(t *testing.T) { | |||
| 105 | + | writer := NewMemoryTraceWriter() | |||
| 106 | + | t.Cleanup(func() { _ = writer.Close() }) | |||
| 107 | + | supervisor := &Supervisor{trace: writer} | |||
| 108 | + | started := time.Date(2026, time.August, 13, 1, 52, 0, 0, time.UTC) | |||
| 109 | + | ||||
| 110 | + | writer.Record(TraceEvent{ | |||
| 111 | + | Timestamp: started, | |||
| 112 | + | Type: "turn.started", | |||
| 113 | + | AgentID: "agent-1", | |||
| 114 | + | TurnID: "turn-1", | |||
| 115 | + | }) | |||
| 116 | + | for index := 0; index < memoryTraceEventLimit; index++ { | |||
| 117 | + | writer.Record(TraceEvent{ | |||
| 118 | + | Timestamp: started.Add(time.Duration(index+1) * time.Millisecond), | |||
| 119 | + | Type: "assistant.stream.observed", | |||
| 120 | + | AgentID: "agent-1", | |||
| 121 | + | TurnID: "turn-1", | |||
| 122 | + | }) | |||
| 123 | + | if (index+1)%traceQueueInitialCapacity == 0 { | |||
| 124 | + | if err := writer.Flush(); err != nil { | |||
| 125 | + | t.Fatal(err) | |||
| 126 | + | } | |||
| 127 | + | } | |||
| 128 | + | } | |||
| 129 | + | requestStarted := started.Add(time.Minute) | |||
| 130 | + | writer.Record(TraceEvent{ | |||
| 131 | + | Timestamp: requestStarted, | |||
| 132 | + | Type: "provider.request.started", | |||
| 133 | + | AgentID: "agent-1", | |||
| 134 | + | TurnID: "turn-1", | |||
| 135 | + | }) | |||
| 136 | + | writer.Record(TraceEvent{ | |||
| 137 | + | Timestamp: requestStarted.Add(time.Second), | |||
| 138 | + | Type: "provider.request.finished", | |||
| 139 | + | AgentID: "agent-1", | |||
| 140 | + | TurnID: "turn-1", | |||
| 141 | + | }) | |||
| 142 | + | if err := writer.Flush(); err != nil { | |||
| 143 | + | t.Fatal(err) | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | view := supervisor.TraceViews()["agent-1"] | |||
| 147 | + | if view.PrimaryOperation == nil || view.PrimaryOperation.Type != "turn.started" { | |||
| 148 | + | t.Fatalf("primary operation after request = %#v, want open delegated turn", view.PrimaryOperation) | |||
| 149 | + | } | |||
| 150 | + | if !view.PrimaryOperation.StartedAt.Equal(started) { | |||
| 151 | + | t.Fatalf("delegated turn started at %v, want %v", view.PrimaryOperation.StartedAt, started) | |||
| 152 | + | } | |||
| 153 | + | ||||
| 154 | + | nextRequestStarted := requestStarted.Add(2 * time.Second) | |||
| 155 | + | writer.Record(TraceEvent{ | |||
| 156 | + | Timestamp: nextRequestStarted, | |||
| 157 | + | Type: "provider.request.started", | |||
| 158 | + | AgentID: "agent-1", | |||
| 159 | + | TurnID: "turn-1", | |||
| 160 | + | }) | |||
| 161 | + | if err := writer.Flush(); err != nil { | |||
| 162 | + | t.Fatal(err) | |||
| 163 | + | } | |||
| 164 | + | view = supervisor.TraceViews()["agent-1"] | |||
| 165 | + | if view.PrimaryOperation == nil || view.PrimaryOperation.Type != "provider.request.started" { | |||
| 166 | + | t.Fatalf("primary operation during next request = %#v, want provider request", view.PrimaryOperation) | |||
| 167 | + | } | |||
| 168 | + | if got := view.TurnStartedAt(*view.PrimaryOperation); !got.Equal(started) { | |||
| 169 | + | t.Fatalf("elapsed timer anchor = %v, want delegated turn start %v", got, started) | |||
| 170 | + | } | |||
| 171 | + | } | |||
| 172 | + | ||||
| 173 | + | func TestTraceViewsObserveIngestedBoundariesBeforeAsyncWrite(t *testing.T) { | |||
| 174 | + | writer := &TraceWriter{ | |||
| 175 | + | pending: make([]traceRecord, 0, traceQueueInitialCapacity), | |||
| 176 | + | } | |||
| 177 | + | writer.cond = sync.NewCond(&writer.mu) | |||
| 178 | + | writer.Record(TraceEvent{ | |||
| 179 | + | Timestamp: time.Date(2026, time.August, 13, 2, 0, 0, 0, time.UTC), | |||
| 180 | + | Type: "turn.started", | |||
| 181 | + | AgentID: "agent-1", | |||
| 182 | + | TurnID: "turn-1", | |||
| 183 | + | }) | |||
| 184 | + | ||||
| 185 | + | view := writer.Views()["agent-1"] | |||
| 186 | + | if view.PrimaryOperation == nil || view.PrimaryOperation.Type != "turn.started" { | |||
| 187 | + | t.Fatalf("primary operation before async write = %#v, want delegated turn", view.PrimaryOperation) | |||
| 188 | + | } | |||
| 189 | + | } | |||
| 190 | + | ||||
| 103 | func TestTraceWriterNormalRedactsSensitiveValues(t *testing.T) { | 191 | func TestTraceWriterNormalRedactsSensitiveValues(t *testing.T) { | ||
| 104 | writer, err := NewTraceWriter(filepath.Join(t.TempDir(), "trace"), TraceModeNormal) | 192 | writer, err := NewTraceWriter(filepath.Join(t.TempDir(), "trace"), TraceModeNormal) | ||
| 105 | if err != nil { | 193 | if err != nil { | ||
M packages/agent/tools/subagent_spawn.go +9 -31
| @@ -61,10 +61,11 @@ type subagentSpawnArgs struct { | |||
| 61 | 61 | FastMode *bool `json:"fast_mode,omitempty"` | |
| 62 | 62 | Required bool `json:"required,omitempty"` | |
| 63 | 63 | Isolation string `json:"isolation,omitempty"` | |
| 64 | - | // Timeout remains decode-only for calls produced from an older schema. | |
| 65 | - | // Deadline policy is manager-owned, so Execute deliberately ignores it. | |
| 66 | - | Timeout string `json:"timeout,omitempty"` | |
| 67 | - | MaxTurns *int `json:"max_turns,omitempty"` | |
| 64 | + | // Decode removed fields so stale callers receive an explicit policy error | |
| 65 | + | // instead of silently spawning with a different, unlimited budget. | |
| 66 | + | Timeout json.RawMessage `json:"timeout,omitempty"` | |
| 67 | + | MaxTurns json.RawMessage `json:"max_turns,omitempty"` | |
| 68 | + | MaxSteps json.RawMessage `json:"max_steps,omitempty"` | |
| 68 | 69 | } | |
| 69 | 70 | ||
| 70 | 71 | const subagentSpawnSchemaTemplate = `{ | |
| @@ -103,12 +104,6 @@ const subagentSpawnSchemaTemplate = `{ | |||
| 103 | 104 | "type": "string", | |
| 104 | 105 | "enum": ["shared", "worktree"], | |
| 105 | 106 | "description": "Workspace mode. Shared preserves existing behavior; worktree captures a patch without merging it." | |
| 106 | - | }, | |
| 107 | - | "max_turns": { | |
| 108 | - | "type": "integer", | |
| 109 | - | "minimum": 1, | |
| 110 | - | "maximum": %d, | |
| 111 | - | "description": "Optional maximum prompt-level turns for this worker. Omit to use the supervisor default; the effective policy allows 1 through %d." | |
| 112 | 107 | } | |
| 113 | 108 | }, | |
| 114 | 109 | "required": ["task"] | |
| @@ -119,13 +114,7 @@ func (t *SubagentSpawnTool) Description() string { | |||
| 119 | 114 | return "Delegate work to a sub-agent. Every spawn returns immediately and completion is host-event-driven through [auto-subagents update]. Set required=true when the outcome is mandatory before the parent's terminal response; failures remain recoverable through subagent_resume. Never use bash sleep, watch, tail -f, polling loops, repeated subagent_status, or dashboard/metadata/event-log/file checks solely to wait. Work on unrelated independent tasks or end/yield your turn. Legitimate waits inside user-requested commands, provider flows, extensions, or tests are allowed." | |
| 120 | 115 | } | |
| 121 | 116 | func (t *SubagentSpawnTool) Schema() json.RawMessage { | |
| 122 | - | maxTurns := 3 | |
| 123 | - | if t.Supervisor != nil { | |
| 124 | - | if limit := t.Supervisor.MaxTurns(); limit > 0 { | |
| 125 | - | maxTurns = limit | |
| 126 | - | } | |
| 127 | - | } | |
| 128 | - | return json.RawMessage(fmt.Sprintf(subagentSpawnSchemaTemplate, maxTurns, maxTurns)) | |
| 117 | + | return json.RawMessage(subagentSpawnSchemaTemplate) | |
| 129 | 118 | } | |
| 130 | 119 | ||
| 131 | 120 | func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ func(string)) (core.ToolResult, error) { | |
| @@ -140,6 +129,9 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ | |||
| 140 | 129 | if err := json.Unmarshal(raw, &a); err != nil { | |
| 141 | 130 | return core.ToolResult{}, fmt.Errorf("invalid args: %w", err) | |
| 142 | 131 | } | |
| 132 | + | if len(a.Timeout) != 0 || len(a.MaxTurns) != 0 || len(a.MaxSteps) != 0 { | |
| 133 | + | return protocolToolError(prefix + ": timeout, max_turns, and max_steps are manager-owned budgets and cannot be set by this tool") | |
| 134 | + | } | |
| 143 | 135 | task := strings.TrimSpace(a.Task) | |
| 144 | 136 | if task == "" { | |
| 145 | 137 | return protocolToolError(prefix + ": task is required") | |
| @@ -152,15 +144,6 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ | |||
| 152 | 144 | return protocolToolError(prefix + ": isolation must be shared or worktree") | |
| 153 | 145 | } | |
| 154 | 146 | } | |
| 155 | - | if a.MaxTurns != nil { | |
| 156 | - | if *a.MaxTurns < 1 { | |
| 157 | - | return protocolToolError(prefix + ": max_turns must be positive") | |
| 158 | - | } | |
| 159 | - | if limit := t.Supervisor.MaxTurns(); limit > 0 && *a.MaxTurns > limit { | |
| 160 | - | return protocolToolError(fmt.Sprintf("%s: max_turns must be 1 through %d; omit it to use the supervisor default", prefix, limit)) | |
| 161 | - | } | |
| 162 | - | } | |
| 163 | - | ||
| 164 | 147 | agentName := strings.TrimSpace(a.Agent) | |
| 165 | 148 | var profile *subagents.Profile | |
| 166 | 149 | var fastModeOverride *bool | |
| @@ -231,10 +214,6 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ | |||
| 231 | 214 | } | |
| 232 | 215 | } | |
| 233 | 216 | ||
| 234 | - | maxTurns := 0 | |
| 235 | - | if a.MaxTurns != nil { | |
| 236 | - | maxTurns = *a.MaxTurns | |
| 237 | - | } | |
| 238 | 217 | agent, err := t.Supervisor.SpawnReq(ctx, subagents.SpawnRequest{ | |
| 239 | 218 | Task: task, | |
| 240 | 219 | Model: model, | |
| @@ -243,7 +222,6 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ | |||
| 243 | 222 | FastMode: fastModeOverride, | |
| 244 | 223 | Subagent: agentName, | |
| 245 | 224 | Required: a.Required, | |
| 246 | - | MaxTurns: maxTurns, | |
| 247 | 225 | WorkspaceMode: workspaceMode, | |
| 248 | 226 | Tools: profileTools, | |
| 249 | 227 | }) | |
| @@ -61,10 +61,11 @@ type subagentSpawnArgs struct { | |||||
| 61 | FastMode *bool `json:"fast_mode,omitempty"` | 61 | FastMode *bool `json:"fast_mode,omitempty"` | ||
| 62 | Required bool `json:"required,omitempty"` | 62 | Required bool `json:"required,omitempty"` | ||
| 63 | Isolation string `json:"isolation,omitempty"` | 63 | Isolation string `json:"isolation,omitempty"` | ||
| 64 | - | // Timeout remains decode-only for calls produced from an older schema. | 64 | + | // Decode removed fields so stale callers receive an explicit policy error |
| 65 | - | // Deadline policy is manager-owned, so Execute deliberately ignores it. | 65 | + | // instead of silently spawning with a different, unlimited budget. |
| 66 | - | Timeout string `json:"timeout,omitempty"` | 66 | + | Timeout json.RawMessage `json:"timeout,omitempty"` |
| 67 | - | MaxTurns *int `json:"max_turns,omitempty"` | 67 | + | MaxTurns json.RawMessage `json:"max_turns,omitempty"` |
| 68 | + | MaxSteps json.RawMessage `json:"max_steps,omitempty"` | |||
| 68 | } | 69 | } | ||
| 69 | 70 | ||||
| 70 | const subagentSpawnSchemaTemplate = `{ | 71 | const subagentSpawnSchemaTemplate = `{ | ||
| @@ -103,12 +104,6 @@ const subagentSpawnSchemaTemplate = `{ | |||||
| 103 | "type": "string", | 104 | "type": "string", | ||
| 104 | "enum": ["shared", "worktree"], | 105 | "enum": ["shared", "worktree"], | ||
| 105 | "description": "Workspace mode. Shared preserves existing behavior; worktree captures a patch without merging it." | 106 | "description": "Workspace mode. Shared preserves existing behavior; worktree captures a patch without merging it." | ||
| 106 | - | }, | |||
| 107 | - | "max_turns": { | |||
| 108 | - | "type": "integer", | |||
| 109 | - | "minimum": 1, | |||
| 110 | - | "maximum": %d, | |||
| 111 | - | "description": "Optional maximum prompt-level turns for this worker. Omit to use the supervisor default; the effective policy allows 1 through %d." | |||
| 112 | } | 107 | } | ||
| 113 | }, | 108 | }, | ||
| 114 | "required": ["task"] | 109 | "required": ["task"] | ||
| @@ -119,13 +114,7 @@ func (t *SubagentSpawnTool) Description() string { | |||||
| 119 | return "Delegate work to a sub-agent. Every spawn returns immediately and completion is host-event-driven through [auto-subagents update]. Set required=true when the outcome is mandatory before the parent's terminal response; failures remain recoverable through subagent_resume. Never use bash sleep, watch, tail -f, polling loops, repeated subagent_status, or dashboard/metadata/event-log/file checks solely to wait. Work on unrelated independent tasks or end/yield your turn. Legitimate waits inside user-requested commands, provider flows, extensions, or tests are allowed." | 114 | return "Delegate work to a sub-agent. Every spawn returns immediately and completion is host-event-driven through [auto-subagents update]. Set required=true when the outcome is mandatory before the parent's terminal response; failures remain recoverable through subagent_resume. Never use bash sleep, watch, tail -f, polling loops, repeated subagent_status, or dashboard/metadata/event-log/file checks solely to wait. Work on unrelated independent tasks or end/yield your turn. Legitimate waits inside user-requested commands, provider flows, extensions, or tests are allowed." | ||
| 120 | } | 115 | } | ||
| 121 | func (t *SubagentSpawnTool) Schema() json.RawMessage { | 116 | func (t *SubagentSpawnTool) Schema() json.RawMessage { | ||
| 122 | - | maxTurns := 3 | 117 | + | return json.RawMessage(subagentSpawnSchemaTemplate) |
| 123 | - | if t.Supervisor != nil { | |||
| 124 | - | if limit := t.Supervisor.MaxTurns(); limit > 0 { | |||
| 125 | - | maxTurns = limit | |||
| 126 | - | } | |||
| 127 | - | } | |||
| 128 | - | return json.RawMessage(fmt.Sprintf(subagentSpawnSchemaTemplate, maxTurns, maxTurns)) | |||
| 129 | } | 118 | } | ||
| 130 | 119 | ||||
| 131 | func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ func(string)) (core.ToolResult, error) { | 120 | func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ func(string)) (core.ToolResult, error) { | ||
| @@ -140,6 +129,9 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ | |||||
| 140 | if err := json.Unmarshal(raw, &a); err != nil { | 129 | if err := json.Unmarshal(raw, &a); err != nil { | ||
| 141 | return core.ToolResult{}, fmt.Errorf("invalid args: %w", err) | 130 | return core.ToolResult{}, fmt.Errorf("invalid args: %w", err) | ||
| 142 | } | 131 | } | ||
| 132 | + | if len(a.Timeout) != 0 || len(a.MaxTurns) != 0 || len(a.MaxSteps) != 0 { | |||
| 133 | + | return protocolToolError(prefix + ": timeout, max_turns, and max_steps are manager-owned budgets and cannot be set by this tool") | |||
| 134 | + | } | |||
| 143 | task := strings.TrimSpace(a.Task) | 135 | task := strings.TrimSpace(a.Task) | ||
| 144 | if task == "" { | 136 | if task == "" { | ||
| 145 | return protocolToolError(prefix + ": task is required") | 137 | return protocolToolError(prefix + ": task is required") | ||
| @@ -152,15 +144,6 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ | |||||
| 152 | return protocolToolError(prefix + ": isolation must be shared or worktree") | 144 | return protocolToolError(prefix + ": isolation must be shared or worktree") | ||
| 153 | } | 145 | } | ||
| 154 | } | 146 | } | ||
| 155 | - | if a.MaxTurns != nil { | |||
| 156 | - | if *a.MaxTurns < 1 { | |||
| 157 | - | return protocolToolError(prefix + ": max_turns must be positive") | |||
| 158 | - | } | |||
| 159 | - | if limit := t.Supervisor.MaxTurns(); limit > 0 && *a.MaxTurns > limit { | |||
| 160 | - | return protocolToolError(fmt.Sprintf("%s: max_turns must be 1 through %d; omit it to use the supervisor default", prefix, limit)) | |||
| 161 | - | } | |||
| 162 | - | } | |||
| 163 | - | ||||
| 164 | agentName := strings.TrimSpace(a.Agent) | 147 | agentName := strings.TrimSpace(a.Agent) | ||
| 165 | var profile *subagents.Profile | 148 | var profile *subagents.Profile | ||
| 166 | var fastModeOverride *bool | 149 | var fastModeOverride *bool | ||
| @@ -231,10 +214,6 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ | |||||
| 231 | } | 214 | } | ||
| 232 | } | 215 | } | ||
| 233 | 216 | ||||
| 234 | - | maxTurns := 0 | |||
| 235 | - | if a.MaxTurns != nil { | |||
| 236 | - | maxTurns = *a.MaxTurns | |||
| 237 | - | } | |||
| 238 | agent, err := t.Supervisor.SpawnReq(ctx, subagents.SpawnRequest{ | 217 | agent, err := t.Supervisor.SpawnReq(ctx, subagents.SpawnRequest{ | ||
| 239 | Task: task, | 218 | Task: task, | ||
| 240 | Model: model, | 219 | Model: model, | ||
| @@ -243,7 +222,6 @@ func (t *SubagentSpawnTool) Execute(ctx context.Context, raw json.RawMessage, _ | |||||
| 243 | FastMode: fastModeOverride, | 222 | FastMode: fastModeOverride, | ||
| 244 | Subagent: agentName, | 223 | Subagent: agentName, | ||
| 245 | Required: a.Required, | 224 | Required: a.Required, | ||
| 246 | - | MaxTurns: maxTurns, | |||
| 247 | WorkspaceMode: workspaceMode, | 225 | WorkspaceMode: workspaceMode, | ||
| 248 | Tools: profileTools, | 226 | Tools: profileTools, | ||
| 249 | }) | 227 | }) | ||
M packages/agent/tools/subagent_spawn_test.go +29 -108
| @@ -81,52 +81,18 @@ func TestSubagentSpawnGuidanceUsesHostCompletionUpdatesWithoutPolling(t *testing | |||
| 81 | 81 | } | |
| 82 | 82 | } | |
| 83 | 83 | ||
| 84 | - | func TestSubagentSpawnSchemaAdvertisesEffectiveMaxTurns(t *testing.T) { | |
| 85 | - | for _, tc := range []struct { | |
| 86 | - | name string | |
| 87 | - | policy int | |
| 88 | - | want int | |
| 89 | - | }{ | |
| 90 | - | {name: "default", want: 3}, | |
| 91 | - | {name: "negative-default", policy: -1, want: 3}, | |
| 92 | - | {name: "configured", policy: 7, want: 7}, | |
| 93 | - | } { | |
| 94 | - | t.Run(tc.name, func(t *testing.T) { | |
| 95 | - | root := t.TempDir() | |
| 96 | - | manager := subagents.New(subagents.Config{ | |
| 97 | - | Root: filepath.Join(root, "subagents"), | |
| 98 | - | RepoRoot: root, | |
| 99 | - | Policy: subagents.SubagentPolicy{MaxTurns: tc.policy}, | |
| 100 | - | NewRunner: func(*subagents.Agent) subagents.Runner { | |
| 101 | - | return noopSupervisorRunner{} | |
| 102 | - | }, | |
| 103 | - | }) | |
| 104 | - | t.Cleanup(manager.StopAll) | |
| 105 | - | tool := &SubagentSpawnTool{Supervisor: manager} | |
| 106 | - | ||
| 107 | - | var schema struct { | |
| 108 | - | Properties struct { | |
| 109 | - | MaxTurns struct { | |
| 110 | - | Minimum int `json:"minimum"` | |
| 111 | - | Maximum int `json:"maximum"` | |
| 112 | - | Description string `json:"description"` | |
| 113 | - | } `json:"max_turns"` | |
| 114 | - | } `json:"properties"` | |
| 115 | - | } | |
| 116 | - | if err := json.Unmarshal(tool.Schema(), &schema); err != nil { | |
| 117 | - | t.Fatal(err) | |
| 118 | - | } | |
| 119 | - | maxTurns := schema.Properties.MaxTurns | |
| 120 | - | if maxTurns.Minimum != 1 { | |
| 121 | - | t.Fatalf("max_turns minimum = %d, want 1", maxTurns.Minimum) | |
| 122 | - | } | |
| 123 | - | if maxTurns.Maximum != tc.want { | |
| 124 | - | t.Fatalf("max_turns maximum = %d, want %d", maxTurns.Maximum, tc.want) | |
| 125 | - | } | |
| 126 | - | if !strings.Contains(strings.ToLower(maxTurns.Description), "omit") { | |
| 127 | - | t.Fatalf("max_turns description = %q, want omit-to-default guidance", maxTurns.Description) | |
| 128 | - | } | |
| 129 | - | }) | |
| 84 | + | func TestSubagentSpawnSchemaKeepsManagerBudgetsOutOfModelControl(t *testing.T) { | |
| 85 | + | tool := &SubagentSpawnTool{Supervisor: newTestSupervisor(t)} | |
| 86 | + | var schema struct { | |
| 87 | + | Properties map[string]json.RawMessage `json:"properties"` | |
| 88 | + | } | |
| 89 | + | if err := json.Unmarshal(tool.Schema(), &schema); err != nil { | |
| 90 | + | t.Fatal(err) | |
| 91 | + | } | |
| 92 | + | for _, property := range []string{"timeout", "max_turns", "max_steps"} { | |
| 93 | + | if _, found := schema.Properties[property]; found { | |
| 94 | + | t.Fatalf("spawn schema exposes manager-owned property %q", property) | |
| 95 | + | } | |
| 130 | 96 | } | |
| 131 | 97 | } | |
| 132 | 98 | ||
| @@ -162,8 +128,8 @@ func TestSubagentSpawnInheritsHostModelAndProviderWhenOmitted(t *testing.T) { | |||
| 162 | 128 | if got := details["timeout"]; got != "unlimited" { | |
| 163 | 129 | t.Fatalf("timeout detail = %v, want unlimited", got) | |
| 164 | 130 | } | |
| 165 | - | if got := details["max_steps"]; got != 128 { | |
| 166 | - | t.Fatalf("max_steps detail = %v, want 128", got) | |
| 131 | + | if got := details["max_steps"]; got != 0 { | |
| 132 | + | t.Fatalf("max_steps detail = %v, want unlimited", got) | |
| 167 | 133 | } | |
| 168 | 134 | text := textResult(res.Content) | |
| 169 | 135 | if !strings.Contains(text, "model: gpt-5") || !strings.Contains(text, "provider: openai-codex") || !strings.Contains(text, "reasoning: medium") { | |
| @@ -177,8 +143,8 @@ func TestSubagentSpawnInheritsHostModelAndProviderWhenOmitted(t *testing.T) { | |||
| 177 | 143 | if agents[0].Model != "gpt-5" || agents[0].Provider != "openai-codex" || agents[0].Reasoning != "medium" { | |
| 178 | 144 | t.Fatalf("agent model/provider/reasoning = %q/%q/%q, want gpt-5/openai-codex/medium", agents[0].Model, agents[0].Provider, agents[0].Reasoning) | |
| 179 | 145 | } | |
| 180 | - | if agents[0].MaxTurns != 3 { | |
| 181 | - | t.Fatalf("omitted max_turns = %d, want default 3", agents[0].MaxTurns) | |
| 146 | + | if agents[0].MaxTurns != 0 { | |
| 147 | + | t.Fatalf("omitted max_turns = %d, want unlimited", agents[0].MaxTurns) | |
| 182 | 148 | } | |
| 183 | 149 | } | |
| 184 | 150 | ||
| @@ -224,78 +190,33 @@ func TestSubagentSpawnDetailsUseEffectiveTimeoutAndTurns(t *testing.T) { | |||
| 224 | 190 | } | |
| 225 | 191 | } | |
| 226 | 192 | ||
| 227 | - | func TestSubagentSpawnIgnoresLegacyModelTimeoutOverride(t *testing.T) { | |
| 193 | + | func TestSubagentSpawnRejectsLegacyModelTimeoutOverride(t *testing.T) { | |
| 228 | 194 | manager := newTestSupervisor(t) | |
| 229 | 195 | tool := &SubagentSpawnTool{Supervisor: manager, Enabled: func() bool { return true }} | |
| 230 | 196 | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs","timeout":"20m"}`), nil) | |
| 231 | 197 | if err != nil { | |
| 232 | 198 | t.Fatal(err) | |
| 233 | 199 | } | |
| 234 | - | if res.IsError { | |
| 235 | - | t.Fatalf("unexpected tool error: %s", textResult(res.Content)) | |
| 200 | + | if !res.IsError || !strings.Contains(textResult(res.Content), "manager-owned") { | |
| 201 | + | t.Fatalf("result = %#v, want manager-owned budget error", res) | |
| 236 | 202 | } | |
| 237 | - | agents := manager.List() | |
| 238 | - | if len(agents) != 1 || agents[0].Timeout != 0 { | |
| 239 | - | t.Fatalf("legacy model timeout changed manager policy: %#v", agents) | |
| 203 | + | if agents := manager.List(); len(agents) != 0 { | |
| 204 | + | t.Fatalf("legacy timeout spawned agents: %#v", agents) | |
| 240 | 205 | } | |
| 241 | 206 | } | |
| 242 | 207 | ||
| 243 | - | func TestSubagentSpawnRejectsExplicitZeroMaxTurns(t *testing.T) { | |
| 244 | - | tool := &SubagentSpawnTool{ | |
| 245 | - | Supervisor: newTestSupervisor(t), | |
| 246 | - | Enabled: func() bool { return true }, | |
| 247 | - | } | |
| 248 | - | ||
| 249 | - | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs","max_turns":0}`), nil) | |
| 208 | + | func TestSubagentSpawnRejectsLegacyModelMaxTurnsOverride(t *testing.T) { | |
| 209 | + | manager := newTestSupervisor(t) | |
| 210 | + | tool := &SubagentSpawnTool{Supervisor: manager, Enabled: func() bool { return true }} | |
| 211 | + | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs","max_turns":1}`), nil) | |
| 250 | 212 | if err != nil { | |
| 251 | 213 | t.Fatal(err) | |
| 252 | 214 | } | |
| 253 | - | if !res.IsError || !strings.Contains(textResult(res.Content), "max_turns must be positive") { | |
| 254 | - | t.Fatalf("result = %#v, want max_turns validation error", res) | |
| 255 | - | } | |
| 256 | - | if got := len(tool.Supervisor.List()); got != 0 { | |
| 257 | - | t.Fatalf("spawned agents = %d, want 0", got) | |
| 215 | + | if !res.IsError || !strings.Contains(textResult(res.Content), "manager-owned") { | |
| 216 | + | t.Fatalf("result = %#v, want manager-owned budget error", res) | |
| 258 | 217 | } | |
| 259 | - | } | |
| 260 | - | ||
| 261 | - | func TestSubagentSpawnRejectsMaxTurnsAbovePolicy(t *testing.T) { | |
| 262 | - | for _, tc := range []struct { | |
| 263 | - | name string | |
| 264 | - | policy int | |
| 265 | - | wantLimit string | |
| 266 | - | }{ | |
| 267 | - | {name: "default", wantLimit: "3"}, | |
| 268 | - | {name: "configured", policy: 7, wantLimit: "7"}, | |
| 269 | - | } { | |
| 270 | - | t.Run(tc.name, func(t *testing.T) { | |
| 271 | - | root := t.TempDir() | |
| 272 | - | manager := subagents.New(subagents.Config{ | |
| 273 | - | Root: filepath.Join(root, "subagents"), | |
| 274 | - | RepoRoot: root, | |
| 275 | - | Policy: subagents.SubagentPolicy{MaxTurns: tc.policy}, | |
| 276 | - | NewRunner: func(*subagents.Agent) subagents.Runner { | |
| 277 | - | return noopSupervisorRunner{} | |
| 278 | - | }, | |
| 279 | - | }) | |
| 280 | - | t.Cleanup(manager.StopAll) | |
| 281 | - | tool := &SubagentSpawnTool{ | |
| 282 | - | Supervisor: manager, | |
| 283 | - | Enabled: func() bool { return true }, | |
| 284 | - | } | |
| 285 | - | ||
| 286 | - | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs","max_turns":8}`), nil) | |
| 287 | - | if err != nil { | |
| 288 | - | t.Fatal(err) | |
| 289 | - | } | |
| 290 | - | text := textResult(res.Content) | |
| 291 | - | want := "max_turns must be 1 through " + tc.wantLimit | |
| 292 | - | if !res.IsError || !strings.Contains(text, want) || !strings.Contains(text, "omit") { | |
| 293 | - | t.Fatalf("result = %#v, want model-visible policy guidance", res) | |
| 294 | - | } | |
| 295 | - | if got := len(tool.Supervisor.List()); got != 0 { | |
| 296 | - | t.Fatalf("spawned agents = %d, want 0", got) | |
| 297 | - | } | |
| 298 | - | }) | |
| 218 | + | if agents := manager.List(); len(agents) != 0 { | |
| 219 | + | t.Fatalf("legacy max_turns spawned agents: %#v", agents) | |
| 299 | 220 | } | |
| 300 | 221 | } | |
| 301 | 222 | ||
| @@ -81,52 +81,18 @@ func TestSubagentSpawnGuidanceUsesHostCompletionUpdatesWithoutPolling(t *testing | |||||
| 81 | } | 81 | } | ||
| 82 | } | 82 | } | ||
| 83 | 83 | ||||
| 84 | - | func TestSubagentSpawnSchemaAdvertisesEffectiveMaxTurns(t *testing.T) { | 84 | + | func TestSubagentSpawnSchemaKeepsManagerBudgetsOutOfModelControl(t *testing.T) { |
| 85 | - | for _, tc := range []struct { | 85 | + | tool := &SubagentSpawnTool{Supervisor: newTestSupervisor(t)} |
| 86 | - | name string | 86 | + | var schema struct { |
| 87 | - | policy int | 87 | + | Properties map[string]json.RawMessage `json:"properties"` |
| 88 | - | want int | 88 | + | } |
| 89 | - | }{ | 89 | + | if err := json.Unmarshal(tool.Schema(), &schema); err != nil { |
| 90 | - | {name: "default", want: 3}, | 90 | + | t.Fatal(err) |
| 91 | - | {name: "negative-default", policy: -1, want: 3}, | 91 | + | } |
| 92 | - | {name: "configured", policy: 7, want: 7}, | 92 | + | for _, property := range []string{"timeout", "max_turns", "max_steps"} { |
| 93 | - | } { | 93 | + | if _, found := schema.Properties[property]; found { |
| 94 | - | t.Run(tc.name, func(t *testing.T) { | 94 | + | t.Fatalf("spawn schema exposes manager-owned property %q", property) |
| 95 | - | root := t.TempDir() | 95 | + | } |
| 96 | - | manager := subagents.New(subagents.Config{ | |||
| 97 | - | Root: filepath.Join(root, "subagents"), | |||
| 98 | - | RepoRoot: root, | |||
| 99 | - | Policy: subagents.SubagentPolicy{MaxTurns: tc.policy}, | |||
| 100 | - | NewRunner: func(*subagents.Agent) subagents.Runner { | |||
| 101 | - | return noopSupervisorRunner{} | |||
| 102 | - | }, | |||
| 103 | - | }) | |||
| 104 | - | t.Cleanup(manager.StopAll) | |||
| 105 | - | tool := &SubagentSpawnTool{Supervisor: manager} | |||
| 106 | - | ||||
| 107 | - | var schema struct { | |||
| 108 | - | Properties struct { | |||
| 109 | - | MaxTurns struct { | |||
| 110 | - | Minimum int `json:"minimum"` | |||
| 111 | - | Maximum int `json:"maximum"` | |||
| 112 | - | Description string `json:"description"` | |||
| 113 | - | } `json:"max_turns"` | |||
| 114 | - | } `json:"properties"` | |||
| 115 | - | } | |||
| 116 | - | if err := json.Unmarshal(tool.Schema(), &schema); err != nil { | |||
| 117 | - | t.Fatal(err) | |||
| 118 | - | } | |||
| 119 | - | maxTurns := schema.Properties.MaxTurns | |||
| 120 | - | if maxTurns.Minimum != 1 { | |||
| 121 | - | t.Fatalf("max_turns minimum = %d, want 1", maxTurns.Minimum) | |||
| 122 | - | } | |||
| 123 | - | if maxTurns.Maximum != tc.want { | |||
| 124 | - | t.Fatalf("max_turns maximum = %d, want %d", maxTurns.Maximum, tc.want) | |||
| 125 | - | } | |||
| 126 | - | if !strings.Contains(strings.ToLower(maxTurns.Description), "omit") { | |||
| 127 | - | t.Fatalf("max_turns description = %q, want omit-to-default guidance", maxTurns.Description) | |||
| 128 | - | } | |||
| 129 | - | }) | |||
| 130 | } | 96 | } | ||
| 131 | } | 97 | } | ||
| 132 | 98 | ||||
| @@ -162,8 +128,8 @@ func TestSubagentSpawnInheritsHostModelAndProviderWhenOmitted(t *testing.T) { | |||||
| 162 | if got := details["timeout"]; got != "unlimited" { | 128 | if got := details["timeout"]; got != "unlimited" { | ||
| 163 | t.Fatalf("timeout detail = %v, want unlimited", got) | 129 | t.Fatalf("timeout detail = %v, want unlimited", got) | ||
| 164 | } | 130 | } | ||
| 165 | - | if got := details["max_steps"]; got != 128 { | 131 | + | if got := details["max_steps"]; got != 0 { |
| 166 | - | t.Fatalf("max_steps detail = %v, want 128", got) | 132 | + | t.Fatalf("max_steps detail = %v, want unlimited", got) |
| 167 | } | 133 | } | ||
| 168 | text := textResult(res.Content) | 134 | text := textResult(res.Content) | ||
| 169 | if !strings.Contains(text, "model: gpt-5") || !strings.Contains(text, "provider: openai-codex") || !strings.Contains(text, "reasoning: medium") { | 135 | if !strings.Contains(text, "model: gpt-5") || !strings.Contains(text, "provider: openai-codex") || !strings.Contains(text, "reasoning: medium") { | ||
| @@ -177,8 +143,8 @@ func TestSubagentSpawnInheritsHostModelAndProviderWhenOmitted(t *testing.T) { | |||||
| 177 | if agents[0].Model != "gpt-5" || agents[0].Provider != "openai-codex" || agents[0].Reasoning != "medium" { | 143 | if agents[0].Model != "gpt-5" || agents[0].Provider != "openai-codex" || agents[0].Reasoning != "medium" { | ||
| 178 | t.Fatalf("agent model/provider/reasoning = %q/%q/%q, want gpt-5/openai-codex/medium", agents[0].Model, agents[0].Provider, agents[0].Reasoning) | 144 | t.Fatalf("agent model/provider/reasoning = %q/%q/%q, want gpt-5/openai-codex/medium", agents[0].Model, agents[0].Provider, agents[0].Reasoning) | ||
| 179 | } | 145 | } | ||
| 180 | - | if agents[0].MaxTurns != 3 { | 146 | + | if agents[0].MaxTurns != 0 { |
| 181 | - | t.Fatalf("omitted max_turns = %d, want default 3", agents[0].MaxTurns) | 147 | + | t.Fatalf("omitted max_turns = %d, want unlimited", agents[0].MaxTurns) |
| 182 | } | 148 | } | ||
| 183 | } | 149 | } | ||
| 184 | 150 | ||||
| @@ -224,78 +190,33 @@ func TestSubagentSpawnDetailsUseEffectiveTimeoutAndTurns(t *testing.T) { | |||||
| 224 | } | 190 | } | ||
| 225 | } | 191 | } | ||
| 226 | 192 | ||||
| 227 | - | func TestSubagentSpawnIgnoresLegacyModelTimeoutOverride(t *testing.T) { | 193 | + | func TestSubagentSpawnRejectsLegacyModelTimeoutOverride(t *testing.T) { |
| 228 | manager := newTestSupervisor(t) | 194 | manager := newTestSupervisor(t) | ||
| 229 | tool := &SubagentSpawnTool{Supervisor: manager, Enabled: func() bool { return true }} | 195 | tool := &SubagentSpawnTool{Supervisor: manager, Enabled: func() bool { return true }} | ||
| 230 | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs","timeout":"20m"}`), nil) | 196 | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs","timeout":"20m"}`), nil) | ||
| 231 | if err != nil { | 197 | if err != nil { | ||
| 232 | t.Fatal(err) | 198 | t.Fatal(err) | ||
| 233 | } | 199 | } | ||
| 234 | - | if res.IsError { | 200 | + | if !res.IsError || !strings.Contains(textResult(res.Content), "manager-owned") { |
| 235 | - | t.Fatalf("unexpected tool error: %s", textResult(res.Content)) | 201 | + | t.Fatalf("result = %#v, want manager-owned budget error", res) |
| 236 | } | 202 | } | ||
| 237 | - | agents := manager.List() | 203 | + | if agents := manager.List(); len(agents) != 0 { |
| 238 | - | if len(agents) != 1 || agents[0].Timeout != 0 { | 204 | + | t.Fatalf("legacy timeout spawned agents: %#v", agents) |
| 239 | - | t.Fatalf("legacy model timeout changed manager policy: %#v", agents) | |||
| 240 | } | 205 | } | ||
| 241 | } | 206 | } | ||
| 242 | 207 | ||||
| 243 | - | func TestSubagentSpawnRejectsExplicitZeroMaxTurns(t *testing.T) { | 208 | + | func TestSubagentSpawnRejectsLegacyModelMaxTurnsOverride(t *testing.T) { |
| 244 | - | tool := &SubagentSpawnTool{ | 209 | + | manager := newTestSupervisor(t) |
| 245 | - | Supervisor: newTestSupervisor(t), | 210 | + | tool := &SubagentSpawnTool{Supervisor: manager, Enabled: func() bool { return true }} |
| 246 | - | Enabled: func() bool { return true }, | 211 | + | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs","max_turns":1}`), nil) |
| 247 | - | } | |||
| 248 | - | ||||
| 249 | - | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs","max_turns":0}`), nil) | |||
| 250 | if err != nil { | 212 | if err != nil { | ||
| 251 | t.Fatal(err) | 213 | t.Fatal(err) | ||
| 252 | } | 214 | } | ||
| 253 | - | if !res.IsError || !strings.Contains(textResult(res.Content), "max_turns must be positive") { | 215 | + | if !res.IsError || !strings.Contains(textResult(res.Content), "manager-owned") { |
| 254 | - | t.Fatalf("result = %#v, want max_turns validation error", res) | 216 | + | t.Fatalf("result = %#v, want manager-owned budget error", res) |
| 255 | - | } | |||
| 256 | - | if got := len(tool.Supervisor.List()); got != 0 { | |||
| 257 | - | t.Fatalf("spawned agents = %d, want 0", got) | |||
| 258 | } | 217 | } | ||
| 259 | - | } | 218 | + | if agents := manager.List(); len(agents) != 0 { |
| 260 | - | 219 | + | t.Fatalf("legacy max_turns spawned agents: %#v", agents) | |
| 261 | - | func TestSubagentSpawnRejectsMaxTurnsAbovePolicy(t *testing.T) { | |||
| 262 | - | for _, tc := range []struct { | |||
| 263 | - | name string | |||
| 264 | - | policy int | |||
| 265 | - | wantLimit string | |||
| 266 | - | }{ | |||
| 267 | - | {name: "default", wantLimit: "3"}, | |||
| 268 | - | {name: "configured", policy: 7, wantLimit: "7"}, | |||
| 269 | - | } { | |||
| 270 | - | t.Run(tc.name, func(t *testing.T) { | |||
| 271 | - | root := t.TempDir() | |||
| 272 | - | manager := subagents.New(subagents.Config{ | |||
| 273 | - | Root: filepath.Join(root, "subagents"), | |||
| 274 | - | RepoRoot: root, | |||
| 275 | - | Policy: subagents.SubagentPolicy{MaxTurns: tc.policy}, | |||
| 276 | - | NewRunner: func(*subagents.Agent) subagents.Runner { | |||
| 277 | - | return noopSupervisorRunner{} | |||
| 278 | - | }, | |||
| 279 | - | }) | |||
| 280 | - | t.Cleanup(manager.StopAll) | |||
| 281 | - | tool := &SubagentSpawnTool{ | |||
| 282 | - | Supervisor: manager, | |||
| 283 | - | Enabled: func() bool { return true }, | |||
| 284 | - | } | |||
| 285 | - | ||||
| 286 | - | res, err := tool.Execute(context.Background(), json.RawMessage(`{"task":"research docs","max_turns":8}`), nil) | |||
| 287 | - | if err != nil { | |||
| 288 | - | t.Fatal(err) | |||
| 289 | - | } | |||
| 290 | - | text := textResult(res.Content) | |||
| 291 | - | want := "max_turns must be 1 through " + tc.wantLimit | |||
| 292 | - | if !res.IsError || !strings.Contains(text, want) || !strings.Contains(text, "omit") { | |||
| 293 | - | t.Fatalf("result = %#v, want model-visible policy guidance", res) | |||
| 294 | - | } | |||
| 295 | - | if got := len(tool.Supervisor.List()); got != 0 { | |||
| 296 | - | t.Fatalf("spawned agents = %d, want 0", got) | |||
| 297 | - | } | |||
| 298 | - | }) | |||
| 299 | } | 220 | } | ||
| 300 | } | 221 | } | ||
| 301 | 222 | ||||
M packages/core/agent.go +29 -12
| @@ -18,11 +18,24 @@ var ( | |||
| 18 | 18 | ErrMaxSteps = errors.New("agent model step limit exceeded") | |
| 19 | 19 | ) | |
| 20 | 20 | ||
| 21 | + | const ( | |
| 22 | + | DefaultStreamIdleTimeout = 5 * time.Minute | |
| 23 | + | DefaultStreamMaxRetries = 5 | |
| 24 | + | ) | |
| 25 | + | ||
| 21 | 26 | type queuedMessage struct { | |
| 22 | 27 | text string | |
| 23 | 28 | accepted time.Time | |
| 24 | 29 | } | |
| 25 | 30 | ||
| 31 | + | // requestOpenError marks a failure returned before a provider stream exists. | |
| 32 | + | // Provider clients own retries for this phase; the agent retry budget is only | |
| 33 | + | // for reconnecting a stream that opened and then failed. | |
| 34 | + | type requestOpenError struct{ err error } | |
| 35 | + | ||
| 36 | + | func (e *requestOpenError) Error() string { return e.err.Error() } | |
| 37 | + | func (e *requestOpenError) Unwrap() error { return e.err } | |
| 38 | + | ||
| 26 | 39 | // Agent is a stateful conversation bound to a provider client, a model, | |
| 27 | 40 | // and a set of tools. | |
| 28 | 41 | type Agent struct { | |
| @@ -94,9 +107,8 @@ type Agent struct { | |||
| 94 | 107 | // model can still see what it said in subsequent turns). | |
| 95 | 108 | BeforeAssistantMessage func(text string) (allowed bool, reason, replacement string) | |
| 96 | 109 | ||
| 97 | - | // MaxRetries controls agent-level retries for transient provider | |
| 98 | - | // failures that arrive after the HTTP stream opens (for example | |
| 99 | - | // Anthropic overloaded_error). Zero disables this retry layer. | |
| 110 | + | // MaxRetries controls reconnection attempts for transient failures after the | |
| 111 | + | // provider stream opens. Zero disables this retry layer. | |
| 100 | 112 | // RetryBaseDelay is doubled for each attempt; zero uses 2s. | |
| 101 | 113 | MaxRetries int | |
| 102 | 114 | RetryBaseDelay time.Duration | |
| @@ -158,14 +170,15 @@ type Agent struct { | |||
| 158 | 170 | // NewAgent returns an Agent with sensible defaults. | |
| 159 | 171 | func NewAgent(client provider.Client, model, system string, tools Registry) *Agent { | |
| 160 | 172 | return &Agent{ | |
| 161 | - | Client: client, | |
| 162 | - | Model: model, | |
| 163 | - | System: system, | |
| 164 | - | Tools: tools, | |
| 165 | - | MaxSteps: 0, // 0 = unlimited | |
| 166 | - | MaxRetries: 3, | |
| 167 | - | RetryBaseDelay: 2 * time.Second, | |
| 168 | - | timeContext: newAgentTimeContext(time.Now()), | |
| 173 | + | Client: client, | |
| 174 | + | Model: model, | |
| 175 | + | System: system, | |
| 176 | + | Tools: tools, | |
| 177 | + | MaxSteps: 0, // 0 = unlimited | |
| 178 | + | MaxRetries: DefaultStreamMaxRetries, | |
| 179 | + | RetryBaseDelay: 2 * time.Second, | |
| 180 | + | StreamIdleTimeout: DefaultStreamIdleTimeout, | |
| 181 | + | timeContext: newAgentTimeContext(time.Now()), | |
| 169 | 182 | } | |
| 170 | 183 | } | |
| 171 | 184 | ||
| @@ -663,6 +676,10 @@ func (a *Agent) canRetryError(err error, attempt int) bool { | |||
| 663 | 676 | if err == nil || a.MaxRetries <= 0 || attempt >= a.MaxRetries { | |
| 664 | 677 | return false | |
| 665 | 678 | } | |
| 679 | + | var openErr *requestOpenError | |
| 680 | + | if errors.As(err, &openErr) { | |
| 681 | + | return false | |
| 682 | + | } | |
| 666 | 683 | if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { | |
| 667 | 684 | return false | |
| 668 | 685 | } | |
| @@ -944,7 +961,7 @@ func (a *Agent) oneTurn(ctx context.Context, sink func(AgentEvent), turnContext | |||
| 944 | 961 | defer cancelStream() | |
| 945 | 962 | stream, err := a.Client.Stream(streamCtx, req) | |
| 946 | 963 | if err != nil { | |
| 947 | - | return provider.StopError, provider.Message{}, err | |
| 964 | + | return provider.StopError, provider.Message{}, &requestOpenError{err: err} | |
| 948 | 965 | } | |
| 949 | 966 | ||
| 950 | 967 | sink(EvAssistantStart{}) | |
| @@ -18,11 +18,24 @@ var ( | |||||
| 18 | ErrMaxSteps = errors.New("agent model step limit exceeded") | 18 | ErrMaxSteps = errors.New("agent model step limit exceeded") | ||
| 19 | ) | 19 | ) | ||
| 20 | 20 | ||||
| 21 | + | const ( | |||
| 22 | + | DefaultStreamIdleTimeout = 5 * time.Minute | |||
| 23 | + | DefaultStreamMaxRetries = 5 | |||
| 24 | + | ) | |||
| 25 | + | ||||
| 21 | type queuedMessage struct { | 26 | type queuedMessage struct { | ||
| 22 | text string | 27 | text string | ||
| 23 | accepted time.Time | 28 | accepted time.Time | ||
| 24 | } | 29 | } | ||
| 25 | 30 | ||||
| 31 | + | // requestOpenError marks a failure returned before a provider stream exists. | |||
| 32 | + | // Provider clients own retries for this phase; the agent retry budget is only | |||
| 33 | + | // for reconnecting a stream that opened and then failed. | |||
| 34 | + | type requestOpenError struct{ err error } | |||
| 35 | + | ||||
| 36 | + | func (e *requestOpenError) Error() string { return e.err.Error() } | |||
| 37 | + | func (e *requestOpenError) Unwrap() error { return e.err } | |||
| 38 | + | ||||
| 26 | // Agent is a stateful conversation bound to a provider client, a model, | 39 | // Agent is a stateful conversation bound to a provider client, a model, | ||
| 27 | // and a set of tools. | 40 | // and a set of tools. | ||
| 28 | type Agent struct { | 41 | type Agent struct { | ||
| @@ -94,9 +107,8 @@ type Agent struct { | |||||
| 94 | // model can still see what it said in subsequent turns). | 107 | // model can still see what it said in subsequent turns). | ||
| 95 | BeforeAssistantMessage func(text string) (allowed bool, reason, replacement string) | 108 | BeforeAssistantMessage func(text string) (allowed bool, reason, replacement string) | ||
| 96 | 109 | ||||
| 97 | - | // MaxRetries controls agent-level retries for transient provider | 110 | + | // MaxRetries controls reconnection attempts for transient failures after the |
| 98 | - | // failures that arrive after the HTTP stream opens (for example | 111 | + | // provider stream opens. Zero disables this retry layer. |
| 99 | - | // Anthropic overloaded_error). Zero disables this retry layer. | |||
| 100 | // RetryBaseDelay is doubled for each attempt; zero uses 2s. | 112 | // RetryBaseDelay is doubled for each attempt; zero uses 2s. | ||
| 101 | MaxRetries int | 113 | MaxRetries int | ||
| 102 | RetryBaseDelay time.Duration | 114 | RetryBaseDelay time.Duration | ||
| @@ -158,14 +170,15 @@ type Agent struct { | |||||
| 158 | // NewAgent returns an Agent with sensible defaults. | 170 | // NewAgent returns an Agent with sensible defaults. | ||
| 159 | func NewAgent(client provider.Client, model, system string, tools Registry) *Agent { | 171 | func NewAgent(client provider.Client, model, system string, tools Registry) *Agent { | ||
| 160 | return &Agent{ | 172 | return &Agent{ | ||
| 161 | - | Client: client, | 173 | + | Client: client, |
| 162 | - | Model: model, | 174 | + | Model: model, |
| 163 | - | System: system, | 175 | + | System: system, |
| 164 | - | Tools: tools, | 176 | + | Tools: tools, |
| 165 | - | MaxSteps: 0, // 0 = unlimited | 177 | + | MaxSteps: 0, // 0 = unlimited |
| 166 | - | MaxRetries: 3, | 178 | + | MaxRetries: DefaultStreamMaxRetries, |
| 167 | - | RetryBaseDelay: 2 * time.Second, | 179 | + | RetryBaseDelay: 2 * time.Second, |
| 168 | - | timeContext: newAgentTimeContext(time.Now()), | 180 | + | StreamIdleTimeout: DefaultStreamIdleTimeout, |
| 181 | + | timeContext: newAgentTimeContext(time.Now()), | |||
| 169 | } | 182 | } | ||
| 170 | } | 183 | } | ||
| 171 | 184 | ||||
| @@ -663,6 +676,10 @@ func (a *Agent) canRetryError(err error, attempt int) bool { | |||||
| 663 | if err == nil || a.MaxRetries <= 0 || attempt >= a.MaxRetries { | 676 | if err == nil || a.MaxRetries <= 0 || attempt >= a.MaxRetries { | ||
| 664 | return false | 677 | return false | ||
| 665 | } | 678 | } | ||
| 679 | + | var openErr *requestOpenError | |||
| 680 | + | if errors.As(err, &openErr) { | |||
| 681 | + | return false | |||
| 682 | + | } | |||
| 666 | if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { | 683 | if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { | ||
| 667 | return false | 684 | return false | ||
| 668 | } | 685 | } | ||
| @@ -944,7 +961,7 @@ func (a *Agent) oneTurn(ctx context.Context, sink func(AgentEvent), turnContext | |||||
| 944 | defer cancelStream() | 961 | defer cancelStream() | ||
| 945 | stream, err := a.Client.Stream(streamCtx, req) | 962 | stream, err := a.Client.Stream(streamCtx, req) | ||
| 946 | if err != nil { | 963 | if err != nil { | ||
| 947 | - | return provider.StopError, provider.Message{}, err | 964 | + | return provider.StopError, provider.Message{}, &requestOpenError{err: err} |
| 948 | } | 965 | } | ||
| 949 | 966 | ||||
| 950 | sink(EvAssistantStart{}) | 967 | sink(EvAssistantStart{}) | ||
M packages/core/agent_retry_test.go +41 -5
| @@ -61,6 +61,15 @@ func (silentStreamClient) Stream(ctx context.Context, _ provider.Request) (<-cha | |||
| 61 | 61 | return out, nil | |
| 62 | 62 | } | |
| 63 | 63 | ||
| 64 | + | type requestOpenFailureClient struct{ calls int32 } | |
| 65 | + | ||
| 66 | + | func (c *requestOpenFailureClient) Name() string { return "request-open-failure" } | |
| 67 | + | ||
| 68 | + | func (c *requestOpenFailureClient) Stream(context.Context, provider.Request) (<-chan provider.Event, error) { | |
| 69 | + | atomic.AddInt32(&c.calls, 1) | |
| 70 | + | return nil, errors.New("provider returned error: 503 service unavailable") | |
| 71 | + | } | |
| 72 | + | ||
| 64 | 73 | type readableAfterCancellationClient struct { | |
| 65 | 74 | cancel context.CancelFunc | |
| 66 | 75 | } | |
| @@ -74,6 +83,19 @@ func (c readableAfterCancellationClient) Stream(_ context.Context, _ provider.Re | |||
| 74 | 83 | return out, nil | |
| 75 | 84 | } | |
| 76 | 85 | ||
| 86 | + | func TestNewAgentUsesCodexStreamGuardDefaults(t *testing.T) { | |
| 87 | + | a := NewAgent(&retryFakeClient{}, "fake-model", "system", Registry{}) | |
| 88 | + | if a.MaxSteps != 0 { | |
| 89 | + | t.Fatalf("max steps = %d, want unlimited", a.MaxSteps) | |
| 90 | + | } | |
| 91 | + | if a.MaxRetries != 5 { | |
| 92 | + | t.Fatalf("stream retries = %d, want 5", a.MaxRetries) | |
| 93 | + | } | |
| 94 | + | if a.StreamIdleTimeout != 5*time.Minute { | |
| 95 | + | t.Fatalf("stream idle timeout = %s, want 5m", a.StreamIdleTimeout) | |
| 96 | + | } | |
| 97 | + | } | |
| 98 | + | ||
| 77 | 99 | func TestAgentStopsReadingStreamAfterCancellation(t *testing.T) { | |
| 78 | 100 | ctx, cancel := context.WithCancel(context.Background()) | |
| 79 | 101 | a := NewAgent(readableAfterCancellationClient{cancel: cancel}, "fake-model", "system", Registry{}) | |
| @@ -142,6 +164,20 @@ func TestAgentRetriesOverloadedStreamError(t *testing.T) { | |||
| 142 | 164 | } | |
| 143 | 165 | } | |
| 144 | 166 | ||
| 167 | + | func TestAgentDoesNotReplayProviderRequestRetriesAtStreamLayer(t *testing.T) { | |
| 168 | + | client := &requestOpenFailureClient{} | |
| 169 | + | a := NewAgent(client, "fake-model", "system", Registry{}) | |
| 170 | + | a.RetryBaseDelay = time.Millisecond | |
| 171 | + | ||
| 172 | + | err := a.Prompt(context.Background(), "hello", nil, nil) | |
| 173 | + | if err == nil || !strings.Contains(err.Error(), "503") { | |
| 174 | + | t.Fatalf("Prompt error = %v, want provider failure", err) | |
| 175 | + | } | |
| 176 | + | if got := atomic.LoadInt32(&client.calls); got != 1 { | |
| 177 | + | t.Fatalf("Stream calls = %d, want one provider-owned request sequence", got) | |
| 178 | + | } | |
| 179 | + | } | |
| 180 | + | ||
| 145 | 181 | func TestAgentRetriesUnexpectedEOFStreamError(t *testing.T) { | |
| 146 | 182 | client := &retryFakeClient{firstErr: fmt.Errorf("read SSE: %w", io.ErrUnexpectedEOF)} | |
| 147 | 183 | a := NewAgent(client, "fake-model", "system", Registry{}) | |
| @@ -173,10 +209,10 @@ func TestAgentEmitsRetryLifecycleEvents(t *testing.T) { | |||
| 173 | 209 | } | |
| 174 | 210 | ||
| 175 | 211 | want := []string{ | |
| 176 | - | "request:agent:1/4", | |
| 212 | + | "request:agent:1/6", | |
| 177 | 213 | "request:provider:1/1", | |
| 178 | - | "retry:agent:2/4", | |
| 179 | - | "request:agent:2/4", | |
| 214 | + | "retry:agent:2/6", | |
| 215 | + | "request:agent:2/6", | |
| 180 | 216 | "request:provider:1/1", | |
| 181 | 217 | } | |
| 182 | 218 | if strings.Join(lifecycle, ",") != strings.Join(want, ",") { | |
| @@ -200,11 +236,11 @@ func TestAgentReportsSanitizedRetryLifecycle(t *testing.T) { | |||
| 200 | 236 | want := []RetryLifecycleRecord{ | |
| 201 | 237 | { | |
| 202 | 238 | Event: RetryLifecycleRequestFailed, Scope: RetryScopeAgent, | |
| 203 | - | Attempt: 1, MaxAttempts: 4, Reason: RetryReasonOverload, | |
| 239 | + | Attempt: 1, MaxAttempts: 6, Reason: RetryReasonOverload, | |
| 204 | 240 | }, | |
| 205 | 241 | { | |
| 206 | 242 | Event: RetryLifecycleRetryScheduled, Scope: RetryScopeAgent, | |
| 207 | - | Attempt: 2, MaxAttempts: 4, Reason: RetryReasonOverload, DelayMS: 1, | |
| 243 | + | Attempt: 2, MaxAttempts: 6, Reason: RetryReasonOverload, DelayMS: 1, | |
| 208 | 244 | }, | |
| 209 | 245 | } | |
| 210 | 246 | if !reflect.DeepEqual(records, want) { | |
| @@ -61,6 +61,15 @@ func (silentStreamClient) Stream(ctx context.Context, _ provider.Request) (<-cha | |||||
| 61 | return out, nil | 61 | return out, nil | ||
| 62 | } | 62 | } | ||
| 63 | 63 | ||||
| 64 | + | type requestOpenFailureClient struct{ calls int32 } | |||
| 65 | + | ||||
| 66 | + | func (c *requestOpenFailureClient) Name() string { return "request-open-failure" } | |||
| 67 | + | ||||
| 68 | + | func (c *requestOpenFailureClient) Stream(context.Context, provider.Request) (<-chan provider.Event, error) { | |||
| 69 | + | atomic.AddInt32(&c.calls, 1) | |||
| 70 | + | return nil, errors.New("provider returned error: 503 service unavailable") | |||
| 71 | + | } | |||
| 72 | + | ||||
| 64 | type readableAfterCancellationClient struct { | 73 | type readableAfterCancellationClient struct { | ||
| 65 | cancel context.CancelFunc | 74 | cancel context.CancelFunc | ||
| 66 | } | 75 | } | ||
| @@ -74,6 +83,19 @@ func (c readableAfterCancellationClient) Stream(_ context.Context, _ provider.Re | |||||
| 74 | return out, nil | 83 | return out, nil | ||
| 75 | } | 84 | } | ||
| 76 | 85 | ||||
| 86 | + | func TestNewAgentUsesCodexStreamGuardDefaults(t *testing.T) { | |||
| 87 | + | a := NewAgent(&retryFakeClient{}, "fake-model", "system", Registry{}) | |||
| 88 | + | if a.MaxSteps != 0 { | |||
| 89 | + | t.Fatalf("max steps = %d, want unlimited", a.MaxSteps) | |||
| 90 | + | } | |||
| 91 | + | if a.MaxRetries != 5 { | |||
| 92 | + | t.Fatalf("stream retries = %d, want 5", a.MaxRetries) | |||
| 93 | + | } | |||
| 94 | + | if a.StreamIdleTimeout != 5*time.Minute { | |||
| 95 | + | t.Fatalf("stream idle timeout = %s, want 5m", a.StreamIdleTimeout) | |||
| 96 | + | } | |||
| 97 | + | } | |||
| 98 | + | ||||
| 77 | func TestAgentStopsReadingStreamAfterCancellation(t *testing.T) { | 99 | func TestAgentStopsReadingStreamAfterCancellation(t *testing.T) { | ||
| 78 | ctx, cancel := context.WithCancel(context.Background()) | 100 | ctx, cancel := context.WithCancel(context.Background()) | ||
| 79 | a := NewAgent(readableAfterCancellationClient{cancel: cancel}, "fake-model", "system", Registry{}) | 101 | a := NewAgent(readableAfterCancellationClient{cancel: cancel}, "fake-model", "system", Registry{}) | ||
| @@ -142,6 +164,20 @@ func TestAgentRetriesOverloadedStreamError(t *testing.T) { | |||||
| 142 | } | 164 | } | ||
| 143 | } | 165 | } | ||
| 144 | 166 | ||||
| 167 | + | func TestAgentDoesNotReplayProviderRequestRetriesAtStreamLayer(t *testing.T) { | |||
| 168 | + | client := &requestOpenFailureClient{} | |||
| 169 | + | a := NewAgent(client, "fake-model", "system", Registry{}) | |||
| 170 | + | a.RetryBaseDelay = time.Millisecond | |||
| 171 | + | ||||
| 172 | + | err := a.Prompt(context.Background(), "hello", nil, nil) | |||
| 173 | + | if err == nil || !strings.Contains(err.Error(), "503") { | |||
| 174 | + | t.Fatalf("Prompt error = %v, want provider failure", err) | |||
| 175 | + | } | |||
| 176 | + | if got := atomic.LoadInt32(&client.calls); got != 1 { | |||
| 177 | + | t.Fatalf("Stream calls = %d, want one provider-owned request sequence", got) | |||
| 178 | + | } | |||
| 179 | + | } | |||
| 180 | + | ||||
| 145 | func TestAgentRetriesUnexpectedEOFStreamError(t *testing.T) { | 181 | func TestAgentRetriesUnexpectedEOFStreamError(t *testing.T) { | ||
| 146 | client := &retryFakeClient{firstErr: fmt.Errorf("read SSE: %w", io.ErrUnexpectedEOF)} | 182 | client := &retryFakeClient{firstErr: fmt.Errorf("read SSE: %w", io.ErrUnexpectedEOF)} | ||
| 147 | a := NewAgent(client, "fake-model", "system", Registry{}) | 183 | a := NewAgent(client, "fake-model", "system", Registry{}) | ||
| @@ -173,10 +209,10 @@ func TestAgentEmitsRetryLifecycleEvents(t *testing.T) { | |||||
| 173 | } | 209 | } | ||
| 174 | 210 | ||||
| 175 | want := []string{ | 211 | want := []string{ | ||
| 176 | - | "request:agent:1/4", | 212 | + | "request:agent:1/6", |
| 177 | "request:provider:1/1", | 213 | "request:provider:1/1", | ||
| 178 | - | "retry:agent:2/4", | 214 | + | "retry:agent:2/6", |
| 179 | - | "request:agent:2/4", | 215 | + | "request:agent:2/6", |
| 180 | "request:provider:1/1", | 216 | "request:provider:1/1", | ||
| 181 | } | 217 | } | ||
| 182 | if strings.Join(lifecycle, ",") != strings.Join(want, ",") { | 218 | if strings.Join(lifecycle, ",") != strings.Join(want, ",") { | ||
| @@ -200,11 +236,11 @@ func TestAgentReportsSanitizedRetryLifecycle(t *testing.T) { | |||||
| 200 | want := []RetryLifecycleRecord{ | 236 | want := []RetryLifecycleRecord{ | ||
| 201 | { | 237 | { | ||
| 202 | Event: RetryLifecycleRequestFailed, Scope: RetryScopeAgent, | 238 | Event: RetryLifecycleRequestFailed, Scope: RetryScopeAgent, | ||
| 203 | - | Attempt: 1, MaxAttempts: 4, Reason: RetryReasonOverload, | 239 | + | Attempt: 1, MaxAttempts: 6, Reason: RetryReasonOverload, |
| 204 | }, | 240 | }, | ||
| 205 | { | 241 | { | ||
| 206 | Event: RetryLifecycleRetryScheduled, Scope: RetryScopeAgent, | 242 | Event: RetryLifecycleRetryScheduled, Scope: RetryScopeAgent, | ||
| 207 | - | Attempt: 2, MaxAttempts: 4, Reason: RetryReasonOverload, DelayMS: 1, | 243 | + | Attempt: 2, MaxAttempts: 6, Reason: RetryReasonOverload, DelayMS: 1, |
| 208 | }, | 244 | }, | ||
| 209 | } | 245 | } | ||
| 210 | if !reflect.DeepEqual(records, want) { | 246 | if !reflect.DeepEqual(records, want) { | ||
M packages/provider/retry.go +12 -15
| @@ -12,13 +12,10 @@ import ( | |||
| 12 | 12 | "time" | |
| 13 | 13 | ) | |
| 14 | 14 | ||
| 15 | - | // streamRetryAttempts is the maximum number of silent retries for | |
| 16 | - | // transient connect failures before the streaming request is opened. | |
| 17 | - | // 2 retries (so up to 3 total attempts) covers the common case where | |
| 18 | - | // the upstream provider's edge proxy briefly returns 502/503/504 or | |
| 19 | - | // resets the connection before headers, without making the user wait | |
| 20 | - | // noticeably long if the outage is real. | |
| 21 | - | const streamRetryAttempts = 2 | |
| 15 | + | // requestMaxRetries is the maximum number of retries for transient connect or | |
| 16 | + | // HTTP failures before a streaming response opens. It matches Codex's default | |
| 17 | + | // request retry guard and remains separate from agent-level stream reconnects. | |
| 18 | + | const requestMaxRetries = 4 | |
| 22 | 19 | ||
| 23 | 20 | // maxServerRetryDelay caps how long we are willing to honor a | |
| 24 | 21 | // server-requested Retry-After delay. When a provider asks for a | |
| @@ -27,9 +24,9 @@ const streamRetryAttempts = 2 | |||
| 27 | 24 | const maxServerRetryDelay = 60 * time.Second | |
| 28 | 25 | ||
| 29 | 26 | // streamRetryBackoff returns the wait duration before retry attempt n | |
| 30 | - | // (1-based). Short, fixed backoff: 250ms, then 750ms. Anything longer | |
| 31 | - | // would feel like the agent froze; anything shorter starts hammering | |
| 32 | - | // the provider when it's actually struggling. | |
| 27 | + | // (1-based). Short, fixed backoff: 250ms, then 750ms for later retries. | |
| 28 | + | // Anything longer would feel like the agent froze; anything shorter starts | |
| 29 | + | // hammering the provider when it's actually struggling. | |
| 33 | 30 | func streamRetryBackoff(attempt int) time.Duration { | |
| 34 | 31 | switch attempt { | |
| 35 | 32 | case 1: | |
| @@ -40,7 +37,7 @@ func streamRetryBackoff(attempt int) time.Duration { | |||
| 40 | 37 | } | |
| 41 | 38 | ||
| 42 | 39 | // doStreamWithRetry performs an HTTP request that begins a streaming | |
| 43 | - | // response, with up to streamRetryAttempts silent retries for transient | |
| 40 | + | // response, with up to requestMaxRetries silent retries for transient | |
| 44 | 41 | // connect failures. Successful responses (status 200) and non-transient | |
| 45 | 42 | // failures (4xx, malformed bodies, etc.) are returned immediately. | |
| 46 | 43 | // | |
| @@ -66,8 +63,8 @@ func streamRetryBackoff(attempt int) time.Duration { | |||
| 66 | 63 | func doStreamWithRetry(ctx context.Context, client *http.Client, newReq func() (*http.Request, error), lifecycle RequestLifecycle) (*http.Response, error) { | |
| 67 | 64 | var lastErr error | |
| 68 | 65 | serverDelay := time.Duration(-1) // <0: use the default fixed backoff | |
| 69 | - | maxAttempts := streamRetryAttempts + 1 | |
| 70 | - | for attempt := 0; attempt <= streamRetryAttempts; attempt++ { | |
| 66 | + | maxAttempts := requestMaxRetries + 1 | |
| 67 | + | for attempt := 0; attempt <= requestMaxRetries; attempt++ { | |
| 71 | 68 | if attempt > 0 { | |
| 72 | 69 | delay := streamRetryBackoff(attempt) | |
| 73 | 70 | if serverDelay >= 0 { | |
| @@ -106,7 +103,7 @@ func doStreamWithRetry(ctx context.Context, client *http.Client, newReq func() ( | |||
| 106 | 103 | return nil, err | |
| 107 | 104 | } | |
| 108 | 105 | transient := isTransientConnectError(err) | |
| 109 | - | reportRequestFailure(lifecycle, attempt+1, maxAttempts, requestFailureReasonForError(err), !transient || attempt == streamRetryAttempts) | |
| 106 | + | reportRequestFailure(lifecycle, attempt+1, maxAttempts, requestFailureReasonForError(err), !transient || attempt == requestMaxRetries) | |
| 110 | 107 | if !transient { | |
| 111 | 108 | return nil, err | |
| 112 | 109 | } | |
| @@ -129,7 +126,7 @@ func doStreamWithRetry(ctx context.Context, client *http.Client, newReq func() ( | |||
| 129 | 126 | return synthesizeResponse(resp.StatusCode, body), nil | |
| 130 | 127 | } | |
| 131 | 128 | lastErr = &transientHTTPError{Status: resp.StatusCode, Body: trimmed} | |
| 132 | - | if attempt == streamRetryAttempts { | |
| 129 | + | if attempt == requestMaxRetries { | |
| 133 | 130 | reportRequestFailure(lifecycle, attempt+1, maxAttempts, reason, true) | |
| 134 | 131 | // Wrap as a real *http.Response shape the caller | |
| 135 | 132 | // expects so it formats the error identically to a | |
| @@ -12,13 +12,10 @@ import ( | |||||
| 12 | "time" | 12 | "time" | ||
| 13 | ) | 13 | ) | ||
| 14 | 14 | ||||
| 15 | - | // streamRetryAttempts is the maximum number of silent retries for | 15 | + | // requestMaxRetries is the maximum number of retries for transient connect or |
| 16 | - | // transient connect failures before the streaming request is opened. | 16 | + | // HTTP failures before a streaming response opens. It matches Codex's default |
| 17 | - | // 2 retries (so up to 3 total attempts) covers the common case where | 17 | + | // request retry guard and remains separate from agent-level stream reconnects. |
| 18 | - | // the upstream provider's edge proxy briefly returns 502/503/504 or | 18 | + | const requestMaxRetries = 4 |
| 19 | - | // resets the connection before headers, without making the user wait | |||
| 20 | - | // noticeably long if the outage is real. | |||
| 21 | - | const streamRetryAttempts = 2 | |||
| 22 | 19 | ||||
| 23 | // maxServerRetryDelay caps how long we are willing to honor a | 20 | // maxServerRetryDelay caps how long we are willing to honor a | ||
| 24 | // server-requested Retry-After delay. When a provider asks for a | 21 | // server-requested Retry-After delay. When a provider asks for a | ||
| @@ -27,9 +24,9 @@ const streamRetryAttempts = 2 | |||||
| 27 | const maxServerRetryDelay = 60 * time.Second | 24 | const maxServerRetryDelay = 60 * time.Second | ||
| 28 | 25 | ||||
| 29 | // streamRetryBackoff returns the wait duration before retry attempt n | 26 | // streamRetryBackoff returns the wait duration before retry attempt n | ||
| 30 | - | // (1-based). Short, fixed backoff: 250ms, then 750ms. Anything longer | 27 | + | // (1-based). Short, fixed backoff: 250ms, then 750ms for later retries. |
| 31 | - | // would feel like the agent froze; anything shorter starts hammering | 28 | + | // Anything longer would feel like the agent froze; anything shorter starts |
| 32 | - | // the provider when it's actually struggling. | 29 | + | // hammering the provider when it's actually struggling. |
| 33 | func streamRetryBackoff(attempt int) time.Duration { | 30 | func streamRetryBackoff(attempt int) time.Duration { | ||
| 34 | switch attempt { | 31 | switch attempt { | ||
| 35 | case 1: | 32 | case 1: | ||
| @@ -40,7 +37,7 @@ func streamRetryBackoff(attempt int) time.Duration { | |||||
| 40 | } | 37 | } | ||
| 41 | 38 | ||||
| 42 | // doStreamWithRetry performs an HTTP request that begins a streaming | 39 | // doStreamWithRetry performs an HTTP request that begins a streaming | ||
| 43 | - | // response, with up to streamRetryAttempts silent retries for transient | 40 | + | // response, with up to requestMaxRetries silent retries for transient |
| 44 | // connect failures. Successful responses (status 200) and non-transient | 41 | // connect failures. Successful responses (status 200) and non-transient | ||
| 45 | // failures (4xx, malformed bodies, etc.) are returned immediately. | 42 | // failures (4xx, malformed bodies, etc.) are returned immediately. | ||
| 46 | // | 43 | // | ||
| @@ -66,8 +63,8 @@ func streamRetryBackoff(attempt int) time.Duration { | |||||
| 66 | func doStreamWithRetry(ctx context.Context, client *http.Client, newReq func() (*http.Request, error), lifecycle RequestLifecycle) (*http.Response, error) { | 63 | func doStreamWithRetry(ctx context.Context, client *http.Client, newReq func() (*http.Request, error), lifecycle RequestLifecycle) (*http.Response, error) { | ||
| 67 | var lastErr error | 64 | var lastErr error | ||
| 68 | serverDelay := time.Duration(-1) // <0: use the default fixed backoff | 65 | serverDelay := time.Duration(-1) // <0: use the default fixed backoff | ||
| 69 | - | maxAttempts := streamRetryAttempts + 1 | 66 | + | maxAttempts := requestMaxRetries + 1 |
| 70 | - | for attempt := 0; attempt <= streamRetryAttempts; attempt++ { | 67 | + | for attempt := 0; attempt <= requestMaxRetries; attempt++ { |
| 71 | if attempt > 0 { | 68 | if attempt > 0 { | ||
| 72 | delay := streamRetryBackoff(attempt) | 69 | delay := streamRetryBackoff(attempt) | ||
| 73 | if serverDelay >= 0 { | 70 | if serverDelay >= 0 { | ||
| @@ -106,7 +103,7 @@ func doStreamWithRetry(ctx context.Context, client *http.Client, newReq func() ( | |||||
| 106 | return nil, err | 103 | return nil, err | ||
| 107 | } | 104 | } | ||
| 108 | transient := isTransientConnectError(err) | 105 | transient := isTransientConnectError(err) | ||
| 109 | - | reportRequestFailure(lifecycle, attempt+1, maxAttempts, requestFailureReasonForError(err), !transient || attempt == streamRetryAttempts) | 106 | + | reportRequestFailure(lifecycle, attempt+1, maxAttempts, requestFailureReasonForError(err), !transient || attempt == requestMaxRetries) |
| 110 | if !transient { | 107 | if !transient { | ||
| 111 | return nil, err | 108 | return nil, err | ||
| 112 | } | 109 | } | ||
| @@ -129,7 +126,7 @@ func doStreamWithRetry(ctx context.Context, client *http.Client, newReq func() ( | |||||
| 129 | return synthesizeResponse(resp.StatusCode, body), nil | 126 | return synthesizeResponse(resp.StatusCode, body), nil | ||
| 130 | } | 127 | } | ||
| 131 | lastErr = &transientHTTPError{Status: resp.StatusCode, Body: trimmed} | 128 | lastErr = &transientHTTPError{Status: resp.StatusCode, Body: trimmed} | ||
| 132 | - | if attempt == streamRetryAttempts { | 129 | + | if attempt == requestMaxRetries { |
| 133 | reportRequestFailure(lifecycle, attempt+1, maxAttempts, reason, true) | 130 | reportRequestFailure(lifecycle, attempt+1, maxAttempts, reason, true) | ||
| 134 | // Wrap as a real *http.Response shape the caller | 131 | // Wrap as a real *http.Response shape the caller | ||
| 135 | // expects so it formats the error identically to a | 132 | // expects so it formats the error identically to a | ||
M packages/provider/retry_test.go +17 -16
| @@ -19,7 +19,7 @@ func TestOpenAIStreamRetriesOn503(t *testing.T) { | |||
| 19 | 19 | var attempts atomic.Int32 | |
| 20 | 20 | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 21 | 21 | n := attempts.Add(1) | |
| 22 | - | if n < 3 { | |
| 22 | + | if n < 5 { | |
| 23 | 23 | w.WriteHeader(http.StatusServiceUnavailable) | |
| 24 | 24 | _, _ = w.Write([]byte("upstream connect error")) | |
| 25 | 25 | return | |
| @@ -53,14 +53,14 @@ func TestOpenAIStreamRetriesOn503(t *testing.T) { | |||
| 53 | 53 | if gotText != "ok" { | |
| 54 | 54 | t.Fatalf("text=%q", gotText) | |
| 55 | 55 | } | |
| 56 | - | if got := attempts.Load(); got != 3 { | |
| 57 | - | t.Fatalf("attempts=%d want 3", got) | |
| 56 | + | if got := attempts.Load(); got != 5 { | |
| 57 | + | t.Fatalf("attempts=%d want 5", got) | |
| 58 | 58 | } | |
| 59 | - | if got := lifecycle.attempts; len(got) != 3 || got[0] != (lifecycleAttempt{attempt: 1, max: 3}) || got[2] != (lifecycleAttempt{attempt: 3, max: 3}) { | |
| 60 | - | t.Fatalf("lifecycle attempts = %#v, want 1 through 3", got) | |
| 59 | + | if got := lifecycle.attempts; len(got) != 5 || got[0] != (lifecycleAttempt{attempt: 1, max: 5}) || got[4] != (lifecycleAttempt{attempt: 5, max: 5}) { | |
| 60 | + | t.Fatalf("lifecycle attempts = %#v, want 1 through 5", got) | |
| 61 | 61 | } | |
| 62 | - | if got := lifecycle.retries; len(got) != 2 || got[0].attempt != 2 || got[1].attempt != 3 { | |
| 63 | - | t.Fatalf("lifecycle retries = %#v, want retries before attempts 2 and 3", got) | |
| 62 | + | if got := lifecycle.retries; len(got) != 4 || got[0].attempt != 2 || got[3].attempt != 5 { | |
| 63 | + | t.Fatalf("lifecycle retries = %#v, want retries before attempts 2 through 5", got) | |
| 64 | 64 | } | |
| 65 | 65 | } | |
| 66 | 66 | ||
| @@ -85,8 +85,8 @@ func TestOpenAIStreamSurfaces503AfterRetriesExhausted(t *testing.T) { | |||
| 85 | 85 | if !strings.Contains(err.Error(), "503") { | |
| 86 | 86 | t.Fatalf("error %q should mention 503", err) | |
| 87 | 87 | } | |
| 88 | - | if got := attempts.Load(); got != streamRetryAttempts+1 { | |
| 89 | - | t.Fatalf("attempts=%d want %d", got, streamRetryAttempts+1) | |
| 88 | + | if got := attempts.Load(); got != requestMaxRetries+1 { | |
| 89 | + | t.Fatalf("attempts=%d want %d", got, requestMaxRetries+1) | |
| 90 | 90 | } | |
| 91 | 91 | } | |
| 92 | 92 | ||
| @@ -273,13 +273,14 @@ func TestDoStreamWithRetryReportsLifecycle(t *testing.T) { | |||
| 273 | 273 | } | |
| 274 | 274 | resp.Body.Close() | |
| 275 | 275 | ||
| 276 | - | if got := lifecycle.attempts; len(got) != 2 || got[0] != (lifecycleAttempt{attempt: 1, max: 3}) || got[1] != (lifecycleAttempt{attempt: 2, max: 3}) { | |
| 277 | - | t.Fatalf("attempts = %#v, want attempts 1 and 2 of 3", got) | |
| 276 | + | maxAttempts := requestMaxRetries + 1 | |
| 277 | + | if got := lifecycle.attempts; len(got) != 2 || got[0] != (lifecycleAttempt{attempt: 1, max: maxAttempts}) || got[1] != (lifecycleAttempt{attempt: 2, max: maxAttempts}) { | |
| 278 | + | t.Fatalf("attempts = %#v, want attempts 1 and 2 of %d", got, maxAttempts) | |
| 278 | 279 | } | |
| 279 | - | if got := lifecycle.retries; len(got) != 1 || got[0].attempt != 2 || got[0].max != 3 || got[0].delay != 250*time.Millisecond { | |
| 280 | - | t.Fatalf("retries = %#v, want attempt 2 of 3 after 250ms", got) | |
| 280 | + | if got := lifecycle.retries; len(got) != 1 || got[0].attempt != 2 || got[0].max != maxAttempts || got[0].delay != 250*time.Millisecond { | |
| 281 | + | t.Fatalf("retries = %#v, want attempt 2 of %d after 250ms", got, maxAttempts) | |
| 281 | 282 | } | |
| 282 | - | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: 3, reason: RequestFailureOverload}) { | |
| 283 | + | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: maxAttempts, reason: RequestFailureOverload}) { | |
| 283 | 284 | t.Fatalf("failures = %#v, want one non-terminal overload", got) | |
| 284 | 285 | } | |
| 285 | 286 | } | |
| @@ -302,7 +303,7 @@ func TestDoStreamWithRetryReportsTerminalClientFailure(t *testing.T) { | |||
| 302 | 303 | if resp.StatusCode != http.StatusUnauthorized { | |
| 303 | 304 | t.Fatalf("status = %d, want 401", resp.StatusCode) | |
| 304 | 305 | } | |
| 305 | - | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: 3, reason: RequestFailureClient, terminal: true}) { | |
| 306 | + | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: requestMaxRetries + 1, reason: RequestFailureClient, terminal: true}) { | |
| 306 | 307 | t.Fatalf("failures = %#v, want one terminal client failure", got) | |
| 307 | 308 | } | |
| 308 | 309 | if len(lifecycle.retries) != 0 { | |
| @@ -329,7 +330,7 @@ func TestDoStreamWithRetryReportsTerminalQuota(t *testing.T) { | |||
| 329 | 330 | if resp.StatusCode != http.StatusTooManyRequests { | |
| 330 | 331 | t.Fatalf("status = %d, want 429", resp.StatusCode) | |
| 331 | 332 | } | |
| 332 | - | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: 3, reason: RequestFailureQuota, terminal: true}) { | |
| 333 | + | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: requestMaxRetries + 1, reason: RequestFailureQuota, terminal: true}) { | |
| 333 | 334 | t.Fatalf("failures = %#v, want one terminal quota failure", got) | |
| 334 | 335 | } | |
| 335 | 336 | } | |
| @@ -19,7 +19,7 @@ func TestOpenAIStreamRetriesOn503(t *testing.T) { | |||||
| 19 | var attempts atomic.Int32 | 19 | var attempts atomic.Int32 | ||
| 20 | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | 20 | srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| 21 | n := attempts.Add(1) | 21 | n := attempts.Add(1) | ||
| 22 | - | if n < 3 { | 22 | + | if n < 5 { |
| 23 | w.WriteHeader(http.StatusServiceUnavailable) | 23 | w.WriteHeader(http.StatusServiceUnavailable) | ||
| 24 | _, _ = w.Write([]byte("upstream connect error")) | 24 | _, _ = w.Write([]byte("upstream connect error")) | ||
| 25 | return | 25 | return | ||
| @@ -53,14 +53,14 @@ func TestOpenAIStreamRetriesOn503(t *testing.T) { | |||||
| 53 | if gotText != "ok" { | 53 | if gotText != "ok" { | ||
| 54 | t.Fatalf("text=%q", gotText) | 54 | t.Fatalf("text=%q", gotText) | ||
| 55 | } | 55 | } | ||
| 56 | - | if got := attempts.Load(); got != 3 { | 56 | + | if got := attempts.Load(); got != 5 { |
| 57 | - | t.Fatalf("attempts=%d want 3", got) | 57 | + | t.Fatalf("attempts=%d want 5", got) |
| 58 | } | 58 | } | ||
| 59 | - | if got := lifecycle.attempts; len(got) != 3 || got[0] != (lifecycleAttempt{attempt: 1, max: 3}) || got[2] != (lifecycleAttempt{attempt: 3, max: 3}) { | 59 | + | if got := lifecycle.attempts; len(got) != 5 || got[0] != (lifecycleAttempt{attempt: 1, max: 5}) || got[4] != (lifecycleAttempt{attempt: 5, max: 5}) { |
| 60 | - | t.Fatalf("lifecycle attempts = %#v, want 1 through 3", got) | 60 | + | t.Fatalf("lifecycle attempts = %#v, want 1 through 5", got) |
| 61 | } | 61 | } | ||
| 62 | - | if got := lifecycle.retries; len(got) != 2 || got[0].attempt != 2 || got[1].attempt != 3 { | 62 | + | if got := lifecycle.retries; len(got) != 4 || got[0].attempt != 2 || got[3].attempt != 5 { |
| 63 | - | t.Fatalf("lifecycle retries = %#v, want retries before attempts 2 and 3", got) | 63 | + | t.Fatalf("lifecycle retries = %#v, want retries before attempts 2 through 5", got) |
| 64 | } | 64 | } | ||
| 65 | } | 65 | } | ||
| 66 | 66 | ||||
| @@ -85,8 +85,8 @@ func TestOpenAIStreamSurfaces503AfterRetriesExhausted(t *testing.T) { | |||||
| 85 | if !strings.Contains(err.Error(), "503") { | 85 | if !strings.Contains(err.Error(), "503") { | ||
| 86 | t.Fatalf("error %q should mention 503", err) | 86 | t.Fatalf("error %q should mention 503", err) | ||
| 87 | } | 87 | } | ||
| 88 | - | if got := attempts.Load(); got != streamRetryAttempts+1 { | 88 | + | if got := attempts.Load(); got != requestMaxRetries+1 { |
| 89 | - | t.Fatalf("attempts=%d want %d", got, streamRetryAttempts+1) | 89 | + | t.Fatalf("attempts=%d want %d", got, requestMaxRetries+1) |
| 90 | } | 90 | } | ||
| 91 | } | 91 | } | ||
| 92 | 92 | ||||
| @@ -273,13 +273,14 @@ func TestDoStreamWithRetryReportsLifecycle(t *testing.T) { | |||||
| 273 | } | 273 | } | ||
| 274 | resp.Body.Close() | 274 | resp.Body.Close() | ||
| 275 | 275 | ||||
| 276 | - | if got := lifecycle.attempts; len(got) != 2 || got[0] != (lifecycleAttempt{attempt: 1, max: 3}) || got[1] != (lifecycleAttempt{attempt: 2, max: 3}) { | 276 | + | maxAttempts := requestMaxRetries + 1 |
| 277 | - | t.Fatalf("attempts = %#v, want attempts 1 and 2 of 3", got) | 277 | + | if got := lifecycle.attempts; len(got) != 2 || got[0] != (lifecycleAttempt{attempt: 1, max: maxAttempts}) || got[1] != (lifecycleAttempt{attempt: 2, max: maxAttempts}) { |
| 278 | + | t.Fatalf("attempts = %#v, want attempts 1 and 2 of %d", got, maxAttempts) | |||
| 278 | } | 279 | } | ||
| 279 | - | if got := lifecycle.retries; len(got) != 1 || got[0].attempt != 2 || got[0].max != 3 || got[0].delay != 250*time.Millisecond { | 280 | + | if got := lifecycle.retries; len(got) != 1 || got[0].attempt != 2 || got[0].max != maxAttempts || got[0].delay != 250*time.Millisecond { |
| 280 | - | t.Fatalf("retries = %#v, want attempt 2 of 3 after 250ms", got) | 281 | + | t.Fatalf("retries = %#v, want attempt 2 of %d after 250ms", got, maxAttempts) |
| 281 | } | 282 | } | ||
| 282 | - | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: 3, reason: RequestFailureOverload}) { | 283 | + | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: maxAttempts, reason: RequestFailureOverload}) { |
| 283 | t.Fatalf("failures = %#v, want one non-terminal overload", got) | 284 | t.Fatalf("failures = %#v, want one non-terminal overload", got) | ||
| 284 | } | 285 | } | ||
| 285 | } | 286 | } | ||
| @@ -302,7 +303,7 @@ func TestDoStreamWithRetryReportsTerminalClientFailure(t *testing.T) { | |||||
| 302 | if resp.StatusCode != http.StatusUnauthorized { | 303 | if resp.StatusCode != http.StatusUnauthorized { | ||
| 303 | t.Fatalf("status = %d, want 401", resp.StatusCode) | 304 | t.Fatalf("status = %d, want 401", resp.StatusCode) | ||
| 304 | } | 305 | } | ||
| 305 | - | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: 3, reason: RequestFailureClient, terminal: true}) { | 306 | + | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: requestMaxRetries + 1, reason: RequestFailureClient, terminal: true}) { |
| 306 | t.Fatalf("failures = %#v, want one terminal client failure", got) | 307 | t.Fatalf("failures = %#v, want one terminal client failure", got) | ||
| 307 | } | 308 | } | ||
| 308 | if len(lifecycle.retries) != 0 { | 309 | if len(lifecycle.retries) != 0 { | ||
| @@ -329,7 +330,7 @@ func TestDoStreamWithRetryReportsTerminalQuota(t *testing.T) { | |||||
| 329 | if resp.StatusCode != http.StatusTooManyRequests { | 330 | if resp.StatusCode != http.StatusTooManyRequests { | ||
| 330 | t.Fatalf("status = %d, want 429", resp.StatusCode) | 331 | t.Fatalf("status = %d, want 429", resp.StatusCode) | ||
| 331 | } | 332 | } | ||
| 332 | - | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: 3, reason: RequestFailureQuota, terminal: true}) { | 333 | + | if got := lifecycle.failures; len(got) != 1 || got[0] != (lifecycleFailure{attempt: 1, max: requestMaxRetries + 1, reason: RequestFailureQuota, terminal: true}) { |
| 333 | t.Fatalf("failures = %#v, want one terminal quota failure", got) | 334 | t.Fatalf("failures = %#v, want one terminal quota failure", got) | ||
| 334 | } | 335 | } | ||
| 335 | } | 336 | } | ||