fix(runner): release a container without needing one (DEV-2556)

[demtario]

The root cause, inverted

The ticket's headline claim is backwards, and correcting it is most of the change. An instance the platform refuses to allocate is not occupying one of the five slots, so a failed DELETE /api/session/:id cannot leak a session — a refused teardown is a symptom of a full pool and never a contributor to one. Nothing leaked in the Sentry events behind this ticket.

The real defect was an asymmetry. await sandbox.destroy() in the DELETE /api/session/:id handler (runner/workers/api/src/index.ts:876 before this change) was the only unguarded destroy in the file, while both siblings — closedWhileCreating at index.ts:635 and the closed-tier teardown at index.ts:494 — already swallowed. So a platform hiccup on the one path whose entire job is release became a thrown 500 that no caller could read: deleteSession() in runner/packages/runtime/src/container.ts:434 is void fetch(…, { keepalive: true }).catch(() => {}), fired from pagehide. The browser discards the response and the e2e helper ignores the status, so the 500 bought a Sentry event and nothing else.

What changed

A declined teardown is now classified rather than blanket-caught. runner/workers/api/src/session-lifecycle.ts holds the two decisions — isExpectedTeardownFailure (three recognised platform messages) and isAtCapacityFailure (only the capacity message) — in a module free of Cloudflare imports and written in erasable syntax, so pipeline/ can import the .ts directly under --experimental-strip-types. index.ts is a Worker entrypoint and cannot be imported that way at all, which is why both decisions are lifted out of it: this module is the only seam either one can be tested through. Anything the predicate does not recognise still rethrows and keeps today's status and today's Sentry event, so a Cloudflare rewording degrades to noisy, never to silent.

The tombstone marker gained a second state. session-tombstone:<id> now carries "1" (a teardown was attempted, outcome unknown) or "destroyed" (we watched destroy() resolve), and only the second lets a later DELETE answer from KV without a sandbox RPC. The "1" literal is kept byte-identical so a rolling deploy and any legacy marker still in KV keep working. The confirmation is written only after destroy() resolves, so the skip can never be reached for a teardown that did not complete.

On the create side, a capacity failure on POST /api/session now answers 503 { error: "at_capacity", message } instead of letting the platform's own words — "…try configuring a higher value for max_instances", an instruction to us — reach a visitor inside a 500. sessionStartMessage in container.ts:214 passes that code through unwrapped like the budget_* refusals, and the sentence is checked against the describeRuntimeError alternation in apps/authoring/src/App.tsx:130 (/failed to fetch|networkerror|load failed|session start failed|fetch/i) so a production visitor is not told to install Docker and run a local worker.

The review finding: the swallow had traded its own signal away

The thing worth being sceptical about on this ticket is whether the change fixes the fault or merely stops the symptom being visible. The honest answer is a split, and the review caught one half of it failing.

The teardown swallow is symptom suppression, and it is the right call — 204 is the correct answer to a caller that discards the response. But suppression is only defensible if the signal moves somewhere durable, and as originally written it did not: the thrown 500 and its Sentry event were replaced by a console.warn. observability.head_sampling_rate is 0.1 in runner/workers/api/wrangler.jsonc:14, so nine of ten of those lines are never retained. At two capacity events in 90 days the expected number of surviving log lines is a fraction of one — the signal the swallow was traded for would effectively not have existed. A declined teardown now also files a warning-level Sentry event under its own tier2-teardown-declined fingerprint (index.ts), alongside the log line rather than instead of it. The fingerprint is load-bearing: the outer catch at index.ts:1699 reports bare and this project groups on the culprit Object.fetch(index), so an unfingerprinted event would land straight back in the DEMOS-1 grab-bag it is meant to be readable apart from. beforeSend (rehomeBudgetAlert) only re-homes context: "budget-alert" and never returns null, so a warning arrives; tagged context: "tier2-teardown" it stays in the main environment rather than being routed to budget-alerts, which is right — a declined teardown is not a spend alert.

This matters most for container service is unreachable, the weakest member of isExpectedTeardownFailure. Unlike the other two it does not imply that no slot is held: a container may well be running and billing and the teardown simply could not reach it. That is bounded by the sleepAfter backstop and is exactly today's outcome (a 500 nobody retries either), so it is not a regression — but it is the case a reviewer should expect to see turning up in the new issue.

The same defect appeared once more, client-side. reportRuntimeError in apps/authoring/src/App.tsx:205 fingerprinted on ["tier2-session-start", status], so the new at_capacity 503 — named in the Worker comment as the surviving create-side capacity evidence — shared a single Sentry issue with an envelope-less platform 503 ("The sandbox service is unavailable right now"), same status and a completely unrelated cause. The server's machine-readable code now joins the fingerprint when it sent one. It is appended rather than substituted, so uncoded failures keep the exact fingerprint they have today and no live issue regroups.

What the skip actually covers

The destroyConfirmed skip is a genuine fix, but narrower than "release a container without needing one" implies, and it is worth saying plainly. The pagehide DELETE is sent once, so the skip is only reachable on the create/delete-race arms — container.ts:434 (a failed create tears down by the local id) and container.ts:444 (a dispose() that raced container creation sends a second DELETE). The two events in the Sentry group were DELETEs for two different session ids, 328 ms apart, so they never touched this path at all. What the skip buys is that the race arms stop asking for a container slot in order to destroy something that is not there.

Capacity numbers

No limit was raised: max_instances: 5 at runner/workers/api/wrangler.jsonc:149 is untouched. Two real capacity events in 90 days, 328 ms apart on 2026-08-07 14:00:24, both DELETEs. No POST /api/session has produced this message in that window — and since this project groups on the culprit Object.fetch(index), a create that hit the cap would have landed in the same group, so on the evidence only the teardown path has surfaced the cap. The pool genuinely was saturated then (three different visitors got session start failed (504) at 13:56, and a preview reported "container is not listening" at 13:59). Two events in 90 days does not yet justify paying for more instances; the honest 503 and the new fingerprinted issue are what will tell us if that changes.

Sentry issue DEMOS-1

Do not close this with Fixes DEMOS-1. That group is a grab-bag, not four capacity events: it also holds a 2026-07-27 "TEMP-VERIFY sentry smoke error" fired from a local wrangler dev run (its location is a .wrangler/tmp path on a developer machine) and an unrelated "container service is unreachable" event. Closing it by keyword would resolve those too. It should be resolved manually after the deploy. The underlying cause — every unexpected throw from the API Worker's fetch handler collapsing into one issue, because the outer catch at index.ts:1699 captures bare — is worth its own change; sentry-gate.ts already owns the beforeSend seam a fingerprint would go through.

ClickUp

https://app.clickup.com/t/86cb68q78 (DEV-2556).

Verified

pnpm build then pnpm typecheck — clean across all four projects (packages/runtime, workers/api, packages/editor-shell, apps/authoring). pnpm test — 584/584, # fail 0 in the raw TAP. pnpm --filter @handsontable/demo-authoring build, then playwright against a preview on my own --strictPort port 4291 via E2E_BASE_URL (the config hardcodes 4173 with reuseExistingServer, which silently tests whatever another worktree left running): the bind was confirmed with lsof, the served bundle hash was matched against the build output, and the run was 166 expected / 0 unexpected / 0 flaky. Server confirmed dead afterwards; 4173 never bound. All commands run raw, not through an rtk filter.

One thing is deliberately untested and should not be read as covered by the 584: the App.tsx fingerprint itself has no test, because App.tsx is not importable from pipeline/ and building a harness for a two-line Sentry grouping change is not worth it. What is pinned is the fingerprint's only input — session-start-failure.test.mjs asserts err.code === "at_capacity" survives the round trip, and session-lifecycle.test.mjs pins the sentence, the envelope code, and that the sentence trips none of the describeRuntimeError alternation.

Found and deliberately not fixed

DELETE /api/session/<invented-id> runs an unauthenticated, un-budget-gated sandbox.destroy(), and that RPC is itself slot-consuming — the resurrection gate at index.ts:775 covers parts.length >= 4 only, and the session DELETE is length 3 and explicitly outside it. This is the ticket's own headline sentence applied to the one case where it is fully avoidable, so it is tempting. The obvious gate — skip the RPC unless a meter or a tombstone exists — introduces a worse leak than it removes: a newly written meter key is not guaranteed visible in another colo for up to 60 seconds, and a successful read of an unpropagated key returns null rather than an error, so hasSessionMeter's fail-open on KV errors (budget.ts:249) does not cover it. A tab closing within the first minute of its session after an anycast reroute would have its real destroy skipped and leak a slot for the full sleepAfter. That is a new leak from a fix whose whole purpose is not leaking slots, against a DoS surface that has produced zero observed events. It wants its own ticket.

Absolute session lifetime, recorded and not implemented. A visible-but-abandoned tab holds a container indefinitely: the client pings /status every 60s while visibilityState !== "hidden", each ping is the meter's tick and keeps the container awake, sleepAfter is 5m and there is no absolute cap. One open-and-forgotten tab occupies 1 of 5 slots for as long as the machine is awake, and five of them are a permanently full pool with nobody using it. This is the most plausible mechanism behind the 13:56–14:00 saturation, more so than genuine concurrent demand. A cap needs a product decision on what a visitor sees when their still-open tab is reclaimed — the 410 tombstone path already exists and the client handles it, so the machinery is there.

At-capacity retry for anonymous visitors, recorded and not implemented. At ≥80% of budget the guardrail already restricts live sessions to signed-in users (anon_blocked), so a capacity refusal and a budget refusal now land on the same shape (503, envelope, a sentence written for the user) with different codes. Whether an anonymous visitor should be offered a retry, told to sign in, or queued is a product call. What ships is honest and terminal, with the error card's existing "Restart preview" button as the only retry affordance.

Pre-existing and not fixed here: meterSession(env, id, { final: true }) runs before the destroy attempt and deletes the meter key, so a failed teardown closes the awake window on a container that may still be billing. It is bounded by MAX_UNSEEN_AWAKE_SECONDS and overwritten nightly by reconcileBilling with Cloudflare's own figures. Moving it after the destroy would risk skipping the meter entirely on a throw, which is the worse trade. Also unchanged: startSessionMeter stays open on a create that throws, which is true on today's 500 path too and is closed by the client's follow-up DELETE at container.ts:434.

Unverified, stated plainly

I could not prove which platform call raises the capacity message during a teardown. In @cloudflare/sandbox 0.12.3, doDestroy() is DO-storage work plus container.destroy(), whose only container-HTTP path is caught internally. Either ctx.container.destroy() itself needs an instance, or a duplicate destroy re-enters a container path. Confirming it needs a wrangler dev reproduction with the pool held full. The fix is correct either way: teardown must not fail the request, and a confirmed-destroyed session must never touch the sandbox again.

🤖 Generated with Claude Code