Repository navigation
sdk/python: bound a claim by the caller's deadline - #212
Merged
Merged
Conversation
Every control-plane request took a fresh client timeout, redirect targets included, so the LangChain toolkit's five-minute promise bought 300s per black-holed candidate: two of them cost 600s before the tool's own budget check ran. Claims now accept an absolute time.monotonic() deadline that bounds each request by what is left and refuses once it is spent, and the toolkit passes the deadline it already computes for the run. Measured against a node that redirects to two addresses which accept TCP and never answer, client timeout 2s: 4.01s and an APIError before, 0.30s and a TimeoutError with a 0.3s deadline.
Keep early transport failures as APIError for redirect fallback, but surface TimeoutError once an absolute claim deadline is exhausted so toolkit calls retain their bounded cutoff result. Keep the deadline tests ahead of their helper.
recording_claim already records every claim body a fake node sees; the deadline test rebuilt it inline.
CMGS
added a commit
that referenced
this pull request
Sep 20, 2026
* fix(store/s3): report a meta without an export as not found Delete keeps meta.json until the exports are gone, so a failed export cleanup leaves a record with a meta and no export. The dir backend answers store.ErrNotFound for that record and resolveGolden cold-boots; s3 answered a plain error and every claim of the template key failed until someone retried the delete. All four Fetch callers already map ErrNotFound to their unknown-record outcome. * fix(mcp): cap the logs tool's output like exec and read_file toolLogs collected the guest's replay in a bare strings.Builder. silkd bounds a log ring at 256 KiB, so only a replaced agent streams more, but that agent runs in the guest and the cap on exec and read_file exists for the same reason: guest-fed output must not grow this process. * fix(server): audit the buffered exec frame without its delimiter handleExec appended the newline before it handed the frame to Audit, so an exec frame of exactly AuditLineCap bytes was journaled as oversized while the relay tap, which strips the delimiter, records the same frame in full. * fix(pool): log a failed template listing at startup NewManager dropped the Metas error: the template set stayed empty for the life of the process and the node gossiped no promoted template, with nothing in the log. Warn and carry on, as the startup archive listing in reconcile already does. * fix(sdk/go): keep the relay connection when a stream open is refused DialPort, Watch and OpenPty closed the connection when the guest answered the open with an error frame; every other verb parks it through lease.done. silkd returns to its serve loop after each of those error frames, so a DialPort poll on a port that is not listening yet paid a dial, an upgrade and a TLS handshake per attempt. * fix(sdk/python): report a guest frame without its field as a ProtocolError The stdio pump and the data drain indexed frame["data"] and frame["code"] directly, so a guest that omits the field raised a bare KeyError outside the SandboxError hierarchy callers catch. Every sibling reader already goes through _need. * fix(sdk/python): retire proxy_port's accept thread when the listener closes close() from another thread does not wake a blocked accept(2) on Linux, so every proxy_port call left its accept thread parked for the life of the process; macOS does wake it, which kept the suite green. The loop now accepts under a poll timeout and leaves on the closed socket's OSError. Measured in python:3.11-slim and python:3.9-slim: five closed listeners left 5 threads before, 0 after. * fix(sdk/openai): keep one sandbox handle per session _sandbox() built a fresh Client and Sandbox for every operation, so the SDK's keep-alive pool never served a second call: each operation paid a relay upgrade and an info probe, then parked its connection for the 30 s keep-alive window under its own timer, holding the sandbox's idle clock. The session state is set once at construction; delete releases through the same handle, which drains its pool. * fix(sdk/langchain): bound every tool's first claim by the call budget The claim deadline from #212 reached sandbox_exec only. write_file, read_file and list_dir claimed with no deadline, so a run whose first tool call is a file write still walked a black-holed redirect at one client timeout per hop. The toolkit's sandbox() now defaults the claim deadline to CALL_TIMEOUT from the call. * fix(sdk/langchain): return SDK failures to the model as tool errors handle_tool_error=True makes LangChain catch ToolException only, and the toolkit raised one for a missing read_file path alone. Every other failure — write_file into a missing directory, an expired claim, a guest error — escaped tool.invoke() and ended the agent run. _tool now maps SandboxError to ToolException in one place, which also replaces read_file's special case. docs/langchain.md states this and the claim budget every tool's first call runs under. * fix(ci): lock the released silkd build and run the Go gate on its own config release.yml built the amd64 silkd asset without --locked while the arm64 leg and every image build use it, so one release could ship two dependency sets. sandboxd.yml did not trigger on Makefile or .golangci.yml, the two files that define its lint and test commands. The golangci-lint install ran under the host's go: on a host older than go.mod's directive the fresh linter refuses the config, so the install now pins GOTOOLCHAIN to that directive. * fix(images): track the released silkd in the android default, assert the pty symbol The android flavor's SILKD_IMAGE default stayed at 0.1.0 through twelve releases; CI overrides it, a local build does not. The kernel's post-olddefconfig assertion list did not cover UNIX98_PTYS, which silkd's pty RPC needs and which sits under EXPERT with VT and LEGACY_PTYS off. * fix(e2e): fail loudly instead of passing on an unmet premise The egress, intercept and socks scripts waited for a warm pool and fell through on timeout, so the smoke then passed at the cold tier; a kit without jq takes exactly that path. They now exit the way sandboxd-e2e.sh does. meshsmoke printed PASS with both owner assertions skipped when -peer was empty; it now requires the flag. * review: godoc on the func it names, annotated constructor, tests ahead of helpers runBounded's godoc sat on destroyAll (/code comment budget: a comment describes the declaration it sits on). Template.__init__ lacked its return annotation (/code-py item 4). The langchain suite's last test followed its helpers. * docs: the sandbox index fields and the intercepting proxy's idle ceiling docs/sdk.md and docs/sdk-python.md listed four of the eight SandboxSummary fields as if complete; both now point at GET /v1/sandboxes, which owns the list. docs/egress.md gave 64 idle upstream connections per sandbox; an intercepting pool's proxy holds a second transport of the same size. * fix(sdk/python): keep the relay connection when a stream open is refused The Python twin of the Go SDK fix: _open_stream closed the connection on a guest error frame while _lease parks it, so a dial_port poll on a port that is not listening yet re-dialed and re-upgraded on every attempt. * fix(pool): let a warm 0 pool under warm_max grow, and return to empty The target is ceil(rate x lead x safety). The lead is measured only by a refill and a refill needs a target above zero, so a pool configured warm 0 with a warm_max stayed at zero under any load: every claim took the provision path. With no measured lead, demand now asks for one VM, whose refill measures it. The ceiling of any positive rate is 1 and the rate only decays exponentially, so an empty-floor pool would then hold one idle VM until the float underflowed, about twelve hours; under one arrival per decay period now counts as silence. Both branches are the identity for a floor of one or more. * fix(pool): close an archived claim's usage interval when retention purges it The purge arm of the reap journaled archive_delete and nothing else, so the claim never got the release or reap that docs/sandboxd-api.md folds compute seconds over, and reaps_total missed it. Release of an archived claim already emits archive_delete then release; the purge now emits archive_delete then reap. * fix(pool): journal a hibernate that reconcile adopts A daemon killed between the journaled hibernate intent and the usage event restarts, adopts the claim as hibernated, and later wakes it: the journal then held a wake with no hibernate and the whole hibernated span folded into billable compute. settlePendingSnap already journals the adoption it makes in-process; reconcile now does the same, after it drops the manager mutex. * fix(store/s3): do not tie a shared download to the caller that started it Fetch ran populate inside singleflight.Do under the first caller's request context. When that client hung up the download was cancelled and discarded, and every concurrent brancher of the same checkpoint got its context canceled back as a 500. A download that outlasts the client timeout never completed at all: each retry restarted from zero and died at the same point. The flight now runs detached under its own 30 minute budget, the shape healCheckpoint already uses. * fix(pool): run the store generation sweep off the housekeeping loop sweepStoreGenerations was the one long task Run still called inline in its select; the TTL sweep, the archive-delete retries and every engine fan-out already run in their own goroutines. On a shared FUSE checkpoint root the hourly walk held refill, reap and the claim path's refill kick for its whole duration. * fix(packaging): stop sandboxd without signalling its service cgroup docs/deploy.md promises that stopping sandboxd leaves VMs alive, but the unit left KillMode at control-group, so the promise held only for VMMs that cocoon had moved into a scope of their own. A host survey found one VMM, up for days, still inside the sandboxd service cgroup, where a systemctl stop would have killed it. KillMode=process signals the daemon alone, as libvirtd, containerd and dockerd do. * fix(sdk/langchain, sdk/openai): default the claim to a one hour lease Both adapters sent no ttl_seconds, so the node applied its 5 minute default and nothing renews a lease: a run that thought for five minutes between two tool calls lost its sandbox, and every later call failed for the rest of the run. The MCP server already defaults to an hour for this reason (mcp/tools.go:113). An explicit ttl_seconds is unchanged. * docs: state what the code does where the pages said otherwise silkd.md, sdk.md, sdk-python.md: a dropped connection kills a foreground exec (only detached processes and sessions survive); exec with a session uses argv alone, sends no started frame and merges stderr; a command's environment is cleared to PATH, TERM and the lane's proxy variables; an exited detached process leaves the table after 5 minutes; after the child exits an undrained client has 2 seconds before the tail is dropped; one oversized match ends a find; replace is atomic per file, not per list. sdk-python.md: only run and exec take a timeout, unlike the Go SDK's contexts; Template.new takes no claim_ref. mcp.md: logs forgets an exited process after 5 minutes. sandboxd-api.md, security.md: preview has no operator path, unlike fork, promote and checkpoint; an archived claim kept forever has no lease to clamp a preview URL to. deploy.md: a volume claim refused at max_claims answers 429 with no redirect. egress.md: an absolute-form https request on the forward door injects without intercept; an egress pool's warm VMs and golden build sit on the bridge unlocked until claim. * review: keep a test fixture's type and its method together The generation-sweep test's stallingSweepStore landed between stallingMetasStore and its Metas method (/code item 1: a type is immediately followed by its complete method set). * fix(pool): keep the empty-floor cutoff off a pool whose lead sizes it dcbfd6f returned the floor whenever the decayed rate fell under one arrival per decay period, and claimed that was the identity for a floor of one or more. It is not: warm 1, warm_max 3, 0.01/s and a two minute lead target 3, and the cutoff made it 1. The cutoff and the unmeasured lead bootstrap now only ever turn a dynamic target of 1 into 0 or of 0 into 1, which max(floor, ...) absorbs for any floor of one or more. * fix(store/s3): let a cancelled caller leave a stalled shared download bf38fd9 detached the flight but kept the synchronous Do, so a caller whose client hung up stayed blocked, holding its record read lock, until the download ended or its 30 minute budget ran out. DoChan plus a select on the caller's context is the shape healCheckpoint uses. * fix(pool): run one store generation sweep at a time Moving the sweep off the housekeeping loop let an hourly tick start a second walk while one on a wedged mount was still blocked. Every other sweep this loop starts is single-flight through an atomic flag. * build: list zerolog as a direct sandboxd dependency log.go has imported it directly since the journald priority prefix landed; go mod tidy moves it out of the indirect block. The other four modules are already tidy. * fix(pool): keep one claims.json retrier while the write keeps failing recommit started a detached retry per failed persist. On a data_dir that stays unwritable the reap tick alone rolls back and calls it every 5 seconds, so the daemon grew one goroutine and one error line per tick with no client activity. commit always writes the current full state, so a single retrier converges every outstanding change. * fix(types, server): saturate wire seconds instead of wrapping them ttl_seconds and the buffered exec's timeout_seconds were multiplied by time.Second unbounded. A large value wraps negative: a ttl_seconds in [9223372037, 2^63) read as unset and got the 5 minute default instead of the documented 24h cap, and a timeout_seconds such as 10000000000 woke a hibernated sandbox and then answered 504 without running the command. * perf(engine): size the preview port reader from the frame it reads portReadBuf was 64 KiB under a comment saying it fits one silkd data frame, but a port_forward frame carries BULK_CHUNK = 256 KiB and is 349,554 bytes on the wire, so every frame took bufio's fragment path. The constant now derives from wire.BulkChunk. Interleaved A/B (ABBA x3, 2000 frames per run): 9 allocations per frame to 1, median 75 ms to 47 ms. Preview path only; nothing on the claim or SDK relay path reads here. * fix(sdk/python): raise a typed error when a send fails Conn.recv wraps a socket failure in ProtocolError; Conn.send let a bare BrokenPipeError or ConnectionResetError out, the one SDK failure outside the SandboxError hierarchy. A write into a relay the node had just closed therefore escaped every caller that catches SandboxError, including the LangChain toolkit's tool-error mapping. The two places that tolerated a failed send now name the typed error. * fix(ci): run the shell gate when the Makefile changes shell.yml's only step is `make sh-lint`; a761567 added the Makefile to the Go workflow's paths and missed this one. * docs: correct this round's own sentences and six more promises Corrects two sentences earlier commits on this branch added: a dropped connection kills a foreground command only once it next writes output (a silent one runs on; silkd notices a hang-up through a failed write), and on a claim with no lease ttl_seconds 0 mints a one hour preview URL. deploy.md: the fuller config's idle_hibernate_seconds 300 equalled the 5 minute lease a woken archive gets, so that sandbox could never hibernate again before the reap destroyed it (now 120, with the constraint stated); the minimal config's policy-less egress pool has no network. sandboxd-api.md, egress.md: PUT /v1/pools decides which policy layers a key has, and live claims follow at their next wake or restart; require_promoted belongs to volume claims only. mcp.md: ending a session releases its sandboxes. silkd.md: fs_push creates dest before the stream. langchain.md: from_checkpoint pins template and net. sdk.md: Close and Stop answer not_found for a session or server already gone. * fix(pool): drain the latest claims sequence before the retrier lets go a33b56a made recommit single-flight, which dropped a recommit that found the retrier busy. If the retrier had already cloned the state for its write — commitWake's temporary record, say — and the rollback's recommit arrived before it finished, claims.json kept the stale record with no one left to replace it; an unclean restart then lost the archived claim. The retrier now clears its flag, re-reads the sequence and carries on while a newer one is unwritten. TestRecommitDrainsAChangeItWasTooBusyToSee holds the first write on a FIFO to make the interleaving deterministic. * fix(pool): a woken archive gets the lease its claim asked for The wake stamped a fresh server-default 5 minute lease whatever TTL the claim was made with, and nothing renews a lease, so a sandbox archived after an hour of idling and woken by its next call was destroyed five minutes later — mid-use if still busy — with its archive checkpoint already consumed. The claim record now keeps the lease it asked for and a wake grants that again; records written before the field still get the default. * fix(pool): pin a claim's egress policy layer when it is made effectivePolicy asked whether the key was pooled at every arm, so a PUT /v1/pools that added a policy-less pool for a key tenants already held claims on took their egress away at the next wake or restart, and one that dropped such a pool widened the same claims to the tenant's whole allow-list. The claim record now carries the layer decided at claim time; a record written before the field keeps resolving against the live pool set, so an upgrade neither widens nor narrows a claim. * fix(sdk): kill the command a canceled Run or a run timeout leaves behind The guest kills a foreground exec whose connection dropped only when a write to the client fails, so a silent command (sleep, a quiet build) outlived every canceled ctx and every run(timeout=) cut. Run and run now keep the pid the started frame carried and, when the ctx or the timeout ends the stream, send it a kill over a connection of their own, bounded to 5 s and best effort, before returning the cancel — the shape of sandboxd's killExec for buffered exec. A session command reports no pid and is untouched. Hot path: one ctx.Err check on the success path, no extra RPC. * docs: lease, policy layer and cancel semantics as the code has them A wake grants the lease the claim was made with, a claim's policy layer is settled when it is made, a session command is the session's and Session.Close ends it, and a canceled Run or a run timeout kills the command it cuts. * fix(sdk/python): bound the kill a cut sends by one absolute deadline A socket timeout bounds each read, not the reply: a guest trickling the kill's done frame a byte at a time kept run() past its timeout for as long as it liked. The cleanup now arms the same watchdog run() uses, which aborts the connection at deadline whatever arrives, and keeps only the last started pid instead of every started frame a guest cares to send. * docs: the exec cap may hold a reply up to 5 s while the kill lands * fix(types): clamp a negative wire seconds count to the server default Seconds saturated only the positive half: min(n, MaxInt64/1e9) returns a negative n unchanged, so ttl_seconds -10000000000 still wrapped into a positive duration and clampTTL granted the 24 h cap where 0, the server default, was meant. A small negative already meant "default" through clampTTL's <= 0 branch; the low clamp keeps that and closes the wrap. * fix(sdk/python): cancel the kill's watchdog before its connection is parked The watchdog was cancelled outside the lease, so a timer firing between the park and the cancel shut down a connection already in the pool. The lease now wraps the whole cleanup: the cancel runs before the park, and a watchdog that fails to start leaves the connection to the lease's close. * review: say what recommit and LeaseSeconds do now recommit is one retrier per manager that writes the current state until nothing newer is pending, not a per-call background persist; LeaseSeconds is the lease the claim was granted, the clamped value, not the one it asked for. The test name follows the field. * test(engine): pin the port reader to one bulk data frame * docs: a wake regrants the lease the claim was granted * cut: inline the redirect walk DeleteTemplate alone used * cut: one routable-host check in config * cut: drop the pre-generation record layout readers Nothing has written the flat <id>/export layout or a .old sibling since generations landed: every publish renames <staging>/export to its generation dir. The dir Fetch, sweep and touch fallbacks, the s3 prefix fallback, the .old removals and the four tests that planted the old layout go with it. Records published before generations are no longer readable; there is no compatibility window to keep. * cut: one stub server per status in the peer probe tests * cut: one attach rendezvous fixture in the volume tests * fix(sdk/python): raise typed errors from the agent handshake The connect and TLS phases already turned an OSError into ProtocolError, but the upgrade exchange (send, status line, headers) let a socket timeout or a connection reset out raw, so a node that accepts TCP and stalls the 101 escaped the adapters' SandboxError handling and broke the agent run. Every handshake failure now maps through one helper: a timeout is SandboxTimeout, anything else ProtocolError; a SandboxTimeout raised by remaining_timeout inside the exchange passes through unwrapped. docs/deploy.md: the egress pool example said the pool "or its tenants" needs an egress block; a tenant's claim of a pooled key needs both blocks and gets their intersection, so the sentence now says so.
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.
Closes #182.
Client._requestgave every control-plane request a freshclient.timeout, and the redirect walk on a warm miss is a loop of such requests. The LangChain toolkit promises a five-minute wall clock persandbox_execand, since #179, honours it for the run — but not for the claim: two black-holed redirect candidates cost 600 s with a 300 s client timeout, so the tool could exceed its promise on a cluster with unreachable peers.Claims now take an absolute
time.monotonic()deadline:Client.new,Template.newandCheckpoint.newaccept keyword-onlydeadline, threaded through_claim_from,_post_jsonand the redirect walk into_request_requestbounds each attempt bymin(timeout, deadline - now)and raisesTimeoutErroronce the deadline is spent, before the request is built —_try_eachand_redirect_fallbackonly catchAPIError, so the walk stops there instead of trying the next candidateconn.remaining_timeoutthe dial path already used, now shared rather than duplicated (it gained the operation name for the error message)CocoonToolkit._execpasses the deadline it already computes, throughsandbox()into_claimMeasured against a node that redirects to two addresses which accept TCP and never answer, client timeout 2 s:
Default behavior is unchanged: without a
deadlineevery request still gets the fulltimeout.Tests:
test_claim_refuses_a_spent_deadline(the node sees no request at all),test_claim_deadline_bounds_the_redirect_walk(two black holes, fails in 0.3 s instead of 2 x 2 s), plustest_exec_claims_inside_the_call_budgetandtest_exec_reports_a_claim_that_outlives_the_budgeton the toolkit.conftestgained ablack_holefixture — a listening socket that never accepts.Left alone deliberately:
Client.lookupscatters its probes concurrently undermin(_PEERS_TIMEOUT, timeout)= 5 s, so it cannot run away the same way, and the OpenAI adapter makes no wall-clock promise over its claim (itstimeoutis caller-supplied and applies torun).Gates:
ruff format --checkandruff checkclean insdk/python,sdk/openaiandsdk/langchain;pytest195 passed insdk/pythonand 8 insdk/langchain.sdk/openai's suite does not collect on this machine (openai-agentsis not installed); it is untouched by this change and CI runs it.