Skip to content

fix(tui): retry sub-agent API timeouts with backoff; raise default timeout to 600s - #5210

Merged
Hmbown merged 1 commit into
agent/v094-release-train-20260802from
agent/v094-subagent-retry-20260803
Aug 3, 2026
Merged

fix(tui): retry sub-agent API timeouts with backoff; raise default timeout to 600s#5210
Hmbown merged 1 commit into
agent/v094-release-train-20260802from
agent/v094-subagent-retry-20260803

Conversation

@Hmbown

@Hmbown Hmbown commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Problem

Dogfood-confirmed 2026-08-03: a 6-agent fan-out was wiped out one by one — every sub-agent died with Sub-agent interrupted before completion (API call timed out after 120000ms; checkpoint preserved for continuation).

Root cause: in crates/tui/src/tools/subagent/mod.rs, the per-step create_message call is wrapped in tokio::time::timeout(step_api_timeout, …), and the timeout arm returned Interrupted with zero retries. Transient provider errors already retry (SUBAGENT_TRANSIENT_PROVIDER_MAX_RETRIES), but timeouts did not — so one live-but-slow provider call killed an entire child. Full findings: codewhale-ops/FINISH-0.9.4.md entries #39/#40.

kimi-code comparison

A scout compared against kimi-code (packages/agent-core): it retries every provider step up to 10 attempts with exponential backoff 0.5s→32s + 25% jitter, honors Retry-After, classifies timeouts as retryable, uses streaming so live-but-slow calls never hit a wall-clock cap, and has first-class resume. Owner directive: "learn from kimicode."

What this PR does

(a) Timeout arm is now retryable. A per-step API timeout folds into the retry machinery:

  • New budget SUBAGENT_API_TIMEOUT_MAX_RETRIES = 5 per step.
  • Exponential backoff: base 1s, ×2 per retry, capped at 30s, ±20% jitter (UUID-entropy idiom shared with llm_client::RetryConfig) so a fan-out that times out together doesn't re-fire in lockstep.
  • Emits the same style of ModelWait progress event as the existing "retrying API request" transient path, so the TUI shows the retry (…: API call timed out after Nms; retrying API request k/5 in Xms).
  • After exhaustion, behavior is unchanged: Interrupted with checkpoint_reason: "api_timeout" and the checkpoint preserved for continuation.

(c) Config limits (crates/tui/src/config/subagent_limits.rs):

  • DEFAULT_SUBAGENT_API_TIMEOUT_SECS: 120 → 600 (matches the mitigation operators already applied by hand).
  • Clamp ceiling MAX_SUBAGENT_API_TIMEOUT_SECS: 1800 → 3600.
  • Doc comments updated, including the stale ~/.deepseek/config.toml reference (product home is ~/.codewhale); docs/SUBAGENTS.md, docs/CONFIGURATION.md, and config.example.toml updated to match.
  • Knock-on effect (test updated, no code change): with the 600s API default, the heartbeat resolver's "at least api_timeout + 30s" floor lifts the resolved default heartbeat from 300s to 630s (subagent_heartbeat_timeout_defaults_clamps_and_respects_api_timeout now asserts the resolved value; docs updated).

Tests

  • Backoff sequence is deterministic and bounded: 1s/2s/4s/8s/16s then capped at 30s; jittered delay stays within ±20% of the base (32 samples per retry level).
  • Integration: server slow on attempt 1 only → retried exactly once, then Completed (subagent_retries_api_timeout_before_succeeding).
  • Integration: server slow on every attempt → exactly 1 + SUBAGENT_API_TIMEOUT_MAX_RETRIES calls, then Interrupted with api_timeout checkpoint, waiting_for_user projection, no parking (updated api_timeout_preserves_checkpoint_and_returns_needs_input_without_parking; new always_delayed_chat_client helper mirrors the existing fake-server pattern; production backoff is shrunk via a #[cfg(test)] runtime knob so the test runs in milliseconds).
  • Config: default is 600, 3600 accepted, 3601 clamps to 3600 (added literal assertions to subagent_api_timeout_defaults_and_clamps).
  • child_runtime()/background_runtime() preserve the new backoff field (extended child_and_background_runtimes_preserve_step_api_timeout).
  • Gates: cargo fmt --all clean; cargo test -p codewhale-tui — 9621 passed; the only 19 failures are pre-existing environment-dependent tests (missing DeepSeek/MiniMax/Anthropic keys, provider-catalog fixtures) verified to fail identically on base d53f4f998.

Not covered at unit level: the exact wall-clock interleaving of timeout → progress event → sleep inside request_subagent_model_response_with_retries (exercised end-to-end by the two integration tests instead); the TUI rendering of the progress event (same event path as the existing transient retry).

Follow-ups (NOT in this PR)

  • (b) Idle-based streaming keepalive for sub-agent calls, reusing the engine's stream_chunk_timeout (crates/tui/src/core/engine.rs:467), so a live-but-slow streaming call never trips a wall-clock cap at all (kimi's actual defense-in-depth).
  • (d) Automated checkpoint resume on re-dispatch (kimi's resume=<agent_id> semantics) instead of relying on the parent to re-dispatch manually.

…meout to 600s

A per-step create_message call that exceeded step_api_timeout went
straight to Interrupted with zero retries, so one live-but-slow provider
call killed an entire child (dogfood: a 6-agent fan-out wiped out one by
one at the 120s wall, FINISH-0.9.4 entries #39/#40). Fold the timeout
arm into the retry machinery:

- SUBAGENT_API_TIMEOUT_MAX_RETRIES (5) per-step timeout budget with
  exponential backoff (1s base, x2, 30s cap, +/-20% jitter via the
  llm_client UUID-entropy idiom) and the same ModelWait progress event
  style as the transient-provider retry path.
- After exhaustion, behavior is unchanged: Interrupted with the
  checkpoint preserved for continuation (api_timeout).
- DEFAULT_SUBAGENT_API_TIMEOUT_SECS 120 -> 600 and clamp ceiling
  1800 -> 3600; doc comments, config.example.toml, and docs updated
  (including the stale ~/.deepseek/config.toml reference). The resolved
  default heartbeat rises to 630s via the existing api+30s floor.
- Tests: deterministic backoff sequence + jitter bounds, retry-then-
  success and retry-then-exhaustion integration coverage (new
  always_delayed_chat_client helper; backoff shrinkable in tests),
  config clamp literals (600 default, 3600 accepted, 3601 clamped).
Copilot AI review requested due to automatic review settings August 3, 2026 09:18
@cursor

cursor Bot commented Aug 3, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves sub-agent reliability in the TUI by treating per-step create_message API timeouts as retryable failures (with exponential backoff + jitter) and by increasing the default/configured timeout ceilings to better accommodate slow-but-live reasoning calls.

Changes:

  • Add timeout-retry behavior for sub-agent per-step API calls, including exponential backoff + jitter and progress events during retries.
  • Raise the default sub-agent API timeout to 600s and increase the configured maximum to 3600s, updating related config resolution behavior and tests.
  • Update documentation to reflect the new defaults/retry behavior and the knock-on heartbeat resolution change.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
docs/SUBAGENTS.md Updates sub-agent timeout guidance and explains the new default/behavior.
docs/CONFIGURATION.md Updates configuration docs for sub-agent API/heartbeat timeouts and retry behavior.
crates/tui/src/tools/subagent/tests.rs Adds/updates integration + unit tests for timeout retries/backoff and runtime propagation.
crates/tui/src/tools/subagent/mod.rs Implements timeout retry budget + backoff/jitter, adds runtime field for test backoff shrinking.
crates/tui/src/core/engine.rs Updates sub-agent API timeout clamp documentation in engine config.
crates/tui/src/config/tests.rs Updates config tests for new defaults and clamping behavior.
crates/tui/src/config/subagent_limits.rs Raises API timeout defaults/max constants used for clamping.
crates/tui/src/config.rs Updates config docs and resolution behavior to match new sub-agent timeout defaults.
crates/tui/src/commands/groups/config/config.rs Updates config command test expectations for resolved sub-agent timeout values.
config.example.toml Updates example config to show the new default/clamp values.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +8871 to +8875
let jitter = base.as_secs_f64()
* SUBAGENT_API_TIMEOUT_BACKOFF_JITTER_FACTOR
* (2.0 * random_factor - 1.0); // -20% to +20%
Duration::from_secs_f64((base.as_secs_f64() + jitter).max(0.0))
}
Comment thread docs/SUBAGENTS.md
Comment on lines +455 to +457
transcript handle and persisted worker record. The default is 5 minutes
(resolved to at least 30 seconds above `api_timeout_secs`, so 630 seconds
with the 600-second default API timeout):
Comment thread docs/CONFIGURATION.md
Comment on lines +1602 to 1607
`1..=3600`, with `0` or unset preserving the 600 second default; a timed-out
attempt is retried with exponential backoff (up to 5 retries) before the
step interrupts with a preserved checkpoint.
`[subagents] heartbeat_timeout_secs` controls stale running agent cleanup,
defaults to `300`, and is clamped to `30..=3600` while staying above the
resolved API timeout. `[subagents.providers.<provider>]` accepts the same
@Hmbown
Hmbown merged commit 86b4e23 into agent/v094-release-train-20260802 Aug 3, 2026
5 of 6 checks passed
@Hmbown
Hmbown deleted the agent/v094-subagent-retry-20260803 branch August 3, 2026 10:15
Hmbown added a commit that referenced this pull request Aug 4, 2026
…meout to 600s (#5210)

A per-step create_message call that exceeded step_api_timeout went
straight to Interrupted with zero retries, so one live-but-slow provider
call killed an entire child (dogfood: a 6-agent fan-out wiped out one by
one at the 120s wall, FINISH-0.9.4 entries #39/#40). Fold the timeout
arm into the retry machinery:

- SUBAGENT_API_TIMEOUT_MAX_RETRIES (5) per-step timeout budget with
  exponential backoff (1s base, x2, 30s cap, +/-20% jitter via the
  llm_client UUID-entropy idiom) and the same ModelWait progress event
  style as the transient-provider retry path.
- After exhaustion, behavior is unchanged: Interrupted with the
  checkpoint preserved for continuation (api_timeout).
- DEFAULT_SUBAGENT_API_TIMEOUT_SECS 120 -> 600 and clamp ceiling
  1800 -> 3600; doc comments, config.example.toml, and docs updated
  (including the stale ~/.deepseek/config.toml reference). The resolved
  default heartbeat rises to 630s via the existing api+30s floor.
- Tests: deterministic backoff sequence + jitter bounds, retry-then-
  success and retry-then-exhaustion integration coverage (new
  always_delayed_chat_client helper; backoff shrinkable in tests),
  config clamp literals (600 default, 3600 accepted, 3601 clamped).
Hmbown added a commit that referenced this pull request Aug 4, 2026
…meout to 600s (#5210)

A per-step create_message call that exceeded step_api_timeout went
straight to Interrupted with zero retries, so one live-but-slow provider
call killed an entire child (dogfood: a 6-agent fan-out wiped out one by
one at the 120s wall, FINISH-0.9.4 entries #39/#40). Fold the timeout
arm into the retry machinery:

- SUBAGENT_API_TIMEOUT_MAX_RETRIES (5) per-step timeout budget with
  exponential backoff (1s base, x2, 30s cap, +/-20% jitter via the
  llm_client UUID-entropy idiom) and the same ModelWait progress event
  style as the transient-provider retry path.
- After exhaustion, behavior is unchanged: Interrupted with the
  checkpoint preserved for continuation (api_timeout).
- DEFAULT_SUBAGENT_API_TIMEOUT_SECS 120 -> 600 and clamp ceiling
  1800 -> 3600; doc comments, config.example.toml, and docs updated
  (including the stale ~/.deepseek/config.toml reference). The resolved
  default heartbeat rises to 630s via the existing api+30s floor.
- Tests: deterministic backoff sequence + jitter bounds, retry-then-
  success and retry-then-exhaustion integration coverage (new
  always_delayed_chat_client helper; backoff shrinkable in tests),
  config clamp literals (600 default, 3600 accepted, 3601 clamped).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants