Skip to content

fix(cloud-agent-next) skip container wake on idle stop - #4902

Merged
St0rmz1 merged 2 commits into
mainfrom
fix/skip-container-wake-on-idle-stop
Jul 30, 2026
Merged

fix(cloud-agent-next) skip container wake on idle stop#4902
St0rmz1 merged 2 commits into
mainfrom
fix/skip-container-wake-on-idle-stop

Conversation

@St0rmz1

@St0rmz1 St0rmz1 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

stopWrappers inspects the sandbox by calling listProcesses, which is a
container fetch and therefore boots a sleeping container. On the idle timeout
path that boot is wasted work: cleanupIdleKiloServer only requests a stop
after hasWrapperRuntimeOrPendingWork() has already confirmed from Durable
Object state that no wrapper runtime and no pending work remain. The inspection
then wakes a stopped container to re-derive that same fact, and the woken
container sits idle until the 900s sleepAfter expires it.

In 90 minutes of production logs, reason=idle-timeout produced 233 stop
inspections. Every one returned absent. None returned present.

This confirms absence from container state instead, when the container is
stopped. A wrapper is a process, and a process cannot outlive its container
(activity expiry SIGTERMs the container, exit code 143), so a stopped container
cannot be hiding a leaked wrapper.

Changes

  • container-usage.ts: adds MeteredSandbox.isContainerRunning(), returning
    this.ctx.container?.running === true. It reads Durable Object state only, so
    calling it over RPC does not boot the container.
  • container-usage-context.ts: adds the method to MeteredSandboxInstance and
    adds isSandboxContainerRunning(), using the same runtime guarded cast as
    configureSandboxBillingInput. It returns boolean | undefined, yielding
    undefined when the method is missing or throws, so an unknown state is never
    read as stopped.
  • cloudflare-agent-sandbox.ts: in stopWrappers, when the reason is
    idle-timeout and the container is confirmed not running, return
    { status: 'absent' } without inspecting. Every other stop reason is
    unchanged.
  • cloudflare-agent-sandbox.ts: the wrapper_stop_inspection log is now emitted
    once for all outcomes rather than once per branch, with a local
    StopInspection type adding an absent-no-container status for the new case.

Verification

  • No manual testing of the fix. Reproducing it needs a sandbox container that
    has already gone to sleep plus a Durable Object alarm firing the idle sweep,
    which is not reproducible locally. The problem itself was measured from
    production logs (233 idle timeout inspections, all absent).

Visual Changes

N/A

Reviewer Notes

  • The narrow scope is deliberate. The stopWrappers inspection was added by
    fix(cloud-agent-next): recover leaked sandbox wrappers safely #3555 ("recover leaked sandbox wrappers safely") to catch wrapper processes the
    Durable Object lease does not track, including duplicates in shared sandboxes.
    That recovery is preserved: it can only apply when a container is running, and
    in that case this change falls through to the existing inspection untouched.
    Only idle-timeout short circuits.
  • isSandboxContainerRunning sits behind a && on the reason check, so the probe
    does not run at all for other stop reasons. There is a test asserting this.
  • Returning bare { status: 'absent' } drops stoppedInstanceIds. That field is
    optional on StopWrappersResult and has no readers anywhere in the codebase
    (produced in stopWrappers, never consumed).
  • Residual risk worth a look: this trades a container fetch for trusting
    ctx.container.running. If that flag were ever stale and reported false while a
    container was up, a legitimate leak check would be skipped. The same flag
    already gates billing adoption in configureBilling.
  • Four tests cover the branch matrix: idle timeout with a stopped container
    (asserts listProcesses is never called), idle timeout with a running
    container, a non idle timeout reason with a stopped container, and a sandbox
    that does not expose isContainerRunning.
  • Effect is checkable after deploy with existing logging: reason=idle-timeout
    should report observation=absent-no-container, and container_stopped with
    reason=activity_expired should fall relative to reason=exit.

St0rmz1 added 2 commits July 30, 2026 11:33
  The idle sweep requests a physical wrapper stop only after confirming via
  DO state that no wrapper runtime or pending work remains. stopWrappers then
  inspected the container to re-derive that, and listProcesses is a container
  fetch, so a sleeping container was cold-booted to look for processes that
  activity expiry had already SIGTERMd. It then idled to the 900s sleepAfter.
  In 90 minutes of production logs this path was 233 inspections, all absent,
  none present.

  Confirm absence from container state instead when the container is stopped.
  A wrapper is a process and cannot outlive its container, so a stopped
  container cannot hide a leaked wrapper. Scoped to idle-timeout: every other
  stop reason still inspects, preserving the leaked wrapper recovery added in
  #3555.
@kilo-code-bot

kilo-code-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The idle-timeout stop path now short-circuits on DO-only container state (ctx.container.running) before any container fetch, and the behavior degrades safely: an unknown or failing probe returns undefined and falls back to inspection, every non-idle stop reason still inspects, and the absent result returned in the skip path is identical to what the previous inspection produced for a stopped container, so lease reconciliation in wrapper-supervisor.ts is unaffected.

Verification notes
  • isSandboxContainerRunning mirrors the existing configureSandboxBillingInput capability-probe pattern (typeof guard plus try/catch), so a sandbox class without the RPC method cannot be misread as "stopped".
  • All sandbox DO classes in src/sandbox-outbound.ts extend MeteredSandbox, so the new RPC method is present on every metered class; MeteredSandboxInstance has no object-literal construction sites, so the added required member does not break other callers.
  • StopInspection narrows correctly: absent-no-container is returned before the initial.status !== 'present' branch, and observedWrapperCount still only reads observed on present.
  • Added tests cover stopped/running/non-idle-reason/unknown-capability paths; default metadata() uses a ses- sandbox id, so discoverSessionWrappers only needs the mocked listProcesses.
  • Not run in this read-only review: pnpm --filter cloud-agent-next test / typecheck.
Files Reviewed (4 files)
  • services/cloud-agent-next/src/agent-sandbox/cloudflare/cloudflare-agent-sandbox.ts
  • services/cloud-agent-next/src/agent-sandbox/cloudflare/cloudflare-agent-sandbox.test.ts
  • services/cloud-agent-next/src/container-usage-context.ts
  • services/cloud-agent-next/src/container-usage.ts

Reviewed by claude-opus-5 · Input: 64 · Output: 15.6K · Cached: 2.3M

Review guidance: REVIEW.md from base branch main

@St0rmz1
St0rmz1 merged commit 215c488 into main Jul 30, 2026
16 checks passed
@St0rmz1
St0rmz1 deleted the fix/skip-container-wake-on-idle-stop branch July 30, 2026 19:47
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