feat(sdk): expose box network tunnels#994
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:
📝 WalkthroughWalkthroughAdds box-scoped network tunnel APIs to the C, Go, Python, and Node SDKs. Tunnels expose URI or file-descriptor endpoints, support one-shot socket connections, and are validated through unit, integration, and HTTP/WebSocket/binary end-to-end tests. ChangesNetwork tunnel SDK stack
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SDKClient
participant NetworkHandle
participant BoxTunnel
participant GuestService
SDKClient->>NetworkHandle: tunnel(port)
NetworkHandle->>BoxTunnel: create one-shot tunnel
SDKClient->>BoxTunnel: endpoint() or connect()
BoxTunnel->>GuestService: bridge socket to guest port
GuestService-->>SDKClient: tunneled response
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
G4614
marked this pull request as ready for review
July 17, 2026 05:26
📦 BoxLite review — couldn't completepowered by BoxLite |
![boxlite-agent[bot]](https://avatars.githubusercontent.com/in/4244458?s=60&v=4){kind=small}
G4614
force-pushed
the
codex/c-go-box-tunnel
branch
from
July 17, 2026 05:48
0eb7595 to
a66ff19
Compare
This was referenced Jul 17, 2026
G4614
force-pushed
the
codex/c-go-box-tunnel
branch
from
July 21, 2026 13:51
a66ff19 to
690c471
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
scripts/test/e2e/sdks/node/e2e_tunnel.ts (1)
14-45: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider bounding
endpoint()/connect()with a timeout.Only the HTTP response has an explicit 5s timer; if
tunnel.endpoint()ortunnel.connect()hangs,waitForHttp's 30s deadline check is never reached and the test relies on the outer subprocesstimeout=180intest_node_tunnel.pyto fail — with a less actionable error.♻️ Suggested guard
async function getOverTunnel(box: SimpleBox): Promise<string> { const tunnel = await box.network.tunnel(GUEST_PORT) - const endpoint = await tunnel.endpoint() + const endpoint = await Promise.race([ + tunnel.endpoint(), + delay(5_000).then(() => { throw new Error('endpoint() timed out') }), + ]) if (typeof endpoint !== 'string') { throw new Error('expected REST tunnel endpoint URL for the cloud box') } - const socket = await tunnel.connect() + const socket = await Promise.race([ + tunnel.connect(), + delay(5_000).then(() => { throw new Error('connect() timed out') }), + ])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test/e2e/sdks/node/e2e_tunnel.ts` around lines 14 - 45, Add an explicit timeout around the awaited tunnel.endpoint() and tunnel.connect() calls in getOverTunnel, using the same 5-second request timeout or an appropriate shared timeout, and reject with a clear timeout error. Ensure these operations cannot hang indefinitely before the existing HTTP response timer and preserve the current successful tunnel flow.sdks/c/src/tests.rs (1)
190-200: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd null-pointer validation tests for the tunnel functions too.
Only
boxlite_box_networkgets a null-pointer test here;boxlite_network_tunnel,boxlite_tunnel_endpoint, andboxlite_tunnel_connecteach have equivalent null-guard branches but no matching coverage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/c/src/tests.rs` around lines 190 - 200, Add equivalent null-pointer validation tests alongside test_box_network_null_pointer_validation for boxlite_network_tunnel, boxlite_tunnel_endpoint, and boxlite_tunnel_connect. For each function, pass null pointers as applicable, assert BoxliteErrorCode::InvalidArgument, verify the FFIError message is non-null, and free the error with boxlite_error_free.sdks/node/src/network.rs (1)
45-47: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valuePanics via
.expect()instead of returning a recoverable error, unlike the equivalent C ABI path.
sdks/c/src/network.rs'sboxlite_network_tunneltreats aGUEST_IPparse failure as a gracefulBoxliteError::Internalreturn; this napi binding instead panics via.expect(...). In practiceGUEST_IPis a fixed constant so this can't realistically fail, but for consistency with the C layer's defensive handling of the same computation, consider returningErr(...)instead of panicking.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/node/src/network.rs` around lines 45 - 47, The network tunnel path currently panics when parsing the guest socket address; make it return a recoverable error instead. Update the target address construction in the relevant napi network function to propagate the parse failure as the binding’s established internal error type, matching the graceful handling in boxlite_network_tunnel while preserving successful parsing behavior.sdks/c/src/network.rs (1)
53-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNewly-added public network/tunnel functions lack docstrings. All of the new
pub unsafe extern "C" fnin the C ABI, and the new napi methodstunnel()/endpoint()in the Node binding, are missing doc comments, while sibling structs/enums and the existingconnect_fd()do have them.
sdks/c/src/network.rs#L53-L252: add///doc comments toboxlite_box_network,boxlite_network_free,boxlite_network_tunnel,boxlite_tunnel_free,boxlite_tunnel_endpoint, andboxlite_tunnel_connect.sdks/node/src/network.rs#L40-L68: add///doc comments toJsNetworkHandle::tunnelandJsBoxTunnel::endpoint.As per coding guidelines,
sdks/**/*.{js,ts,jsx,tsx,py,java,go,rs,rb,php,cs,cpp,c,h,swift,kt}: "Write comprehensive docstrings for all public functions and classes".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/c/src/network.rs` around lines 53 - 252, Add comprehensive Rust doc comments to the public C ABI functions boxlite_box_network, boxlite_network_free, boxlite_network_tunnel, boxlite_tunnel_free, boxlite_tunnel_endpoint, and boxlite_tunnel_connect in sdks/c/src/network.rs (lines 53-252), and to JsNetworkHandle::tunnel and JsBoxTunnel::endpoint in sdks/node/src/network.rs (lines 40-68), documenting their purpose, parameters, outputs, and relevant safety/ownership behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdks/c/include/boxlite.h`:
- Around line 546-568: Add comprehensive API documentation comments for
boxlite_box_network, boxlite_network_free, boxlite_network_tunnel,
boxlite_tunnel_free, boxlite_tunnel_endpoint, and boxlite_tunnel_connect.
Document each function’s purpose, parameters, return status, output values,
error handling, and ownership/lifetime rules for returned network, tunnel, URI,
and file-descriptor resources.
In `@sdks/c/src/network.rs`:
- Around line 20-31: The UnixStream bridging logic in connection_fd is
duplicated across SDK crates; extract the pair/copy_bidirectional/FD-export flow
into a shared helper in the core boxlite crate, then update
sdks/c/src/network.rs lines 20-31 and sdks/node/src/network.rs lines 12-24 to
call it instead of maintaining local implementations. Preserve the existing
error propagation and returned OwnedFd behavior at both sites.
- Around line 152-213: Initialize *out_type to a defined sentinel immediately
after validating the output pointers and alongside the existing *out_uri and
*out_fd initialization in boxlite_tunnel_endpoint. Preserve the
endpoint-specific assignments on success while ensuring the error path leaves
all out-parameters deterministically initialized.
In `@sdks/go/tunnel.go`:
- Around line 67-85: Update Network.Tunnel and Tunnel.Connect so context
cancellation is honored while the native cgo calls are in flight, not only
before invocation. Avoid leaving the caller blocked on synchronous bridge calls
when the guest network hangs; use the existing native/API mechanisms to return
promptly on ctx cancellation while preserving current validation and error
propagation.
In `@sdks/python/tests/test_tunnel.py`:
- Line 89: Remove the module-wide pytestmark integration marker from
test_tunnel.py, then apply the integration marker only to tests that require a
real box or external service. Keep pure unit tests such as
test_endpoint_returns_local_file_descriptor, test_connect_consumes_tunnel_once,
test_tunnel_requires_a_started_box, and test_sync_tunnel_rejects_invalid_ports
unmarked so they remain in the non-integration test lane.
---
Nitpick comments:
In `@scripts/test/e2e/sdks/node/e2e_tunnel.ts`:
- Around line 14-45: Add an explicit timeout around the awaited
tunnel.endpoint() and tunnel.connect() calls in getOverTunnel, using the same
5-second request timeout or an appropriate shared timeout, and reject with a
clear timeout error. Ensure these operations cannot hang indefinitely before the
existing HTTP response timer and preserve the current successful tunnel flow.
In `@sdks/c/src/network.rs`:
- Around line 53-252: Add comprehensive Rust doc comments to the public C ABI
functions boxlite_box_network, boxlite_network_free, boxlite_network_tunnel,
boxlite_tunnel_free, boxlite_tunnel_endpoint, and boxlite_tunnel_connect in
sdks/c/src/network.rs (lines 53-252), and to JsNetworkHandle::tunnel and
JsBoxTunnel::endpoint in sdks/node/src/network.rs (lines 40-68), documenting
their purpose, parameters, outputs, and relevant safety/ownership behavior.
In `@sdks/c/src/tests.rs`:
- Around line 190-200: Add equivalent null-pointer validation tests alongside
test_box_network_null_pointer_validation for boxlite_network_tunnel,
boxlite_tunnel_endpoint, and boxlite_tunnel_connect. For each function, pass
null pointers as applicable, assert BoxliteErrorCode::InvalidArgument, verify
the FFIError message is non-null, and free the error with boxlite_error_free.
In `@sdks/node/src/network.rs`:
- Around line 45-47: The network tunnel path currently panics when parsing the
guest socket address; make it return a recoverable error instead. Update the
target address construction in the relevant napi network function to propagate
the parse failure as the binding’s established internal error type, matching the
graceful handling in boxlite_network_tunnel while preserving successful parsing
behavior.
🪄 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 Plus
Run ID: 964a9057-fd30-4380-b8ed-fbc73632747a
📒 Files selected for processing (31)
scripts/test/e2e/cases/test_node_tunnel.pyscripts/test/e2e/cases/test_sdk_tunnel.pyscripts/test/e2e/sdks/node/e2e_tunnel.tssdks/c/Cargo.tomlsdks/c/include/boxlite.hsdks/c/src/lib.rssdks/c/src/network.rssdks/c/src/tests.rssdks/go/constants.gosdks/go/tunnel.gosdks/node/Cargo.tomlsdks/node/lib/index.tssdks/node/lib/native-contracts.tssdks/node/lib/simplebox.tssdks/node/src/box_handle.rssdks/node/src/lib.rssdks/node/src/network.rssdks/node/tests/tunnel.test.tssdks/python/Cargo.tomlsdks/python/boxlite/__init__.pysdks/python/boxlite/simplebox.pysdks/python/boxlite/sync_api/__init__.pysdks/python/boxlite/sync_api/_box.pysdks/python/boxlite/sync_api/_network.pysdks/python/boxlite/sync_api/_simplebox.pysdks/python/pytest.inisdks/python/src/box_handle.rssdks/python/src/lib.rssdks/python/src/network.rssdks/python/tests/test_dev_tunnel_e2e.pysdks/python/tests/test_tunnel.py
G4614
changed the title
2 tasks
G4614
force-pushed
the
codex/c-go-box-tunnel
branch
from
July 22, 2026 04:19
405d870 to
1fbde98
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
sdks/python/src/box_handle.rs (1)
44-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the public network getter contract.
Expand the docstring to state that
network.tunnel(port)is asynchronous and lazy, rejects port0, and exposes URI/FD endpoints with one-shot connections.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/src/box_handle.rs` around lines 44 - 50, Expand the docstring for the public network getter method network in PyBoxHandle to document that network.tunnel(port) is asynchronous and lazy, rejects port 0, and exposes URI and file-descriptor endpoints using one-shot connections.Source: Coding guidelines
sdks/node/lib/native-contracts.ts (1)
257-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the exported tunnel API documentation.
Document the non-zero port requirement, the endpoint union (
stringURI versus numeric FD), and that_connectFd()consumes the one-shot tunnel connection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/node/lib/native-contracts.ts` around lines 257 - 266, Update the exported JsNetworkHandle.tunnel and JsBoxTunnel endpoint and _connectFd documentation to state that ports must be non-zero, endpoint() returns either a string URI or numeric file descriptor, and _connectFd() consumes the tunnel’s one-shot connection. Keep the existing API signatures unchanged.Source: Coding guidelines
sdks/python/tests/test_dev_tunnel_e2e.py (2)
294-368: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd docstrings to test functions and helpers.
None of the
test_*functions or module-level helpers carry a docstring describing the scenario under test (e.g. one-shot tunnel reconnection, binary integrity with concurrent streams).As per coding guidelines, "Write comprehensive docstrings for all public functions and classes" for
sdks/**/*.py.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/tests/test_dev_tunnel_e2e.py` around lines 294 - 368, The test functions and module-level helpers in test_dev_tunnel_e2e.py lack scenario documentation. Add concise docstrings to every test_* function and helper, including the tunnel endpoint/one-shot reconnection and binary-integrity concurrent-stream scenarios, describing the behavior each validates.Source: Coding guidelines
82-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate duplicated background-server wrapper logic.
_serve_marker_httpand_serve_binary_echoeach define a near-identicalLocalServerclass and PID-capture/backgrounding shell command for theSimpleBoxbranch, differing only by the embedded code and log path. Extracting a shared_run_background_process(box, code, log_path)helper would remove the duplication.As per coding guidelines, "Keep functions small and focused on a single responsibility" for
sdks/python/**.♻️ Suggested consolidation
async def _run_background_process(box, code: str, log_path: str): if isinstance(box, boxlite.SimpleBox): command = f"python3 -u -c {shlex.quote(code)} >{log_path} 2>&1 & echo $!" result = await box.exec("sh", "-c", command) pid = result.stdout.strip() class LocalServer: async def kill(self): await box.exec("kill", pid) return LocalServer() return await box.exec("python3", ["-u", "-c", code])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/tests/test_dev_tunnel_e2e.py` around lines 82 - 156, Extract the duplicated SimpleBox background-process and PID cleanup logic from _serve_marker_http and _serve_binary_echo into a shared _run_background_process(box, code, log_path) helper. Have both server functions delegate to it with their respective code and log paths, while preserving the existing non-SimpleBox execution and kill behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdks/c/src/tests.rs`:
- Around line 194-196: Extend the test around boxlite_box_network to exercise
both ABI null-pointer guards independently: retain the null-handle case with a
valid output pointer, then add a separate call using a valid handle and null
out_network, asserting the expected InvalidArgument result and non-null error
message for each case.
---
Nitpick comments:
In `@sdks/node/lib/native-contracts.ts`:
- Around line 257-266: Update the exported JsNetworkHandle.tunnel and
JsBoxTunnel endpoint and _connectFd documentation to state that ports must be
non-zero, endpoint() returns either a string URI or numeric file descriptor, and
_connectFd() consumes the tunnel’s one-shot connection. Keep the existing API
signatures unchanged.
In `@sdks/python/src/box_handle.rs`:
- Around line 44-50: Expand the docstring for the public network getter method
network in PyBoxHandle to document that network.tunnel(port) is asynchronous and
lazy, rejects port 0, and exposes URI and file-descriptor endpoints using
one-shot connections.
In `@sdks/python/tests/test_dev_tunnel_e2e.py`:
- Around line 294-368: The test functions and module-level helpers in
test_dev_tunnel_e2e.py lack scenario documentation. Add concise docstrings to
every test_* function and helper, including the tunnel endpoint/one-shot
reconnection and binary-integrity concurrent-stream scenarios, describing the
behavior each validates.
- Around line 82-156: Extract the duplicated SimpleBox background-process and
PID cleanup logic from _serve_marker_http and _serve_binary_echo into a shared
_run_background_process(box, code, log_path) helper. Have both server functions
delegate to it with their respective code and log paths, while preserving the
existing non-SimpleBox execution and kill behavior.
🪄 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 Plus
Run ID: a0e10bb7-937a-49c4-99e7-e047b390ed52
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (48)
openapi/box.openapi.yamlscripts/build/build-runtime.shscripts/test/e2e/cases/test_node_tunnel.pyscripts/test/e2e/cases/test_sdk_tunnel.pyscripts/test/e2e/sdks/node/e2e_tunnel.tssdks/c/Cargo.tomlsdks/c/include/boxlite.hsdks/c/src/lib.rssdks/c/src/network.rssdks/c/src/tests.rssdks/go/constants.gosdks/go/tunnel.gosdks/node/Cargo.tomlsdks/node/lib/index.tssdks/node/lib/native-contracts.tssdks/node/lib/simplebox.tssdks/node/src/box_handle.rssdks/node/src/lib.rssdks/node/src/network.rssdks/node/tests/tunnel.test.tssdks/python/Cargo.tomlsdks/python/boxlite/__init__.pysdks/python/boxlite/simplebox.pysdks/python/boxlite/sync_api/__init__.pysdks/python/boxlite/sync_api/_box.pysdks/python/boxlite/sync_api/_network.pysdks/python/boxlite/sync_api/_simplebox.pysdks/python/pytest.inisdks/python/src/box_handle.rssdks/python/src/lib.rssdks/python/src/network.rssdks/python/tests/test_dev_tunnel_e2e.pysdks/python/tests/test_tunnel.pysrc/boxlite/Cargo.tomlsrc/boxlite/src/litebox/box_impl.rssrc/boxlite/src/litebox/mod.rssrc/boxlite/src/litebox/network.rssrc/boxlite/src/net/gvproxy/services.rssrc/boxlite/src/net/mod.rssrc/boxlite/src/rest/client.rssrc/boxlite/src/rest/litebox.rssrc/boxlite/src/rest/runtime.rssrc/boxlite/src/runtime/backend.rssrc/boxlite/tests/gvproxy_backend.rssrc/cli/src/cli.rssrc/cli/src/commands/mod.rssrc/cli/src/commands/tunnel.rssrc/cli/src/main.rs
💤 Files with no reviewable changes (2)
- src/boxlite/src/net/gvproxy/services.rs
- src/boxlite/tests/gvproxy_backend.rs
🚧 Files skipped from review as they are similar to previous changes (26)
- sdks/go/constants.go
- sdks/python/boxlite/sync_api/init.py
- sdks/node/Cargo.toml
- sdks/python/pytest.ini
- sdks/node/src/lib.rs
- sdks/python/Cargo.toml
- scripts/build/build-runtime.sh
- sdks/python/boxlite/sync_api/_network.py
- sdks/python/boxlite/init.py
- sdks/python/src/lib.rs
- sdks/node/tests/tunnel.test.ts
- sdks/c/src/lib.rs
- sdks/node/src/box_handle.rs
- sdks/python/boxlite/sync_api/_simplebox.py
- scripts/test/e2e/cases/test_sdk_tunnel.py
- sdks/python/tests/test_tunnel.py
- sdks/node/src/network.rs
- sdks/python/boxlite/sync_api/_box.py
- sdks/go/tunnel.go
- sdks/python/src/network.rs
- sdks/node/lib/index.ts
- sdks/c/include/boxlite.h
- sdks/python/boxlite/simplebox.py
- scripts/test/e2e/sdks/node/e2e_tunnel.ts
- sdks/c/src/network.rs
- sdks/node/lib/simplebox.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/test/e2e/cases/test_sdk_tunnel.py`:
- Around line 105-109: Update the WebSocket reads in the test flow around
frame_header and echoed to accumulate data across repeated loop.sock_recv calls
until the exact expected byte counts are reached. Ensure the header always
contains 2 bytes before reading its length byte, then collect the full echoed
frame length before comparing it with marker + b":" + payload.
🪄 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 Plus
Run ID: ebe7a50a-6097-45a7-a38e-baf64acc73dc
📒 Files selected for processing (9)
scripts/test/e2e/cases/test_sdk_tunnel.pyscripts/test/e2e/fixtures/service_in_box_server.pyscripts/test/e2e/sdks/node/e2e_tunnel.tssdks/node/lib/native-contracts.tssdks/node/lib/simplebox.tssdks/python/boxlite/simplebox.pysdks/python/boxlite/sync_api/_box.pysdks/python/boxlite/sync_api/_network.pysdks/python/boxlite/sync_api/_simplebox.py
🚧 Files skipped from review as they are similar to previous changes (6)
- sdks/python/boxlite/sync_api/_network.py
- sdks/python/boxlite/sync_api/_simplebox.py
- sdks/node/lib/native-contracts.ts
- sdks/node/lib/simplebox.ts
- sdks/python/boxlite/sync_api/_box.py
- sdks/python/boxlite/simplebox.py
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
G4614
force-pushed
the
codex/c-go-box-tunnel
branch
from
July 22, 2026 08:22
58dc0e6 to
50b53d0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (4)
sdks/node/src/network.rs (1)
40-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the public N-API methods.
tunnelandendpointare public SDK APIs but have no Rust docstrings. Document their one-shot semantics, endpoint variants, and error conditions.Proposed documentation
+ /// Create a one-shot tunnel to `port` on the guest. + /// + /// Returns an error when `port` is zero or the tunnel cannot be created. #[napi] pub async fn tunnel(&self, port: u16) -> Result<JsBoxTunnel> { ... + /// Return this tunnel's URI or Unix file-descriptor endpoint. + /// + /// Returns an error after the tunnel has been consumed by `_connectFd`. #[napi] pub async fn endpoint(&self) -> Result<Either<String, i32>> {As per coding guidelines: “Write comprehensive docstrings for all public functions and classes.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/node/src/network.rs` around lines 40 - 63, Document the public N-API methods `JsBoxNetwork::tunnel` and `JsBoxTunnel::endpoint` with Rust doc comments. Describe that `tunnel` creates a one-shot tunnel, rejects port 0, and can return tunnel-creation errors; describe that `endpoint` returns either a string endpoint or integer port and document its relevant errors and one-shot behavior.Source: Coding guidelines
sdks/python/tests/test_dev_tunnel_e2e.py (3)
55-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate retry/readiness-polling loop.
_wait_for_http_serverand_wait_for_binary_serverimplement the same deadline/retry/fresh-tunnel pattern with only the readiness check differing. Consider extracting a shared "poll until ready" helper parameterized by an async predicate.As per coding guidelines, "Keep functions small and focused on a single responsibility" and "Avoid deeply nested code structures; refactor into separate functions if needed."
♻️ Sketch of a shared polling helper
async def _poll_until_ready(box, port, check, *, timeout=20, interval=0.5): deadline = asyncio.get_running_loop().time() + timeout last_result = "no connection attempt completed" while True: tunnel = await box.network.tunnel(port) try: result = await check(tunnel) if result is not None: return result except (ConnectionError, OSError, RuntimeError, TimeoutError) as error: last_result = f"{type(error).__name__}: {error}" if asyncio.get_running_loop().time() >= deadline: raise AssertionError(f"server did not become ready; last result: {last_result}") await asyncio.sleep(interval)Also applies to: 210-241
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/tests/test_dev_tunnel_e2e.py` around lines 55 - 73, Extract the duplicated deadline, retry, error-tracking, sleep, and tunnel-refresh logic from _wait_for_http_server and _wait_for_binary_server into a shared async polling helper, parameterized by an async readiness check and preserving the existing timeout, interval, exception handling, and result semantics. Update both server-specific functions to supply only their protocol-specific checks and use the helper’s returned ready result.Source: Coding guidelines
322-346: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate cloud runtime/box bootstrap.
test_dev_cloud_tunnel_endpoint_and_one_shot_connectsandtest_dev_cloud_tunnel_binary_integrityrepeat the same env-var lookup,Boxlite.rest(...)construction, and box-fetch/assert sequence. Extract a shared_dev_cloud_box()helper.As per coding guidelines, "Keep functions small and focused on a single responsibility."
♻️ Proposed shared helper
async def _dev_cloud_box(): from boxlite import ApiKeyCredential, Boxlite, BoxliteRestOptions api_key = _required_env("BOXLITE_API_KEY") box_id = _required_env("BOXLITE_E2E_BOX_ID") rest_url = os.getenv("BOXLITE_REST_URL", "https://dev.boxlite.ai/api") path_prefix = os.getenv("BOXLITE_REST_PATH_PREFIX") runtime = Boxlite.rest( BoxliteRestOptions(url=rest_url, credential=ApiKeyCredential(api_key), path_prefix=path_prefix) ) box = await runtime.get(box_id) assert box is not None, f"dev box {box_id!r} was not found" return boxAlso applies to: 350-367
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/tests/test_dev_tunnel_e2e.py` around lines 322 - 346, Extract the duplicated cloud runtime and box-fetch setup from test_dev_cloud_tunnel_endpoint_and_one_shot_connects and test_dev_cloud_tunnel_binary_integrity into a shared async _dev_cloud_box() helper. Move the environment lookups, Boxlite.rest construction, runtime.get call, and missing-box assertion into that helper, then have both tests await it while retaining their existing test-specific port and tunnel assertions.Source: Coding guidelines
82-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate background-process launch/kill logic.
_serve_marker_httpand_serve_binary_echorepeat the identicalisinstance(box, boxlite.SimpleBox)branching and inlineLocalServerclass, differing only in thecode/log path. Extract a shared_launch_background_process(box, code, log_path)helper.As per coding guidelines, "Keep functions focused and cohesive - each function should do one thing well."
♻️ Proposed shared helper
async def _launch_background_process(box, code: str, log_path: str): if isinstance(box, boxlite.SimpleBox): command = f"python3 -u -c {shlex.quote(code)} >{log_path} 2>&1 & echo $!" result = await box.exec("sh", "-c", command) pid = result.stdout.strip() class LocalServer: async def kill(self): await box.exec("kill", pid) return LocalServer() return await box.exec("python3", ["-u", "-c", code])Then
_serve_marker_http/_serve_binary_echojust buildcodeand delegate:- if isinstance(box, boxlite.SimpleBox): - command = ( - f"python3 -u -c {shlex.quote(code)} >/tmp/tunnel-e2e.log 2>&1 & echo $!" - ) - result = await box.exec("sh", "-c", command) - pid = result.stdout.strip() - - class LocalServer: - async def kill(self): - await box.exec("kill", pid) - - return LocalServer() - return await box.exec("python3", ["-u", "-c", code]) + return await _launch_background_process(box, code, "/tmp/tunnel-e2e.log")Also applies to: 145-155
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/python/tests/test_dev_tunnel_e2e.py` around lines 82 - 102, Extract the duplicated SimpleBox background-process launch and inline LocalServer kill behavior from _serve_marker_http and _serve_binary_echo into a shared _launch_background_process(box, code, log_path) helper. Keep each serve function responsible only for constructing its server code and delegating with its appropriate log path, while preserving the existing non-SimpleBox execution path and kill behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@sdks/node/src/network.rs`:
- Around line 40-63: Document the public N-API methods `JsBoxNetwork::tunnel`
and `JsBoxTunnel::endpoint` with Rust doc comments. Describe that `tunnel`
creates a one-shot tunnel, rejects port 0, and can return tunnel-creation
errors; describe that `endpoint` returns either a string endpoint or integer
port and document its relevant errors and one-shot behavior.
In `@sdks/python/tests/test_dev_tunnel_e2e.py`:
- Around line 55-73: Extract the duplicated deadline, retry, error-tracking,
sleep, and tunnel-refresh logic from _wait_for_http_server and
_wait_for_binary_server into a shared async polling helper, parameterized by an
async readiness check and preserving the existing timeout, interval, exception
handling, and result semantics. Update both server-specific functions to supply
only their protocol-specific checks and use the helper’s returned ready result.
- Around line 322-346: Extract the duplicated cloud runtime and box-fetch setup
from test_dev_cloud_tunnel_endpoint_and_one_shot_connects and
test_dev_cloud_tunnel_binary_integrity into a shared async _dev_cloud_box()
helper. Move the environment lookups, Boxlite.rest construction, runtime.get
call, and missing-box assertion into that helper, then have both tests await it
while retaining their existing test-specific port and tunnel assertions.
- Around line 82-102: Extract the duplicated SimpleBox background-process launch
and inline LocalServer kill behavior from _serve_marker_http and
_serve_binary_echo into a shared _launch_background_process(box, code, log_path)
helper. Keep each serve function responsible only for constructing its server
code and delegating with its appropriate log path, while preserving the existing
non-SimpleBox execution path and kill behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c33c8dfa-6272-4177-ab2c-3f8163377647
📒 Files selected for processing (32)
scripts/test/e2e/cases/test_node_tunnel.pyscripts/test/e2e/cases/test_sdk_tunnel.pyscripts/test/e2e/fixtures/service_in_box_server.pyscripts/test/e2e/sdks/node/e2e_tunnel.tssdks/c/Cargo.tomlsdks/c/include/boxlite.hsdks/c/src/lib.rssdks/c/src/network.rssdks/c/src/tests.rssdks/go/constants.gosdks/go/tunnel.gosdks/node/Cargo.tomlsdks/node/lib/index.tssdks/node/lib/native-contracts.tssdks/node/lib/simplebox.tssdks/node/src/box_handle.rssdks/node/src/lib.rssdks/node/src/network.rssdks/node/tests/tunnel.test.tssdks/python/Cargo.tomlsdks/python/boxlite/__init__.pysdks/python/boxlite/simplebox.pysdks/python/boxlite/sync_api/__init__.pysdks/python/boxlite/sync_api/_box.pysdks/python/boxlite/sync_api/_network.pysdks/python/boxlite/sync_api/_simplebox.pysdks/python/pytest.inisdks/python/src/box_handle.rssdks/python/src/lib.rssdks/python/src/network.rssdks/python/tests/test_dev_tunnel_e2e.pysdks/python/tests/test_tunnel.py
🚧 Files skipped from review as they are similar to previous changes (29)
- sdks/python/pytest.ini
- sdks/go/constants.go
- sdks/python/Cargo.toml
- scripts/test/e2e/fixtures/service_in_box_server.py
- sdks/python/boxlite/sync_api/init.py
- sdks/node/Cargo.toml
- sdks/c/src/lib.rs
- sdks/node/src/box_handle.rs
- sdks/c/src/tests.rs
- sdks/node/src/lib.rs
- sdks/python/boxlite/init.py
- sdks/node/lib/native-contracts.ts
- sdks/c/Cargo.toml
- sdks/python/boxlite/sync_api/_network.py
- sdks/python/boxlite/sync_api/_simplebox.py
- sdks/node/lib/index.ts
- scripts/test/e2e/cases/test_sdk_tunnel.py
- sdks/python/src/box_handle.rs
- sdks/c/include/boxlite.h
- sdks/node/tests/tunnel.test.ts
- sdks/python/boxlite/simplebox.py
- scripts/test/e2e/sdks/node/e2e_tunnel.ts
- sdks/python/tests/test_tunnel.py
- sdks/go/tunnel.go
- sdks/python/src/network.rs
- sdks/python/src/lib.rs
- sdks/python/boxlite/sync_api/_box.py
- sdks/node/lib/simplebox.ts
- sdks/c/src/network.rs
G4614
force-pushed
the
codex/c-go-box-tunnel
branch
from
July 22, 2026 09:35
89bbea3 to
f6ac4f2
Compare
G4614
enabled auto-merge
July 22, 2026 10:07
DorianZheng
approved these changes
Jul 22, 2026
DorianZheng
disabled auto-merge
July 22, 2026 10:21
5 tasks
DorianZheng
added a commit
that referenced
this pull request
Jul 23, 2026
## Problem PR #988 split the guest's fused `Container.Init` into `Init` (create) + `Start` (run init). The guest agent is baked into the versioned guest-rootfs and reused per-box across restarts, so a box created before #988 boots its old agent — which has no `Container.Start`. The post-#988 host calls it, tonic returns `Unimplemented`, and it surfaced as `gRPC/tonic error … Unimplemented … (code=16)`, breaking restart/exec of pre-#988 boxes. ## Fix `ContainerInterface::start` now reads the gRPC status before `?` flattens it and treats `Unimplemented` as "already started": on a pre-#988 agent the fused `Init` (still called every boot) already ran init, so `Start` is a redundant no-op. Real errors still propagate. Also removes the spurious `boxlite/gvproxy` feature from `sdks/c` (introduced in #994), which linked the shim-side `libgvproxy-sys` CGO library into the C SDK — the SDK/CLI must not, per the existing `net::tests` guard. Matches Python/Node/CLI, which expose the same tunnel API with `rest` only. ## Test An in-process tonic stub guest returning `Unimplemented` reproduces the exact failure through `ContainerInterface::start`; the degrade turns it green while a genuine `Start` error still surfaces. Full boxlite + boxlite-shared unit suites pass (852 + 47). Deferred: the durable fix (re-inject/recreate the guest so restarts run the current agent, kata/Daytona-style) is not included here. https://claude.ai/code/session_01Vu4qDgCnMjnLPpNsroxeAP <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Container startup now succeeds when the service reports that the container start operation is unsupported, treating it as already started. * Genuine container startup errors continue to be reported normally. * **Compatibility** * Updated SDK configuration to use the REST feature without the optional networking feature. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: tester <[email protected]>
This was referenced Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Expose one-shot box network tunnels through the C and Go SDKs while preserving direct local descriptors.
Test plan:
cargo check -p boxlite-con Linux debug hostcargo test -p boxlite-c --libon Linux debug host (59 passed, 1 ignored)go test -tags boxlite_dev ./...(blocked by existing unresolved auto-pause/resume/delete Cgo symbols on latest main)