Repository navigation
fix(cloud-agent-next): correct containers status, terminal and cgroups - #6698
Merged
Merged
Conversation
Fix three defects in the Cloudflare Containers control-plane runtime: - Report the real container instance (containers-standard-3/4) in the sandbox status runtime metadata and overlay it on read, so the web Sandbox popover stops showing the legacy isolated-small type/capacity. - Drop the copied containment rejection in the containers terminal billing validator; containment does not select a containers class. - Delegate the populated containers cgroup root so kilo-workloads can be created, supply CONTROL_WORKLOAD_LIMIT_MB from the instance memory, and release the gated child with the stream API so managed placement works. Verified locally end to end (popover, terminal, live cgroup membership).
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryIncremental re-review of commit Files Reviewed (12 files)
Notes
Previous Review Summaries (2 snapshots, latest commit 7b144a5)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 7b144a5)Status: No Issues Found | Recommendation: Merge Executive SummaryIncremental re-review of commit Files Reviewed (20 files)
Notes
Previous review (commit 9e45b09)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the three Cloudflare Containers control-plane fixes (status runtime-type projection, containers terminal billing validator, and cgroup delegation plus gated-child release); the changed lines are internally consistent and I found no high-confidence defects. Files Reviewed (20 files)
Notes
Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
pandemicsyn
approved these changes
Sep 24, 2026
| // `provider_configuration`, so project its type on read; a stored | ||
| // `isolated-small` from the id classification never reaches the client. | ||
| let projectedRuntime = runtime; | ||
| const configuration = sandboxProviderConfigurationSchema.safeParse(providerConfiguration); |
Contributor
There was a problem hiding this comment.
bot: Status projection misses legacy provider-kind-only containers records
Suggested fix: Resolve the effective provider configuration using the same legacy provider_kind fallback used by readProviderConfiguration before deciding whether to overlay the containers type. If the instance cannot be recovered, avoid exposing the stale ID-derived type as though it were authoritative.
Name Cloudflare, Cloudflare Containers and Vercel sandboxes by one memory ladder (Small <= 6 GiB, Medium 8 GiB, Large 12 GiB), so the status popover and the new-session picker use the same words across providers and the containers type row stops repeating the capacity row. Label only: stored identifiers, the sandboxType enum, runtime metadata and capacity numbers are unchanged. - popover: containers standard-3/4 render Medium/Large; provider reads Cloudflare Containers - shared tenancy reads Large . Shared so it stays distinct from the dedicated Large at the same capacity - picker keeps vCPU/memory numbers by resolving capacity from the billing service rather than the label
This was referenced Sep 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes three defects reported for the new Cloudflare Containers control-plane runtime (provider
cloudflare-containers, Durable ObjectSandboxContainers).isolated-smallbecause the runtime metadata derives the type from the session id, so a 4 vCPU / 12 GiB container rendered asSandbox type: Small,Capacity: 2 vCPU / 6 GiB. The metadata type now carriescontainers-standard-3/containers-standard-4(set where the instance is known) and is overlaid on read from the persisted instance, so the popover shows the real instance under one cross-provider memory ladder (Small≤ 6 GiB,Medium8 GiB,Large12 GiB), and the Provider row now readsCloudflare Containersrather thanCloudflare. No metadata migration, andgetSandboxStatusdoes not write.billing_runtime_mismatch.validateContainersTerminalBillingRuntimehad copied the legacy containment rejection, where containment selects a distinct container class. Containers use oneSandboxContainersclass and enforce containment with the outbound intercept proxy, so the default (containment-enabled) session was rejected before the real checks ran. Only that branch is removed; class, service, sku, logical instance and Durable Object checks remain.kilo-toolshad no cgroup memory cap. The containers cgroup namespace root is populated by PID 1 and the control wrapper, and cgroup v2 refuses to enable controllers on a populated cgroup, sofindUsableParentreturnednot_delegatedand nokilo-workloadsparent was created. The wrapper now evacuates the root intokilo-runtime(only after ordinary self-enable fails, and only when membership is the mount root) and then enables the controllers; the containers adapter suppliesCONTROL_WORKLOAD_LIMIT_MBfrom the instance memory size. End-to-end verification then showed the cap was still inert because managed placement fell back to unmanaged:releaseGateread Bun's private_handle.fd, which is absent on the gateSocket. It now releases the gate withgate.end('start\n')— the standard writable-stream release.Verification
Manual, local (dev stack on port offset 700; the Cloudflare Containers image was rebuilt from this branch's source, and the wrapper bundle inside it was confirmed to match).
standard-4showsCloudflare Containers / Large / 4 vCPU / 12 GiB;standard-3showsCloudflare Containers / Medium / 2 vCPU / 8 GiB; legacycloudflarestill showsCloudflare / Small / 2 vCPU / 6 GiB. The type and capacity rows are now distinct strings for containers (they previously printed the same capacity twice).Kilo · Cloudflare Containers · 2 vCPU / 8 GiB, menu entry2 vCPU / 8 GiB · Medium.standard-4terminal connects and streams a command; the worker log has 0billing_runtime_mismatch.server/cgroup.procsholds thekilo servePID;tools/cgroup.procsholds a Kilo-started tool PID; the scope'scgroup.eventsispopulated 1;tools/memory.maxis10737418240; the wrapper is0::/kilo-runtimeand outsidekilo-workloads;control.workloadisappliedwithworkloadLimitSource: explicitand noworkloadFailure: unavailable.collectWorktreeChangescase that passes in isolation and is unrelated); web Jest 106 pass; service unit tests plus the Workers-runtime terminal-billing integration test pass. No new lint errors.Limitations: production Cloudflare Containers
container.execplacement after delegation is not verified locally; it is a rollout gate — if a post-delegation log-tail or recovery-probe exec targets the controller-enabled namespace root and fails, this must not ship. The containment-enabled terminal branch is covered by unit tests only because local dev runs with containment disabled.toolsis empty at steady state (fake-LLM file tools run in-process and Kilo-started helpers are short-lived), so tool membership was proven by polling.Visual Changes
Before: containers sessions showed
Sandbox type: Small/Capacity: 2 vCPU / 6 GiB, and the terminal showedTerminal access denied: billing_runtime_mismatch.standard-4standard-3cloudflareunchanged (regression)Reviewer Notes
owned-processes.tsreleaseGate, used by both the managed and the legacy placement paths; the new release path is the standard writable-stream API, and the legacy path was re-checked via the full wrapper suite.findUsableParentevacuation only runs after ordinary self-enable fails, only for membership/, and only when the root is populated; a nested membership never evacuates, and a delegated ancestor returns before it. Failure staysnot_delegatedand never fails allocation.sandboxTypevalues are additive, and the web client strict-parses the status snapshot, so an older web would render the whole popover as Unknown until it is deployed.sandboxTypeenum value, Durable Object metadata or capacity number changed, and no migration is needed —containers-standard-3/containers-standard-4remain the persisted and API values. The webhook-trigger allocation picker (TriggerForm) deliberately keeps itsDedicated Standardlabel: it is an allocation-policy control that contrasts withAutomaticand states its capacity in its help text, not the cross-provider size picker.test/integration/sandbox-control.test.ts.