Skip to content

fix(portal): tolerate pre-#988 guest missing Container.Start#1020

Merged
DorianZheng merged 1 commit into
mainfrom
worktree-partitioned-greeting-firefly
Jul 23, 2026
Merged

fix(portal): tolerate pre-#988 guest missing Container.Start#1020
DorianZheng merged 1 commit into
mainfrom
worktree-partitioned-greeting-firefly

Conversation

@DorianZheng

@DorianZheng DorianZheng commented Jul 23, 2026

Copy link
Copy Markdown
Member

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

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.

A box created before #988 boots a guest agent that has no
Container.Start RPC (the agent is baked into the reused guest rootfs).
The post-#988 host calls Start on restart/exec; tonic answers the
unknown method with Unimplemented, which surfaced as BoxliteError::Rpc
(code=16) and failed the box. That agent's fused Container.Init -- still
run on every boot -- already started the container, so treat
Unimplemented as already-started instead of an error. The status code is
inspected before `?` flattens it to Rpc(String); real Start errors still
propagate.

Also drop the boxlite/gvproxy feature from sdks/c (added in #994): it
links the shim-side libgvproxy-sys CGO library into the C SDK, which the
SDK/CLI must never do per the existing guard test. The tunnel API is
host-side and needs only the rest feature, matching Python/Node/CLI.

Claude-Session: https://claude.ai/code/session_01Vu4qDgCnMjnLPpNsroxeAP
@DorianZheng
DorianZheng requested a review from a team July 23, 2026 07:24
@DorianZheng DorianZheng added the e2e-local Triggers the local (in-process) E2E suite on the self-hosted runner label Jul 23, 2026
@boxlite-agent

boxlite-agent Bot commented Jul 23, 2026

Copy link
Copy Markdown

📦 BoxLite review — couldn't complete

claude exited 1

stdout:
{"is_error":true,"duration_api_ms":0,"num_turns":1,"stop_reason":"stop_sequence","session_id":"abb061ce-5747-41b2-86bd-7007e707be9f","total_cost_usd":0,"usage":{"input_tokens":0,"cache_creation_input_tokens":0,"cache_read_input_tokens":0,"output_tokens":0,"server_tool_use":{"web_search_requests":0,"web_fetch_requests":0},"service_tier":"standard","cache_creation":{"ephemeral_1h_input_tokens":0,"ephemeral_5m_input_tokens":0},"inference_geo":"","iterations":[],"speed":"standard"},"modelUsage":{},"permission_denials":[],"terminal_reason":"api_error","fast_mode_state":"off","subtype":"success","api_error_status":403,"result":"Your organization has disabled Claude subscription access for Claude Code · Use an Anthropic API key instead, or ask your admin to enable access","type":"result","duration_ms":375,"uuid":"3b38f726-125f-44f9-a944-769bda4bac21"}

stderr:
<empty>

powered by BoxLite

@cla-assistant

cla-assistant Bot commented Jul 23, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


tester seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@cla-assistant

cla-assistant Bot commented Jul 23, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


tester seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8bc60ea3-c5d7-41aa-9875-90fe7593cce7

📥 Commits

Reviewing files that changed from the base of the PR and between 015503d and 392631e.

📒 Files selected for processing (2)
  • sdks/c/Cargo.toml
  • src/boxlite/src/portal/interfaces/container.rs

📝 Walkthrough

Walkthrough

Container startup now treats gRPC Unimplemented responses as successful, preserves propagation of other errors, and adds tests for unimplemented, failure, and success responses. The C SDK no longer enables the gvproxy feature for BoxLite dependencies.

Changes

Container startup compatibility

Layer / File(s) Summary
C SDK dependency features
sdks/c/Cargo.toml
The regular and development BoxLite dependencies enable only the rest feature; the build dependency remains unchanged.
Container start response handling
src/boxlite/src/portal/interfaces/container.rs
ContainerInterface::start returns success for Unimplemented, propagates other start errors, and tests unimplemented, failing, and successful server responses.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main compatibility fix in the PR.
Description check ✅ Passed It covers the problem, fix, and test evidence, though it doesn't follow the template headings or include a formal risks/rollout section.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-partitioned-greeting-firefly

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@DorianZheng
DorianZheng merged commit a5da5b4 into main Jul 23, 2026
38 of 40 checks passed
@DorianZheng
DorianZheng deleted the worktree-partitioned-greeting-firefly branch July 23, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e-local Triggers the local (in-process) E2E suite on the self-hosted runner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant