Skip to content

fix(cloud-agent): wire Code Reviewer sandbox cleanup#4292

Open
eshurakov wants to merge 1 commit into
mainfrom
flossy-scion
Open

fix(cloud-agent): wire Code Reviewer sandbox cleanup#4292
eshurakov wants to merge 1 commit into
mainfrom
flossy-scion

Conversation

@eshurakov

Copy link
Copy Markdown
Contributor

Summary

  • Route wrapper terminal events from ingest through the session lifecycle so Code Reviewer ephemeral sandbox cleanup runs after normal completions.
  • Add Code Reviewer ephemeral sandbox routing and cleanup policy around crv-* sandboxes, including status reporting and alarm-driven destruction.
  • Gate destructive recovery and terminal cleanup on the allocated sandbox ID being crv-* so rollout allowlist changes do not tear down shared sandboxes.

Verification

N/A - backend lifecycle change with no manual UI path.

Visual Changes

N/A

Reviewer Notes

Focus on the terminal-event lifecycle wiring and the route-based crv-* cleanup guard. The guard intentionally uses the allocated sandbox ID in addition to current org policy so rollout config changes cannot make older shared allocations destructible.

const policy = resolveEphemeralSandboxPolicy(this.env, metadata);
if (!policy.enabled) return;
if (!isCodeReviewEphemeralSandboxId(metadata.workspace?.sandboxId)) return;
await this.getWrapperSupervisor().requestPhysicalWrapperStop('terminal-completed', {

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.

WARNING: Cleanup also runs after failed and interrupted terminal events

handleWrapperTerminalEvent() schedules the Code Reviewer wrapper stop and delayed sandbox destruction for every terminal event once the session is on a crv-* sandbox, because this new block never checks params.status. That means failed and interrupted runs will also be torn down 60 seconds later, removing the retry/debug window that the supervisor still preserves for unsuccessful runs and conflicting with the PR's stated "after normal completions" intent.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/src/persistence/CloudAgentSession.ts 3154 Cleanup is scheduled for failed and interrupted terminal events too

Fix these issues in Kilo Cloud

Files Reviewed (25 files)
  • services/cloud-agent-next/src/agent-sandbox/cloudflare/cloudflare-agent-sandbox.ts - 0 issues
  • services/cloud-agent-next/src/agent-sandbox/protocol.ts - 0 issues
  • services/cloud-agent-next/src/code-review-ephemeral-sandbox.test.ts - 0 issues
  • services/cloud-agent-next/src/code-review-ephemeral-sandbox.ts - 0 issues
  • services/cloud-agent-next/src/execution/orchestrator.test.ts - 0 issues
  • services/cloud-agent-next/src/execution/orchestrator.ts - 0 issues
  • services/cloud-agent-next/src/index.ts - 0 issues
  • services/cloud-agent-next/src/kilo-facade/session-proxy.ts - 0 issues
  • services/cloud-agent-next/src/persistence/CloudAgentSession.ts - 1 issue
  • services/cloud-agent-next/src/persistence/schemas.ts - 0 issues
  • services/cloud-agent-next/src/persistence/session-metadata.ts - 0 issues
  • services/cloud-agent-next/src/persistence/types.ts - 0 issues
  • services/cloud-agent-next/src/router.test.ts - 0 issues
  • services/cloud-agent-next/src/router/handlers/session-management.ts - 0 issues
  • services/cloud-agent-next/src/sandbox-id.test.ts - 0 issues
  • services/cloud-agent-next/src/sandbox-id.ts - 0 issues
  • services/cloud-agent-next/src/session-service.ts - 0 issues
  • services/cloud-agent-next/src/session/session-registration.ts - 0 issues
  • services/cloud-agent-next/src/session/wrapper-runtime-state.ts - 0 issues
  • services/cloud-agent-next/src/types.ts - 0 issues
  • services/cloud-agent-next/src/websocket/ingest.test.ts - 0 issues
  • services/cloud-agent-next/src/websocket/ingest.ts - 0 issues
  • services/cloud-agent-next/test/integration/session/code-review-ephemeral-sandbox.test.ts - 0 issues
  • services/cloud-agent-next/worker-configuration.d.ts - 0 issues
  • services/cloud-agent-next/wrangler.jsonc - 0 issues

Reviewed by gpt-5.4-20260305 · Input: 104K · Output: 15.5K · Cached: 1.1M

Review guidance: REVIEW.md from base branch main

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.

1 participant