SDK integration tests, auto-generated docs, and snapshot support - #1242
Merged
Conversation
…ypes from meta pkg - New `cua_sandbox/runtime/compat.py` with `check_local_support(image) -> RuntimeSupport` that detects whether the image's runtime is installed/auto-installable and whether hardware acceleration (HVF, KVM, Hyper-V) is available for the host/guest OS+arch combo - `Image.local_support()` method delegates to check_local_support (lazy import, no circulars) - `skip_if_unsupported(image)` pytest helper replaces all hardcoded skipif/env-var gates - `cua_sandbox/__init__` exports RuntimeSupport, check_local_support, skip_if_unsupported - `cua` meta package: eager compat re-export, lazy __getattr__ for all runtime classes (DockerRuntime, QEMURuntime, LumeRuntime, AndroidEmulatorRuntime, HyperVRuntime, RuntimeInfo) and interface types (Shell, CommandResult, Mouse, Keyboard, Screen, Clipboard, Tunnel, TunnelInfo, Mobile, Terminal, Window) — no import-time overhead - `cua/pyproject.toml`: add uv.sources pointing to local editable siblings - Updated test_android_multitouch.py to use skip_if_unsupported instead of manual checks Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…CG on Apple Silicon QEMU supports HVF (-accel hvf) for x86_64 guests on Intel Macs. On Apple Silicon, x86_64 guests must use TCG software emulation (no cross-arch HVF). - Replace _has_hvf() with _has_hvf_for_arm64_guest / _has_hvf_for_x86_guest - Add _x86_guest_hw_accel() covering KVM (Linux), HVF (Intel Mac), Hyper-V (Windows) - Linux VM and Windows VM sections now use _x86_guest_hw_accel() - Android: Intel Mac gets HVF for x86_64 Android system images - Apple Silicon correctly reported as software-only for Windows/Linux VMs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- tests/test_interfaces.py — 117 tests covering every sb.* interface method (shell, clipboard, screen, mouse, keyboard, tunnel, terminal, window, mobile) across Linux container, Linux VM, macOS VM, Windows VM, Android VM - tests/test_image_builder.py — 37 tests covering every Image builder method (apt/brew/choco/winget/apk/pwa/pip/uv install, run, env, copy, expose, from_registry) across all guest OS types - tests/pytest.ini — add pythonpath so `from cua import ...` resolves - scripts/gen_interface_docs.py — auto-generate interfaces.mdx from pydocstrings - docs/.../interfaces.mdx — regenerated from source (replaces AI-generated copy) All tests use local=True + skip_if_unsupported(image) — no hardcoded skips. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SSH sessions on macOS often have a minimal PATH (/usr/bin:/bin:/usr/sbin:/sbin) that omits /usr/local/bin where OrbStack/Docker Desktop install the docker CLI. _has_docker() now falls back through common install locations so Linux tests don't incorrectly skip on machines where Docker is installed but not in PATH. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… modules Instead of reimplementing binary/path detection in compat.py (which diverged from the actual runtime logic), each _has_*() probe now calls into the runtime module that owns that detection: _has_docker() → docker._has_docker() (moved multi-path logic here) _has_lume() → lume._has_lume() _has_qemu() → qemu_installer.qemu_bin() (now finds Homebrew/MacPorts/cached) _has_android_sdk()→ android_emulator._sdk_path() _has_java() → android_emulator._java_env() _has_hyperv() → hyperv._has_hyperv() Also promotes docker._has_docker() to use the same multi-path probe that compat.py introduced, so the runtime itself finds Docker in stripped SSH PATH environments (e.g. OrbStack on macOS via SSH). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All subprocess calls in DockerRuntime now use _docker_bin() which probes common install locations (/usr/local/bin, /opt/homebrew/bin, etc.) so the runtime works in SSH sessions where PATH is stripped (e.g. cua.local). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tarts DockerRuntime.start() was missing layer execution — apt_install, pip_install, run, env, etc. layers were stored in image._layers but never applied for Docker containers (unlike VMs where create_session_disk() handles this). Now uses LayerExecutor to apply layers via the running computer-server API. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The computer-server /cmd endpoint expects params as {"params": {"command": ...}}
not top-level "command_args". Also fix result parsing to check "return_code"
(server field name) before "returncode", and use "success" flag for failure
detection when return_code is absent (e.g. on handler exceptions).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- executor: fix run/env layers to use sudo for system access - executor: fix uv_install for Linux (uv pip install --system) - executor: fix pip_install for Linux (--break-system-packages) - executor: implement copy layer via computer-server write_bytes - executor: implement expose layer as no-op (handled at docker run) - executor: add os_type param to select correct commands per platform - docker: handle image._env and image._files after container starts - docker: map image._ports as additional -p flags at docker run time Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Environment variables from image.env() are now passed as -e KEY=VAL at docker run time, ensuring they're available in subprocess shells. Previously they were written to /etc/environment post-start which doesn't work since subprocesses don't source that file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…om_registry - docker: apply _files before _layers so copy → run(chmod) ordering works - docker: write env vars to /etc/profile.d/cua-env.sh for sudo access - docker: fix expose port range (port to port+1000 not 8000-9000) - executor: source /etc/profile.d/cua-env.sh in run layer bash invocation - tests: fix from_registry tests to use cua image (needs computer-server) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… expose test - executor: write copy files to /tmp first then sudo mv for root-owned paths - docker: write env profile via sudo tee (direct write_bytes can't write to /etc/profile.d) - tests: simplify expose test — just verify container starts, not socket internals Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Lume's Swift HTTP server closes the TCP connection after sending a 4xx
response (e.g. VM-not-found on GET /lume/vms/{name}). When httpx reuses
that connection for the subsequent POST /lume/pull/start, the body arrives
empty at the server, causing 'Invalid request body' 400 errors.
Fix: open a fresh AsyncClient for the pull POST instead of reusing the
client that made the initial VM-status GET.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LumeRuntime.start() was returning without applying _layers, _env, or _files — unlike DockerRuntime which runs LayerExecutor after is_ready(). Add _apply_image_layers() helper and call it in both start() return paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- sandbox.destroy(): call runtime.delete() instead of stop() for ephemeral sandboxes, so lume VMs are permanently removed (not just stopped) - executor._exec_run: don't use sudo on macOS/Android VMs — macOS VMs require a password for sudo; only Linux containers have passwordless sudo Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
macOS lume VMs have the default password 'lume'. Pipe it to sudo -S so run layers and env var setup can write to system paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Lume's custom HTTP server uses minimumIncompleteLength=1, so on a fresh TCP connection the request body can arrive after the first receive() returns, causing a spurious 400. Add _pull_start_with_retry() that retries up to 3 times with a 1s pause between attempts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tart Writing to /etc/profile.d/cua-env.sh is only sourced by login shells on macOS. Instead write to ~/.zshenv (sourced by all zsh invocations) and set via launchctl setenv, then restart computer-server so it inherits the new environment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
computer-server's launchd plist has an explicit EnvironmentVariables dict, so launchctl setenv is ignored. Use PlistBuddy to add/set vars directly in the plist, then unload/load the service to pick them up. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Running launchctl unload from inside computer-server kills it before the load step runs. Use lume ssh from the host instead — it runs outside the VM so the unload+load sequence completes successfully. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
launchctl unload/load doesn't work correctly for Aqua session agents from non-GUI SSH sessions. Use bootout/bootstrap which properly handles the GUI session context (gui/<uid>). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
macOS VMs need 'echo lume | sudo -S' for passwordless sudo. Apply this to mkdir/mv in copy layers and to /etc/environment writes in env layers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of pulling (decompressing ~30GB) for every ephemeral VM, pull once into a stopped golden base VM and then APFS-clone it for each use. Clone is instant via clonefile(2); subsequent test runs skip the ~155s decompression step entirely. Also adds CheckpointInfo dataclass and checkpoint/fork/ensure_base primitives to the Runtime base class, forming the foundation for a user-facing snapshot/fork API. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The custom NWConnection server read with minimumIncompleteLength=1, causing 'Invalid request body' whenever TCP delivered the POST body in a second segment (common after several connections). Replace with ServerBootstrap + NIOHTTP1 pipeline, which handles Content-Length / chunked reassembly correctly before dispatching to route handlers. No more spurious 400s. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Bridge ChannelHandlerContext back via EventLoopPromise so it never crosses actor boundaries (fixes 'sending ctx risks data races') - Remove @mainactor from Server (handlers create LumeController locally) - Bump swift-sdk from 0.10.0 to 0.12.0 which fixes the data race in NetworkTransport that broke clean builds under Swift 6.3 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- In _apply_layers, write image._env to /data/local/tmp/.cua_env on the device so env vars survive across independent adb shell invocations - Prefix every adb transport shell command with `. /data/local/tmp/.cua_env 2>/dev/null` - Update test_apk_install to use F-Droid APK URL instead of skipping Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix env var quoting by writing to a local tempfile and pushing via `adb push` instead of shell string escaping - test_pwa_install now uses android-example-gym-pwa-app manifest URL instead of requiring ANDROID_TEST_PWA_URL env var Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tall - Source .cua_env in GRPCEmulatorTransport shell commands (this is the actual transport used by ephemeral Android sandboxes, not ADBTransport) - Augment PATH with /opt/homebrew/bin so node/npm are found when the process is launched without a login shell - test_pwa_install downloads keystore from android-example-gym-pwa-app repo Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add docs/content/docs/cua/guide/sandbox/snapshots.mdx covering the
snapshot API, use cases, and performance characteristics
- Fix gen_interface_docs.py: single-brace import, escape {}<> outside
code spans with HTML entities instead of backslashes
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces snapshot support for Cua sandboxes, allowing users to capture running sandbox states as reusable images. It integrates SwiftNIO into the Lume HTTP server, extends the Python SDK with runtime compatibility detection and multi-OS layer execution support, and implements new checkpoint/fork runtime APIs for managing snapshots. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant NIO as NIO<br/>ServerBootstrap
participant Handler as HTTPChannel<br/>Handler
participant Router as Route<br/>Handler
participant App as Application<br/>Logic
Client->>NIO: TCP connect
NIO->>Handler: channel active
Client->>Handler: send HTTP request bytes
Handler->>Handler: decode with NIOHTTP1
Handler->>Handler: accumulate body
Handler->>Router: handleRequest(HTTPRequest)
Router->>App: route to handler (async)
App->>App: execute handler
App-->>Router: response result
Router->>Handler: write response
Handler->>Client: HTTP response bytes
sequenceDiagram
participant User
participant Sandbox as Sandbox
participant Transport as Cloud<br/>Transport
participant API as Cloud API
participant Runtime as Runtime
participant NewImage as Image
User->>Sandbox: snapshot(name, stateful)
Sandbox->>Transport: create_snapshot(name, stateful)
Transport->>API: POST /v1/vms/{vm}/snapshot
API-->>Transport: image descriptor
Transport-->>Sandbox: image dict
Sandbox->>NewImage: Image(_snapshot_source=descriptor, ...)
NewImage-->>User: Image object
User->>Sandbox: Sandbox.create(image=NewImage)
Sandbox->>Transport: _create_vm(source="snapshot", ...)
Transport->>API: POST /v1/vms (fork from snapshot)
API-->>Transport: new VM details
Transport->>Runtime: apply layers to new VM
Runtime-->>Sandbox: running sandbox (reusing installed state)
Sandbox-->>User: Sandbox instance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…d eventLoopGroup cleanup Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📦 Publishable packages changed
Add |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
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 (2)
libs/python/cua-sandbox/cua_sandbox/interfaces/mouse.py (1)
35-42:⚠️ Potential issue | 🔴 CriticalQMPTransport._drag does not support the new path payload format and will fail at runtime.
The
Mouse.drag()method sendspath=[[start_x, start_y], [end_x, end_y]], butQMPTransport._drag(line 298) expects separatestart_x,start_y,end_x,end_yparameters. Whensend("drag", path=..., button=...)dispatches to_drag, the required parameters will be missing, causing aTypeError.
LocalTransporthandles both formats correctly (checks for"path"in params and extracts coordinates), butQMPTransport._draghas no such conversion logic. Update_dragto accept and unpack thepathparameter:async def _drag( self, path: list | None = None, start_x: int | None = None, start_y: int | None = None, end_x: int | None = None, end_y: int | None = None, button: str = "left", **_: Any ) -> None: if path: start_x, start_y = path[0] end_x, end_y = path[-1] # ... rest of implementation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/interfaces/mouse.py` around lines 35 - 42, QMPTransport._drag currently expects start_x/start_y/end_x/end_y and will fail when Mouse.drag sends path=[[...],[...]]; update the _drag method signature to accept a path parameter (and optional start_x/start_y/end_x/end_y) and, at the start of _drag, if path is provided unpack start and end coordinates from path[0] and path[-1] respectively before continuing with the existing implementation; ensure the method still accepts button and ignores extra kwargs so it remains compatible with send dispatching.libs/python/cua-sandbox/cua_sandbox/runtime/android_emulator.py (1)
503-510:⚠️ Potential issue | 🟠 MajorPass augmented PATH to both npm subprocess and post-install lookup.
Line 503's
subprocess.run()lacksenv=env, so the npm process inherits the original PATH instead of the augmented one. Line 510'sshutil.which("bubblewrap")also omits thepathparameter, causing it to search the original PATH. On stripped-PATH hosts, this inconsistency allows the install to succeed while the subsequent lookup fails.Suggested fix
subprocess.run( [npm, "install", "-g", "@bubblewrap/cli"], check=True, capture_output=True, timeout=300, + env=env, ) - bw = shutil.which("bubblewrap") + bw = shutil.which("bubblewrap", path=env["PATH"])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/runtime/android_emulator.py` around lines 503 - 510, The npm install subprocess call and the subsequent bubblewrap lookup are using the original PATH instead of the augmented env; update the subprocess.run invocation that installs "@bubblewrap/cli" to include env=env (the same env you build/augment) and change the shutil.which("bubblewrap") call to pass the augmented path (e.g., shutil.which("bubblewrap", path=env["PATH"]) or env.get("PATH")) so both the install and the post-install lookup use the same modified PATH.
🟠 Major comments (18)
libs/lume/src/Server/Server.swift-126-126 (1)
126-126:⚠️ Potential issue | 🟠 Major
@unchecked Sendablerequires thread-safe access to mutable state.
Serveris marked@unchecked SendablebutserverChannelis accessed without synchronization—written instart()(line 427) and read instop()(line 440). Ifstop()is called from a different thread/task beforestart()assigns the channel, this creates a data race.🔒 Option 1: Use a lock for serverChannel access
+import NIOConcurrencyHelpers final class Server: `@unchecked` Sendable { // ... - private var serverChannel: (any Channel)? + private let serverChannelLock = NIOLock() + private var _serverChannel: (any Channel)? + private var serverChannel: (any Channel)? { + get { serverChannelLock.withLock { _serverChannel } } + set { serverChannelLock.withLock { _serverChannel = newValue } } + }🔒 Option 2: Document single-threaded usage contract
If
stop()is guaranteed to only be called afterstart()returns or from the same isolation context, add a doc comment clarifying this constraint.Also applies to: 171-171
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/lume/src/Server/Server.swift` at line 126, Server is marked `@unchecked` Sendable but accesses to mutable serverChannel in start() and stop() are unsynchronized, risking a data race; add a synchronization primitive (e.g., a private NSLock or DispatchQueue property like serverChannelLock) to the Server class and wrap all reads/writes of serverChannel in lock() / unlock() (or sync on the queue) inside start() and stop(); alternatively, if you choose the single-threaded contract, replace `@unchecked` Sendable with a doc comment on Server documenting that stop() must only be called after start() returns (or from the same isolation) and ensure callers abide by that contract.libs/lume/Package.swift-17-17 (1)
17-17:⚠️ Potential issue | 🟠 MajorAddress breaking changes in swift-sdk 0.12.0.
The version bump from 0.10.0 to 0.12.0 includes breaking API changes aligned with the updated MCP specification. Review the impact of:
- Tool content representation changes (
Tool.Contentassociated values)- Resource field renamed from
metadatatosize- Prompt and completion context argument typing changed from
ValuetoString- Elicitation
requestedSchemais now requiredVerify current MCP product usage of these types is compatible with the changes before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/lume/Package.swift` at line 17, The package update to swift-sdk 0.12.0 introduces breaking API changes: update all usages of Tool.Content (adjust associated-value patterns/constructors to the new representation), rename any Resource.metadata accesses/fields to Resource.size, change prompt and completion context argument types from generic Value to String (update signatures and call sites for types like Prompt.init / CompletionContext or methods that accept context), and ensure every Elicitation instance supplies the now-required requestedSchema field; search for symbols Tool.Content, Resource.metadata, Prompt/Completion context parameters, and Elicitation.requestedSchema to locate and update each affected site accordingly.libs/python/cua-sandbox/cua_sandbox/image.py-123-123 (1)
123-123:⚠️ Potential issue | 🟠 MajorPreserve
_snapshot_sourcewhen cloning anImage.Line 123 adds the snapshot descriptor, but
_add_layer()and_with()do not copy it forward. Anysnapshot_image.run(...),.env(...),.copy(...), or.expose(...)call silently drops the snapshot source, soCloudTransportstops treating the result as a forkable snapshot image.Suggested fix
def _add_layer(self, layer: Dict[str, Any]) -> Image: return Image( os_type=self.os_type, distro=self.distro, version=self.version, kind=self.kind, _layers=self._layers + (layer,), _env=self._env, _ports=self._ports, _files=self._files, _registry=self._registry, _disk_path=self._disk_path, _agent_type=self._agent_type, + _snapshot_source=self._snapshot_source, ) def _with(self, **kwargs) -> Image: """Return a new Image with specific fields overridden.""" fields = { "os_type": self.os_type, "distro": self.distro, "version": self.version, "kind": self.kind, "_layers": self._layers, "_env": self._env, "_ports": self._ports, "_files": self._files, "_registry": self._registry, "_disk_path": self._disk_path, "_agent_type": self._agent_type, + "_snapshot_source": self._snapshot_source, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/image.py` at line 123, The _snapshot_source attribute added to Image is not preserved when creating modified Image instances, so methods that build on an image (notably _add_layer and _with, which back run, env, copy, expose) drop _snapshot_source causing CloudTransport to lose snapshot semantics; update _add_layer() and _with() to copy the original instance’s _snapshot_source into the new Image before returning (i.e., if self._snapshot_source is set, set new_image._snapshot_source = self._snapshot_source) so all chainable methods (run, env, copy, expose) created via those helpers retain the snapshot descriptor.libs/python/cua-sandbox/cua_sandbox/sandbox.py-297-301 (1)
297-301:⚠️ Potential issue | 🟠 MajorPopulate
os_typefrom OS metadata, not fromkind.Line 298 currently turns descriptors like
{"kind": "vm"}intoImage(os_type="vm"). That misclassifies the snapshot image for OS-gated builder methods and compatibility checks, and the current constructor also dropskindentirely when_transport._imageis missing.Suggested fix
return ImageCls( - os_type=image_desc.get("kind", src_image.os_type if src_image else "linux"), - distro=src_image.distro if src_image else "ubuntu", - version=src_image.version if src_image else "24.04", + os_type=( + image_desc.get("os_type") + or image_desc.get("os") + or (src_image.os_type if src_image else "linux") + ), + distro=image_desc.get("distro", src_image.distro if src_image else "ubuntu"), + version=image_desc.get("version", src_image.version if src_image else "24.04"), + kind=image_desc.get("kind", src_image.kind if src_image else "vm"), _snapshot_source=image_desc, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/sandbox.py` around lines 297 - 301, The code is using image_desc.get("kind") to populate ImageCls.os_type which misclassifies images; change the os_type assignment to read OS metadata from the descriptor (e.g. image_desc.get("os", {}).get("type") or image_desc.get("os_type")) and only fallback to src_image.os_type or "linux" if that metadata is absent, leaving image_desc unchanged for _snapshot_source; update the ImageCls(...) call so os_type= image_desc.get("os", {}).get("type", image_desc.get("os_type", src_image.os_type if src_image else "linux")) instead of using image_desc.get("kind").libs/python/cua-sandbox/cua_sandbox/runtime/docker.py-198-225 (1)
198-225:⚠️ Potential issue | 🟠 MajorTear the container down if post-start provisioning fails.
Lines 198-225 add several new failure points after
docker runsucceeds. If env propagation, file copy, or a build layer errors,start()raises before aSandboxexists, and the half-provisioned container is left running.Suggested fix
await self.is_ready(info) - # Apply image layers and files via computer-server - env_items = getattr(image, "_env", ()) - file_items = getattr(image, "_files", ()) - has_work = image._layers or file_items - if has_work or env_items: - from cua_sandbox.builder.executor import LayerExecutor - - executor = LayerExecutor(f"http://{info.host}:{info.api_port}", os_type=image.os_type) - - # Write env vars to a sourceable profile script so run layers can access them - if env_items and image.os_type != "windows": - ... - - # Apply files before layers so later run layers can reference copied files - for src, dst in file_items: - await executor.execute_layers([{"type": "copy", "src": src, "dst": dst}]) - - if image._layers: - await executor.execute_layers(list(image._layers)) + try: + # Apply image layers and files via computer-server + env_items = getattr(image, "_env", ()) + file_items = getattr(image, "_files", ()) + has_work = image._layers or file_items + if has_work or env_items: + from cua_sandbox.builder.executor import LayerExecutor + + executor = LayerExecutor( + f"http://{info.host}:{info.api_port}", + os_type=image.os_type, + ) + + # Write env vars to a sourceable profile script so run layers can access them + if env_items and image.os_type != "windows": + ... + + for src, dst in file_items: + await executor.execute_layers([{"type": "copy", "src": src, "dst": dst}]) + + if image._layers: + await executor.execute_layers(list(image._layers)) + except Exception: + subprocess.run([docker, "rm", "-f", name], capture_output=True) + raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/runtime/docker.py` around lines 198 - 225, The new provisioning steps (env/file/layers) in start() can fail after docker run succeeds, leaving a half-provisioned container running; wrap the block that creates LayerExecutor and runs env/file/layer work in a try/except and on any exception ensure the just-started container is torn down before re-raising. Specifically, around the code that instantiates LayerExecutor and uses executor.run_command/execute_layers, catch exceptions and call the existing cleanup routine used elsewhere (e.g., the Sandbox teardown/stop method or the internal container removal helper—refer to the instance field that stores the started container like self.container_id or self._container and call the matching cleanup method such as self.teardown()/self.stop()/self._remove_container(...) before raising), then re-raise the error.libs/python/cua-sandbox/tests/test_android_multitouch.py-146-146 (1)
146-146:⚠️ Potential issue | 🟠 MajorMove the compatibility gate out of the session fixture.
Line 146 executes when pytest instantiates
local_android_sb, which happens because_reset_between_tests(autouse=True) has it as a direct parameter. On hosts without local Android support, this causes the entire test session to skip—including cloud tests that have their own API-key gate and don't depend on local emulator support.Move
skip_if_unsupported(Image.android(str(_API_LEVEL)))to thesbfixture inTestAndroidMultitouchLocalonly, or refactor_reset_between_teststo requestlocal_android_sblazily (e.g., viarequest.getfixturevalue("local_android_sb")) so it's only resolved when actually needed by local tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/tests/test_android_multitouch.py` at line 146, The compatibility gate skip_if_unsupported(Image.android(str(_API_LEVEL))) is being evaluated during pytest fixture instantiation (because _reset_between_tests directly depends on local_android_sb), causing the whole session to skip on hosts without local Android; fix by moving that call out of the session-level fixture: either add skip_if_unsupported(Image.android(str(_API_LEVEL))) inside the sb fixture of TestAndroidMultitouchLocal (so it only runs for local tests) or change _reset_between_tests to obtain local_android_sb lazily (use request.getfixturevalue("local_android_sb") inside the fixture body) so local_android_sb is only resolved when a test actually needs it. Ensure references to skip_if_unsupported, Image.android, local_android_sb, _reset_between_tests, and the sb fixture are updated accordingly.libs/python/cua-sandbox/cua_sandbox/runtime/docker.py-207-218 (1)
207-218:⚠️ Potential issue | 🟠 MajorUse
shlex.quote()to safely emit environment variables to/etc/profile.d/cua-env.sh.Lines 213–217 only escape single quotes in double-quoted strings. Values containing
$(),$VAR, backticks, or newlines will be re-evaluated or mangled when the script is sourced. Useshlex.quote()to properly escape arbitrary strings, and validate variable names withre.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", k)to prevent injection.Note: The same vulnerability exists in
libs/python/cua-sandbox/cua_sandbox/runtime/lume.pyat line 403.Suggested fix
+ import re + import shlex for k, v in env_items: - safe_v = v.replace("'", "'\\''") + if not re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", k): + raise ValueError(f"Invalid environment variable name: {k!r}") + line = f"export {k}={shlex.quote(v)}" await executor.run_command( - f"printf 'export {k}=\"{safe_v}\"\\n' " + f"printf '%s\\n' {shlex.quote(line)} " f"| sudo tee -a /etc/profile.d/cua-env.sh > /dev/null" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/runtime/docker.py` around lines 207 - 218, The current code in the block that writes env vars to /etc/profile.d/cua-env.sh (guarded by image.os_type != "windows" and using executor.run_command) manually escapes single quotes and is unsafe for values with $(), backticks, newlines, etc.; replace the manual escaping with Python's shlex.quote() for the value and ensure the variable name k is validated with re.fullmatch(r"[A-Za-z_][A-Za-z0-9_]*", k) before emitting it; keep writing via executor.run_command to handle sudo/tee but construct the exported line as export {k}={shlex.quote(v)} (and skip or raise on invalid names) and apply the same fix in the corresponding block in runtime/lume.py (around the noted line).libs/python/cua-sandbox/cua_sandbox/builder/executor.py-210-214 (1)
210-214:⚠️ Potential issue | 🟠 MajorQuote privileged copy destinations.
Lines 213-214 splice
dst_diranddstdirectly intomkdir/mv. A destination like/Applications/My App/binwill break, and shell metacharacters in the path become command injection._sh_quote()is already in this file; use it for both arguments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/builder/executor.py` around lines 210 - 214, The current commands build shell strings by splicing dst_dir, dst and tmp_path directly which breaks on spaces and allows injection; update the block in executor.py to use the existing _sh_quote() helper for any user-supplied path when calling run_command: quote dst_dir for the mkdir -p invocation and quote both tmp_path and dst for the mv invocation (keep using the computed sudo variable and the run_command method). Ensure you still guard the mkdir call with the dst_dir and "/" checks, but pass the quoted path to run_command rather than the raw variables.tests/test_interfaces.py-52-63 (1)
52-63:⚠️ Potential issue | 🟠 MajorThe tunnel helper is POSIX-only, but Windows reuses it.
python3 -c "... &"assumes a POSIX shell, background operator, and apython3binary in the guest.TestTunnelWindowscalls this helper unchanged, so those tests can fail before the tunnel code is exercised. Pick the bootstrap command per guest OS, or provision Python explicitly in the non-Linux images.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_interfaces.py` around lines 52 - 63, The helper _start_http_server currently invokes a POSIX-specific bootstrap (uses "python3 -c ... &") which breaks Windows tests like TestTunnelWindows; update _start_http_server to choose the bootstrap command based on the guest OS or provide an explicit Windows-compatible command when called from TestTunnelWindows: detect the guest OS from the sandbox API or add a parameter (e.g., shell_type or is_windows) to _start_http_server, and when Windows is detected use a PowerShell/start-process or cmd /C start /B variant (or install/provision python in the Windows image) instead of the POSIX background "&" and "python3" binary; update the TestTunnelWindows invocation to pass the Windows flag if you add a parameter so the correct boot command is used.libs/python/cua-sandbox/cua_sandbox/transport/cloud.py-94-106 (1)
94-106:⚠️ Potential issue | 🟠 MajorDon't block on normal
.cua.shendpoints.Line 99 treats every
.cua.shURL as “not ready yet”, but_resolve_endpoint()already returns.cua.shfor the normal hosted reverse-proxy path. On prod this can add the full 120s wait beforeHTTPTransporteven tries to connect. Gate this loop behind a local-devbase_url/host check instead of the endpoint suffix itself.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/transport/cloud.py` around lines 94 - 106, The loop currently treats any .cua.sh endpoint as “not ready” and polls for 120s; change it to only poll when running against a local-dev host by guarding the loop with a local-dev base_url/host check (e.g., check self.base_url or self._base_url for localhost/127.0.0.1 or your dev domain) and otherwise skip the polling so production .cua.sh endpoints are not delayed; keep using _resolve_endpoint(vm_info), _get_vm(self._name) and let _wait_for_server_ready handle errors as before.libs/python/cua-sandbox/cua_sandbox/builder/executor.py-174-185 (1)
174-185:⚠️ Potential issue | 🟠 MajorDon't interpolate raw env text into
/etc/environment.Line 184 uses an unquoted heredoc around raw
k/vstrings. Quotes,$VAR, backticks, or newlines in user input will either expand during the shell write or leave/etc/environmentmalformed. Validate the key and escape the persisted content before writing it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/builder/executor.py` around lines 174 - 185, _in _exec_env_: do not interpolate raw keys/values into an unquoted heredoc; instead validate each env key (e.g. enforce pattern like ^[A-Z_][A-Z0-9_]*$), reject or fail on invalid keys, and sanitize/escape values (e.g. replace or fail on raw newlines, and escape quotes/backslashes) before persisting. When writing to /etc/environment use a non-expanding heredoc (<<'EOF') or write with a safe printf/append approach run under sudo to avoid shell expansion; update the construction around variables, lines and cmd in _exec_env to validate keys, escape values, and use a quoted heredoc or printf to safely append KEY="escaped_value" entries.libs/python/cua-sandbox/tests/test_snapshots.py-31-66 (1)
31-66:⚠️ Potential issue | 🟠 MajorMeasure “create+install” after the install finishes.
Line 34 captures
t_createimmediately after boot, so Line 64 compares fork time against raw VM startup even though the assertion message says “create+install”. That makes the test fail for the wrong reason when fork is slower than boot but still faster than boot+package install.Suggested fix
async with Sandbox.ephemeral(Image.linux("ubuntu", "24.04")) as sb: - t_create = time.monotonic() - t_create_start - # Install something unique result = await sb.shell.run( "apt-get update -qq && apt-get install -y -qq cowsay", timeout=120 ) assert result.success, f"Install failed: {result.stderr}" @@ result = await sb.shell.run("/usr/games/cowsay hello") assert result.success assert "hello" in result.stdout + t_create = time.monotonic() - t_create_start🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/tests/test_snapshots.py` around lines 31 - 66, The test measures t_create too early; move the t_create = time.monotonic() - t_create_start assignment to after the install and verification steps so t_create reflects "create+install" instead of just VM startup. Specifically, in the Sandbox.ephemeral block that uses t_create_start, set t_create after the sb.shell.run calls and the assertions that verify cowsay (i.e., after the install result/asserts and the "/usr/games/cowsay hello" check), before calling await sb.snapshot(...), so the later comparison with t_fork correctly compares fork time to create+install time.libs/python/cua-sandbox/cua_sandbox/runtime/compat.py-419-437 (1)
419-437:⚠️ Potential issue | 🟠 MajorMark Android unsupported when Java is missing.
When
java_okis false, Line 434 still returnssupported=True.skip_if_unsupported(Image.android())therefore won't skip, but the emulator start path immediately fails in_java_env().supportedshould stay false unless the host can actually boot or auto-install the remaining pieces.Suggested fix
return RuntimeSupport( - supported=True, # SDK auto-installs on macOS/Linux if Java is present + supported=java_ok, hw_accel=hw, runtime_installed=installed, auto_installable=auto and java_ok, runtime_name="android_emulator",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/runtime/compat.py` around lines 419 - 437, The code currently returns RuntimeSupport(supported=True) even when java_ok is False; update the supported flag to reflect actual boot capability by passing supported=java_ok (so Android is marked unsupported when Java is missing), keeping the existing hw_accel, runtime_installed, and auto_installable logic (references: java_ok, sdk_ok, installed, auto, RuntimeSupport).libs/python/cua/cua/__init__.py-46-55 (1)
46-55:⚠️ Potential issue | 🟠 MajorDon't export compat helpers as
None.If
cua_sandbox.runtime.compatis absent,from cua import skip_if_unsupportedstill succeeds because the name exists in the module and__all__, but callers only discover the problem whenNoneis invoked. Either omit these exports in the fallback path or replace them with stubs that raiseImportError.Also applies to: 119-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua/cua/__init__.py` around lines 46 - 55, The module currently assigns RuntimeSupport, check_local_support, and skip_if_unsupported to None when cua_sandbox.runtime.compat is missing, causing silent import success; instead, change the fallback so these names are not exported (remove them from __all__) or replace them with stubs that immediately raise ImportError when called/instantiated; update the same treatment for the additional compatibility names referenced later (the other symbols you set to None around the later block), and ensure __all__ only lists names that are actually usable so importing from the package fails fast with a clear ImportError.libs/python/cua-sandbox/cua_sandbox/runtime/lume.py-188-216 (1)
188-216:⚠️ Potential issue | 🟠 MajorCheckpoint listing is too broad.
checkpoint()lets callers choose anycheckpoint_name, sostatus == "suspended"is not enough to identify checkpoints. As written,list_checkpoints()will surface ordinary stopped sandboxes, anddelete_checkpoint()can then delete them.libs/python/cua-sandbox/cua_sandbox/runtime/lume.py-372-377 (1)
372-377:⚠️ Potential issue | 🟠 MajorTreat env values as literals, not shell fragments.
Both branches interpolate
kandvdirectly into shell/PlistBuddy commands. Even benign values like$HOMEorfoo barstop being literals here; worse, command substitutions/newlines can write broken config or execute unintended shell when/etc/profile.d/cua-env.shis later sourced bylibs/python/cua-sandbox/cua_sandbox/builder/executor.py:109-125. Validate variable names and emit properly quoted literals instead of composing shell with raw strings.Also applies to: 402-406
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/runtime/lume.py` around lines 372 - 377, The loop that builds and passes PlistBuddy commands currently interpolates env key/value directly into a shell string (see env_items loop and executor.run_command calls), which allows shell/meta characters to be interpreted; validate keys with a strict regex like r'^[A-Za-z_][A-Za-z0-9_]*$' and reject or sanitize invalid names, and stop composing raw shell fragments for values—emit properly quoted literals (e.g., use shlex.quote on keys/values if executor.run_command must receive a shell string) or, better, call PlistBuddy via a safe API (pass argv lists to subprocess/ executor.run_command if it supports lists) or write the EnvironmentVariables dict using plistlib to avoid shell entirely; apply the same fixes to the similar block at lines 402-406.libs/python/cua-sandbox/cua_sandbox/runtime/lume.py-149-154 (1)
149-154:⚠️ Potential issue | 🟠 MajorRequire the cached base VM to be stopped.
The docstring says the base cache is a stopped golden VM, but this branch also accepts
running. The next step clones from that base, so reusing a live base can produce dirty clones or clone failures.Possible fix
if resp.status_code == 200: vm = resp.json() - if vm.get("status") in ("stopped", "running"): + if vm.get("status") == "stopped": logger.info(f"Base VM '{base_name}' already exists — skipping pull") return CheckpointInfo(name=base_name, runtime_type="lume", created_at=time.time()) + if vm.get("status") == "running": + raise RuntimeError( + f"Base VM '{base_name}' is running; stop it before cloning" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/runtime/lume.py` around lines 149 - 154, The branch that treats an existing base VM as usable should only accept a stopped golden VM, not "running": change the condition that currently checks vm.get("status") in ("stopped", "running") to require equality to "stopped" so only stopped bases are reused; update the log/message to reflect that we are skipping pull because the base VM is stopped and return the same CheckpointInfo (references: client.get(...)/resp, vm.get("status"), base_name, and CheckpointInfo).libs/python/cua-sandbox/cua_sandbox/runtime/lume.py-86-109 (1)
86-109:⚠️ Potential issue | 🟠 MajorHandle existing stopped VMs before cloning a new one.
Only
runningis special-cased here. If/lume/vms/{name}already exists in a stopped state, this falls through tofork(base_name, name), which collides with the existing VM name instead of resuming or intentionally recreating it.Possible fix
if vm.get("status") == "running": logger.info(f"Lume VM {name} already running") ip = await self._wait_for_ip(name, lume_url) await self._deliver_vnc_config(name, lume_url) info = RuntimeInfo(host=ip, api_port=self.api_port, name=name) await self.is_ready(info) await self._apply_image_layers(image, info) return info + if vm.get("status") in ("stopped", "stop"): + logger.info(f"Lume VM {name} already exists but is stopped; resuming") + return await self.resume(image, name, **opts) oci_ref = image._registry or MACOS_VERSION_IMAGES.get(image.version or "") or MACOS_SEQUOIA🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua-sandbox/cua_sandbox/runtime/lume.py` around lines 86 - 109, The current fast-path only handles status "running" and ignores an existing VM in "stopped" state which causes fork(base_name, name) to collide; update the logic that checks vm = resp.json() so that if vm exists and vm.get("status") == "stopped" you treat it like a resume rather than cloning: call self._run_vm(name, lume_url, opts) to start the stopped VM, then follow the same post-start steps (await self._wait_for_ip(name, lume_url), await self._deliver_vnc_config(name, lume_url), construct RuntimeInfo, await self.is_ready(info), await self._apply_image_layers(image, info), and return info) instead of calling fork(base_name, name). Ensure you still keep the existing handling for "running" and the pull/clone path for non-existent VMs (functions referenced: _wait_for_ip, _deliver_vnc_config, _run_vm, ensure_base, fork, _apply_image_layers).
🟡 Minor comments (3)
docs/content/docs/cua/guide/sandbox/snapshots.mdx-47-49 (1)
47-49:⚠️ Potential issue | 🟡 MinorClarify that local snapshot support is not a public
sb.snapshot()flow yet.Line 48 says local sandboxes have "limited support", but
Sandbox.snapshot()still raisesNotImplementedErrorfor non-cloud transports inlibs/python/cua-sandbox/cua_sandbox/sandbox.pyLines 288-289. As written, this reads like a supported local user path when the public API still rejects it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/cua/guide/sandbox/snapshots.mdx` around lines 47 - 49, The doc text incorrectly implies local sandboxes have a usable snapshot flow; update the copy to clarify that local sandbox transports (Docker, Lume, QEMU) do not expose a public snapshot API yet because Sandbox.snapshot() currently raises NotImplementedError for non-cloud transports. Change the Callout to say snapshots are supported on Cua Cloud sandboxes and local sandbox snapshot functionality is experimental/internal or not available via the public Sandbox.snapshot() method for non-cloud transports (referencing Sandbox.snapshot() in cua_sandbox/sandbox.py), so users should rely on Cua Cloud for the public snapshot flow.tests/test_image_builder.py-367-389 (1)
367-389:⚠️ Potential issue | 🟡 MinorThis test doesn't prove the PWA was installed.
Lines 387-388 only assert that
pm list packagesexecuted successfully; that command returns 0 even when the target package is absent. Pass a deterministicpackage_nametopwa_install()and assert that exact package shows up in stdout so the test actually exercises the install path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_image_builder.py` around lines 367 - 389, The test test_pwa_install currently only checks that "pm list packages" ran successfully but not that the PWA was installed; update the Image.android().pwa_install call to pass a deterministic package_name (e.g., the expected app id) and then run pm list packages and assert that the package_name appears in the command stdout (use sb.shell.run(...).stdout or equivalent) instead of only asserting r.success so the test verifies the installed package. Reference Image.android().pwa_install, test_pwa_install, Sandbox.ephemeral and sb.shell.run when making the change.docs/content/docs/cua/reference/sandbox-sdk/interfaces.mdx-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorFix the encoding artifact in the generated description.
The replacement character
�will show up in page metadata/search snippets and suggests the generator is emitting invalid text. Since this page is generated, please fix the generator output rather than hand-editing the MDX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/docs/cua/reference/sandbox-sdk/interfaces.mdx` at line 3, The generated frontmatter description contains a replacement character '�' from the generator; fix scripts/gen_interface_docs.py so it emits valid UTF-8 text: open all source files and templates with explicit encoding='utf-8', avoid using decode/encode with errors='replace' (use 'strict' or 'ignore' where appropriate), sanitize or normalize any non-ASCII characters (e.g. replace smart quotes or control chars) before writing, and ensure the description string written to the MDX frontmatter is the cleaned/normalized Unicode value rather than a fallback replacement character.
🧹 Nitpick comments (5)
libs/lume/src/Server/Server.swift (3)
47-50: HTTP headers with duplicate names are overwritten.HTTP allows multiple headers with the same name (e.g.,
Set-Cookie,Cookie). The current conversion to[String: String]keeps only the last value. Consider whether any routes need access to all header values.♻️ Alternative: Join duplicate header values
var headers: [String: String] = [:] for (name, value) in head.headers { - headers[name.description] = value + if let existing = headers[name.description] { + headers[name.description] = existing + ", " + value + } else { + headers[name.description] = value + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/lume/src/Server/Server.swift` around lines 47 - 50, The current conversion in Server.swift collects request headers into a [String: String] named headers by iterating head.headers and assigning headers[name.description] = value, which drops duplicate header names; change this to preserve all values (either by making headers a [String: [String]] and appending value to headers[name.description], or by joining duplicates with ", " for a [String: String] representation) so routes that need multiple values (e.g., Set-Cookie/Cookie) can access them; update any downstream consumers of headers accordingly (look for the headers variable and places that read it to adapt to the new shape or joined format).
57-63: Duplicate request/response logging.The request is logged here and again in
handleRequest()(line 446). Similarly, the response is logged inwriteResponse()(line 100) andhandleRequest()(line 460). Consider removing one set to reduce log noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/lume/src/Server/Server.swift` around lines 57 - 63, The request/response are being logged twice—once at the top of the request pipeline (the Logger.info that logs "Received request" with metadata including method/path/body) and again inside handleRequest(), and likewise responses are logged in writeResponse() and again in handleRequest(); remove the duplicate logging to reduce noise by keeping a single canonical log location: either remove the Logger.info call that logs the request/body (the one outside handleRequest()) or remove the request/response logs inside handleRequest()/writeResponse() so that only handleRequest() (or only writeResponse()/the pipeline entry) emits request and response logs; update or document which functions (handleRequest, writeResponse, and the earlier Logger.info block) will be responsible for logging and ensure any necessary metadata (method, path, body, status) is preserved in the retained log location.
262-268: Consider logging JSON parse failures at debug level.The empty
catch {}silently ignores malformed JSON in the/stoprequest body. While acceptable for an optional parameter, a debug-level log would aid troubleshooting.♻️ Add debug logging
- } catch {} + } catch { + Logger.debug("Failed to parse /stop body JSON", metadata: ["error": "\(error)"]) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/lume/src/Server/Server.swift` around lines 262 - 268, The empty catch silently swallows JSON parsing errors for the /stop request; update the catch after JSONSerialization.jsonObject to log the parse failure at debug level (include the error and optionally the raw bodyData), referencing the existing JSONSerialization.jsonObject call and the storage variable so you only log when parsing fails; use the project's logger API (e.g., logger.debug("Failed to parse /stop body: \(error)", ...) or equivalent) instead of leaving catch empty.scripts/gen_interface_docs.py (1)
78-112: Minor: Signature formatting may be incorrect for keyword-only arguments.When a method has keyword-only arguments but no
*args, the generated signature won't include the bare*separator that Python requires to distinguish keyword-only args. For example,def foo(a, *, b=1)would render as(a, b = 1)instead of(a, *, b = 1).For documentation purposes this is likely acceptable, but if you want strict Python signature accuracy:
♻️ Proposed fix to add bare * separator
for arg in all_args: if arg.arg == "self": continue parts.append(_format_arg(arg, defaults)) + # Add bare * separator if there are kwonlyargs but no vararg + if args.kwonlyargs and not args.vararg: + parts.append("*") + for arg in args.kwonlyargs: parts.append(_format_arg(arg, defaults)) if args.vararg: parts.append(f"*{args.vararg.arg}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/gen_interface_docs.py` around lines 78 - 112, The generated signature in _method_signature omits the bare '*' when there are keyword-only args but no vararg; update the logic in _method_signature to insert a standalone "*" into parts before appending args.kwonlyargs whenever args.kwonlyargs is non-empty and args.vararg is None (use the existing parts list and args.kwonlyargs/args.vararg symbols to detect this), then continue formatting kw-only args as before so signatures like (a, *, b=1) are produced correctly.libs/python/cua/pyproject.toml (1)
69-73: Consider aligning source resolution with the root workspace configuration.The root
pyproject.tomldefinescua-agentwithworkspace = true, while this file uses path-based references. This creates different resolution behavior depending on whether you install from the workspace root or fromlibs/python/cuadirectly.This works for local development but may cause confusion. Consider adding
cua-sandboxandcua-clito the root workspace members for consistency, or document this intentional divergence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/python/cua/pyproject.toml` around lines 69 - 73, The [tool.uv.sources] entries in this pyproject.toml (cua-sandbox, cua-agent, cua-cli) create path-based resolution that diverges from the root workspace's use of workspace = true for cua-agent; to fix, either add cua-sandbox and cua-cli as members in the root workspace section of the root pyproject.toml (so all three packages use workspace resolution consistently) or add a short comment in this [tool.uv.sources] block explaining the intentional divergence and when path vs workspace resolution should be used; reference the symbols cua-sandbox, cua-agent, cua-cli and the workspace = true setting when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f50b674d-5f07-42ea-af8c-c57084be1924
📒 Files selected for processing (34)
docs/content/docs/cua/guide/sandbox/meta.jsondocs/content/docs/cua/guide/sandbox/snapshots.mdxdocs/content/docs/cua/reference/sandbox-sdk/interfaces.mdxlibs/lume/Package.resolvedlibs/lume/Package.swiftlibs/lume/src/Server/HTTP.swiftlibs/lume/src/Server/Server.swiftlibs/python/cua-sandbox/cua_sandbox/__init__.pylibs/python/cua-sandbox/cua_sandbox/_config.pylibs/python/cua-sandbox/cua_sandbox/builder/executor.pylibs/python/cua-sandbox/cua_sandbox/image.pylibs/python/cua-sandbox/cua_sandbox/interfaces/clipboard.pylibs/python/cua-sandbox/cua_sandbox/interfaces/mouse.pylibs/python/cua-sandbox/cua_sandbox/runtime/android_emulator.pylibs/python/cua-sandbox/cua_sandbox/runtime/base.pylibs/python/cua-sandbox/cua_sandbox/runtime/compat.pylibs/python/cua-sandbox/cua_sandbox/runtime/docker.pylibs/python/cua-sandbox/cua_sandbox/runtime/lume.pylibs/python/cua-sandbox/cua_sandbox/sandbox.pylibs/python/cua-sandbox/cua_sandbox/transport/adb.pylibs/python/cua-sandbox/cua_sandbox/transport/cloud.pylibs/python/cua-sandbox/cua_sandbox/transport/grpc_emulator.pylibs/python/cua-sandbox/cua_sandbox/transport/local.pylibs/python/cua-sandbox/tests/image/__init__.pylibs/python/cua-sandbox/tests/interfaces/__init__.pylibs/python/cua-sandbox/tests/test_android_multitouch.pylibs/python/cua-sandbox/tests/test_runtime.pylibs/python/cua-sandbox/tests/test_snapshots.pylibs/python/cua/cua/__init__.pylibs/python/cua/pyproject.tomlscripts/gen_interface_docs.pytests/pytest.initests/test_image_builder.pytests/test_interfaces.py
💤 Files with no reviewable changes (1)
- libs/python/cua-sandbox/tests/test_runtime.py
- Add Windows paths to _sdk_path() and check for emulator.exe - Remove hard unsupported bail on Windows; detect existing SDK + WHPX - Use sh -c and source /data/local/tmp/.cua_env for Android run layers - Shell-quote env var values with shlex.quote and validate key names Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📦 Publishable packages changed
Add |
- Add Windows commandlinetools download URL to _ensure_sdk() - Use .exe/.bat extensions for binary detection on Windows - Remove Windows-only bail in compat.py — auto-install works everywhere Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📦 Publishable packages changed
Add |
- _find_bin() now checks .exe and .bat extensions on Windows - grpc_emulator _find_adb() checks for adb.exe on Windows Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📦 Publishable packages changed
Add |
Contributor
📦 Publishable packages changed
Add |
…view Security: - executor.py, docker.py, lume.py: use shlex.quote() for env var values and validate names with re.fullmatch() to prevent shell injection in profile scripts - executor.py: quote dst/dst_dir with _sh_quote() in privileged copy commands Correctness: - image.py: propagate _snapshot_source through _add_layer() and _with() so chained image mutations don't silently drop the snapshot descriptor - sandbox.py: use src_image.os_type/distro/version/kind for snapshot Image instead of image_desc["kind"] which is snapshot kind, not OS type - lume.py: resume existing stopped VMs instead of re-cloning (name collision); require base VM to be stopped before cloning; stop→clone→restart in checkpoint() to match cloud behaviour; scope list/delete_checkpoint to cua-base-* / cua-ckpt-* prefix; fix cloud.py .cua.sh polling loop to only run in local-dev mode (not prod reverse-proxy) - compat.py: return supported=java_ok for Android (Java is hard prerequisite) - cua/__init__.py: replace None fallbacks with stubs that raise ImportError - docker.py: tear down container on post-start provisioning failure - test_snapshots.py: capture t_create after install completes, not after boot Concurrency: - Server.swift: protect serverChannel and eventLoopGroup with NSLock to eliminate data race between start() writer and stop() reader Tests: - test_android_multitouch.py: resolve local_android_sb lazily in autouse fixture so cloud tests don't trigger emulator boot or compat skip - test_interfaces.py: fix _start_http_server docstring and formatting Style: run isort + black across affected files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
📦 Publishable packages changed
Add |
Contributor
📦 Publishable packages changed
|
Contributor
📦 Publishable packages changed
Add |
5 tasks
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
scripts/gen_interface_docs.pyintrospects all interface classes and emitsinterfaces.mdxwith proper MDX escaping (HTML entities for<>{}outside code spans)sb.snapshot()→Image), snapshot guide docs, and test cases for Linux + Android snapshot workflows covering create/fork/isolationTest plan
pytest tests/test_image_builder.py -v— image builder tests against Dockerpytest tests/test_interfaces.py -v— interface tests against DockerCUA_API_KEY=... pytest tests/test_snapshots.py -v— snapshot tests against cloudpnpm -C docs build— docs build passes without MDX errors🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation
Bug Fixes & Improvements