Repository navigation
fix(runtime): recover long checks with durable process receipts - #185
Conversation
![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 — 💬 Comment
Risk tier: full · 0 critical · 1 warnings · 1 suggestions
Reviewers: security 0 · performance 1 · code-quality
1. ⚠️ Warning — Finalization performs thousands of sequential R2 requests
📍 packages/runtime-cf/src/check-command-live.ts:191-209
Finalization calls 'bucket.head' once per persisted chunk and then performs another sequential 'bucket.get' for every chunk before publishing the aggregate object. A command producing the allowed 4096 chunks therefore requires up to 8192 serial R2 operations, creating substantial latency and request cost on the workflow's critical path. Batch or parallelize bounded requests (with a concurrency limit), or publish/track a durable aggregate without re-reading every chunk during each finalize retry.
2. 💡 Suggestion — Update stale namespace type in JSDoc
📍 packages/runtime-cf/src/cache-r2.ts:57
The '@param ns' documentation still says 'DurableObjectNamespaceSandbox', but the function now accepts the generic 'DurableObjectNamespaceT'. Update the comment to describe the generic namespace type so generated API docs match the signature.
There was a problem hiding this comment.
AI code review — 🛑 Request changes
Risk tier: full · 1 critical · 0 warnings · 0 suggestions
Reviewers: security
1. 🛑 Critical — Environment secrets can be persisted in command logs
📍 packages/runtime-cf/src/check-command-live.ts:64
Output scrubbing only uses the optional 'opts.redactValues' list. Environment values passed to the process are not automatically added to that list, so a caller that supplies credentials via 'env' but omits 'redactValues' can cause those credentials to be uploaded to R2 through 'writeLog' and included in the returned stdout/stderr tails. Automatically redact all environment values (or require an explicit redaction policy before starting the command).
There was a problem hiding this comment.
AI code review — 🛑 Request changes
Risk tier: full · 1 critical · 0 warnings · 0 suggestions
Reviewers: security
1. 🛑 Critical — Import the RunSandbox type used by proxyToSandbox
📍 apps/dispatcher/src/index.ts:45-47
The new generic call references 'RunSandbox', but this file only imports 'Env'; 'RunSandbox' is not declared in scope and will fail TypeScript compilation. Add 'import type { RunSandbox } from "./sandbox";'.
1 participant
Long SDK-backed checks recover the same command after a lost response. A retry preserves the original deadline and publishes the command's actual exit result with a complete scrubbed log.
The synchronous execution path can lose its response after the command starts. Retrying that callback can launch the command again, and its output reaches durable storage only after execution returns.
The long-check path uses short preparation, launch, observation and finalization checkpoints. A shared storage transaction reserves launch intent before starting a stable SDK process. Command identity binds the command, directory, environment and redaction-policy digests, timeout and container; changed identity refuses before output capture. The typed API and all four runtime boundaries require an explicit string-array redaction policy before process, owner or log access; an empty array declares no secret values. Missing uncertain processes refuse recovery without a second launch.
A native supervisor bounds the combined output spool at 64 MiB and enforces the absolute deadline. Its receipt requires the direct child to close and every owned process-group member to stop. Linux zombies count as stopped; live members, unavailable or malformed probes, and cleanup exceeding its finite wait refuse publication. Escaped sessions remain subject to the existing container lifecycle. Scrubbed chunks reach stable storage keys before offsets advance. Streaming final assembly precedes an immutable, actual exit receipt.
Short commands, checkout, application boot and the substrate backend retain their existing paths. Dependencies, SDK, images, deployment configuration and migrations remain unchanged.
Validation: 2,505 tests pass with one existing skip; typecheck, lint and the dispatcher dry-run bundle pass. Native and workerd tests cover lost launch responses, concurrent reservation, replay identity changes, missing or malformed policies, exit 0/7, redirected and TERM-ignoring descendants, delayed signal delivery, failed or stalled probes, deadlines, output floods, split secrets and lost storage acknowledgments. These tests do not claim a live-container eviction experiment or universal exactly-once execution. A deliberate restart that discards preparation requires a new execution ID for a fresh command.