Fix webexec WS: heartbeat past idle timeout, fail loud on lost Python session - #2879
Open
rafel-roboflow wants to merge 14 commits into
Open
Fix webexec WS: heartbeat past idle timeout, fail loud on lost Python session#2879rafel-roboflow wants to merge 14 commits into
rafel-roboflow wants to merge 14 commits into
Conversation
- App-level heartbeat frames replace protocol-level ws pings: the server's
idle timeout is an asyncio.wait_for on receive_bytes, which protocol
pings (answered by the ASGI layer) never reset, so any >10s gap between
custom Python calls guaranteed a server-side close. Heartbeat interval
derives from the idle timeout the server now advertises in a hello
handshake instead of a hardcoded guess.
- Every recv is type-checked: a text frame (the payload of a close on a
dead connection) raises ConnectionError instead of reaching
msgpack.unpackb as str ("bytes-like object is required" crash).
- Execution frames carry a request_id the server echoes and dedups
(per-container LRU), making resend-after-failure safe; against a v1
server the legacy no-resend-after-send rule is preserved.
- Session continuity is explicit: the hello carries a session_id, the
server answers whether this container still knows it, and a reconnect
that lands on a fresh container after successful executions raises a
session-lost error instead of silently continuing with reset Python
state (wrong results for stateful blocks).
- Backward compatible both ways: v2 client detects a v1 server from the
hello reply and falls back; v1 clients ignore the new fields.
- Tests: handshake negotiation and fallback, session-lost, text-frame
typing, v2 resend vs v1 no-resend, response-id guard, heartbeat frames.
rafel-roboflow
requested review from
PawelPeczek-Roboflow,
dkosowski87,
grzegorz-roboflow,
hansent,
probicheaux and
yeldarby
as code owners
August 27, 2026 08:46
Contributor
|
👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely. 🚦 This PR is marked Ready for review, so automated Claude review will run — and every pass spends real credits. Warning 💸 The Claude reviewer bills in credits, not vibesAutomated review spins up a real agent that reads real code and spends real credits on every pass. It is glad to help — but it is not a rubber duck, a linter you poke in a loop, or a substitute for reading the contributing guide. Treat it like an expensive senior reviewer whose time you booked, and show up prepared. Draft when unsure, Ready when you mean it:
However you get there, arrive prepared:
Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.
|
Contributor
|
🤖 Claude review started at commit New commits are not auto-reviewed. Add the |
The websocket endpoint imports msgpack at ASGI construction; on base images that predate msgpack in inference's requirements the wsapp container crash-loops on boot and every client connect times out at the proxy. Install it explicitly next to fastapi so the transport works on any INFERENCE_VERSION image.
- Register a session on the server only after user code succeeds, and rotate the client session id when raising WebexecSessionLostError, so session loss fails loudly exactly once instead of silently continuing with reset runtime state on the next reconnect - Track in-flight requests on the server (request_id -> task) so a resend during execution awaits the original run instead of executing user code twice; hello now returns a container_id and the client refuses to resend when the reconnect lands on a different container (per-container dedup cache cannot answer there) - Stop keepalive after WEBEXEC_WS_IDLE_RELEASE_SECONDS (new env, default 600) and release the connection + session, so idle executors no longer pin billed Modal containers for the full 1h connection cap - Await heartbeat acks under a 10s timeout instead of the 720s read timeout, so a half-open connection cannot hold _io_lock and stall real frames for minutes - Bound the server response cache (128 entries, 64MB, 120s TTL) and prune _ws_sessions; commit handshake results atomically so the keepalive thread never observes mid-handshake protocol state - Extend protocol tests: session rotation, container pinning on resend, heartbeat ack timeout and timeout restore
Follow-up review of the previous commit found the two guarantees still bypassable in several ways. Duplicate execution: - execute_request now never raises: a result the server cannot serialize comes back as a packed, cached error instead of killing the connection with nothing cached, which invited a resend of an already-run request - add an executed-request registry, marked BEFORE user code runs, that outlives the response cache; a resend whose payload was evicted (an oversized response evicts itself, and concurrent ones evict each other) now gets a loud ResponseNoLongerAvailable error instead of re-executing - the same-container resend guard now fails closed on an unidentified container and is evaluated inside _io_lock, immediately before the send, since _connect publishes the socket before the handshake commits the new container id; _drop_ws_connection clears the id it no longer speaks for Silent state loss: - the v1 fallback path now runs the session-lost check too: a legacy server cannot confirm the session survived, so a reconnect after a prior success fails loudly rather than silently rebuilding a fresh namespace - the keepalive recomputes idleness under the lock, so a frame completing between the unlocked read and the acquire no longer releases and rotates a live session Also: hold the handshake to a 60s reply timeout rather than the 720s execution timeout (it runs under _io_lock, so a stalled server starved every request thread for the workspace); refresh a known session on hello so an erroring-but-live stream cannot age out of the registry; document that "fail loudly" is per-executor, not per-run, and that the hourly connection cap makes session loss scheduled for long stateful runs. Tests: server-side dedup/at-most-once suite driven through the real websocket app (resend from cache, resend after eviction, unserializable result, session registration timing), plus client coverage for the v1 session check, fail-closed container guard, and the idle-release race.
Third review round found the previous commit's v1 change to be a net
regression, plus three smaller issues.
Revert the v1-fallback session check. A v1 server cannot confirm a session,
but it also cannot hold a connection open: its idle timeout closes the socket
every WEBEXEC_WS_IDLE_TIMEOUT_SECONDS (10s) and v1's only keepalive is a
protocol ping, which the ASGI layer answers without resetting that app-level
timer. Reconnects are therefore the normal state of a v1 connection, so
failing each one after a prior success failed roughly every other request
(the rotation cleared the latch, so it alternated pass/fail). That is far
worse than the rare silent state reset it prevented, especially since most
blocks are stateless. The guarantee needs protocol v2 to be affordable.
Also:
- publish handshake results as one immutable _ServerInfo instead of three
separate attributes. The keepalive reads them unlocked, and proto=2 seen
with a not-yet-written idle timeout falls back to the 25s v1 interval,
which a v2 server closes under. Rebinding one tuple makes the torn read
unrepresentable rather than merely ordered-by-convention.
- on a resend whose reconnect lost the session, report the ambiguous outcome
("may have already executed") instead of letting the session error win and
instruct a checkpoint replay that would re-run side effects.
- decode request inputs outside the "the block ran" error path: a malformed
payload was reported as a user-code failure of a block that never ran, and
must stay retryable since nothing executed.
- correct the connection-cap comment: Modal's 700s per-input timeout may
bound a websocket before the 3600s cap, so the previous "replay per hour"
claim was unverified.
Tests: v1 reconnect after success stays transparent; handshake state traced
line-by-line for torn reads; resend-after-session-loss message; undecodable
inputs reported honestly and left unmarked. All four fixes verified by
mutation (each reverts to a failing test).
Contributor
|
🤖 Claude review started at commit New commits are not auto-reviewed. Add the |
Contributor
|
🤖 Claude review started at commit New commits are not auto-reviewed. Add the |
Contributor
|
⏳ This PR is on hold pending your answers — the review will not advance to sign-off until the IMPORTANT question below is answered. Unanswered questions may keep this out of a release. IMPORTANT — does the loud session-lost failure intentionally apply to stateless custom‑Python blocks too?The v2 session-lost check in Two facts make the blast radius larger than the "stateful video run" framing in the description:
Why this is the question that decides the outcome: if failing every stateless custom‑Python job once per container recycle is an accepted trade-off (fail-safe over the rare stateful correctness bug), this ships as-is; if not, the check needs to be narrowed (e.g. gated on evidence the block actually mutates Re-review is not automatic: add the Reviewed at HEAD: b0c0172 |
Contributor
📓 Release coordination — Execution EngineThis PR changes run-time behavior of remotely-executed Custom Python Blocks (new
This notice does not itself block the PR. Reviewed at HEAD: b0c0172 |
Contributor
Review summarySkills: review-workflows-execution-engine, review-topic-concurrency-and-resource-safety, review-topic-local-vs-remote-execution, review-topic-test-hygiene, review-core-infra. No dedicated surface skill covers This is a carefully-engineered protocol change and the test coverage is genuinely strong (server-side dedup driven through the real FastAPI websocket app, torn-read tracing of the handshake, idle-release race, resend/at-most-once, v1 fallback). I traced the concurrency-sensitive paths — the atomic Not a sign-off. The one IMPORTANT question above (does the loud session-lost failure intentionally apply to stateless blocks, given Minor doubts (non-blocking): the connection-cap semantics (700s per-input vs 3600s cap) are self-described as unverified in Commands that informed this review: Reviewed at HEAD: b0c0172 |
- Add WEBEXEC_WS_FAIL_ON_SESSION_LOSS (default True) so the loud session-lost failure can be disabled in seconds instead of requiring an inference-ref rollback across every consumer; forwarded to the webrtc Modal worker along with the other WS timing knobs. - Drop WEBEXEC_WS_MAX_CONNECTION_SECONDS from 3600 to 600 so the server always closes a connection cleanly before Modal's 700s per-input timeout kills it mid-execution, and correct the contradictory comments in modal_app.py and env.py. - Server: catch BaseException in execute_request so SystemExit from user code (and CancelledError) returns a cached error frame instead of tearing down the connection with a permanently poisoned request_id; roll back the executed marker when the worker thread never entered the user code; handle non-dict frames, bogus chunk headers and any escaping loop error as responses rather than opaque disconnects. - Server: make _TtlKeySet agree on what a key is (a non-str request_id was stored but never found, voiding the at-most-once backstop), validate request_id/session_id at the wire boundary, refuse to cache an oversized payload rather than draining the whole response cache, and stop the cache TTL sliding on every hit. - Client: report server-infrastructure failures as DynamicBlockError rather than DynamicBlockCodeError, latch _had_success only on protocol v2, keep the "may have already executed" warning on retry exhaustion, take _io_lock in _drop_ws_connection, stop and join the keepalive thread in close() before freeing the fd, and treat WEBEXEC_WS_IDLE_RELEASE_SECONDS <= 0 as disabled to match its sibling knob. - Client: require the echoed request_id on v2 so a late heartbeat_ack cannot masquerade as a response, clamp the advertised idle timeout, validate chunk counts, and turn malformed responses into transport errors. - Tests: new test_modal_ws_contract.py pins every constant and wire key that crosses the client/server boundary by driving the real client against the real server route; shared loader moved to conftest.py. Adds coverage for in-flight concurrent dedup, SystemExit, malformed frames, boundary types, container identity and retry exhaustion. 104 websocket tests, 188 in the dynamic-blocks suite.
The server closes every connection at the top of its receive loop - on the
idle timeout and on WEBEXEC_WS_MAX_CONNECTION_SECONDS - and never reads
again after deciding to. A frame the client wrote concurrently was therefore
provably unprocessed, but the client could not tell that from mid-execution
death and reported it as "the block may have already executed". At the 600s
cap that failed real work on a fixed ~10 minute cycle.
- Server: send {"_kind": "closing"} immediately before close(1000) on both
paths, gated on the connection having sent a hello so a v1 client never
receives an unexpected binary frame. Client: raise _ServerClosingError,
reset the delivery state and retry on any container.
- Server: run input decode, result serialization and packb inside
asyncio.to_thread. All three were on the event loop shared by up to
max_inputs=10 connections, where seconds of GIL-held CPU (cv2.imdecode,
ndarray.tolist) stall the other connections past their idle deadline -
starving the very heartbeats this protocol adds. _InputsDecodeError keeps
a malformed payload reported as never-run and safely resendable.
- Server: bound reassembly by BYTES, not just chunk count; refuse to
announce more than WEBEXEC_WS_MAX_CHUNKS; stamp server_error on the
server's own UnknownCodeHash/InvalidRequest control responses; echo the
in-flight request_id on error frames so the v2 response-id guard does not
discard the diagnostic.
- Client: install the heartbeat deadline BEFORE the write - settimeout
bounds sends too, so a blocked write held _io_lock for the 720s execution
timeout and stalled every request for the workspace.
- Client: decide infrastructure-vs-user failures by the server_error flag on
v2 instead of the error_type name, which is an exception class name
untrusted user code controls; a block raising UnknownCodeHash could
trigger the resend-with-full-code path and execute twice. Names remain the
v1-only fallback.
- Client: clamp the advertised idle timeout downward only (clamping up made
the client believe it had headroom the server does not give, reinstating
the idle close) and move the floor onto the derived interval; give chunk
continuation reads their own deadline; refuse an over-limit payload before
it reaches the wire.
- env: parse WEBEXEC_WS_FAIL_ON_SESSION_LOSS with str2bool like the other
117 booleans, so a typo fails at boot instead of silently disabling the
safety net. Declare websocket-client, previously only present transitively
via the docker SDK despite being required by the websocket transport.
- Tests: keepalive backoff while a request genuinely holds _io_lock,
graceful close in the execution and heartbeat slots, end-to-end
graceful-close retry across two containers, chunk count at exactly the
limit and one over, and the full-code resend driven for real instead of
grepping the client source for a literal. TTL expiry runs on an injected
clock rather than a wall-clock busy wait. 254 passed.
Known limitation, unchanged: the session-lost guard is armed by
_had_success, which lives on a per-workspace executor, so it can still fire
for a stateless block after an unrelated run succeeded. Confining it to the
run that lost state needs a per-stream session identity and per-stream
connection stickiness. WEBEXEC_WS_FAIL_ON_SESSION_LOSS=False disables it.
Conflict: tests/.../dynamic_blocs/conftest.py was added independently on both sides — two equivalent harnesses for importing the real modal_app.py with a stubbed modal package. Resolved by keeping one implementation (this branch's, which parameterises the module name so each test gets a FRESH module — the server keeps container-local state in module/class scope, so sharing one leaks namespaces, session and dedup registries between tests) and re-exporting main's public names as aliases: FakeModalApp, FakeModalImage, identity_decorator and the modal_app_with_fake_modal fixture. Neither side's tests needed rewriting. main's #2870 (Batch/list-shaped data across the Modal boundary) touches the HTTP serialization path and auto-merged cleanly with the websocket work here; both changes verified present. Full dynamic-blocks suite: 264 passed. Whole tests/workflows/unit_tests: 6139 passed, with the same 17 failures that already fail on a clean tree (semantic segmentation, RLE, detections stitch).
The graceful-close handling added in b405bb2 was itself unsafe. A closing frame proves only that THIS attempt's write went unread; it says nothing about an earlier attempt that was delivered. Clearing the cross-attempt delivery state disarmed the same-container guard: attempt 1: frame -> C1, C1 executes, response lost -> resend_pending, C1 attempt 2: reconnect C1, resend, server closes before reading -> closing -> handler wiped resend_pending attempt 3: guard disarmed, lands on C2, C2 has no dedup record -> runs again Keep the ambiguity once incurred: only an attempt that never followed a delivery may retry on any container. A second entry point existed via _handshake, where _ServerClosingError escapes _ensure_connection and the container check never ran at all. The regression test drives the resend_pending=True path and was confirmed to fail before this change. Also closed, all in the same at-most-once/resource family: - HTTP transport (the DEFAULT) still selected its resend-with-full-code path by matching the error_type NAME, so a block raising `class UnknownCodeHash(Exception)` re-executed - and HTTP has no request_id or dedup to catch it. Both transports now share one helper: the server_error flag decides whenever present, names only for servers predating it. The name list moved to module scope so the two cannot drift. - Server cancellation could cache "the block was not run" while the pool thread went on to run it: asyncio.to_thread cancellation cancels the awaiting future, not the work item. Only a positive decode_failed signal from the worker now clears the executed marker; an unknown outcome keeps it so a resend gets a loud error instead of a second run. - Request reassembly was capped at MAX_CHUNKS * MAX_FRAME_BYTES (1 GiB) and unpacked on the shared event loop with msgpack's size-derived limits, so one frame could expand to tens of GB of GIL-held work and push all ten sibling connections past their idle deadline. Capped at 64 MiB, explicit collection limits, decode moved into a worker thread. Mirrored client-side, where reassembly runs in the shared inference server process. - request_id / session_id had no length bound, so the 32768-entry executed registry could hold gigabytes. Bounded at the wire boundary with an explicit error rather than in _TtlKeySet alone: a silently rejected key would execute with no dedup record, which is worse than refusing it. - conn_last_request_id is cleared per iteration so a failure before the id is parsed cannot echo the previous request's id. Tests: 266 passed. Two chunk-boundary tests were allocating 1 GiB each to exercise a count comparison; they now monkeypatch the limits down. sys.settrace restores the prior tracer instead of None (it was silently disabling coverage for everything after it), and the dedup poll loop is bounded and daemonised so a regression fails fast instead of hanging CI. .gitignore: c.key and modal/deploy_modal_*.sh were untracked but NOT ignored, so a single `git add -A` would have committed live credentials. Never in history; ignoring them does not remove the local files.
…TP hole BLOCKER — the session-loss check was armed for every block and its trigger was guaranteed on a schedule. `_had_success` latches on any v2 success, this branch lowered the server cap 3600->600, and a forced close whose `_ServerClosingError` was swallowed by the keepalive's generic handler left the latch armed. So a long-running pipeline using a STATELESS block got a hard DynamicBlockError roughly every 10 minutes on work that previously succeeded. Two fixes: - Rotate the session on a graceful close, in both the keepalive and the retry path. A close the server scheduled is not evidence state was lost unexpectedly, and the at-most-once guard is unaffected (it keys on sent_to_container, not the session id). - Default WEBEXEC_WS_FAIL_ON_SESSION_LOSS to False. The check cannot tell a stateful block from a stateless one, and container-local state cannot survive a long run by construction: a connection is one Modal input capped at 700s, namespaces are keyed by code hash, and reconnects have no affinity. It is an opt-in diagnostic, not a guarantee the platform can keep. BLOCKER — the HTTP transport (the DEFAULT) still had the forged-UnknownCodeHash double execution. `_is_server_originated_response` fell back to matching error_type NAMES whenever the server_error key was absent — but a stamping server omits that key on user-code failures, so the fallback governed exactly the responses it must not. A block raising `class UnknownCodeHash(Exception)` triggered the resend-with-full-code and ran twice, with no request_id or dedup to catch it. The flag is now the only discriminator, on both transports; the name list is deleted. Servers predating the flag are out of scope by this PR's own server-first deploy contract. BLOCKER — close() could free the fd under the keepalive's blocked recv(), the exact hazard its ordering exists to prevent. The ack deadline is installed on the socket so it bounds send AND recv; the join covered only one of them. Join now bounds 2 x ack + slack, and when the lock is still held close() drops only its own reference instead of closing a socket another thread owns. Also: - _ws_executed / _ws_sessions eviction was TTL-blind: the size cap popped the oldest entry regardless of age, so under load markers were evicted inside the client's retry window and a same-container resend re-ran user code. Entries younger than a retention window are now never evicted for size, and the cap being exceeded is reported rather than silently papering over it. _TtlKeySet. refresh() now prunes first, so an expired entry is not revived. - Byte caps are mirrored in both directions. Previously each side bounded only the chunk COUNT (1 GiB) while the peer bounded bytes (64 MiB), so a payload in between surfaced as "may have already executed" or as a frame desync - verbatim the failure modes those guards exist to prevent. - _ws_inflight cleanup is identity-checked; popping by key alone let a stale callback delete a newer task under the same id. - _check_response_id now surfaces server-stamped unaddressed error frames instead of reporting them as a desync, so chunk-abort and decode-limit diagnostics reach the user. - Provable non-executions (UnknownCodeHash, InvalidRequest) discard the executed marker. A namespace-init failure does NOT: exec()ing the user's module scope IS running their code. - A non-str request_id is refused rather than downgraded to "no dedup", which would have executed with no at-most-once record. - Chunk receive re-derives the connection budget each chunk; b"".join moved inside the worker thread; WEBEXEC_WS_MAX_REQUEST_BYTES propagated to the container env; response-cache TTL raised to cover the client's actual retry window; _container_id uniqueness no longer depends on MODAL_TASK_ID being set. - Heartbeat floor is capped by idle/3, so the "worst gap is 2 x interval" invariant holds at every advertised timeout the clamp accepts, including 1s. - Continuation chunks read the socket the deadline was installed on. - WEBEXEC_WS_IDLE_RELEASE_SECONDS 600 -> 120: at 600 it could never fire, since connection age is >= client idle time by construction. - Pool size > 1 with the session guard on now warns; websocket-client pinned. Two corrected docstrings that were actively wrong: _rotate_session claimed a per-run identity was available (workflow_execution_id is minted per FRAME, so using it would report a lost session on every frame) and omitted that a positive session_known says nothing about state integrity, since namespaces are keyed by code hash and share one globals dict. decode_failed is no longer described as the only proof of non-execution. Tests: 274 passed. New contract tests drive the REAL server's own close triggers, so the `closing` non-delivery proof the client's retry depends on is verified rather than assumed, and a genuinely v1-shaped frame (no _kind, no request_id) is exercised for the backward-compat claim. Retry-exhaustion now pins the attempt count, the heartbeat invariant test covers the value that failed, and the dedup test no longer leaks an ASGI portal thread.
… into fix/webexec-ws-session-protocol
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
2 participants
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.
Description
Fixes the custom-Python WebSocket failures behind recent video-processing incidents (Linear: DG-737).
Root cause: the webexec server closes the socket after 10s idle via an app-level
asyncio.wait_for(receive_bytes), while the client keepalive sent protocol-level ws pings — which the ASGI layer answers below the application, so they never reset that timer. Any workflow step with >10s between Python calls guaranteed a dead socket; the client then reused it and fed the text close-frame to msgpack (a bytes-like object is required, not 'str').Protocol v2, backward compatible in both directions:
session_id; server replies with its idle timeout, its container id, and whether this container still knows the session.idle_timeout/3— real messages the handler receives, so they actually reset the server's timer. The heartbeat interval is capped byidle_timeout/3so the worst-case gap (the loop skips a tick whenidle < interval, making it2 x interval) stays under the server's deadline for every advertised value.TypeError.request_id+ per-container dedup: a resend after a lost response is answered from cache rather than re-executing. The client only ever resends to the container it originally reached, so the registries only see same-container resends. Against a v1 server the legacy no-resend-after-send rule is preserved.{"_kind": "closing"}immediately beforeclose(1000)on its idle and connection-cap paths. Both are decided at the top of the receive loop, before any read, so the frame is proof the client's in-flight write was never processed — it can be retried on any container instead of being reported as "may have already executed". Aclosingalso rotates the client's session, so the scheduled cap-close is a clean restart rather than a session-loss error.Server hardening in the same change: msgpack collection limits and a byte ceiling on reassembly, chunk count/byte bounds enforced on both send and receive on both sides, decode/encode moved off the shared event loop, a
server_errorflag so error classification never depends on a user-chosen exception name,SystemExit/CancelledErrorhandling, and TTL-respecting eviction in the dedup registries.Type of change
How has this change been tested?
tests/workflows/unit_tests/execution_engine/dynamic_blocs/— 274 passed, 0 skipped. Of those, 131 passed across the three new websocket files (103 test functions):test_modal_ws_protocol.py— handshake negotiation and v1 fallback, session-lost enforcement, text-frame typing, v2 resend vs v1 no-resend, the response-id guard, heartbeat timing invariants, graceful-close retry, keepalive backoff while a request genuinely holds the io lock, and that aclosingarriving after an earlier delivery does not disarm the same-container guard.test_modal_ws_server_dedup.py— in-flight dedup with two real threads racing onerequest_id, TTL/LRU eviction on an injected clock,SystemExitfrom user code, malformed frames, wire-boundary id validation.test_modal_ws_contract.py— drives the real client against the real server route to pin every constant and wire key crossing the boundary, including the chunk-count boundary in both directions, the full-code resend actually firing, the server'sclosingframe emitted from its own idle and cap triggers (and not emitted to a v1 connection), and a genuinely v1-shaped frame (no_kind, norequest_id) still executing.New/changed configuration
WEBEXEC_WS_FAIL_ON_SESSION_LOSSFalsestr2bool, so a bad value fails at boot.WEBEXEC_WS_IDLE_RELEASE_SECONDS120<= 0disables.WEBEXEC_WS_MAX_CONNECTION_SECONDS3600→600WEBEXEC_WS_MAX_REQUEST_BYTES64 MiBwebsocket-clientis now declared (pinned) inrequirements/_requirements.txt; it was previously only present transitively via the docker SDK, despite being a hard requirement ofWEBEXEC_TRANSPORT=websocket..gitignoregained patterns for local credential files that were untracked but not ignored.Any specific deployment considerations
WEBEXEC_TRANSPORTdefaults tohttp, so deployments that have not opted into the websocket transport are unaffected.Deploy order:
modal deploythe webexec server app first (staging, then prod). Both directions are safe, so neither window is dangerous:hello, so a v1 client never receives one.workflow_dispatchwhile the client ships with any inference ref bump): the handshake gets a non-helloreply, discards it, and falls back to v1 semantics including the no-resend-after-send rule.Consumers (preview worker, webrtc image, async-serverless) pick up the client on their next inference ref bump.
Known limitations (not addressed here)
_had_successon a per-workspace executor, so it cannot be confined to the run that actually lost state. Doing that needs a per-stream session identity and connection stickiness per stream. It is off by default for that reason.session_knownproves the container still holds a namespace for this code hash — not that a run's state is intact. Namespaces are keyed by code hash and share one globals dict, so two concurrent runs of the same block on one container already read and overwrite each other's state. That predates this change and is unaffected by it.np.array(value, dtype=...)on both transports takes a client-supplied dtype string, so a small frame can request an arbitrarily large allocation. Pre-existing on both paths and not addressed here; it bypasses the byte caps above and is worth a follow-up.