feat(runtime-cf): pin a container's transport per repo, so rpc can be proved on one - #140
Conversation
…be proved on one The R2 dependency cache has missed on every run since it was added, and the reason is a transport: `installCached`'s restore hands `writeFile` a `ReadableStream`, the SDK routes any stream to `writeFileStream`, and that method is a bare `throw` off the `rpc` client. The SDK's own comment is the tell — `rpc` is the "primary container-control client", `http` and `websocket` are the "route-based compatibility client", and the streaming file APIs only exist on the primary one. `SANDBOX_TRANSPORT=rpc` as a Worker var would fix it in one line and change the control path for every repo this dispatcher serves, at once. `sandbox.transport:<repo>` in CONFIG_KV is the same choice scoped to one consumer, threaded through the runtime to `makeSandboxCloudflareLive` and applied per CONTAINER at acquire — a keyed container is a different DO, so pinning only the first would leave the stage containers on the compatibility client. Best-effort by construction: a `setTransport` that fails leaves the container on the default, which works and merely cannot stream a file. Failing acquisition over a transport preference would trade a slow cache for a dead run. Worth knowing before setting it: the pin PERSISTS. `setTransport` writes to the container's DO storage and the SDK prefers a stored transport over the env-derived default on cold start, so a container keeps what it was given for its lifetime. Ids are per execution, so the pin is re-applied per run at the cost of one DO call.
![flaredispatch-fractalboxdev[bot]](https://avatars.githubusercontent.com/in/4273665?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI code review — ✅ Approve
Risk tier: lite · 0 critical · 0 warnings · 3 suggestions
Reviewers: security 0 · code-quality 0 · performance 2 · documentation 1
1. 💡 Suggestion — Per-run KV lookup adds avoidable latency
📍 apps/dispatcher/src/workflow.ts:453-455
Every workflow execution performs a CONFIG_KV read before constructing the runtime, even when no transport override is configured. This adds a network round trip to all repositories' runs; consider caching the per-repository setting or loading it only when the feature is enabled.
2. 💡 Suggestion — Transport pin adds a DO call for every container acquisition
📍 packages/runtime-cf/src/sandbox-cf.ts:541-550
Each acquire invokes setTransport, including repeated acquisitions of the same container and executions whose container is already pinned. Because this is a persisted setting, avoid redundant calls where possible or document/measure the extra DO round trip on high-acquisition workflows.
3. 💡 Suggestion — Invalid values are filtered before reaching the SDK
The documentation says values other than the three transports are ignored with an SDK warning, but 'workflow.ts' validates 'sandboxTransport' and omits invalid values before constructing the runtime. As a result, invalid KV values are silently ignored by the dispatcher and do not produce the SDK warning described here; update the documentation to reflect the actual behavior.
…for it where it cannot apply All three from pr-review, and the middle one had teeth. `acquire` is not only how a run gets a container — it is how a caller derives an id without provisioning anything. `ensureWorkspace` re-acquires on a rebuild, and `offload-test`'s stage reaper acquires purely to name the container it is about to destroy. Every one of those was waking a Durable Object to re-assert a transport it already had. A `Set` of pinned ids in the Layer closure ends it; the new test pins the property with four acquires over two containers. The CONFIG_KV read now happens only on the container path — a run on the substrate should not pay a round trip to be told about a transport it cannot use. And the README claimed invalid values reach the SDK and produce its warning. They do not: `workflow.ts` filters them first, so the run proceeds on the default in silence. Documented as the silence it is, with the advice that follows from it — read the key back if a transport change appears to have done nothing.
1 participant
Toward #132. This does not fix the cache — it makes the fix testable on one consumer instead of all of them at once.
Problem & Insight
The R2 dependency cache has missed on every run since #118, and the reason is a transport.
installCached's restore handswriteFileaReadableStream; the SDK routes any stream towriteFileStream; and that method is a barethrow:composeRestoreOrcatches it into a miss, warns into the dispatcher's own logs, and runs the full install. Deterministic, and unrelated to the archive, the key, or R2 — which is why it read as a size problem for so long.The SDK's own comment on transport selection is the part worth knowing:
So the transport in use is the compatibility path, and the streaming file APIs exist only on the primary one. The SDK's own
restoreBackupbranches on exactly this flag — streaming underrpc, andarrayBuffer()→ base64 otherwise, which for a 114 MB archive is ~266 MB peak against a 128 MB isolate.Take
SANDBOX_TRANSPORT=rpcas a Worker var fixes this in one line and changes the control path for every repo this dispatcher serves, at once.sandbox.transport:<repo>in CONFIG_KV is the same choice scoped to one consumer.makeSandboxCloudflareLive, applied per container at acquire. A keyed container is a different DO, so pinning only the execution's own would leave the stage containers on the compatibility client — exactly where the streaming APIs are absent.setTransportthat fails leaves the container on the default, which works and merely cannot stream a file. Failing acquisition over a transport preference would trade a slow cache for a dead run.setTransportwrites to the container's DO storage, and the SDK prefers a stored transport over the env-derived default on cold start. Ids are per execution, so it is re-applied per run at the cost of one DO call — and a container never silently reverts mid-run.Anything other than
http/websocket/rpcis ignored by the SDK with a warning, so a typo degrades to the default.Key actions
sandbox.transport:<consumer> = rpcon one repo and check its next run's log inventory: a cache hit has nocargo fetch --lockedlog at all, becausecomposeRestoreOrskipsonMissentirely