Repository navigation
fix(cloud-agent-next): retry git rate-limited workspace setup - #5060
Conversation
Classify clone/checkout 429 and rate-limit errors as git_rate_limited, give them a short backed-off pending-message retry budget, and surface a clear safe failure message instead of failing after one redelivery.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryIncremental review of commit 426f381 confirms the previously reported rate-limit classification false positive is fixed: the 429 pattern is now anchored to an HTTP-error context and a regression test covers git progress object counts, with high confidence in the changed scope. Files Reviewed (2 files)
Previous Review Summary (commit 9bd24cf)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 9bd24cf)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (7 files)
The retry-budget logic (reset-eligibility, alternation accumulation, exhaustion), subtype plumbing (wrapper → session-ready error → WrapperError → ExecutionError → pending flush), telemetry classification, safe-message projection, and code-review terminal-reason mapping were all verified end-to-end and are consistent. No memory-leak vectors introduced (constants and classification only; no new timers/listeners/caches). Reviewed by kimi-k3 · Input: 46.9K · Output: 9.9K · Cached: 361.5K Review guidance: REVIEW.md from base branch |
|
@kilocode-bot address PR feedback |
|
One or more Cloud Agent sessions failed:
|
Clone/fetch run with --progress, so stderr of a clone that failed for other reasons can contain standalone object counts (e.g. "remote: Total 429 (delta 12)"). A bare \b429\b matched those and, being evaluated before git_network_failed, misclassified network failures as git_rate_limited with the wrong safe message, telemetry, and retry budget. Anchor the numeric alternative to error/http/status context and add a regression case.
Summary
Why
Cold Cloud Agent workspace bootstrap can fail when GitHub throttles clone/ref discovery with HTTP 429 (or equivalent “too many requests” / rate-limit text). Those failures were previously unclassified (
workspace_setup_unknown) and only got a single generic pending-message redelivery, so concurrent or bursty clone traffic could fail sessions that would succeed moments later.What was done
\b429\b,too many requests, orrate limit(ed)as workspace subtypegit_rate_limited(after auth patterns so 401/403 still win).rate_limited, project the safe user message “Repository request was rate limited”, and map code-review terminal reason to existingrepository_clone_failed(no new DB enum).15s,45s— two retries / three total attempts) forWORKSPACE_SETUP_FAILED+git_rate_limited, reusing the full cold bootstrap path instead of adding an inner clone retry loop.High-level architecture
sequenceDiagram participant DO as Session DO participant Pending as Pending flush participant Wrapper as Sandbox wrapper participant GitHub as Git remote DO->>Pending: Drain queued cold-start message Pending->>Wrapper: Prepare workspace (clone/checkout) Wrapper->>GitHub: git clone / fetch GitHub-->>Wrapper: 429 / rate limited Wrapper-->>Pending: WORKSPACE_SETUP_FAILED git_rate_limited Note over Pending: Schedule retry at +15s then +45s<br/>(reset-eligible budget) Pending->>Wrapper: Retry full cold bootstrap alt Throttle cleared Wrapper->>GitHub: clone succeeds Wrapper-->>DO: Workspace ready else Budget exhausted Pending-->>DO: Terminal safe failure projection endArchitecture decision
Decision: Detect git rate limits at wrapper classification and retry via the existing pending-flush mechanism with a dedicated short backoff budget, rather than adding an inner retry loop inside
cloneRepository.Context: Pending flush already owns durable redelivery of cold bootstrap, and other transient workspace conditions (e.g. sandbox storage full) already use subtype-specific reset-eligible budgets. Clone 429s are short-lived platform throttling that clear without a new product path.
Rationale: Reusing pending flush keeps retry durability, attempt accounting, and terminalization in one place, mirrors the storage-full pattern, and avoids duplicating bootstrap side effects inside the wrapper.
Alternatives considered:
Consequences: Sessions survive brief GitHub clone throttling with clearer diagnostics and platform ownership. Detection remains best-effort regex on sanitized process output (no captured production 429 stderr sample in-repo); permanent or long throttles still exhaust after two backed-off retries.