Repository navigation
feat(sdk): route Go SDK exec and forward to the sandbox owner replica - #4852
Merged
FrostGod merged 3 commits intoOct 29, 2026
Merged
Conversation
EmilienM
reviewed
Oct 22, 2026
EmilienM
left a comment
EmilienM
left a comment
Contributor
There was a problem hiding this comment.
Needs a human read: 8 minor (inline, 1 below)
No blocking or major findings: this is a faithful port of the CLI's shipped design, and the grpc-go Header()/RecvMsg(nil) handling checks out against v1.83.2. Deep-tier review on code changes, so the verdict stays with a human.
Outside the diff:
- Minor:
docs/kubernetes/ingress.mdx:260, the docs still say only the CLI sets the replica header; a one-sentence update should include the Go SDK.
How this was reviewed
- Commit
32b34f8f, round 1 - Deep review, because of sdk/go/openshell/v1/routing.go, exec_client.go, tcp_client.go
- Passes: architecture, performance, security, scalability, then correctness
- Independent skeptics checked each candidate finding (2 for the major, 1 for minor ones); 5 of 7 held up
- Repo rules read from
9a85667d: AGENTS.md, CLAUDE.md, CONTRIBUTING.md, docs/CONTRIBUTING.mdx - Not covered: nothing skipped
This was referenced Oct 23, 2026
FrostGod
force-pushed
the
feat/4223-go-sdk-replica-routing/FrostGod
branch
from
October 27, 2026 04:21
199dc52 to
afd6b1e
Compare
Collaborator
Author
|
/ok to test afd6b1e |
pimlock
previously approved these changes
Oct 28, 2026
FrostGod
enabled auto-merge
October 28, 2026 21:49
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Oct 28, 2026
The Go SDK now does what the CLI already does for long-lived sandbox calls. The GetSandbox check that Exec.Run, Exec.Stream, Exec.Interactive, TCP.Forward and SSH.Tunnel already make also asks the gateway for the owner replica. The call then sends it in the x-openshell-replica header and retries once without the header when the edge proxy reports that the owner's route has no ready endpoints. No extra round trip is added. Without a reported owner the calls are unchanged. Part of NVIDIA#4223 Signed-off-by: divesh <dgude@nvidia.com>
Share the header check between the routed attempt and its retry, keep the first-frame EOF rule in one bidi helper, reuse Get's request for the owner lookup, and document the header contract next to the replica routing docs. Part of NVIDIA#4223 Signed-off-by: divesh <dgude@nvidia.com>
FrostGod
force-pushed
the
feat/4223-go-sdk-replica-routing/FrostGod
branch
from
October 29, 2026 00:43
afd6b1e to
34ad211
Compare
Collaborator
|
/ok to test 34ad211 |
TaylorMutch
reviewed
Oct 29, 2026
Collaborator
|
Otherwise LGTM |
TaylorMutch
previously approved these changes
Oct 29, 2026
Part of NVIDIA#4223 Signed-off-by: divesh <dgude@nvidia.com>
Collaborator
Author
|
/ok to test 4655179 |
Compatibility review (advisory)New or updated breaking changes; see the full report for all findings. 1. go-sdk — No explicit risk acceptance foundExec.Interactive, TCP.Forward, and SSH.Tunnel previously returned handles after sending initialization; owner-routed calls now wait for response headers.
Review details
Advisory review; publication gates are unchanged. |
pimlock
approved these changes
Oct 29, 2026
2 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Go SDK now sends exec, port-forward, and SSH tunnel calls straight to the gateway replica that owns the sandbox's supervisor session, as the CLI already does. Behind an edge proxy with replica routing enabled, this skips the relay hop through a peer replica; without one, the header is ignored and behavior is unchanged.
Related Issue
Part of #4223 (the "Every SDK asks for the owner and sends the replica hint, like the CLI" item, Go portion). Rust is #4790; TypeScript and Python follow in separate PRs.
Changes
Exec.Run,Exec.Stream,Exec.Interactive,TCP.Forward, andSSH.Tunnelalready callGetSandboxfirst; that call now also asks for the owner, so no round trip is added.routing.go: sendsx-openshell-replicaon those streams and retries once without it when Envoy reports the owner's route has no ready endpoints (same rule as the CLI). Forward and interactive calls resend their init frame on retry.Release Notes
Go SDK exec, port-forward, and SSH tunnel calls now take the direct route to the sandbox's owner replica when replica routing is enabled.
Testing
mise run go:ci: format, build, lint, tests, proto and docs checks)Checklist