refactor async#7
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRestructures the SDK into parallel synchronous and asynchronous packages, centralizes wire schemas under Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant App
participant AsyncLeap0Client
participant AsyncTransport
participant AsyncSandbox
participant AsyncCodeInterpreter
end
App->>AsyncLeap0Client: AsyncLeap0Client(api_key...)
AsyncLeap0Client->>AsyncTransport: init transport (api_key, base_url)
App->>AsyncLeap0Client: await create_sandbox(...)
AsyncLeap0Client->>AsyncTransport: POST /v1/sandbox
AsyncTransport-->>AsyncLeap0Client: 201 SandboxData
AsyncLeap0Client->>AsyncSandbox: wrap SandboxData -> AsyncSandbox
App->>AsyncSandbox: await sandbox.code_interpreter.execute_stream(code)
AsyncSandbox->>AsyncCodeInterpreter: POST /v1/sandbox/{id}/execute/async
AsyncCodeInterpreter->>AsyncTransport: stream SSE response
AsyncTransport-->>App: streaming lines -> aiter_sse_events -> StreamEvent
App->>AsyncSandbox: await sandbox.delete()
AsyncSandbox->>AsyncTransport: DELETE /v1/sandbox/{id}
AsyncTransport-->>AsyncLeap0Client: 204 OK
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
leap0/_sync/git.py (4)
45-64:⚠️ Potential issue | 🟡 MinorDocstring has duplicate and incorrectly merged
Argssections.The
clonemethod docstring contains twoArgs:sections (lines 47-57 and 59-60), with the second one only documentinghttp_timeout. This appears to be a generation/merge error.📝 Suggested fix
"""Clone a remote repository into the sandbox. Args: sandbox: Sandbox ID or object. url: Repository URL. path: Destination path inside the sandbox. branch: Branch to clone. commit_id: Specific commit to checkout after cloning. depth: Shallow clone depth. username: Auth username (for private repos). password: Auth password or token (for private repos). - - http_timeout: Optional HTTP request timeout in seconds for this SDK call. - - Args: http_timeout: Optional HTTP request timeout in seconds for this SDK call. - Returns: + Returns: GitResult: Command output and exit status from the clone operation. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/git.py` around lines 45 - 64, The clone method docstring in leap0/_sync/git.py has duplicated Args sections due to a merge error; fix it by consolidating into a single Args block that lists each parameter (sandbox, url, path, branch, commit_id, depth, username, password, http_timeout) once with their descriptions, remove the repeated/extra Args header and ensure the Returns section remains intact and correctly describes GitResult for the clone function.
318-342:⚠️ Potential issue | 🟡 MinorDocstring contains parameters from a different method.
The
commitmethod docstring includes parameters forpush(remote, branch, set_upstream, username, password) which don't belong here. This appears to be a copy-paste or generation error.📝 Suggested fix — remove incorrect parameters
Args: sandbox: Sandbox ID or object. path: Path to the git repo. message: Commit message. author: Author name. email: Author email. allow_empty: Allow creating an empty commit. - - http_timeout: Optional HTTP request timeout in seconds for this SDK call. - - Args: - sandbox: Sandbox ID or object. - path: Path to the git repo. - remote: Remote name (default ``"origin"``). - branch: Branch name. - set_upstream: Set upstream tracking. - username: Auth username. - password: Auth password or token. http_timeout: Optional HTTP request timeout in seconds for this SDK call. Returns: GitCommitResult: Commit result including commit ID when successful.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/git.py` around lines 318 - 342, The commit method docstring in git.py incorrectly contains parameters from the push method (remote, branch, set_upstream, username, password) and duplicated Args section; edit the docstring for the commit function to remove all push-related parameters and the duplicate Args block so only the actual commit parameters (sandbox, path, message, author, email, allow_empty, http_timeout) and the correct Returns description for GitCommitResult remain; locate the docstring for the commit function (function name: commit) and clean up the Args/Returns text accordingly.
231-260:⚠️ Potential issue | 🟡 Minor
http_timeoutparameter accepted but not forwarded.The
create_branchmethod acceptshttp_timeoutbut doesn't pass it to_git_result.🐛 Proposed fix
- return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/create-branch", payload) + return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/create-branch", payload, http_timeout=http_timeout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/git.py` around lines 231 - 260, The create_branch method accepts an http_timeout parameter but never forwards it to the underlying request; update the call in create_branch that invokes self._git_result("/v1/sandbox/{sandbox_id}/git/create-branch", payload) to pass the http_timeout (e.g., as the http_timeout/timeout argument) so the timeout is honored; locate create_branch and ensure the call to _git_result includes the http_timeout argument (use sandbox_id_of(sandbox) for the path as already used).
184-215:⚠️ Potential issue | 🟡 Minor
http_timeoutparameter accepted but not forwarded.The
logmethod acceptshttp_timeoutbut doesn't pass it to_git_result.🐛 Proposed fix
- return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/log", payload) + return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/log", payload, http_timeout=http_timeout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/git.py` around lines 184 - 215, The log method accepts an http_timeout but never forwards it to the underlying call; update the call in log to pass the timeout through to _git_result (e.g., return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/log", payload, http_timeout=http_timeout)). Modify the invocation in the log function so _git_result receives the http_timeout parameter (keep payload construction and sandbox_id_of(sandbox) as-is).
🟠 Major comments (16)
leap0/_schemas/snapshot.py-1-15 (1)
1-15:⚠️ Potential issue | 🟠 MajorAdd missing imports for type checking:
SandboxStateandNetworkPolicyDictmust be imported for type checkers to resolve the schema.The schema uses
SandboxState(line 13) andNetworkPolicyDict(line 15) in annotations, but does not import them. Whilefrom __future__ import annotationsdefers annotation evaluation and prevents runtime errors, type checkers like Pyright (configured in this project) require these names to be resolvable. The codebase establishes a clear pattern for this inleap0/_schemas/sandbox.py— use aTYPE_CHECKINGblock to import model types without runtime overhead.Proposed fix: Add TYPE_CHECKING imports
from __future__ import annotations +from typing import TYPE_CHECKING, TypedDict -from typing import TypedDict + +if TYPE_CHECKING: + from ..models.sandbox import SandboxState + from .sandbox import NetworkPolicyDict🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_schemas/snapshot.py` around lines 1 - 15, The TypedDict SnapshotCreateResponseDict references SandboxState and NetworkPolicyDict but those names are not imported for type checkers; add a TYPE_CHECKING block by importing TYPE_CHECKING from typing and, inside an if TYPE_CHECKING: block, import SandboxState and NetworkPolicyDict from their defining modules (matching the pattern used in leap0/_schemas/sandbox.py) so static analyzers like Pyright can resolve the types while avoiding runtime imports.examples/desktop.py-19-21 (1)
19-21:⚠️ Potential issue | 🟠 MajorClient can leak if sandbox creation fails before entering
try/finally.If Line 20 throws,
client.close()at Line 41 is never reached. Wrap sandbox creation inside an outertry/finally(or initializesandboxtoNone) so client shutdown is always guaranteed.Proposed fix
def main() -> None: client = Leap0(Leap0Config()) - sandbox: Sandbox = client.sandboxes.create(template_name=DEFAULT_DESKTOP_TEMPLATE_NAME) - + sandbox: Sandbox | None = None try: + sandbox = client.sandboxes.create(template_name=DEFAULT_DESKTOP_TEMPLATE_NAME) sandbox.desktop.wait_until_ready(timeout=60.0) print("Desktop:", sandbox.desktop.desktop_url()) @@ - try: - sandbox.delete() - except Leap0Error: - pass - finally: - client.close() + if sandbox is not None: + try: + sandbox.delete() + except Leap0Error: + pass + client.close()Also applies to: 35-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/desktop.py` around lines 19 - 21, The client may leak if sandbox creation fails before the finally that calls client.close(); ensure the Leap0 client is always closed by either initializing sandbox = None and wrapping the sandbox creation (client.sandboxes.create(...)) and subsequent use in a try/finally that calls client.close() in the outer finally, or by creating an outer try/finally around the call to client.sandboxes.create so that client.close() is executed regardless of exceptions; reference the Leap0(Leap0Config()) instantiation, the sandbox variable, and client.sandboxes.create to locate where to add the outer try/finally and the guaranteed client.close() call.leap0/_async/ssh.py-18-23 (1)
18-23:⚠️ Potential issue | 🟠 MajorAdd
awaitkeywords to both coroutine calls in the async example.The example in the
AsyncSshClientdocstring showsclient.sandboxes.create()andsandbox.ssh.create_access()withoutawait, but both methods are async and return coroutines. Without awaiting these calls, users copying this example will get coroutine objects instead of actual SSH credentials.✏️ Proposed fix
- sandbox = client.sandboxes.create() - access = sandbox.ssh.create_access() + sandbox = await client.sandboxes.create() + access = await sandbox.ssh.create_access() print(access.command)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/ssh.py` around lines 18 - 23, The docstring example in AsyncSshClient calls the async methods client.sandboxes.create() and sandbox.ssh.create_access() without awaiting them; update the example to await both calls (await client.sandboxes.create() and await sandbox.ssh.create_access()) so callers receive actual Sandbox and Access objects instead of coroutine objects, and ensure the surrounding example code is in an async context (e.g., inside an async def) to reflect proper usage.leap0/__init__.py-174-176 (1)
174-176:⚠️ Potential issue | 🟠 MajorKeep
__all__and_DYNAMIC_IMPORTSin sync forRegistryCredentialsDict.Line 175 exports
RegistryCredentialsDict, but the template block in_DYNAMIC_IMPORTSnever resolves it. The firstfrom leap0 import RegistryCredentialsDictwill fall through__getattr__and raiseAttributeError.Suggested fix
"CreateTemplateParams": (".models.template", "CreateTemplateParams"), "ImageConfig": (".models.template", "ImageConfig"), + "RegistryCredentialsDict": (".models.template", "RegistryCredentialsDict"), "Template": (".models.template", "Template"), "RenameTemplateParams": (".models.template", "RenameTemplateParams"),Also applies to: 261-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/__init__.py` around lines 174 - 176, The __all__ list exports RegistryCredentialsDict but _DYNAMIC_IMPORTS' template mapping never includes it, causing the first from leap0 import RegistryCredentialsDict to trigger __getattr__ and raise AttributeError; update the _DYNAMIC_IMPORTS structure (the template block that defines lazy import targets) to include "RegistryCredentialsDict" (and mirror any other names exported on lines ~261-264) so the dynamic import resolver will produce the attribute, keeping __all__ and _DYNAMIC_IMPORTS in sync and preventing the fallback __getattr__ failure.leap0/_sync/pty.py-58-65 (1)
58-65:⚠️ Potential issue | 🟠 MajorQuote
session_idbefore building PTY URLs.Line 58 accepts arbitrary non-empty session IDs, but Lines 108, 120, 135, and 148 splice them straight into REST/WebSocket path segments. A caller can create
session_id="foo/bar"and then make the session unreachable throughget(),delete(),resize(), orconnect(). Usequote(..., safe="")on the path segment or reject reserved characters up front.Also applies to: 98-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/pty.py` around lines 58 - 65, The session_id passed into PtySession-related calls is inserted directly into REST/WebSocket path segments (affecting get(), delete(), resize(), connect(), etc.), so sanitize it before building URLs: either validate/reject session_id values containing reserved characters or call urllib.parse.quote(session_id, safe="") when composing path segments so reserved characters are percent-encoded; update all places that splice session_id into paths (the URL-building code used by get(), delete(), resize(), connect() and the PtySession factory) to use the quoted value (or perform the validation check) so callers cannot break routing by supplying characters like "/" or "?".leap0/_sync/client.py-129-140 (1)
129-140:⚠️ Potential issue | 🟠 MajorDon't install and shut down global OTEL providers per client.
Lines 138–140 set process-wide tracer and meter providers, but lines 174–177 shut them down when this client closes. Creating multiple SDK clients (or a client when the host app has already configured OpenTelemetry) causes the first client to initialize global providers that the second client will overwrite, and closing the first client will shut down providers that the second client now depends on. Only manage these providers if this instance truly owns them, or reuse existing global providers instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/client.py` around lines 129 - 140, The _init_otel method is unconditionally installing global OTEL providers and the client's close path unconditionally shuts them down; change this so the client only sets global providers if it truly owns them and only shuts them down when owned. In _init_otel, detect existing global providers (via trace.get_tracer_provider() and metrics.get_meter_provider()) and if none or explicitly requested, create providers, call trace.set_tracer_provider and metrics.set_meter_provider and set self._owns_otel = True; otherwise reuse the existing providers and set self._owns_otel = False. In the close/shutdown path (the code currently calling shutdown on tracer/meter providers), check self._owns_otel and only call shutdown on the providers when True to avoid impacting other clients or host-configured OpenTelemetry.leap0/_sync/pty.py-151-170 (1)
151-170:⚠️ Potential issue | 🟠 MajorMerge caller
additional_headersinstead of passing it twice.Line 170 explicitly passes
additional_headers=...while also forwarding**kwargsdocumented as additional arguments towebsockets.sync.client.connect. If a caller passesadditional_headersin kwargs (which the docstring allows), this raisesTypeError: connect() got multiple values for keyword argument 'additional_headers'. Pop and merge caller headers with auth headers first. The same issue exists inleap0/_async/pty.pyline 170 and should be fixed identically.Suggested fix
url = self.websocket_url(sandbox, session_id) if http_timeout is not None and "open_timeout" not in kwargs: kwargs["open_timeout"] = http_timeout - websocket = connect(url, additional_headers={self._transport.auth_header: self._transport.auth_value}, **kwargs) + headers = dict(kwargs.pop("additional_headers", {}) or {}) + headers[self._transport.auth_header] = self._transport.auth_value + websocket = connect(url, additional_headers=headers, **kwargs) return PtyConnection(websocket=websocket)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/pty.py` around lines 151 - 170, In the connect method (leap0/_sync/pty.py and apply the same change in leap0/_async/pty.py) avoid passing additional_headers twice by popping any caller-provided additional_headers from kwargs, merging them with the auth header, and then passing the merged dict to connect; e.g., do additional_headers = kwargs.pop("additional_headers", None), build a headers dict that first incorporates caller headers (if any) and then sets headers[self._transport.auth_header] = self._transport.auth_value to ensure the auth header is present, and finally call connect(url, additional_headers=headers, **kwargs).leap0/_sync/_transport.py-254-276 (1)
254-276: 🛠️ Refactor suggestion | 🟠 Major
request_target()doesn't accepttimeoutparameter.Unlike
request()and_request(), therequest_target()method doesn't accept atimeoutparameter, forcing callers to rely solely on the context override or instance default. This is inconsistent with other methods and limits flexibility.♻️ Suggested addition
def request_target( self, method: str, target: str, *, params: JsonObject | None = None, json: JsonObject | None = None, expected_status: int | tuple[int, ...] = 200, + timeout: float | None = None, ) -> httpx.Response: ... - return self._request(method, target, params=params, json=json, expected_status=expected_status) + return self._request(method, target, params=params, json=json, expected_status=expected_status, timeout=timeout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/_transport.py` around lines 254 - 276, The request_target method lacks a timeout parameter unlike request and _request, so update the request_target signature to accept timeout: float | httpx.Timeout | None (or the same type used by request/_request), add timeout to the docstring/Args, and forward it to the internal call by calling self._request(..., expected_status=expected_status, timeout=timeout); reference the request_target function and ensure consistency with request and _request parameter types and behavior.leap0/_sync/_transport.py-200-212 (1)
200-212:⚠️ Potential issue | 🟠 Major
request()ignores_timeout_overridecontext.Same issue as the async transport —
request()usestimeout or self.timeoutwithout checking_timeout_override.get(), makingoverride_timeout()ineffective for calls using this method.🐛 Proposed fix
response = self._client.request( method, url, params=params, json=json, content=content, files=files, headers=actual_headers, - timeout=timeout or self.timeout, + timeout=timeout or self._timeout_override.get() or self.timeout, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/_transport.py` around lines 200 - 212, The synchronous request() method ignores the _timeout_override context; update request() to consult _timeout_override.get() when resolving the timeout (use that value if present, else use the passed timeout or self.timeout) before calling self._client.request; ensure you reference the _timeout_override, request(), and override_timeout() semantics and pass the resolved timeout to self._client.request so _check_response receives the correct behavior after the call.leap0/_async/sandbox.py-248-255 (1)
248-255:⚠️ Potential issue | 🟠 Major
AsyncSandboxesClient.delete()doesn't accepthttp_timeoutparameter.The
AsyncSandbox.delete()method (line 101-107) accepts and forwardshttp_timeout, butAsyncSandboxesClient.delete()doesn't accept this parameter, causing the forwarded timeout to be silently ignored or raise a TypeError.🐛 Proposed fix
`@intercept_errors`("Failed to delete sandbox: ") - async def delete(self, sandbox: SandboxRef) -> None: + async def delete(self, sandbox: SandboxRef, http_timeout: float | None = None) -> None: """Terminate and delete a sandbox. Args: sandbox: Sandbox ID or object. + http_timeout: Optional HTTP request timeout in seconds for this SDK call. """ - await self._transport.request("DELETE", f"/v1/sandbox/{sandbox_id_of(sandbox)}/", expected_status=204) + await self._transport.request("DELETE", f"/v1/sandbox/{sandbox_id_of(sandbox)}/", expected_status=204, timeout=http_timeout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/sandbox.py` around lines 248 - 255, AsyncSandboxesClient.delete currently omits the http_timeout parameter, so when AsyncSandbox.delete forwards http_timeout it is ignored/causes errors; update AsyncSandboxesClient.delete signature to accept the http_timeout parameter and pass it through to self._transport.request (the same way AsyncSandbox.delete does), ensuring you use the same parameter name (http_timeout) and forward it into the _transport.request call that constructs the DELETE to f"/v1/sandbox/{sandbox_id_of(sandbox)}/".leap0/_async/_transport.py-200-214 (1)
200-214:⚠️ Potential issue | 🟠 Major
request()ignores_timeout_overridecontext.The public
request()method usestimeout or self.timeout(line 210), ignoring any activeoverride_timeout()context. In contrast,_request()usestimeout or self._timeout_override.get() or self.timeout(line 144). This inconsistency meansoverride_timeout()won't work for methods that callrequest()directly.🐛 Proposed fix
response = self._client.request( method, url, params=params, json=json, content=content, files=files, headers=actual_headers, - timeout=timeout or self.timeout, + timeout=timeout or self._timeout_override.get() or self.timeout, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/_transport.py` around lines 200 - 214, The public async method request() currently selects timeout via "timeout or self.timeout" and thus ignores any active override set by override_timeout(); update request() to mirror _request() and honor the context by selecting timeout as "timeout or self._timeout_override.get() or self.timeout" before calling _request() (ensure you reference request(), _request(), override_timeout(), and the _timeout_override contextvar to locate the change).leap0/_async/git.py-124-137 (1)
124-137:⚠️ Potential issue | 🟠 MajorSeveral Git methods silently ignore
http_timeout.
diff_unstaged,diff_staged,diff,log,create_branch,checkout_branch,add,push, andpullall accepthttp_timeoutbut call_git_result()without it. That means the caller cannot bound slow Git operations even though the public signature says they can.🐛 Representative fix
- return await self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/diff-unstaged", payload) + return await self._git_result( + f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/diff-unstaged", + payload, + http_timeout=http_timeout, + )Apply the same change to the other affected
_git_result(...)call sites in this file.Also applies to: 140-153, 156-169, 185-215, 231-260, 263-276, 294-304, 359-397, 400-442
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/git.py` around lines 124 - 137, Several Git methods accept an http_timeout but never pass it to _git_result; update each affected method (diff_unstaged, diff_staged, diff, log, create_branch, checkout_branch, add, push, pull) to forward the http_timeout argument to the _git_result call (e.g., await self._git_result(..., payload, http_timeout=http_timeout)) so the timeout actually applies; keep building the payload the same (using sandbox_id_of(sandbox) and payload dicts) and only add the http_timeout kwarg to the existing _git_result invocations across those call sites.leap0/_sync/filesystem.py-115-173 (1)
115-173:⚠️ Potential issue | 🟠 MajorConvenience filesystem methods drop the advertised timeout.
write_file,write_files, andread_fileaccepthttp_timeoutbut do not forward it to the lower-level helper.read_filesalso advertiseshttp_timeout, yetread_files_bytescannot receive it. Callers using the ergonomic APIs will not get the timeout behavior they asked for.🐛 Representative fix
- self.write_bytes(sandbox, path=path, content=content.encode(encoding), permissions=permissions) + self.write_bytes( + sandbox, + path=path, + content=content.encode(encoding), + permissions=permissions, + http_timeout=http_timeout, + ) - self.write_files_bytes(sandbox, files={p: c.encode(encoding) for p, c in files.items()}) + self.write_files_bytes( + sandbox, + files={p: c.encode(encoding) for p, c in files.items()}, + http_timeout=http_timeout, + ) return self.read_bytes( sandbox, path=path, offset=offset, limit=limit, head=head, tail=tail, + http_timeout=http_timeout, ).decode(encoding) - def read_files_bytes(self, sandbox: SandboxRef, *, paths: list[str]) -> dict[str, bytes]: + def read_files_bytes( + self, + sandbox: SandboxRef, + *, + paths: list[str], + http_timeout: float | None = None, + ) -> dict[str, bytes]: - response = self._transport.request("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/filesystem/read-files", json={"paths": paths}) + response = self._transport.request( + "POST", + f"/v1/sandbox/{sandbox_id_of(sandbox)}/filesystem/read-files", + json={"paths": paths}, + timeout=http_timeout, + ) - return {path: content.decode(encoding) for path, content in self.read_files_bytes(sandbox, paths=paths).items()} + return { + path: content.decode(encoding) + for path, content in self.read_files_bytes( + sandbox, + paths=paths, + http_timeout=http_timeout, + ).items() + }Also applies to: 233-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/filesystem.py` around lines 115 - 173, The convenience methods are dropping the caller's http_timeout; update them to forward the timeout to the lower-level helpers: pass the http_timeout argument from write_file into write_bytes (function write_file -> call to write_bytes), pass http_timeout from write_files into write_files_bytes (function write_files -> call to write_files_bytes), and similarly ensure read_file/read_files forward http_timeout into read_bytes/read_files_bytes and add/accept an http_timeout parameter on read_files_bytes if missing so the transport.request timeout is ultimately set; adjust the calls and signatures for read_files_bytes and any other convenience wrappers to propagate the timeout.leap0/_async/filesystem.py-115-173 (1)
115-173:⚠️ Potential issue | 🟠 MajorConvenience filesystem methods drop the advertised timeout.
write_file,write_files, andread_fileaccepthttp_timeoutbut do not forward it to the lower-level helper.read_filesalso advertiseshttp_timeout, yetread_files_bytescannot receive it. That makes the timeout contract ineffective on the higher-level APIs most callers will use.🐛 Representative fix
- await self.write_bytes(sandbox, path=path, content=content.encode(encoding), permissions=permissions) + await self.write_bytes( + sandbox, + path=path, + content=content.encode(encoding), + permissions=permissions, + http_timeout=http_timeout, + ) - await self.write_files_bytes(sandbox, files={p: c.encode(encoding) for p, c in files.items()}) + await self.write_files_bytes( + sandbox, + files={p: c.encode(encoding) for p, c in files.items()}, + http_timeout=http_timeout, + ) return (await self.read_bytes( sandbox, path=path, offset=offset, limit=limit, head=head, tail=tail, + http_timeout=http_timeout, )).decode(encoding) - async def read_files_bytes(self, sandbox: SandboxRef, *, paths: list[str]) -> dict[str, bytes]: + async def read_files_bytes( + self, + sandbox: SandboxRef, + *, + paths: list[str], + http_timeout: float | None = None, + ) -> dict[str, bytes]: - response = await self._transport.request("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/filesystem/read-files", json={"paths": paths}) + response = await self._transport.request( + "POST", + f"/v1/sandbox/{sandbox_id_of(sandbox)}/filesystem/read-files", + json={"paths": paths}, + timeout=http_timeout, + ) - return {path: content.decode(encoding) for path, content in (await self.read_files_bytes(sandbox, paths=paths)).items()} + return { + path: content.decode(encoding) + for path, content in ( + await self.read_files_bytes(sandbox, paths=paths, http_timeout=http_timeout) + ).items() + }Also applies to: 233-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/filesystem.py` around lines 115 - 173, The high-level filesystem methods are not forwarding the http_timeout to their lower-level helpers; update write_file to pass http_timeout into write_bytes (i.e., call self.write_bytes(..., http_timeout=http_timeout)), update write_files to pass http_timeout into write_files_bytes (self.write_files_bytes(..., http_timeout=http_timeout)), and apply the same pattern for the read_file/read_files pair (ensure read_file forwards http_timeout to read_bytes and make read_files_bytes accept/forward http_timeout so read_files can pass it through).leap0/_sync/desktop.py-604-655 (1)
604-655:⚠️ Potential issue | 🟠 Major
wait_until_ready()ignoreshttp_timeout.Line 637 calls
status_stream()withouthttp_timeout. If the SSE read blocks, Tenacity cannot interrupt it until the lower layer times out, so the caller’s timeout setting is effectively ignored.🐛 Proposed fix
- for status in self.status_stream(sandbox, deadline=deadline): + for status in self.status_stream( + sandbox, + deadline=deadline, + http_timeout=http_timeout, + ):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/desktop.py` around lines 604 - 655, The call in wait_until_ready's inner function _poll uses self.status_stream(sandbox, deadline=deadline) but does not pass through the http_timeout parameter; update _poll so status_stream is invoked with http_timeout=http_timeout (preserving the existing deadline argument) to ensure SSE reads respect the caller's http_timeout and Tenacity can interrupt blocked reads. Locate wait_until_ready and its inner _poll and add the http_timeout forwarding to the status_stream call.leap0/_async/desktop.py-565-600 (1)
565-600:⚠️ Potential issue | 🟠 Major
wait_until_ready()ignoreshttp_timeout.Line 588 calls
status_stream()without the caller’shttp_timeout. Becausestatus_stream()only checksdeadlineafter an SSE event arrives, a stalled read can sit past the requested timeout budget.🐛 Proposed fix
- async for status in self.status_stream(sandbox, deadline=deadline): + async for status in self.status_stream( + sandbox, + deadline=deadline, + http_timeout=http_timeout, + ):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/desktop.py` around lines 565 - 600, The wait_until_ready function currently calls self.status_stream(sandbox, deadline=deadline) but omits the caller-supplied http_timeout, allowing stalled SSE reads to exceed the intended timeout; update the call site in wait_until_ready to pass the http_timeout through (e.g., self.status_stream(sandbox, deadline=deadline, http_timeout=http_timeout)) so the underlying stream enforces the HTTP/read timeout bound alongside the deadline.
🟡 Minor comments (19)
leap0/_utils/encoding.py-11-18 (1)
11-18:⚠️ Potential issue | 🟡 MinorDocstrings overstate UTF-8-only behavior.
These functions accept a configurable
encoding, so the docstrings should describe UTF-8 as the default rather than the only behavior.Proposed docstring fix
def b64encode_text(value: str, encoding: str = "utf-8") -> str: - """Encode text as base64 using UTF-8.""" + """Encode text as base64 using the provided encoding (default: UTF-8).""" return b64encode_bytes(value.encode(encoding)) def b64decode_text(value: str, encoding: str = "utf-8") -> str: - """Decode a base64 string into UTF-8 text.""" + """Decode a base64 string into text using the provided encoding (default: UTF-8).""" return base64.b64decode(value).decode(encoding)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_utils/encoding.py` around lines 11 - 18, The docstrings for b64encode_text and b64decode_text incorrectly state they use UTF-8 only; update both docstrings to say UTF-8 is the default but the functions accept an encoding parameter (encoding: str = "utf-8") and will use whatever encoding is passed when encoding/decoding the text; reference the functions b64encode_text and b64decode_text and ensure the wording shows UTF-8 is the default, not the only supported encoding.examples/pty.py-28-29 (1)
28-29:⚠️ Potential issue | 🟡 MinorEnsure
client.close()always runs during cleanup.If
sandbox.delete()fails,client.close()will not execute in the current sequence.Proposed fix
finally: - sandbox.delete() - client.close() + try: + sandbox.delete() + finally: + client.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/pty.py` around lines 28 - 29, The cleanup sequence currently calls sandbox.delete() then client.close(), which means client.close() won't run if sandbox.delete() raises; wrap the sandbox.delete() call in a try/finally (or try/except/finally) so that client.close() is invoked in the finally block regardless of delete errors, referencing sandbox.delete() and client.close() in examples/pty.py; optionally log or re-raise the sandbox.delete() error after closing the client.examples/code_interpreter_stream.py-23-24 (1)
23-24:⚠️ Potential issue | 🟡 MinorGuarantee client shutdown even if sandbox deletion errors.
client.close()can be skipped whensandbox.delete()raises.Proposed fix
finally: - sandbox.delete() - client.close() + try: + sandbox.delete() + finally: + client.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/code_interpreter_stream.py` around lines 23 - 24, sandbox.delete() can raise and prevent client.close() from running; wrap the sandbox deletion so client.close() is always executed. Specifically, call sandbox.delete() inside a try block, ensure client.close() is invoked in a finally block (or call client.close() in a finally after catching the sandbox error), and if you catch the sandbox exception re-raise it after closing the client so the original error is not swallowed; refer to sandbox.delete() and client.close() to locate the code to change.leap0/_async/process.py-42-47 (1)
42-47:⚠️ Potential issue | 🟡 MinorDocstring async example is missing
await.At lines 43–46,
execute()is an async method; the example should useawaitto correctly call it and avoid incorrect copy/paste usage.Proposed fix
- result = client.process.execute( + result = await client.process.execute( sandbox, command="ls -la /workspace", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/process.py` around lines 42 - 47, The docstring example incorrectly calls the async method client.process.execute without awaiting; update the example in leap0/_async/process.py so the call to client.process.execute(sandbox, command="ls -la /workspace") is preceded by await (e.g., result = await client.process.execute(...)) and ensure the surrounding example context is async-compatible (e.g., inside an async function or coroutine) so the await is valid.leap0/_sync/process.py-27-37 (1)
27-37:⚠️ Potential issue | 🟡 MinorClarify timeout default semantics in the API contract.
The method does not set a client-side default timeout (it omits the field when
None), but the docstring states a default of30. This is ambiguous for SDK users.Suggested fix (doc-only)
- timeout: Timeout in seconds (default 30). + timeout: Timeout in seconds. If omitted, the server-side default is used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/process.py` around lines 27 - 37, The docstring for execute (method execute(self, sandbox: SandboxRef, *, command: str, cwd: str | None = None, timeout: int | None = None) -> ProcessResult) is inconsistent with the implementation: it claims a default timeout of 30s but the code omits the field when timeout is None. Either update the signature/implementation to set a concrete client-side default (e.g., timeout: int = 30 and ensure the value is sent) or change the docstring to state that timeout=None means “no client-side timeout / server default is used” and document the actual behavior; adjust the docstring and any related API contract text for execute, SandboxRef usage, and ProcessResult accordingly.leap0/_utils/otel.py-48-55 (1)
48-55:⚠️ Potential issue | 🟡 MinorRace condition in histogram caching.
The
_histogramsdictionary is accessed without synchronization. In a multi-threaded environment, two threads could simultaneously seehistogram is Noneand both create histograms. While OpenTelemetry may handle duplicate histogram names, this is still a data race on the dictionary.🔒 Proposed fix using a lock
+import threading + _tracer = None _meter = None _histograms: dict[str, Any] = {} +_lock = threading.Lock()Then in both wrappers:
- histogram = _histograms.get(histogram_name) - if histogram is None: - histogram = meter.create_histogram( - f"{histogram_name}_duration", - description=f"Duration of {name}", - unit="ms", - ) - _histograms[histogram_name] = histogram + histogram = _histograms.get(histogram_name) + if histogram is None: + with _lock: + histogram = _histograms.get(histogram_name) + if histogram is None: + histogram = meter.create_histogram( + f"{histogram_name}_duration", + description=f"Duration of {name}", + unit="ms", + ) + _histograms[histogram_name] = histogramAlso applies to: 77-84
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_utils/otel.py` around lines 48 - 55, There's a race on the module-level dictionary _histograms when creating/lookups for histogram_name in the wrappers; create a module-level lock (e.g., _histograms_lock) and use it to synchronize the check/create/assignment around _histograms access so only one thread can see histogram is None and call meter.create_histogram for a given histogram_name; apply the same locking pattern to the other identical block (the one around the second histogram creation referenced as also applies to 77-84) and reference the symbols _histograms, _histograms_lock, histogram_name, and meter.create_histogram when making the change.leap0/_sync/ssh.py-80-92 (1)
80-92:⚠️ Potential issue | 🟡 MinorDocstring has formatting issues and incorrect return type.
- The
Args:section has inconsistent extra indentation.- The
Returns:section documentsobjectbut the method returnsSshAccess.📝 Proposed fix
`@intercept_errors`("Failed to regenerate SSH access: ") def regenerate_access(self, sandbox: SandboxRef, http_timeout: float | None = None) -> SshAccess: """Invalidate the current credential and generate a new one. The expiry is also reset. - - Args: - sandbox: Sandbox ID or object. - http_timeout: Optional HTTP request timeout in seconds for this SDK call. - + + Args: + sandbox: Sandbox ID or object. + http_timeout: Optional HTTP request timeout in seconds for this SDK call. + Returns: - object: Result returned by this operation. + SshAccess: Regenerated SSH credential bundle. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/ssh.py` around lines 80 - 92, The regenerate_access method's docstring has bad indentation in the Args section and an incorrect Returns type; update the docstring for regenerate_access (decorated by intercept_errors) to fix the Args indentation for sandbox and http_timeout, and change the Returns section to specify and describe SshAccess (returned via SshAccess.from_dict). Ensure references to SandboxRef and sandbox_id_of(sandbox) remain clear and the short description matches the method behavior.tests/test_docstrings.py-52-52 (1)
52-52:⚠️ Potential issue | 🟡 MinorDead code:
has_returnis alwaysTrueand never used.The expression
not isinstance(item, ast.FunctionDef) or Truealways evaluates toTrue, makinghas_returna constant that's never referenced. This appears to be leftover from an incomplete implementation or a logic error.🔧 Proposed fix: remove the unused variable
doc = ast.get_docstring(item) or "" args = [a.arg for a in item.args.args + item.args.kwonlyargs if a.arg != "self" and a.arg != "cls"] - has_return = not isinstance(item, ast.FunctionDef) or True if args and "Args:" not in doc:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_docstrings.py` at line 52, Remove the dead/unused variable assignment "has_return = not isinstance(item, ast.FunctionDef) or True" in tests/test_docstrings.py; locate the assignment in the loop handling AST nodes (references to ast.FunctionDef and variable name has_return) and delete the line (or replace it with the correct check if a real return-presence test was intended), ensuring no other code expects has_return afterward.leap0/_sync/pty.py-123-136 (1)
123-136:⚠️ Potential issue | 🟡 MinorValidate
colsandrowsinresize()too.Line 123 takes the same dimensions as
create(), but Line 135 forwards zero/negative values directly to the API. That makes the client contract inconsistent and turns obvious input errors into server round-trips.Suggested fix
def resize(self, sandbox: SandboxRef, session_id: str, *, cols: int, rows: int) -> PtySession: """Change the terminal dimensions while connected. @@ - data = cast(PtySessionInfoDict, self._transport.request_json("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/pty/{session_id}/resize", json={"cols": cols, "rows": rows})) + payload = CreatePtySessionParams(cols=cols, rows=rows).to_payload() + data = cast( + PtySessionInfoDict, + self._transport.request_json( + "POST", + f"/v1/sandbox/{sandbox_id_of(sandbox)}/pty/{session_id}/resize", + json=payload, + ), + ) return PtySession.from_dict(data)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/pty.py` around lines 123 - 136, The resize method (PtyClient.resize / function resize) currently forwards cols and rows directly to the API; validate that cols and rows are positive integers (same rules used in create()) before making the request and raise a ValueError with a clear message if invalid; after validation call self._transport.request_json and return PtySession.from_dict as before.leap0/_async/code_interpreter.py-146-185 (1)
146-185:⚠️ Potential issue | 🟡 MinorDocstring has multiple duplicate and incorrectly merged sections.
The
executemethod docstring contains multipleArgs:blocks with duplicated and misplaced content (includingYields:from a different method context). This significantly hinders readability and API documentation generation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/code_interpreter.py` around lines 146 - 185, The execute method's docstring is duplicated and mixed with irrelevant sections; clean it by replacing the multiple repeated "Args:" blocks and the stray "Yields:" section with a single well-structured docstring for execute(self, sandbox: SandboxRef, *, code: str, language: str = "python", context_id: str | None = None, env_vars: dict[str, str] | None = None, timeout_ms: int | None = None, http_timeout: float | None = None) that includes one "Args:" listing each parameter (sandbox, code, language with default, context_id, env_vars, timeout_ms with default, http_timeout) and a single "Returns:" describing CodeExecutionResult; remove duplicate text, correct indentation/formatting, and ensure no unrelated streaming "Yields:" content remains so the docstring is concise and accurate for the execute method.leap0/_async/pty.py-87-106 (1)
87-106:⚠️ Potential issue | 🟡 MinorDocstring contains duplicate
Argssections.The
createmethod docstring has twoArgs:blocks (lines 89-97 and 99-102) with overlapping parameters.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/pty.py` around lines 87 - 106, The docstring for the create method in pty.py contains duplicate "Args:" sections; edit the create method's docstring to remove the duplicated block and merge parameter descriptions into a single Args section listing sandbox, session_id, cols, rows, cwd, envs, lazy_start, and http_timeout (and the Returns description for PtySession) so each parameter appears once with its correct description.leap0/_async/code_interpreter.py-83-100 (1)
83-100:⚠️ Potential issue | 🟡 MinorDocstring has duplicate
Argssections.The
create_contextmethod has twoArgs:blocks with overlapping content.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/code_interpreter.py` around lines 83 - 100, The docstring for create_context contains duplicate "Args:" sections; consolidate them into a single Args block that documents sandbox, language, cwd, and http_timeout once (remove the repeated http_timeout entry), keep the existing Returns description, and preserve the brief one-line summary and decorator `@intercept_errors` for context; update the create_context docstring so each parameter appears only once and formatting is consistent.leap0/_sync/git.py-140-153 (1)
140-153:⚠️ Potential issue | 🟡 Minor
http_timeoutparameter accepted but not forwarded.Same issue as
diff_unstaged— thehttp_timeoutparameter is ignored.🐛 Proposed fix
- return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/diff-staged", payload) + return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/diff-staged", payload, http_timeout=http_timeout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/git.py` around lines 140 - 153, The diff_staged method accepts an http_timeout parameter but never forwards it to the underlying call; update diff_staged (alongside its payload logic) to pass http_timeout through to the _git_result invocation (same fix applied in diff_unstaged): call self._git_result("/v1/sandbox/{sandbox_id}/git/diff-staged", payload, http_timeout=http_timeout) so the timeout is honored by the _git_result request.leap0/_sync/git.py-262-276 (1)
262-276:⚠️ Potential issue | 🟡 Minor
http_timeoutparameter accepted but not forwarded.The
checkout_branchmethod acceptshttp_timeoutbut doesn't pass it to_git_result.🐛 Proposed fix
- return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/checkout-branch", payload) + return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/checkout-branch", payload, http_timeout=http_timeout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/git.py` around lines 262 - 276, The checkout_branch method accepts an http_timeout parameter but never forwards it to the underlying request; update the call to _git_result in checkout_branch to pass the timeout (e.g., _git_result(..., http_timeout=http_timeout)), referencing the checkout_branch method, the _git_result helper, and sandbox_id_of(sandbox) so the request honors the provided timeout.leap0/_sync/git.py-293-304 (1)
293-304:⚠️ Potential issue | 🟡 Minor
http_timeoutparameter accepted but not forwarded.The
addmethod acceptshttp_timeoutbut doesn't pass it to_git_result.🐛 Proposed fix
- return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/add", {"path": path, "files": files}) + return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/add", {"path": path, "files": files}, http_timeout=http_timeout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/git.py` around lines 293 - 304, The add method accepts an http_timeout but never forwards it to the internal caller; update the add implementation so the http_timeout argument is passed through into the call to _git_result (which currently is invoked as _git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/add", {"path": path, "files": files"})). Ensure the third parameter (or the appropriate named parameter used by _git_result for timeouts) receives http_timeout so requests from add honor the provided timeout.leap0/_sync/git.py-124-137 (1)
124-137:⚠️ Potential issue | 🟡 Minor
http_timeoutparameter accepted but not forwarded to_git_result.
diff_unstagedacceptshttp_timeoutas a parameter but doesn't pass it to_git_result, so the timeout will be silently ignored.🐛 Proposed fix
def diff_unstaged(self, sandbox: SandboxRef, *, path: str, context_lines: int | None = None, http_timeout: float | None = None) -> GitResult: ... payload: JsonObject = {"path": path} if context_lines is not None: payload["context_lines"] = context_lines - return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/diff-unstaged", payload) + return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/diff-unstaged", payload, http_timeout=http_timeout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/git.py` around lines 124 - 137, The diff_unstaged method accepts an http_timeout parameter but never forwards it to _git_result, so timeouts are ignored; update the _git_result call in diff_unstaged to pass the http_timeout argument (i.e., call self._git_result(..., http_timeout=http_timeout)) while leaving payload construction (payload and sandbox_id_of(sandbox)) unchanged so the timeout is honored by _git_result.leap0/_sync/git.py-156-169 (1)
156-169:⚠️ Potential issue | 🟡 Minor
http_timeoutparameter accepted but not forwarded.Same issue in
diffmethod.🐛 Proposed fix
- return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/diff", payload) + return self._git_result(f"/v1/sandbox/{sandbox_id_of(sandbox)}/git/diff", payload, http_timeout=http_timeout)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/git.py` around lines 156 - 169, The diff method accepts an http_timeout parameter but doesn't forward it to the underlying HTTP helper; update the diff function (method name: diff, locating the call to self._git_result) to pass the http_timeout through to _git_result (alongside payload and the endpoint built with sandbox_id_of(sandbox)) so the timeout is honored by the request.leap0/_async/pty.py-181-198 (1)
181-198:⚠️ Potential issue | 🟡 MinorClarify explicit connection lifecycle responsibility in docstring.
The
connectmethod awaitswebsockets.asyncio.client.connect()directly without using an async context manager. This pattern is valid and supported, but callers must explicitly callclose()on the returnedAsyncPtyConnectionobject to release resources. While the returned connection object provides aclose()method, the docstring should explicitly document this requirement to prevent connection leaks in exception scenarios.Consider adding: "Important: Always call
close()on the returned connection when done, preferably within a try-finally or async context manager to ensure cleanup."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/pty.py` around lines 181 - 198, The connect method in AsyncPtyConnectionFactory (connect) currently awaits websockets.connect and returns an AsyncPtyConnection but the docstring doesn't warn callers they must close the returned connection to avoid leaks; update the connect docstring to add a clear "Important:" note stating callers must call AsyncPtyConnection.close() (preferably in a try/finally or async context manager) to ensure the websocket is closed and resources are released, and mention that the method does not manage the connection lifecycle for them.leap0/_async/_transport.py-157-165 (1)
157-165:⚠️ Potential issue | 🟡 MinorPass timeout as a parameter to
build_request()instead of via extensions.The
build_request()method accepts atimeoutparameter. Either pass the effective timeout value directly astimeout=effective(float) or construct anhttpx.Timeoutobject with the desired per-phase configuration. Passing timeout viaextensions={"timeout": timeout_dict}uses a low-level mechanism that bypasses the recommended public API and should be avoided.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/_transport.py` around lines 157 - 165, The code currently injects timeouts via extensions (timeout_dict) when calling self._client.build_request; instead use the public API by passing the effective timeout directly (compute effective the same way using timeout or self._timeout_override.get() or self.timeout) and call self._client.build_request(..., timeout=effective) or build an httpx.Timeout object for per-phase settings and pass that as timeout=timeout_obj; update the call that references effective and timeout_dict and remove the extensions={"timeout": ...} usage so build_request receives the timeout via the timeout parameter.
Comment on lines
+263
to
+270
| @intercept_errors("Failed to delete sandbox: ") | ||
| def delete(self, sandbox: SandboxRef) -> None: | ||
| """Terminate and delete a sandbox. | ||
|
|
||
| Args: | ||
| sandbox: Sandbox ID or object. | ||
| """ | ||
| self._transport.request("DELETE", f"/v1/sandbox/{sandbox_id_of(sandbox)}/", expected_status=204) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import ast
from pathlib import Path
path = Path("leap0/_sync/sandbox.py")
tree = ast.parse(path.read_text())
for cls in [n for n in tree.body if isinstance(n, ast.ClassDef) and n.name in {"Sandbox", "SandboxesClient"}]:
for fn in [n for n in cls.body if isinstance(n, ast.FunctionDef) and n.name == "delete"]:
args = [a.arg for a in fn.args.args] + [a.arg for a in fn.args.kwonlyargs]
print(f"{cls.name}.delete(line {fn.lineno}) args={args}")
for call in ast.walk(fn):
if isinstance(call, ast.Call) and isinstance(call.func, ast.Attribute) and call.func.attr == "delete":
print(f" call line {call.lineno}: keywords={[kw.arg for kw in call.keywords]}")
PYRepository: leap0-dev/leap0-python
Length of output: 221
🏁 Script executed:
#!/bin/bash
# Find other lifecycle methods in SandboxesClient to see how they handle http_timeout
rg -A 5 "def (create|list|get|update)" leap0/_sync/sandbox.py | grep -A 5 "SandboxesClient" | head -30Repository: leap0-dev/leap0-python
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Check how transport.request() is called in other methods and if timeout is used
rg "self._transport.request" leap0/_sync/sandbox.py -A 2 -B 1 | head -40Repository: leap0-dev/leap0-python
Length of output: 891
🏁 Script executed:
#!/bin/bash
# Search for other calls to SandboxesClient.delete()
rg "\.delete\(" leap0/_sync/sandbox.py -B 2 -A 2Repository: leap0-dev/leap0-python
Length of output: 313
🏁 Script executed:
#!/bin/bash
# Check if there are direct calls to sandboxes.delete() beyond Sandbox.delete()
rg "sandboxes\.delete\(" leap0/ -rRepository: leap0-dev/leap0-python
Length of output: 479
🏁 Script executed:
#!/bin/bash
# Search for transport.request signature or definition
rg "def request\(" leap0/ -A 3Repository: leap0-dev/leap0-python
Length of output: 428
🏁 Script executed:
#!/bin/bash
# Get full signature of transport.request()
rg "def request\(" leap0/_sync/_transport.py -A 15 | head -30Repository: leap0-dev/leap0-python
Length of output: 570
🏁 Script executed:
#!/bin/bash
# Check for direct calls to sandboxes.delete() method
rg "\.sandboxes\.delete" leap0/ -nRepository: leap0-dev/leap0-python
Length of output: 268
Sandbox.delete() is currently unusable.
Sandbox.delete() forwards http_timeout, but SandboxesClient.delete() does not accept it. Every call to Sandbox.delete() will raise TypeError before sending the DELETE request, even with the default argument.
All other lifecycle methods (create(), pause(), get()) properly accept http_timeout and pass timeout=http_timeout to the transport. The delete() method must follow the same pattern.
Fix required:
- def delete(self, sandbox: SandboxRef) -> None:
+ def delete(self, sandbox: SandboxRef, http_timeout: float | None = None) -> None:
"""Terminate and delete a sandbox.
Args:
sandbox: Sandbox ID or object.
"""
- self._transport.request("DELETE", f"/v1/sandbox/{sandbox_id_of(sandbox)}/", expected_status=204)
+ self._transport.request(
+ "DELETE",
+ f"/v1/sandbox/{sandbox_id_of(sandbox)}/",
+ expected_status=204,
+ timeout=http_timeout,
+ )Note: The async version at leap0/_async/sandbox.py has the identical issue and requires the same fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@leap0/_sync/sandbox.py` around lines 263 - 270, The Sandbox.delete method
currently lacks an http_timeout parameter and therefore doesn't pass a timeout
to the transport; update the sync method (Sandbox.delete in
leap0/_sync/sandbox.py) to accept http_timeout: Optional[float] = None (matching
create/pause/get signatures) and pass timeout=http_timeout into
self._transport.request, and make the identical change in the async counterpart
(Sandbox.delete in leap0/_async/sandbox.py) so both call sites accept and
forward the timeout correctly.
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (1)
leap0/_async/filesystem.py (1)
185-241:⚠️ Potential issue | 🟡 MinorSame docstring issues as sync version: duplicate Args and Returns sections.
This async
read_bytes()has the same malformed docstring as the sync counterpart - duplicate Args blocks withencodingparameter that doesn't exist in this method, and conflicting Returns types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/filesystem.py` around lines 185 - 241, The read_bytes method's docstring contains duplicated Args and Returns sections and mentions a non-existent encoding parameter; update the docstring for async def read_bytes(...) to have a single Args block listing only the actual parameters (sandbox, path, offset, limit, head, tail, http_timeout) and a single Returns block that states "bytes: File contents as raw bytes", remove the stray encoding mention and duplicate paragraphs, and ensure the Example and summary align with returning bytes rather than str; locate the docstring inside read_bytes to make these edits.
🧹 Nitpick comments (7)
leap0/_async/code_interpreter.py (1)
141-176: Inconsistent JSON handling inexecute()vs other methods.
execute()usesself._request()followed byresponse.json()(line 174-175), while other methods likecreate_context()useself._request_json()directly. This inconsistency meansexecute()doesn't benefit from any centralized JSON parsing error handling that_request_jsonmight provide.♻️ Suggested consistency fix
- response = await self._request("POST", sandbox, "/execute", json=payload, http_timeout=http_timeout) - data = cast(CodeExecutionResultDict, response.json()) + data = cast(CodeExecutionResultDict, await self._request_json("POST", sandbox, "/execute", json=payload, http_timeout=http_timeout)) return CodeExecutionResult.from_dict(data)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/code_interpreter.py` around lines 141 - 176, The execute() method calls self._request(...) and then response.json() directly, bypassing centralized JSON parsing/error handling used elsewhere; change execute() to call self._request_json(...) (or otherwise delegate JSON parsing to the same helper) and pass the returned dict into CodeExecutionResult.from_dict so JSON errors are handled consistently with other methods (reference execute(), _request(), _request_json(), and CodeExecutionResult.from_dict).leap0/_async/client.py (2)
193-210: Factory function duplicates config fields instead of passing config directly.
AsyncLeap0()extracts all fields fromLeap0Configand passes them individually toAsyncLeap0Client.__init__, which then creates anotherLeap0Config. This double-parsing is redundant. Consider accepting the config object directly in the client constructor or having the factory pass the config.♻️ Suggested approach
def AsyncLeap0(config: Leap0Config) -> AsyncLeap0Client: - return AsyncLeap0Client( - api_key=config.api_key, - base_url=config.base_url, - sandbox_domain=config.sandbox_domain, - timeout=config.timeout, - auth_header=config.auth_header, - bearer=config.bearer, - otel_enabled=config.otel_enabled, - ) + # Consider adding a _from_config classmethod or internal constructor + # that accepts Leap0Config directly to avoid double validation + return AsyncLeap0Client._from_config(config)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/client.py` around lines 193 - 210, The factory AsyncLeap0 currently unpacks Leap0Config fields and forwards them to AsyncLeap0Client causing redundant reconstruction of Leap0Config; update the factory to pass the Leap0Config object directly (e.g., AsyncLeap0Client(config=config)) or change AsyncLeap0Client.__init__ to accept a config parameter and use it internally; ensure references to AsyncLeap0, AsyncLeap0Client, and Leap0Config are updated consistently so only one config parsing path remains.
172-179: OTEL provider shutdown is synchronous but called from async context.Both
TracerProvider.shutdown()andMeterProvider.shutdown()are synchronous blocking calls. In an async context, these could block the event loop briefly. For most use cases this is acceptable since shutdown happens once at cleanup, but consider usingasyncio.to_thread()if shutdown latency becomes a concern in high-throughput scenarios.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/client.py` around lines 172 - 179, The async close method (with_instrumentation("async_client.close") async def close) currently calls the synchronous TracerProvider.shutdown() and MeterProvider.shutdown() directly; update close to call these blocking shutdown methods on a background thread (e.g., via asyncio.to_thread or loop.run_in_executor) and await those tasks so the event loop isn't blocked; keep the existing ownership checks (_owns_tracer_provider/_tracer_provider and _owns_meter_provider/_meter_provider), import asyncio where needed, and ensure shutdown calls are awaited and exceptions propagated or logged consistently within close.leap0/_async/sandbox.py (1)
61-72: Type annotationclient: objectloses IDE support.Using
client: objectavoids circular imports but sacrifices type checking and IDE autocomplete forself._clientattribute access. Consider usingTYPE_CHECKINGwith a string annotation or a Protocol to preserve type safety.♻️ Alternative using TYPE_CHECKING
from typing import TYPE_CHECKING if TYPE_CHECKING: from .client import AsyncLeap0Client class AsyncSandbox: def __init__(self, client: "AsyncLeap0Client", data: SandboxData | SandboxStatus): self._client: "AsyncLeap0Client" = client # ...🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/sandbox.py` around lines 61 - 72, The constructor uses client: object which loses type hints; update AsyncSandbox.__init__ to use a forward-reference or Protocol so IDEs keep autocomplete: add "from typing import TYPE_CHECKING" and inside a TYPE_CHECKING block import the actual AsyncLeap0Client (or declare a small Protocol with the needed attributes), change the signature to client: "AsyncLeap0Client" (string literal) and annotate self._client: "AsyncLeap0Client" to preserve typing without introducing runtime circular imports; ensure references to _AsyncSandboxServiceProxy(...) remain unchanged.leap0/_async/pty.py (2)
147-167: Consider using a dedicated resize params model instead ofCreatePtySessionParams.
resize()reusesCreatePtySessionParamswhich includes fields likesession_id,cwd,envs, andlazy_startthat are irrelevant for a resize operation. Whileto_payload()likely filters outNonevalues, using the full creation params model for a resize request could cause issues if the API changes or ifto_payload()includes unexpected defaults.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/pty.py` around lines 147 - 167, The resize() method currently builds its payload with CreatePtySessionParams which contains unrelated fields; introduce a dedicated minimal params model (e.g., PtyResizeParams or CreatePtyResizeParams) that only accepts cols and rows, implement its to_payload() to produce the correct JSON, and replace CreatePtySessionParams(cols=cols, rows=rows).to_payload() in resize() with the new model; update any imports and tests to use the new model so the POST to /pty/{id}/resize only sends resize-specific fields.
182-205: Parameter naming:http_timeoutis used for WebSocketopen_timeout.The
http_timeoutparameter is forwarded toopen_timeoutfor the websocket connection, which may confuse users expecting HTTP semantics. Consider renaming toconnect_timeoutoropen_timeoutfor clarity, or document this behavior more explicitly in the docstring.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/pty.py` around lines 182 - 205, The parameter name http_timeout on the connect method is misleading because it is forwarded to the WebSocket open timeout; rename it to open_timeout (or connect_timeout) in the AsyncPty.connect signature and docstring, update the docstring to state it controls the websocket open timeout, and change the kwargs mapping to set kwargs["open_timeout"] from the new parameter; to preserve backward compatibility, accept a deprecated http_timeout alias (if provided, map it to open_timeout and emit a deprecation warning). Update references in the method body (url = self.websocket_url(...), additional_headers handling, and the connect(...) call that passes open_timeout) and add/adjust any tests that rely on the old parameter name.leap0/_async/desktop.py (1)
537-563: Deadline check occurs after event receipt, allowing one extra event.The deadline check at line 555 happens after
async for event in aiter_sse_events(...)yields, meaning one event past the deadline could be processed before the timeout is raised. For most use cases this is fine, but if strict deadline enforcement is needed, consider checking before processing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/desktop.py` around lines 537 - 563, The loop in status_stream currently checks the deadline only after an event is received; rewrite the async for over aiter_sse_events(...) into an explicit async iterator (e.g., ait = aiter_sse_events(...); then loop calling await ait.__anext__()) so you can check deadline (time.monotonic() >= deadline) before awaiting the next event and raise Leap0TimeoutError if expired; keep existing handling of non-dict events, "error" fields (raising Leap0Error), yielding via DesktopProcessStatusList.from_dict, and ensure response.aclose() stays in the finally block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@leap0/__init__.py`:
- Around line 75-90: The root package left partial TYPE_CHECKING-only imports
for NetworkPolicyMode, sandbox_id_of, snapshot_id_of, and RegistryCredentialType
which makes `from leap0 import ...` pass static analysis but causes runtime
AttributeError because they are absent from __all__ and _DYNAMIC_IMPORTS; fix by
either (A) restoring these symbols to the dynamic lazy exports—add their names
to _DYNAMIC_IMPORTS and __all__ where other lazy exports are declared
(referencing the __init__ module's _DYNAMIC_IMPORTS and __all__), or (B) remove
the TYPE_CHECKING-only imports and any TYPE_CHECKING re-exports for those
symbols so they are not advertised; apply the same consistent fix for other
occurrences mentioned (blocks around lines 104-192 and 195-283) referencing the
same symbol names and the TYPE_CHECKING import handling in __init__.py.
In `@leap0/_async/desktop.py`:
- Around line 319-330: type_text is inconsistent with press_key because it lacks
the http_timeout parameter; update the type_text signature to accept an
http_timeout (e.g., http_timeout: Optional[float] = None), update its docstring
to document the timeout, and forward that parameter into the _request_json call
(same way press_key does) so the call becomes await self._request_json("POST",
sandbox, "/api/input/type", json={"text": text}, http_timeout=http_timeout);
keep the return behavior unchanged and use the same SandboxRef type annotation
as press_key.
In `@leap0/_async/filesystem.py`:
- Around line 452-467: The async move method is missing the http_timeout
parameter, causing inconsistency with the sync version; update async def
move(self, sandbox: SandboxRef, *, src_path: str, dst_path: str, overwrite: bool
= False) to accept an http_timeout: Optional[float] = None and pass that through
to self._transport.request (alongside method, URL and json payload) as the
http_timeout argument, ensuring the call signature and behavior match other
async filesystem methods.
In `@leap0/_sync/client.py`:
- Around line 149-153: The new MeterProvider instantiation lacks a MetricReader
so duration histograms never export; update the block that creates
self._meter_provider (where metrics.get_meter_provider() is checked) to attach a
PeriodicExportingMetricReader configured with an OTLPMetricExporter (similar to
how BatchSpanProcessor(OTLPSpanExporter()) is used for tracing), register that
reader with the MeterProvider on creation, and ensure self._owns_meter_provider
remains set to True so it can be shut down properly.
In `@leap0/_sync/desktop.py`:
- Around line 586-590: The deadline check currently runs only after
iter_sse_events yields, allowing response.iter_lines() to block past the
deadline; fix by enforcing the deadline at the read layer inside the streaming
loop: when calling self._transport.stream(...) or when consuming
response.iter_lines() for the SSE stream in the code around
DesktopClient/whatever method that calls iter_sse_events, compute remaining_time
= deadline - time.monotonic() and pass that as the read/HTTP timeout (or use a
short poll/read timeout and repeatedly call response.iter_lines()/read with that
timeout), re-checking time.monotonic() between reads so a Leap0TimeoutError is
raised as soon as the overall deadline is exceeded (refer to
response.iter_lines, iter_sse_events, and the deadline variable to locate and
modify the logic).
- Around line 642-655: The current try/except in wait_until_ready (around the
call to _poll) swallows all exceptions and converts them into Leap0TimeoutError;
remove the broad "except Exception" handler so only Leap0TimeoutError and
Leap0Error are caught/translated, and allow any other exceptions (parsing,
transport, schema errors) raised by _poll to propagate unchanged; update the
except clauses in wait_until_ready to only handle Leap0TimeoutError and
Leap0Error (referencing the function name wait_until_ready and the helper _poll)
so behavior matches leap0/_async/desktop.py.
In `@leap0/_sync/filesystem.py`:
- Around line 452-467: The move method is missing the optional http_timeout
parameter used by other filesystem operations; update move(self, sandbox, *,
src_path: str, dst_path: str, overwrite: bool = False, http_timeout:
Optional[float] = None) and pass http_timeout through to the
self._transport.request(...) call (the POST to
"/v1/sandbox/{sandbox_id}/filesystem/move") so callers can set custom timeouts,
matching the signature/behavior of the other methods that accept http_timeout.
- Around line 185-241: The read_bytes docstring contains duplicated Args and
Returns sections and an incorrect reference to encoding; update the docstring in
the read_bytes function to keep a single Args block listing sandbox: SandboxRef,
path, offset, limit, head, tail, and http_timeout (remove the stray encoding
arg), keep only the correct Returns describing bytes (not str), remove the
duplicated example or ensure it references read_bytes if kept, and leave the
implementation (payload construction and call to self._transport.request with
sandbox_id_of) unchanged.
In `@leap0/_sync/git.py`:
- Around line 167-178: The methods reset, show, and delete_branch still use the
default transport timeout instead of accepting a per-call http_timeout; update
each signature (reset, show, delete_branch) to accept an optional http_timeout:
Optional[int] = None and pass that value through to the internal call (the calls
to self._git_result) so _git_result receives and uses the per-call http_timeout;
ensure the intercept_errors decorator and existing args (sandbox, path, branch,
etc.) remain unchanged while propagating http_timeout to the underlying request
helper.
In `@leap0/_sync/lsp.py`:
- Around line 114-132: The docstring for did_open_path is malformed and only
documents http_timeout; update the docstring for the did_open_path method
(decorated with `@intercept_errors`("Failed to open document: ")) to include brief
parameter descriptions for language_id, path_to_project, path, text, version,
and http_timeout, making sure to keep the existing short summary line and
indicate that this is like did_open but takes a file path; ensure formatting
matches the project's docstring style (e.g., Args: language_id: ...,
path_to_project: ..., path: ..., text: Optional[str] ..., version: int ...,
http_timeout: Optional[float] ...).
- Around line 31-56: The start() docstring mistakenly contains a duplicated Args
section with parameters (uri, text, version, http_timeout) that belong to
did_open(); update the docstring for start() (decorated by `@intercept_errors` and
implemented in start(self, sandbox: SandboxRef, *, language_id: str,
path_to_project: str) -> LspResponse) by removing the erroneous second Args
block and keeping a single correct Args section that documents sandbox,
language_id, and path_to_project; ensure the Returns section remains and that
references to LspResponse, sandbox_id_of, and the transport call remain
unchanged.
In `@leap0/_sync/process.py`:
- Around line 27-55: The execute method is missing a separate HTTP request
timeout so callers cannot extend the underlying HTTP call; add a new optional
parameter (e.g. http_timeout: int | None = None) to the execute signature (keep
the existing timeout which maps into payload), leave payload building unchanged,
and pass the http_timeout into the transport call
(self._transport.request_json(..., json=payload, timeout=http_timeout) or the
transport's equivalent kwarg) so long-running remote commands can extend the
HTTP timeout and trigger the override path in the error handling; update type
hints and docstring to document http_timeout and keep casting to
ProcessResultDict as before.
---
Duplicate comments:
In `@leap0/_async/filesystem.py`:
- Around line 185-241: The read_bytes method's docstring contains duplicated
Args and Returns sections and mentions a non-existent encoding parameter; update
the docstring for async def read_bytes(...) to have a single Args block listing
only the actual parameters (sandbox, path, offset, limit, head, tail,
http_timeout) and a single Returns block that states "bytes: File contents as
raw bytes", remove the stray encoding mention and duplicate paragraphs, and
ensure the Example and summary align with returning bytes rather than str;
locate the docstring inside read_bytes to make these edits.
---
Nitpick comments:
In `@leap0/_async/client.py`:
- Around line 193-210: The factory AsyncLeap0 currently unpacks Leap0Config
fields and forwards them to AsyncLeap0Client causing redundant reconstruction of
Leap0Config; update the factory to pass the Leap0Config object directly (e.g.,
AsyncLeap0Client(config=config)) or change AsyncLeap0Client.__init__ to accept a
config parameter and use it internally; ensure references to AsyncLeap0,
AsyncLeap0Client, and Leap0Config are updated consistently so only one config
parsing path remains.
- Around line 172-179: The async close method
(with_instrumentation("async_client.close") async def close) currently calls the
synchronous TracerProvider.shutdown() and MeterProvider.shutdown() directly;
update close to call these blocking shutdown methods on a background thread
(e.g., via asyncio.to_thread or loop.run_in_executor) and await those tasks so
the event loop isn't blocked; keep the existing ownership checks
(_owns_tracer_provider/_tracer_provider and
_owns_meter_provider/_meter_provider), import asyncio where needed, and ensure
shutdown calls are awaited and exceptions propagated or logged consistently
within close.
In `@leap0/_async/code_interpreter.py`:
- Around line 141-176: The execute() method calls self._request(...) and then
response.json() directly, bypassing centralized JSON parsing/error handling used
elsewhere; change execute() to call self._request_json(...) (or otherwise
delegate JSON parsing to the same helper) and pass the returned dict into
CodeExecutionResult.from_dict so JSON errors are handled consistently with other
methods (reference execute(), _request(), _request_json(), and
CodeExecutionResult.from_dict).
In `@leap0/_async/desktop.py`:
- Around line 537-563: The loop in status_stream currently checks the deadline
only after an event is received; rewrite the async for over
aiter_sse_events(...) into an explicit async iterator (e.g., ait =
aiter_sse_events(...); then loop calling await ait.__anext__()) so you can check
deadline (time.monotonic() >= deadline) before awaiting the next event and raise
Leap0TimeoutError if expired; keep existing handling of non-dict events, "error"
fields (raising Leap0Error), yielding via DesktopProcessStatusList.from_dict,
and ensure response.aclose() stays in the finally block.
In `@leap0/_async/pty.py`:
- Around line 147-167: The resize() method currently builds its payload with
CreatePtySessionParams which contains unrelated fields; introduce a dedicated
minimal params model (e.g., PtyResizeParams or CreatePtyResizeParams) that only
accepts cols and rows, implement its to_payload() to produce the correct JSON,
and replace CreatePtySessionParams(cols=cols, rows=rows).to_payload() in
resize() with the new model; update any imports and tests to use the new model
so the POST to /pty/{id}/resize only sends resize-specific fields.
- Around line 182-205: The parameter name http_timeout on the connect method is
misleading because it is forwarded to the WebSocket open timeout; rename it to
open_timeout (or connect_timeout) in the AsyncPty.connect signature and
docstring, update the docstring to state it controls the websocket open timeout,
and change the kwargs mapping to set kwargs["open_timeout"] from the new
parameter; to preserve backward compatibility, accept a deprecated http_timeout
alias (if provided, map it to open_timeout and emit a deprecation warning).
Update references in the method body (url = self.websocket_url(...),
additional_headers handling, and the connect(...) call that passes open_timeout)
and add/adjust any tests that rely on the old parameter name.
In `@leap0/_async/sandbox.py`:
- Around line 61-72: The constructor uses client: object which loses type hints;
update AsyncSandbox.__init__ to use a forward-reference or Protocol so IDEs keep
autocomplete: add "from typing import TYPE_CHECKING" and inside a TYPE_CHECKING
block import the actual AsyncLeap0Client (or declare a small Protocol with the
needed attributes), change the signature to client: "AsyncLeap0Client" (string
literal) and annotate self._client: "AsyncLeap0Client" to preserve typing
without introducing runtime circular imports; ensure references to
_AsyncSandboxServiceProxy(...) remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94d940f0-bfcf-4968-a0ee-a646f04a747c
📒 Files selected for processing (31)
examples/code_interpreter_stream.pyexamples/desktop.pyexamples/pty.pyleap0/__init__.pyleap0/_async/_transport.pyleap0/_async/client.pyleap0/_async/code_interpreter.pyleap0/_async/desktop.pyleap0/_async/filesystem.pyleap0/_async/git.pyleap0/_async/lsp.pyleap0/_async/process.pyleap0/_async/pty.pyleap0/_async/sandbox.pyleap0/_async/ssh.pyleap0/_schemas/snapshot.pyleap0/_sync/_transport.pyleap0/_sync/client.pyleap0/_sync/desktop.pyleap0/_sync/filesystem.pyleap0/_sync/git.pyleap0/_sync/lsp.pyleap0/_sync/process.pyleap0/_sync/pty.pyleap0/_sync/sandbox.pyleap0/_sync/snapshots.pyleap0/_sync/ssh.pyleap0/_utils/encoding.pyleap0/_utils/errors.pyleap0/_utils/otel.pytests/test_docstrings.py
✅ Files skipped from review due to trivial changes (4)
- leap0/_utils/encoding.py
- examples/desktop.py
- leap0/_schemas/snapshot.py
- leap0/_sync/ssh.py
🚧 Files skipped from review as they are similar to previous changes (10)
- examples/code_interpreter_stream.py
- examples/pty.py
- leap0/_async/ssh.py
- tests/test_docstrings.py
- leap0/_async/process.py
- leap0/_sync/_transport.py
- leap0/_sync/pty.py
- leap0/_async/_transport.py
- leap0/_async/lsp.py
- leap0/_async/git.py
Comment on lines
+31
to
+56
| @intercept_errors("Failed to start LSP server: ") | ||
| def start(self, sandbox: SandboxRef, *, language_id: str, path_to_project: str) -> LspResponse: | ||
| """Start the LSP server for a language and project. | ||
|
|
||
| Spawns the server process and sends the LSP ``initialize`` handshake | ||
| automatically. | ||
|
|
||
| Args: | ||
| sandbox: Sandbox ID or object. | ||
| language_id: Language identifier (``"python"``, ``"typescript"``, or ``"javascript"``). | ||
| path_to_project: Project directory path inside the sandbox. | ||
|
|
||
| Args: | ||
| sandbox: Sandbox ID or object. | ||
| language_id: Language identifier. | ||
| path_to_project: Project directory path. | ||
| uri: Document URI (e.g. ``"file:///home/user/project/main.py"``). | ||
| text: Full document text. | ||
| version: Document version number. | ||
| http_timeout: Optional HTTP request timeout in seconds for this SDK call. | ||
|
|
||
| Returns: | ||
| LspResponse: Server startup result. | ||
| """ | ||
| data = cast(LspSuccessResponseDict, self._transport.request_json("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/lsp/start", json={"language_id": language_id, "path_to_project": path_to_project})) | ||
| return LspResponse.from_dict(data) |
There was a problem hiding this comment.
Docstring for start() contains incorrectly copied Args section.
The docstring has two Args sections. The second one (lines 43-50) contains parameters (uri, text, version, http_timeout) that don't exist in the start() method signature - they appear to be copied from did_open().
📝 Proposed fix
`@intercept_errors`("Failed to start LSP server: ")
def start(self, sandbox: SandboxRef, *, language_id: str, path_to_project: str) -> LspResponse:
"""Start the LSP server for a language and project.
Spawns the server process and sends the LSP ``initialize`` handshake
automatically.
Args:
sandbox: Sandbox ID or object.
language_id: Language identifier (``"python"``, ``"typescript"``, or ``"javascript"``).
path_to_project: Project directory path inside the sandbox.
- Args:
- sandbox: Sandbox ID or object.
- language_id: Language identifier.
- path_to_project: Project directory path.
- uri: Document URI (e.g. ``"file:///home/user/project/main.py"``).
- text: Full document text.
- version: Document version number.
- http_timeout: Optional HTTP request timeout in seconds for this SDK call.
-
Returns:
LspResponse: Server startup result.
"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @intercept_errors("Failed to start LSP server: ") | |
| def start(self, sandbox: SandboxRef, *, language_id: str, path_to_project: str) -> LspResponse: | |
| """Start the LSP server for a language and project. | |
| Spawns the server process and sends the LSP ``initialize`` handshake | |
| automatically. | |
| Args: | |
| sandbox: Sandbox ID or object. | |
| language_id: Language identifier (``"python"``, ``"typescript"``, or ``"javascript"``). | |
| path_to_project: Project directory path inside the sandbox. | |
| Args: | |
| sandbox: Sandbox ID or object. | |
| language_id: Language identifier. | |
| path_to_project: Project directory path. | |
| uri: Document URI (e.g. ``"file:///home/user/project/main.py"``). | |
| text: Full document text. | |
| version: Document version number. | |
| http_timeout: Optional HTTP request timeout in seconds for this SDK call. | |
| Returns: | |
| LspResponse: Server startup result. | |
| """ | |
| data = cast(LspSuccessResponseDict, self._transport.request_json("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/lsp/start", json={"language_id": language_id, "path_to_project": path_to_project})) | |
| return LspResponse.from_dict(data) | |
| `@intercept_errors`("Failed to start LSP server: ") | |
| def start(self, sandbox: SandboxRef, *, language_id: str, path_to_project: str) -> LspResponse: | |
| """Start the LSP server for a language and project. | |
| Spawns the server process and sends the LSP ``initialize`` handshake | |
| automatically. | |
| Args: | |
| sandbox: Sandbox ID or object. | |
| language_id: Language identifier (``"python"``, ``"typescript"``, or ``"javascript"``). | |
| path_to_project: Project directory path inside the sandbox. | |
| Returns: | |
| LspResponse: Server startup result. | |
| """ | |
| data = cast(LspSuccessResponseDict, self._transport.request_json("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/lsp/start", json={"language_id": language_id, "path_to_project": path_to_project})) | |
| return LspResponse.from_dict(data) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@leap0/_sync/lsp.py` around lines 31 - 56, The start() docstring mistakenly
contains a duplicated Args section with parameters (uri, text, version,
http_timeout) that belong to did_open(); update the docstring for start()
(decorated by `@intercept_errors` and implemented in start(self, sandbox:
SandboxRef, *, language_id: str, path_to_project: str) -> LspResponse) by
removing the erroneous second Args block and keeping a single correct Args
section that documents sandbox, language_id, and path_to_project; ensure the
Returns section remains and that references to LspResponse, sandbox_id_of, and
the transport call remain unchanged.
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (3)
leap0/_sync/desktop.py (1)
586-592:⚠️ Potential issue | 🟠 MajorThe SSE deadline can still be exceeded after the first event.
stream_timeoutis computed once before opening the stream, but HTTPX read timeouts apply to each chunk/read. After an early status update, a laternext(events)can still block for that original timeout and overshootdeadline. Recompute the remaining read timeout before each blocking read, or poll with a short read timeout instead. (python-httpx.org)Also applies to: 594-600
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/desktop.py` around lines 586 - 592, The code computes stream_timeout once before opening the SSE stream (variables: stream_timeout, http_timeout, deadline) and then calls self._transport.stream, but subsequent blocking reads (calls like next(events)) can still block past the global deadline; fix by recomputing the remaining_time before each blocking read and using that as the per-read timeout (raise Leap0TimeoutError if remaining_time <= 0), or alternatively poll with a short read timeout and loop until the deadline; update the loop that consumes the stream (the next(events) calls) to compute remaining_time = deadline - time.monotonic() each iteration and pass min(http_timeout, remaining_time) as the read timeout for that single read.leap0/_sync/client.py (1)
143-159:⚠️ Potential issue | 🟠 Major
close()is shutting down process-global OTel providers.
trace.set_tracer_provider()andmetrics.set_meter_provider()install global defaults, butLeap0Client.close()later shuts down the providers it created as if they were instance-scoped. The OTel specs say each provider should only be shut down once and laterget_tracer()/get_meter()calls are no longer valid and should become no-op, so closing one client can silently disable telemetry for other clients in the same process. This lifecycle needs to be application-scoped or shared across client instances. (opentelemetry.io)Also applies to: 193-196
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/client.py` around lines 143 - 159, Leap0Client currently calls trace.set_tracer_provider(...) and metrics.set_meter_provider(...), installing process-global providers, but its close() must not unconditionally shut those down; update Leap0Client.close to only call shutdown or reset the global tracer/meter when the instance actually created them by checking the _owns_tracer_provider and _owns_meter_provider flags before shutting down or calling trace.set_tracer_provider(None)/metrics.set_meter_provider(None); alternatively, if you intend instance-scoped providers, stop calling trace.set_tracer_provider and metrics.set_meter_provider in the constructor and keep providers purely instance-local (avoid touching globals) — ensure the changes reference trace.set_tracer_provider, metrics.set_meter_provider, Leap0Client.close, _owns_tracer_provider, and _owns_meter_provider so lifecycle is not process-wide.leap0/_async/filesystem.py (1)
27-44:⚠️ Potential issue | 🟡 MinorExpose
http_timeoutonls()andstat().Line 28 and Line 47 are the only public filesystem calls here that still cannot override the transport timeout. That leaves listing/metadata calls without the same latency control the rest of this client already exposes.
⏱️ Proposed fix
- async def ls(self, sandbox: SandboxRef, *, path: str, recursive: bool = False, exclude: list[str] | None = None) -> LsResult: + async def ls(self, sandbox: SandboxRef, *, path: str, recursive: bool = False, exclude: list[str] | None = None, http_timeout: float | None = None) -> LsResult: ... - data = cast(LsResponseDict, await self._transport.request_json("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/filesystem/ls", json=payload)) + data = cast(LsResponseDict, await self._transport.request_json("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/filesystem/ls", json=payload, timeout=http_timeout)) ... - async def stat(self, sandbox: SandboxRef, *, path: str) -> FileInfo: + async def stat(self, sandbox: SandboxRef, *, path: str, http_timeout: float | None = None) -> FileInfo: ... - data = cast(FileInfoDict, await self._transport.request_json("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/filesystem/stat", json={"path": path})) + data = cast(FileInfoDict, await self._transport.request_json("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/filesystem/stat", json={"path": path}, timeout=http_timeout))Also applies to: 46-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/filesystem.py` around lines 27 - 44, Add an optional http_timeout parameter to the public filesystem methods ls and stat (e.g., async def ls(..., http_timeout: float | None = None)) and forward it into the transport call by passing it through to self._transport.request_json (e.g., include timeout/http_timeout argument on the request_json call). Update the method signature and docstring to document the new http_timeout parameter, and ensure the call site in ls (and the analogous change in stat) passes the provided http_timeout through so the transport timeout can be overridden while keeping the intercept_errors decorator and return type (LsResult/stat return type) unchanged.
🧹 Nitpick comments (1)
leap0/_sync/pty.py (1)
9-14: UsePtyResizeParamsfor the resize endpoint.This route has a dedicated model now, but
resize()still validates throughCreatePtySessionParams. Reusing the create payload makes it easier for create/resize validation to drift the next time one schema changes.Proposed fix
-from ..models.pty import CreatePtySessionParams, PtyConnection, PtySession +from ..models.pty import CreatePtySessionParams, PtyConnection, PtyResizeParams, PtySession @@ - payload = CreatePtySessionParams(cols=cols, rows=rows).to_payload() + payload = PtyResizeParams(cols=cols, rows=rows).to_payload()Also applies to: 126-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/pty.py` around lines 9 - 14, The resize endpoint currently validates using CreatePtySessionParams; change it to use the dedicated PtyResizeParams model instead to avoid drift—update the resize(...) function to accept/validate with PtyResizeParams (import it from ..models.pty if not already) and replace any type annotations, parsing/validation calls, and usage that reference CreatePtySessionParams with PtyResizeParams; ensure tests and any variables named like create_params or params in resize are renamed/adjusted to reflect PtyResizeParams and preserve the same fields mapping when constructing the resize request to the Transport.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@leap0/_async/client.py`:
- Around line 7-8: The async client creates a MeterProvider without metric
readers so metrics never export; update the MeterProvider initialization (where
MeterProvider(resource=resource) is constructed) to register a
PeriodicExportingMetricReader with an OTLPMetricExporter (e.g.,
MeterProvider(resource=resource,
metric_readers=[PeriodicExportingMetricReader(OTLPMetricExporter())])) and add
the necessary imports for OTLPMetricExporter and PeriodicExportingMetricReader
so metrics recorded by this SDK are collected and exported.
- Around line 149-164: The client currently installs process-global providers
and sets _owns_tracer_provider/_owns_meter_provider when it created them, but
shutting those down in close() can disable telemetry for other code; instead,
implement shared lifecycle management: replace the per-instance ownership
booleans with a process-wide singleton/refcount for providers (e.g.,
module-level _shared_tracer_provider/_shared_tracer_refcount and
_shared_meter_provider/_shared_meter_refcount) and when creating providers in
the constructor increment the refcount and call
trace.set_tracer_provider/metrics.set_meter_provider only when installing the
shared provider; in close() decrement the refcounts and only call shutdown on
the provider and clear the global provider when the refcount reaches zero.
Ensure references to _tracer_provider, _meter_provider, _owns_tracer_provider,
_owns_meter_provider, trace.set_tracer_provider, metrics.set_meter_provider,
close(), and get_tracer()/get_meter() are updated to use the shared manager so
other clients aren’t disabled by a single close().
In `@leap0/_async/code_interpreter.py`:
- Around line 18-20: The code casts every event from aiter_sse_events() directly
to StreamEvent which breaks on plain-text heartbeats or string frames; update
the consumer in leap0/_async/code_interpreter.py to first check each yielded
value is a dict and ignore or handle non-dict frames (and treat dicts with
envelope "error" specially) before creating/typing a StreamEvent, and when
encountering an error envelope convert it into a Leap0Error and surface it via
intercept_errors so the iterator continues/terminates correctly; apply the same
guard logic to the other similar block around the StreamEvent casting (the block
referenced near lines 209-219).
In `@leap0/_async/desktop.py`:
- Around line 598-609: The retry sleep can exceed the remaining deadline in the
wait loop; update the loop in the method that calls self.status_stream (the
wait_until_ready flow) to compute remaining = deadline - time.monotonic() before
sleeping, and if remaining <= 0 raise a Leap0TimeoutError immediately; use
sleep_time = min(delay, remaining) for asyncio.sleep(sleep_time) and then
advance delay = min(delay * 2, 5.0). Ensure the Leap0TimeoutError raised on
timeout includes the timeout value and original exception where applicable and
stop retrying once remaining reaches zero.
In `@leap0/_async/filesystem.py`:
- Around line 93-100: The async example calls currently create coroutines
instead of performing IO; update each example to await the async SDK methods
(e.g., prepend await to client.filesystem.write_bytes and any other async calls
in the examples around the write_bytes usage in leap0/_async/filesystem.py) so
the calls actually run (for the snippets at the occurrences around Line 95,
127–134, and 153–158); ensure each async method invocation in those examples is
awaited and that the examples remain inside an async context if applicable.
- Around line 524-530: The multipart parse error currently embeds raw response
bytes via body_preview in the ValueError raised after the msg.is_multipart()
check; change the exception to avoid including raw body bytes (e.g., remove
body_preview and only include body length and content_type or a fixed
placeholder like "<redacted>") so secrets aren't leaked. Update the error raised
in the block that checks msg.is_multipart() (where body, content_type, and
body_preview are referenced) to log/raise only non-sensitive metadata
(content_type and len(body)) or a redacted preview string instead of the raw
bytes.
- Around line 186-226: In read_bytes, enforce the documented mutual exclusivity
of the head and tail parameters by checking if both head and tail are not None
and raising a ValueError (e.g., "`head` and `tail` are mutually exclusive")
before constructing payload; update the read_bytes function (the async method
with signature read_bytes(self, sandbox: SandboxRef, *, path: str, offset:
int|None = None, limit: int|None = None, head: int|None = None, tail: int|None =
None, ...)) to perform this validation so neither head nor tail get serialized
when both are supplied.
In `@leap0/_async/sandbox.py`:
- Around line 32-43: The proxy currently awaits every callable in
_AsyncSandboxServiceProxy.__getattr__, causing sync utility methods like
AsyncDesktopClient.desktop_url and AsyncPtyClient.websocket_url to raise when
awaited; change the logic to detect non-coroutine callables and return them
unwrapped (e.g., use asyncio.iscoroutinefunction or inspect.iscoroutinefunction
on the retrieved attr) instead of wrapping/awaiting them, while preserving the
existing wrapping for true async callables (bound_attr) so async methods
continue to be proxied correctly.
In `@leap0/_sync/_transport.py`:
- Around line 98-99: The current code uses expressions like "timeout or ..."
which treat falsy but valid timeouts (e.g., 0) as missing and fall back to the
client default; update all such occurrences to explicitly check for None so
callers' supplied falsy values are honored. For each place that sets the request
timeout (e.g., in methods that perform HTTP calls such as _request, any
stream/request wrappers, and other timeout-setting sites referenced around lines
140, 153-159, and 209), replace "timeout or <default>" with an explicit
conditional using "timeout if timeout is not None else <default>" (or equivalent
explicit None check) so only None triggers the fallback; keep the rest of the
logic and error handling unchanged. Ensure tests or call sites that pass 0 or
False still get those values rather than the client default.
In `@leap0/_sync/desktop.py`:
- Around line 602-604: The loop in iter_sse_events (in leap0/_sync/desktop.py)
currently treats any non-dict SSE event as a heartbeat and continues, which
swallows malformed SSE payloads and causes wait_until_ready() to time out;
instead, detect non-dict events and surface them (e.g., raise a descriptive
exception such as ValueError/RuntimeError or a specific ProtocolError) including
the offending event content and context (stream/source/session id) so callers
like wait_until_ready() can fail fast and log useful info; update the branch
that now does "if not isinstance(event, dict): continue" to raise or propagate
an error with the raw event rather than silently continuing.
---
Duplicate comments:
In `@leap0/_async/filesystem.py`:
- Around line 27-44: Add an optional http_timeout parameter to the public
filesystem methods ls and stat (e.g., async def ls(..., http_timeout: float |
None = None)) and forward it into the transport call by passing it through to
self._transport.request_json (e.g., include timeout/http_timeout argument on the
request_json call). Update the method signature and docstring to document the
new http_timeout parameter, and ensure the call site in ls (and the analogous
change in stat) passes the provided http_timeout through so the transport
timeout can be overridden while keeping the intercept_errors decorator and
return type (LsResult/stat return type) unchanged.
In `@leap0/_sync/client.py`:
- Around line 143-159: Leap0Client currently calls
trace.set_tracer_provider(...) and metrics.set_meter_provider(...), installing
process-global providers, but its close() must not unconditionally shut those
down; update Leap0Client.close to only call shutdown or reset the global
tracer/meter when the instance actually created them by checking the
_owns_tracer_provider and _owns_meter_provider flags before shutting down or
calling trace.set_tracer_provider(None)/metrics.set_meter_provider(None);
alternatively, if you intend instance-scoped providers, stop calling
trace.set_tracer_provider and metrics.set_meter_provider in the constructor and
keep providers purely instance-local (avoid touching globals) — ensure the
changes reference trace.set_tracer_provider, metrics.set_meter_provider,
Leap0Client.close, _owns_tracer_provider, and _owns_meter_provider so lifecycle
is not process-wide.
In `@leap0/_sync/desktop.py`:
- Around line 586-592: The code computes stream_timeout once before opening the
SSE stream (variables: stream_timeout, http_timeout, deadline) and then calls
self._transport.stream, but subsequent blocking reads (calls like next(events))
can still block past the global deadline; fix by recomputing the remaining_time
before each blocking read and using that as the per-read timeout (raise
Leap0TimeoutError if remaining_time <= 0), or alternatively poll with a short
read timeout and loop until the deadline; update the loop that consumes the
stream (the next(events) calls) to compute remaining_time = deadline -
time.monotonic() each iteration and pass min(http_timeout, remaining_time) as
the read timeout for that single read.
---
Nitpick comments:
In `@leap0/_sync/pty.py`:
- Around line 9-14: The resize endpoint currently validates using
CreatePtySessionParams; change it to use the dedicated PtyResizeParams model
instead to avoid drift—update the resize(...) function to accept/validate with
PtyResizeParams (import it from ..models.pty if not already) and replace any
type annotations, parsing/validation calls, and usage that reference
CreatePtySessionParams with PtyResizeParams; ensure tests and any variables
named like create_params or params in resize are renamed/adjusted to reflect
PtyResizeParams and preserve the same fields mapping when constructing the
resize request to the Transport.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a9672e7-d3a0-48d5-b27f-692263c83636
📒 Files selected for processing (29)
leap0/__init__.pyleap0/_async/client.pyleap0/_async/code_interpreter.pyleap0/_async/desktop.pyleap0/_async/filesystem.pyleap0/_async/git.pyleap0/_async/lsp.pyleap0/_async/process.pyleap0/_async/pty.pyleap0/_async/sandbox.pyleap0/_async/snapshots.pyleap0/_async/ssh.pyleap0/_async/templates.pyleap0/_sync/_transport.pyleap0/_sync/client.pyleap0/_sync/code_interpreter.pyleap0/_sync/desktop.pyleap0/_sync/filesystem.pyleap0/_sync/git.pyleap0/_sync/lsp.pyleap0/_sync/process.pyleap0/_sync/pty.pyleap0/_sync/sandbox.pyleap0/_sync/snapshots.pyleap0/_sync/ssh.pyleap0/_sync/templates.pyleap0/models/pty.pyleap0/models/sandbox.pyleap0/models/snapshot.py
✅ Files skipped from review due to trivial changes (4)
- leap0/_sync/filesystem.py
- leap0/_sync/code_interpreter.py
- leap0/init.py
- leap0/_async/git.py
🚧 Files skipped from review as they are similar to previous changes (8)
- leap0/_sync/process.py
- leap0/_sync/templates.py
- leap0/_async/templates.py
- leap0/_async/ssh.py
- leap0/_sync/snapshots.py
- leap0/_async/pty.py
- leap0/_async/process.py
- leap0/models/snapshot.py
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
leap0/_sync/desktop.py (1)
586-592:⚠️ Potential issue | 🟠 MajorThe SSE deadline is still only applied once, at connect time.
stream_timeoutis computed before opening the stream, but nothing recomputes the remaining budget before the later blockingnext(events)calls. If the stream goes idle near the end,status_stream()can still wait pastdeadline, sowait_until_ready(timeout=...)may overshoot the caller's timeout.Also applies to: 595-600
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/desktop.py` around lines 586 - 592, The SSE stream timeout is only applied at connect time in status_stream(), so subsequent blocking reads via next(events) can overshoot the overall deadline; update status_stream (and the similar block around the later 595-600 code) to recompute remaining_time = deadline - time.monotonic() before each blocking read and pass that as the per-read timeout (raising Leap0TimeoutError when remaining_time <= 0) or otherwise enforce that per-event waits use min(http_timeout, remaining_time); reference the status_stream logic and the next(events) read sites to locate where to inject the per-iteration timeout check/raise.
🧹 Nitpick comments (3)
leap0/_utils/otel.py (2)
128-138: Usetime.perf_counter()instead oftime.time()for duration measurement.
time.time()is not monotonic and can be affected by NTP adjustments or manual clock changes, which could result in negative or incorrect duration values.time.perf_counter()is specifically designed for measuring elapsed time with high resolution and monotonic guarantees.♻️ Suggested fix for both async and sync wrappers
async def async_wrapper(*args: P.args, **kwargs: P.kwargs) -> R: tracer = get_tracer() histogram = _get_histogram(name) - start = time.time() + start = time.perf_counter() with tracer.start_as_current_span(name) as span: try: result = await func(*args, **kwargs) span.set_status(Status(StatusCode.OK)) - histogram.record((time.time() - start) * 1000, {"status": "success"}) + histogram.record((time.perf_counter() - start) * 1000, {"status": "success"}) return result except Exception as exc: span.set_status(Status(StatusCode.ERROR, str(exc))) span.record_exception(exc) - histogram.record((time.time() - start) * 1000, {"status": "error"}) + histogram.record((time.perf_counter() - start) * 1000, {"status": "error"}) raiseApply the same change to the sync wrapper at lines 148, 153, and 158.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_utils/otel.py` around lines 128 - 138, Replace uses of time.time() with time.perf_counter() for measuring elapsed durations in the async wrapper (where start = time.time(), histogram.record((time.time() - start) * 1000, ...), inside the tracer.start_as_current_span block) and apply the same change to the sync wrapper mentioned (lines referenced in the review). Keep the same arithmetic to convert seconds to milliseconds (multiply by 1000), and ensure the time module is available so time.perf_counter() can be called.
46-56: Unused protocol definition.
_SpanContextManagerProtocolis defined but never used. The_TracerProtocol.start_as_current_spanmethod usesAbstractContextManager[_SpanProtocol]directly instead.🧹 Suggested cleanup
-class _SpanContextManagerProtocol(Protocol): - """Protocol for context managers that yield spans.""" - - def __enter__(self) -> _SpanProtocol: - """Enter the span context.""" - ... - - def __exit__(self, exc_type: object, exc: object, tb: object) -> bool | None: - """Exit the span context.""" - ... - -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_utils/otel.py` around lines 46 - 56, The _SpanContextManagerProtocol is defined but never referenced; remove it to clean up dead code, or alternatively update the _TracerProtocol.start_as_current_span signature to return _SpanContextManagerProtocol instead of AbstractContextManager[_SpanProtocol] so the protocol is actually used; locate the unused class _SpanContextManagerProtocol and either delete the class or change the return/type annotation on _TracerProtocol.start_as_current_span to reference _SpanContextManagerProtocol.leap0/_async/filesystem.py (1)
298-314: Malformed docstring inread_files().The docstring has inconsistent indentation and an incomplete Args section that only documents
http_timeout, omittingsandbox,paths, andencodingparameters.📝 Suggested fix
`@intercept_errors`("Failed to read files: ") async def read_files(self, sandbox: SandboxRef, *, paths: list[str], encoding: str = "utf-8", http_timeout: float | None = None) -> dict[str, str]: - """ - Read multiple files and return decoded text keyed by path. - - Args: - http_timeout: Optional HTTP request timeout in seconds for this SDK call. - - Returns: - object: Result returned by this operation. - """ + """Read multiple files and return decoded text keyed by path. + + Args: + sandbox: Sandbox ID or object. + paths: List of file paths to read. + encoding: Text encoding for decoding file contents. + http_timeout: Optional HTTP request timeout in seconds for this SDK call. + + Returns: + dict[str, str]: Mapping of file path to decoded text content. + """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_async/filesystem.py` around lines 298 - 314, The read_files() docstring is malformed and incomplete: reformat the docstring to consistent indentation and document all parameters (sandbox, paths, encoding, http_timeout) plus the return value; update the docstring in the read_files method (decorated with `@intercept_errors` and which calls read_files_bytes) to include a short description, properly formatted Args section listing types and meanings for sandbox: SandboxRef, paths: list[str], encoding: str = "utf-8", http_timeout: float | None, and a Returns section describing dict[str, str] of decoded file contents.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@leap0/_async/filesystem.py`:
- Around line 376-401: The grep() docstring has duplicated "Args:" sections with
http_timeout documented twice; edit the grep function's docstring to remove the
redundant Args block so each parameter (including http_timeout) is documented
only once and the docstring remains a single coherent section for arguments
(look for the docstring inside the async def grep(...) in
leap0/_async/filesystem.py).
In `@leap0/_sync/_transport.py`:
- Around line 41-43: The ContextVar _timeout_override must be instance-scoped
instead of a class variable to avoid leaking overrides across Transport
instances; move the ContextVar creation into __init__ as self._timeout_override
= ContextVar("leap0_sync_timeout_override", default=None) (preserve the float |
None type), remove the class-level declaration, and update any code that
references _timeout_override (e.g., the override_timeout context
manager/function and any reads/writes) to use self._timeout_override so each
transport instance has its own timeout override state.
In `@leap0/_sync/code_interpreter.py`:
- Around line 88-107: The health() docstring contains copy/paste artifacts
(extra Args like language/cwd and incorrect Returns referencing CodeContext)
that should be corrected; update the docstring for the health(self, sandbox:
SandboxRef, http_timeout: float | None = None) method to document only the
actual parameters (sandbox, http_timeout), remove duplicated Args sections and
unrelated parameters, and change the Returns section to clearly state it returns
bool (True when service reports "ok"); also scan other methods in this file
(e.g., create_context) for similar copy/paste errors and fix their docstrings so
each method's documented parameters and return type match the actual signature
and behavior.
In `@leap0/_sync/desktop.py`:
- Around line 635-647: The current retry predicate _is_transient_leap0 is too
broad; restrict retries to truly reconnectable errors by either (A) creating a
new subclass RetryableLeap0Error (or a dedicated marker attribute on Leap0Error)
and raising that from the code path that represents reconnectable failures
(e.g., the "stream ended without reaching 'running' state" in
status_stream/transport), or (B) tighten _is_transient_leap0 to inspect error
details/attributes on Leap0Error (e.g., error.kind, error.code or a .retryable
flag) and return True only for those reconnectable kinds while returning False
for malformed SSE, server envelope errors, and deterministic API failures;
update the retry=retry_if_exception(...) to check for the new subclass/flag
instead of any Leap0Error.
---
Duplicate comments:
In `@leap0/_sync/desktop.py`:
- Around line 586-592: The SSE stream timeout is only applied at connect time in
status_stream(), so subsequent blocking reads via next(events) can overshoot the
overall deadline; update status_stream (and the similar block around the later
595-600 code) to recompute remaining_time = deadline - time.monotonic() before
each blocking read and pass that as the per-read timeout (raising
Leap0TimeoutError when remaining_time <= 0) or otherwise enforce that per-event
waits use min(http_timeout, remaining_time); reference the status_stream logic
and the next(events) read sites to locate where to inject the per-iteration
timeout check/raise.
---
Nitpick comments:
In `@leap0/_async/filesystem.py`:
- Around line 298-314: The read_files() docstring is malformed and incomplete:
reformat the docstring to consistent indentation and document all parameters
(sandbox, paths, encoding, http_timeout) plus the return value; update the
docstring in the read_files method (decorated with `@intercept_errors` and which
calls read_files_bytes) to include a short description, properly formatted Args
section listing types and meanings for sandbox: SandboxRef, paths: list[str],
encoding: str = "utf-8", http_timeout: float | None, and a Returns section
describing dict[str, str] of decoded file contents.
In `@leap0/_utils/otel.py`:
- Around line 128-138: Replace uses of time.time() with time.perf_counter() for
measuring elapsed durations in the async wrapper (where start = time.time(),
histogram.record((time.time() - start) * 1000, ...), inside the
tracer.start_as_current_span block) and apply the same change to the sync
wrapper mentioned (lines referenced in the review). Keep the same arithmetic to
convert seconds to milliseconds (multiply by 1000), and ensure the time module
is available so time.perf_counter() can be called.
- Around line 46-56: The _SpanContextManagerProtocol is defined but never
referenced; remove it to clean up dead code, or alternatively update the
_TracerProtocol.start_as_current_span signature to return
_SpanContextManagerProtocol instead of AbstractContextManager[_SpanProtocol] so
the protocol is actually used; locate the unused class
_SpanContextManagerProtocol and either delete the class or change the
return/type annotation on _TracerProtocol.start_as_current_span to reference
_SpanContextManagerProtocol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bd261af2-15ae-4b3e-bc88-63a8abded809
📒 Files selected for processing (17)
leap0/_async/client.pyleap0/_async/code_interpreter.pyleap0/_async/desktop.pyleap0/_async/filesystem.pyleap0/_async/sandbox.pyleap0/_sync/_transport.pyleap0/_sync/code_interpreter.pyleap0/_sync/desktop.pyleap0/_sync/pty.pyleap0/_utils/otel.pytests/_async/test_code_interpreter.pytests/_async/test_filesystem.pytests/_async/test_sandbox_proxy.pytests/_sync/test_code_interpreter.pytests/_sync/test_desktop.pytests/_sync/test_pty.pytests/_sync/test_transport.py
✅ Files skipped from review due to trivial changes (4)
- tests/_async/test_sandbox_proxy.py
- tests/_sync/test_pty.py
- leap0/_sync/pty.py
- leap0/_async/desktop.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/_sync/test_transport.py
- tests/_async/test_filesystem.py
- leap0/_async/client.py
Comment on lines
+376
to
+401
| @intercept_errors("Failed to grep: ") | ||
| async def grep(self, sandbox: SandboxRef, *, path: str, pattern: str, include: str | None = None, exclude: list[str] | None = None, http_timeout: float | None = None) -> list[SearchMatch]: | ||
| """Search for a text pattern across files in a directory. | ||
|
|
||
| Args: | ||
| sandbox: Sandbox ID or object. | ||
| path: Base directory to search from. | ||
| pattern: Text pattern to search for. | ||
| include: File pattern filter (e.g. ``"*.py"``). | ||
| exclude: Glob patterns to exclude. | ||
|
|
||
| http_timeout: Optional HTTP request timeout in seconds for this SDK call. | ||
|
|
||
| Args: | ||
| http_timeout: Optional HTTP request timeout in seconds for this SDK call. | ||
|
|
||
| Returns: | ||
| list[SearchMatch]: Matching lines with file and line metadata. | ||
| """ | ||
| payload: JsonObject = {"path": path, "pattern": pattern} | ||
| if include is not None: | ||
| payload["include"] = include | ||
| if exclude is not None: | ||
| payload["exclude"] = exclude | ||
| data = cast(GrepResponseDict, await self._transport.request_json("POST", f"/v1/sandbox/{sandbox_id_of(sandbox)}/filesystem/grep", json=payload, timeout=http_timeout)) | ||
| return [SearchMatch.from_dict(item) for item in data.get("items", [])] |
There was a problem hiding this comment.
Duplicate Args: section in grep() docstring.
The docstring contains two Args: sections (lines 380-387 and 389-390), with http_timeout documented twice. This appears to be a copy-paste artifact.
📝 Proposed fix
Args:
sandbox: Sandbox ID or object.
path: Base directory to search from.
pattern: Text pattern to search for.
include: File pattern filter (e.g. ``"*.py"``).
exclude: Glob patterns to exclude.
-
- http_timeout: Optional HTTP request timeout in seconds for this SDK call.
-
- Args:
http_timeout: Optional HTTP request timeout in seconds for this SDK call.
Returns:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@leap0/_async/filesystem.py` around lines 376 - 401, The grep() docstring has
duplicated "Args:" sections with http_timeout documented twice; edit the grep
function's docstring to remove the redundant Args block so each parameter
(including http_timeout) is documented only once and the docstring remains a
single coherent section for arguments (look for the docstring inside the async
def grep(...) in leap0/_async/filesystem.py).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
leap0/_sync/desktop.py (1)
683-692:⚠️ Potential issue | 🟠 MajorOnly translate retryable failures into
Leap0TimeoutErrorhere.
status_stream()is decorated withintercept_errors(), so malformed payloads, explicit server error envelopes, and other permanent failures already arrive here asLeap0Errorwithretryable=False. The blanketexcept Leap0Errorstill re-labels those as timeouts, which makes immediate protocol/API failures look like a deadline expiry.🛠️ Proposed fix
try: _poll() except Leap0TimeoutError as exc: raise Leap0TimeoutError( f"Desktop did not become ready within {timeout:.0f}s: {exc}" ) from exc except Leap0Error as exc: - raise Leap0TimeoutError( - f"Desktop did not become ready within {timeout:.0f}s: {exc}" - ) from exc + if exc.retryable: + raise Leap0TimeoutError( + f"Desktop did not become ready within {timeout:.0f}s: {exc}" + ) from exc + raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/desktop.py` around lines 683 - 692, The except block that catches Leap0Error around the call to _poll() should only convert retryable errors into Leap0TimeoutError: check the caught exception's retryable attribute (exc.retryable) in the except Leap0Error as exc handler and if True raise Leap0TimeoutError(f"Desktop did not become ready within {timeout:.0f}s: {exc}") from exc; if False simply re-raise the original Leap0Error so permanent/protocol failures (from status_stream() via intercept_errors()) are not mislabeled as timeouts. Ensure you reference the same exception classes Leap0Error and Leap0TimeoutError and the _poll() call when making this change.
🧹 Nitpick comments (1)
leap0/_sync/code_interpreter.py (1)
152-160: Addhttp_timeouttodelete_context()for parity with the rest of the client.This is the only context-management call that can't override the per-call timeout, so deletes fall back to the client default even when callers are explicitly budgeting other operations.
🛠️ Proposed fix
- def delete_context(self, sandbox: SandboxRef, context_id: str) -> None: + def delete_context( + self, + sandbox: SandboxRef, + context_id: str, + http_timeout: float | None = None, + ) -> None: @@ - self._request("DELETE", sandbox, f"/contexts/{context_id}", expected_status=204) + self._request( + "DELETE", + sandbox, + f"/contexts/{context_id}", + expected_status=204, + http_timeout=http_timeout, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@leap0/_sync/code_interpreter.py` around lines 152 - 160, The delete_context method lacks a per-call http_timeout parameter, causing deletes to always use the client default; update delete_context(self, sandbox: SandboxRef, context_id: str, http_timeout: Optional[float] = None) to accept an http_timeout argument, document it in the docstring, and pass it through to the underlying _request call (self._request("DELETE", sandbox, f"/contexts/{context_id}", expected_status=204, http_timeout=http_timeout)); keep the `@intercept_errors` decorator and signature shape consistent with the other client methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@leap0/models/errors.py`:
- Around line 27-33: raise_api_error currently instantiates API errors with
retryable=False causing transient HTTP failures to be treated as non-retryable;
update the raise_api_error function (the factory that builds the API error
instances and any place creating Leap0RateLimitError) to set retryable=True for
status codes that are transient (specifically 429 and 5xx responses, i.e.
status_code == 429 or 500 <= status_code < 600) so the constructed error objects
reflect their retryable nature and downstream code like
DesktopClient.wait_until_ready will back off correctly.
---
Duplicate comments:
In `@leap0/_sync/desktop.py`:
- Around line 683-692: The except block that catches Leap0Error around the call
to _poll() should only convert retryable errors into Leap0TimeoutError: check
the caught exception's retryable attribute (exc.retryable) in the except
Leap0Error as exc handler and if True raise Leap0TimeoutError(f"Desktop did not
become ready within {timeout:.0f}s: {exc}") from exc; if False simply re-raise
the original Leap0Error so permanent/protocol failures (from status_stream() via
intercept_errors()) are not mislabeled as timeouts. Ensure you reference the
same exception classes Leap0Error and Leap0TimeoutError and the _poll() call
when making this change.
---
Nitpick comments:
In `@leap0/_sync/code_interpreter.py`:
- Around line 152-160: The delete_context method lacks a per-call http_timeout
parameter, causing deletes to always use the client default; update
delete_context(self, sandbox: SandboxRef, context_id: str, http_timeout:
Optional[float] = None) to accept an http_timeout argument, document it in the
docstring, and pass it through to the underlying _request call
(self._request("DELETE", sandbox, f"/contexts/{context_id}",
expected_status=204, http_timeout=http_timeout)); keep the `@intercept_errors`
decorator and signature shape consistent with the other client methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5d3618a-b0d7-4069-bc6d-631d809f3f78
📒 Files selected for processing (10)
leap0/_async/filesystem.pyleap0/_sync/_transport.pyleap0/_sync/code_interpreter.pyleap0/_sync/desktop.pyleap0/_utils/errors.pyleap0/_utils/otel.pyleap0/models/errors.pytests/_sync/test_desktop.pytests/_sync/test_transport.pytests/models/test_errors.py
✅ Files skipped from review due to trivial changes (2)
- leap0/_utils/otel.py
- leap0/_sync/_transport.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/_sync/test_desktop.py
- tests/models/test_errors.py
- tests/_sync/test_transport.py
This was referenced Apr 1, 2026
Merged
Merged
Merged
This was referenced Apr 13, 2026
Merged
Merged
This was referenced Apr 22, 2026
Merged
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
1 participant
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.
Summary by CodeRabbit
New Features
Instrumentation
Documentation
Tests