Skip to content

fix(cloud-agent-next): recover exhausted wrapper cleanup leases - #5104

Merged
eshurakov merged 1 commit into
mainfrom
eshurakov/hardy-ember
Aug 18, 2026
Merged

fix(cloud-agent-next): recover exhausted wrapper cleanup leases#5104
eshurakov merged 1 commit into
mainfrom
eshurakov/hardy-ember

Conversation

@eshurakov

@eshurakov eshurakov commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Why

After physical wrapper stop attempts hit WRAPPER_STOP_MAX_ATTEMPTS, the cleanup lease stayed exhausted for the rest of the session TTL. Delivery stayed fenced even when the container runtime later reaped the wedged sandbox, so follow-up messages could not start a fresh wrapper. Separately, prepareSession rejected first-class tilde-prefixed model IDs such as ~x-ai/grok-latest.

What was done

  • Treat cleanup exhaustion as fenced but recoverable: re-observe on a slow cadence and release the lease to none only after a confirmed absent observation.
  • Add observation-only observeWrappersWithoutWaking so recovery never issues another stop and never wakes a stopped container just to inspect a process that cannot outlive it.
  • Bound background rechecks with WRAPPER_CLEANUP_EXHAUSTED_RECHECK_WINDOW_MS; explicit pending flushes force one out-of-cadence probe, then retry on a dedicated WRAPPER_CLEANUP_EXHAUSTED budget before failing closed with an authoritative failure code.
  • Allow ~ in modelIdSchema so gateway latest-alias model IDs validate end-to-end.

High-level architecture

sequenceDiagram
  participant DO as CloudAgentSession DO
  participant Sup as WrapperSupervisor
  participant Queue as SessionMessageQueue
  participant Sandbox as AgentSandbox

  Note over DO,Sandbox: Background recovery
  DO->>Sup: runMaintenance / reconcilePhysicalCleanup
  Sup->>Sup: exhausted lease due for recheck
  Sup->>Sandbox: observeWrappersWithoutWaking
  alt wrapper still present / inspection failed
    Sandbox-->>Sup: present or inspection-failed
    Sup->>Sup: schedule nextRecheckAt (within window)
  else wrapper gone
    Sandbox-->>Sup: absent
    Sup->>Sup: release_exhausted → lease none
    Sup->>Queue: requestPendingDrainIfNeeded
  end

  Note over DO,Sandbox: User waiting on flush
  Queue->>DO: recoverExhaustedDeliveryBlock
  DO->>Sup: recheckExhaustedCleanup (force)
  Sup->>Sandbox: observeWrappersWithoutWaking
  alt still exhausted
    Queue->>Queue: WRAPPER_CLEANUP_EXHAUSTED retry budget, then fail closed
  else lease released
    Queue->>Queue: deliver normally
  end
Loading

Architecture decision

Decision: Recover exhausted wrapper-cleanup leases with observation-only rechecks (cadenced in the background, forced on blocked flush), never by spending more stop attempts.

Context: Stop budget and rollback fence already say the supervisor will not stop again. The container runtime can still reap a wedged sandbox later; without a release path, that permanent fence bricks delivery for the session TTL. A user flush cannot wait on the full background cadence, but indefinite queued without a terminal signal is also unacceptable.

Rationale: Confirmed absence is enough proof to free the lease because a wrapper process cannot outlive its container. Keeping recovery observation-only preserves the stop budget and avoids waking stopped containers. The flush path forces one probe, retries on a dedicated budget, then fails closed with WRAPPER_CLEANUP_EXHAUSTED so terminalization stays authoritative (INTERNAL would not be).

Alternatives considered:

  • Issue more stop attempts after exhaustion. Rejected: the attempt budget and rollback fence exist specifically to stop unbounded teardown thrash.
  • Terminalize blocked messages immediately on first exhausted flush. Rejected: recovery often takes minutes after container reap; early terminalization discards work a later probe would deliver.
  • Leave the lease exhausted until session TTL. Status quo; permanently fences delivery even after the sandbox is gone.
  • Map/strip ~ model aliases at the web client. Rejected: tilde-prefixed latest aliases are first-class public model IDs in the AI gateway.

Consequences: Exhausted sessions can resume after a later reap without weakening stop fencing. Background DO alarms re-arm only inside a one-hour recovery window; unrecoverable exhaustion stops automatic rechecks while explicit sends still probe once. Pending messages blocked on exhaustion get a short retry budget, then a clear terminal failure. modelIdSchema accepts a slightly broader character set.

Verification

  • Manually exercised the local happy path for session prepare/send after the change (including tilde-prefixed model ID acceptance where applicable).
  • Did not manually reproduce a live wedged-sandbox exhaustion → container-reap → lease-release path; that recovery path is covered by unit tests.

Visual Changes

N/A

Reviewer Notes

  • Focus on the ownership split: supervisor owns lease release; message queue owns flush retry/fail-closed; sandbox owns wake-free observation.
  • Recovery must remain observation-only — no extra stopWrappers after exhaustion.
  • Failure code must stay WRAPPER_CLEANUP_EXHAUSTED, not INTERNAL, so recordPendingFlushFailure treats it as authoritative.
  • Background recheck window is 60m with 10m cadence; forced flush probes bypass cadence and window.
  • Branch is behind main by several unrelated commits; this PR is a single commit on top of its merge-base.
  • Local untracked root review notes are intentionally excluded from the PR.

@kilo-code-bot

kilo-code-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

Re-reviewed after rebase: the new wrapper-death reconciliation in wrapper-supervisor.ts mirrors the wrapper's own completion signal (numeric time.completed or terminal error), stays metadata-null-safe, preserves wrapper-failure fallbacks, and introduces no memory-leak vectors; the remaining files are unchanged since the prior passing review.

Files Reviewed (14 files)
  • services/cloud-agent-next/AGENTS.md
  • services/cloud-agent-next/src/agent-sandbox/cloudflare/cloudflare-agent-sandbox.ts
  • services/cloud-agent-next/src/agent-sandbox/protocol.ts
  • services/cloud-agent-next/src/persistence/CloudAgentSession.ts
  • services/cloud-agent-next/src/persistence/schemas.test.ts
  • services/cloud-agent-next/src/persistence/schemas.ts
  • services/cloud-agent-next/src/session/pending-messages.ts
  • services/cloud-agent-next/src/session/session-message-queue.test.ts
  • services/cloud-agent-next/src/session/session-message-queue.ts
  • services/cloud-agent-next/src/session/wrapper-runtime-state.test.ts
  • services/cloud-agent-next/src/session/wrapper-runtime-state.ts
  • services/cloud-agent-next/src/session/wrapper-supervisor.test.ts
  • services/cloud-agent-next/src/session/wrapper-supervisor.ts
  • services/cloud-agent-next/src/terminal/access.test.ts
Previous Review Summary (commit 6da8fe0)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 6da8fe0)

Status: No Issues Found | Recommendation: Merge

Files Reviewed (14 files)
  • services/cloud-agent-next/AGENTS.md
  • services/cloud-agent-next/src/agent-sandbox/cloudflare/cloudflare-agent-sandbox.ts
  • services/cloud-agent-next/src/agent-sandbox/protocol.ts
  • services/cloud-agent-next/src/persistence/CloudAgentSession.ts
  • services/cloud-agent-next/src/persistence/schemas.test.ts
  • services/cloud-agent-next/src/persistence/schemas.ts
  • services/cloud-agent-next/src/session/pending-messages.ts
  • services/cloud-agent-next/src/session/session-message-queue.test.ts
  • services/cloud-agent-next/src/session/session-message-queue.ts
  • services/cloud-agent-next/src/session/wrapper-runtime-state.test.ts
  • services/cloud-agent-next/src/session/wrapper-runtime-state.ts
  • services/cloud-agent-next/src/session/wrapper-supervisor.test.ts
  • services/cloud-agent-next/src/session/wrapper-supervisor.ts
  • services/cloud-agent-next/src/terminal/access.test.ts

Reviewed by kimi-k3 · Input: 81.1K · Output: 13.8K · Cached: 2M

Review guidance: REVIEW.md from base branch main

Recheck exhausted cleanups on a slow cadence (and force one probe when a
user is waiting on flush) so a later-reaped sandbox can release the lease
instead of fencing delivery for the rest of the session TTL.
@eshurakov
eshurakov force-pushed the eshurakov/hardy-ember branch from 6da8fe0 to aa5d045 Compare August 6, 2026 18:27
@eshurakov
eshurakov merged commit b48e3b2 into main Aug 18, 2026
17 checks passed
@eshurakov
eshurakov deleted the eshurakov/hardy-ember branch August 18, 2026 09:48
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