Skip to content

fix(code-reviews): retry transient wrapper startup failures#3290

Merged
jeanduplessis merged 2 commits into
mainfrom
fix/code-review-wrapper-readiness-retries
May 18, 2026
Merged

fix(code-reviews): retry transient wrapper startup failures#3290
jeanduplessis merged 2 commits into
mainfrom
fix/code-review-wrapper-readiness-retries

Conversation

@jeanduplessis
Copy link
Copy Markdown
Contributor

Summary

  • Retry code-review fresh-session fallback for observed transient Cloud Agent wrapper-readiness prepareSession 5xx signatures.
  • Keep configured-session lookup misses, repo-specific checkout failures, billing failures, and cancelled reviews outside widened retry scope.
  • Add structured retry and prepareSession failure classification fields so future alerts can split wrapper startup, sandbox/storage, configured-session, repo, retry outcome, and bot/user sandbox behavior.

Verification

  • N/A - no manual verification performed.

Visual Changes

  • N/A

Reviewer Notes

  • Retry widening stays text-targeted to incident-observed wrapper startup failures rather than treating generic 500 responses as retryable.
  • configured session ... not found remains intentionally non-retryable because wrapper startup treats it as continuity loss, not safe transient infra noise.
  • Observability changes add stable category fields and retry outcome states intended for future monitor/query refinement.

@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented May 17, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit addresses both previously raised review concerns: the repo-failure guard is now ordered before hasRetryableSandboxMarker (preventing misclassification of repo errors that also carry a 500/sandbox signal), and the timeout string match has been generalized to drop the hardcoded 30000ms value. Tests are updated accordingly.

Incremental Changes Reviewed
  • services/cloud-agent-next/src/router/handlers/session-prepare.ts — timeout string match generalized (drop after 30000ms)
  • services/code-review-infra/src/code-review-orchestrator.ts — repo-failure guard moved before hasRetryableSandboxMarker
  • services/code-review-infra/test/integration/code-review-orchestrator.test.ts — tests updated: repo-failure test uses compound SandboxError message to verify precedence; timeout test uses 45000ms to confirm number is no longer hardcoded
Files Reviewed (4 files — full PR)
  • services/cloud-agent-next/src/logger.ts
  • services/cloud-agent-next/src/router/handlers/session-prepare.ts
  • services/code-review-infra/src/code-review-orchestrator.ts
  • services/code-review-infra/test/integration/code-review-orchestrator.test.ts

Reviewed by claude-sonnet-4.6 · 293,626 tokens

Review guidance: REVIEW.md from base branch main

Comment thread services/code-review-infra/src/code-review-orchestrator.ts Outdated
Comment thread services/cloud-agent-next/src/router/handlers/session-prepare.ts Outdated
Comment thread services/cloud-agent-next/src/router/handlers/session-prepare.ts
Copy link
Copy Markdown
Contributor

@eshurakov eshurakov left a comment

Choose a reason for hiding this comment

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

checked cloud agent side

@jeanduplessis jeanduplessis merged commit 9d9406f into main May 18, 2026
13 checks passed
@jeanduplessis jeanduplessis deleted the fix/code-review-wrapper-readiness-retries branch May 18, 2026 13:14
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.

3 participants