feat: add remote session support across all SDKs#1192
Conversation
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1192 · ● 822K
| Assert.Equal(original.Environment, clone.Environment); | ||
| Assert.Equal(original.GitHubToken, clone.GitHubToken); | ||
| Assert.Equal(original.UseLoggedInUser, clone.UseLoggedInUser); | ||
| Assert.Equal(original.CopilotHome, clone.CopilotHome); |
There was a problem hiding this comment.
The CopilotHome property is now covered by the clone test — great! However the Remote property added in this PR is not tested here. Since Remote = other.Remote was added to the clone constructor, it would be worth adding coverage:
// In the original initializer (around line 29):
Remote = true,
// Assertion to add after line 47:
Assert.Equal(original.Remote, clone.Remote);Without this, a regression that accidentally drops the Remote copy in the future won't be caught.
| ping: async (params: PingRequest): Promise<PingResult> => | ||
| connection.sendRequest("ping", params), | ||
| connect: async (params: ConnectRequest): Promise<ConnectResult> => | ||
| connection.sendRequest("connect", params), |
There was a problem hiding this comment.
The connect() server-level RPC method was added here (and in the .NET ServerRpc.ConnectAsync()), but the equivalent is not present in the Go (go/rpc/generated_rpc.go) or Python (python/copilot/generated/rpc.py) server RPC. The PR description notes that Go and Python needed manual additions due to a quicktype bug — connect() appears to have been missed.
For Go, this would look like:
func (c *ServerRpc) Connect(ctx context.Context, token *string) (*ConnectResult, error) {
req := map[string]any{}
if token != nil {
req["token"] = *token
}
raw, err := c.client.Request("connect", req)
if err != nil {
return nil, err
}
var result ConnectResult
if err := json.Unmarshal(raw, &result); err != nil {
return nil, err
}
return &result, nil
}For Python:
async def connect(self, *, token: str | None = None, timeout: float | None = None) -> ConnectResult:
params: dict = {}
if token is not None:
params["token"] = token
return ConnectResult.from_dict(await self._client.request("connect", params, **_timeout_kwargs(timeout)))There was a problem hiding this comment.
Pull request overview
This PR aims to bring the four language SDKs into parity with the Copilot CLI’s remote-session capabilities, so SDK clients can either start the CLI in remote mode or toggle remote access on an existing session. It also includes a second, separate cross-SDK API addition for configuring COPILOT_HOME/copilotHome.
Changes:
- Adds a client-level remote option in Node, Python, Go, and .NET so spawned CLI processes can start with
--remote. - Adds session-scoped remote RPC bindings (
enable/disable) in the generated or hand-maintained RPC clients for each SDK. - Updates shared docs and language docs, and extends some option/clone tests around the new configuration surface.
Show a summary per file
| File | Description |
|---|---|
| python/test_client.py | Adds unit tests for Python copilot_home option storage. |
| python/copilot/generated/rpc.py | Adds Python typed RPC bindings for remote enable/disable. |
| python/copilot/client.py | Adds Python subprocess config fields for copilot_home and remote, and wires them into CLI startup. |
| python/README.md | Documents Python copilot_home option. |
| nodejs/test/client.test.ts | Adds Node unit tests for copilotHome option storage. |
| nodejs/src/types.ts | Adds Node public client options for copilotHome and remote. |
| nodejs/src/generated/rpc.ts | Adds Node RPC types for remote APIs and a generated connect server RPC. |
| nodejs/src/client.ts | Wires Node copilotHome env handling and --remote CLI arg into startup. |
| nodejs/README.md | Documents Node copilotHome option. |
| go/types.go | Adds Go ClientOptions fields for CopilotHome and Remote. |
| go/rpc/generated_rpc.go | Adds Go RPC types and methods for remote enable/disable. |
| go/internal/embeddedcli/embeddedcli.go | Changes embedded CLI install path selection to honor COPILOT_HOME. |
| go/client_test.go | Adds Go unit tests for CopilotHome option storage. |
| go/client.go | Wires Go CopilotHome env handling and --remote CLI arg into startup. |
| go/README.md | Documents Go CopilotHome behavior and cache-path nuance. |
| dotnet/test/Unit/CloneTests.cs | Extends clone coverage for the new .NET options. |
| dotnet/src/Types.cs | Adds .NET CopilotHome and Remote options and updates cloning. |
| dotnet/src/Generated/Rpc.cs | Adds .NET RPC types and methods for remote APIs plus a generated connect RPC. |
| dotnet/src/Client.cs | Wires .NET CopilotHome env handling and --remote CLI arg into startup. |
| dotnet/README.md | Documents .NET CopilotHome option. |
| docs/index.md | Adds Remote Sessions guide to docs index. |
| docs/features/remote-sessions.md | Introduces shared remote-session feature documentation and examples. |
| docs/features/index.md | Adds Remote Sessions to the feature guide index. |
Copilot's findings
- Files reviewed: 20/23 changed files
- Comments generated: 13
| ```go | ||
| result, err := session.RPC.Remote.Enable(ctx) | ||
| fmt.Println("Remote URL:", *result.URL) |
| if (this.options.remote) { | ||
| args.push("--remote"); | ||
| } |
| if (options.Remote) | ||
| { | ||
| args.Add("--remote"); | ||
| } |
| func TestClient_CopilotHome(t *testing.T) { | ||
| t.Run("should accept CopilotHome option", func(t *testing.T) { | ||
| client := NewClient(&ClientOptions{ | ||
| CopilotHome: "/custom/copilot/home", | ||
| }) | ||
|
|
||
| if client.options.CopilotHome != "/custom/copilot/home" { | ||
| t.Errorf("Expected CopilotHome to be '/custom/copilot/home', got %q", client.options.CopilotHome) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("should default CopilotHome to empty string", func(t *testing.T) { | ||
| client := NewClient(&ClientOptions{}) | ||
|
|
||
| if client.options.CopilotHome != "" { | ||
| t.Errorf("Expected CopilotHome to be empty, got %q", client.options.CopilotHome) | ||
| } | ||
| }) |
| CopilotHome = "/custom/copilot/home", | ||
| SessionIdleTimeoutSeconds = 600, | ||
| }; | ||
|
|
||
| var clone = original.Clone(); | ||
|
|
||
| Assert.Equal(original.CliPath, clone.CliPath); | ||
| Assert.Equal(original.CliArgs, clone.CliArgs); | ||
| Assert.Equal(original.Cwd, clone.Cwd); | ||
| Assert.Equal(original.Port, clone.Port); | ||
| Assert.Equal(original.UseStdio, clone.UseStdio); | ||
| Assert.Equal(original.CliUrl, clone.CliUrl); | ||
| Assert.Equal(original.LogLevel, clone.LogLevel); | ||
| Assert.Equal(original.AutoStart, clone.AutoStart); | ||
|
|
||
| Assert.Equal(original.Environment, clone.Environment); | ||
| Assert.Equal(original.GitHubToken, clone.GitHubToken); | ||
| Assert.Equal(original.UseLoggedInUser, clone.UseLoggedInUser); | ||
| Assert.Equal(original.CopilotHome, clone.CopilotHome); | ||
| Assert.Equal(original.SessionIdleTimeoutSeconds, clone.SessionIdleTimeoutSeconds); |
| class TestCopilotHome: | ||
| def test_accepts_copilot_home(self): | ||
| client = CopilotClient( | ||
| SubprocessConfig( | ||
| cli_path=CLI_PATH, | ||
| copilot_home="/custom/copilot/home", | ||
| log_level="error", | ||
| ) | ||
| ) | ||
| assert isinstance(client._config, SubprocessConfig) | ||
| assert client._config.copilot_home == "/custom/copilot/home" | ||
|
|
||
| def test_default_copilot_home_is_none(self): | ||
| client = CopilotClient( | ||
| SubprocessConfig( | ||
| cli_path=CLI_PATH, | ||
| log_level="error", | ||
| ) | ||
| ) | ||
| assert isinstance(client._config, SubprocessConfig) | ||
| assert client._config.copilot_home is None |
| if cfg.remote: | ||
| args.append("--remote") |
| if c.options.Remote { | ||
| args = append(args, "--remote") |
| if copilotHome := os.Getenv("COPILOT_HOME"); copilotHome != "" { | ||
| installDir = filepath.Join(copilotHome, "cache", "copilot-sdk") | ||
| } else { | ||
| var err error | ||
| if installDir, err = os.UserCacheDir(); err != nil { | ||
| // Fall back to temp dir if UserCacheDir is unavailable | ||
| installDir = os.TempDir() | ||
| } | ||
| installDir = filepath.Join(installDir, "copilot-sdk") |
| it("should accept copilotHome option", () => { | ||
| const client = new CopilotClient({ | ||
| copilotHome: "/custom/copilot/home", | ||
| logLevel: "error", | ||
| }); | ||
|
|
||
| expect((client as any).options.copilotHome).toBe("/custom/copilot/home"); | ||
| }); | ||
|
|
||
| it("should leave copilotHome undefined when not provided", () => { | ||
| const client = new CopilotClient({ | ||
| logLevel: "error", | ||
| }); | ||
|
|
||
| expect((client as any).options.copilotHome).toBeUndefined(); | ||
| }); |
CI NotesExpected failures:
Merge order: Runtime PR → publish |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1192 · ● 915.2K
| return &result, nil | ||
| } | ||
|
|
||
| type RemoteApi sessionApi |
There was a problem hiding this comment.
Cross-SDK consistency gap: connect RPC missing from Go ServerRpc
This PR adds a connect server-level RPC method to Node.js (createServerRpc) and .NET (ServerRpc.ConnectAsync), but the equivalent method is absent from Go's ServerRpc. Currently Go only has Ping at the server level.
Suggested addition to ServerRpc (alongside Ping):
func (a *ServerRpc) Connect(ctx context.Context, token *string) (*ConnectResult, error) {
req := map[string]any{}
if token != nil {
req["token"] = *token
}
raw, err := a.common.client.Request("connect", req)
if err != nil {
return nil, err
}
var result ConnectResult
if err := json.Unmarshal(raw, &result); err != nil {
return nil, err
}
return &result, nil
}Along with the corresponding ConnectResult and ConnectRequest types.
| Assert.Equal(original.Environment, clone.Environment); | ||
| Assert.Equal(original.GitHubToken, clone.GitHubToken); | ||
| Assert.Equal(original.UseLoggedInUser, clone.UseLoggedInUser); | ||
| Assert.Equal(original.CopilotHome, clone.CopilotHome); |
There was a problem hiding this comment.
Missing Remote property in clone test
The CopilotClientOptions_Clone_CopiesAllProperties test was updated to cover CopilotHome but the new Remote property added in this same PR is not verified. Since Remote is a bool with a non-obvious default (false), setting it to true in the test would exercise the clone path.
Suggested additions:
// In the original object initializer:
Remote = true,
// In the assertions:
Assert.Equal(original.Remote, clone.Remote);| return UsageGetMetricsResult.from_dict(await self._client.request("session.usage.getMetrics", {"sessionId": self._session_id}, **_timeout_kwargs(timeout))) | ||
|
|
||
|
|
||
| class RemoteApi: |
There was a problem hiding this comment.
Cross-SDK consistency gap: connect RPC missing from Python ServerRpc
This PR adds a server-level connect RPC to Node.js and .NET, but Python's ServerRpc only has ping. The equivalent connect method and its result type should be added here for parity.
Suggested additions:
`@dataclass`
class ConnectResult:
"""Result of the connect RPC call."""
ok: bool
protocol_version: int
version: str
`@staticmethod`
def from_dict(obj: Any) -> 'ConnectResult':
assert isinstance(obj, dict)
return ConnectResult(
ok=from_bool(obj.get("ok")),
protocol_version=from_int(obj.get("protocolVersion")),
version=from_str(obj.get("version")),
)And in ServerRpc:
async def connect(self, token: str | None = None, *, timeout: float | None = None) -> ConnectResult:
params: dict[str, Any] = {}
if token is not None:
params["token"] = token
return ConnectResult.from_dict(await self._client.request("connect", params, **_timeout_kwargs(timeout)))
SteveSandersonMS
force-pushed
the
feat/remote-sessions
branch
from
c6cb6f8 to
9847c1e
Compare
|
@patniko I looked into merging this but I see we're still waiting for runtime-side changes to land, so will hold off for a bit. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1192 · ● 1.4M
| ping: async (params: PingRequest): Promise<PingResult> => | ||
| connection.sendRequest("ping", params), | ||
| connect: async (params: ConnectRequest): Promise<ConnectResult> => | ||
| connection.sendRequest("connect", params), |
There was a problem hiding this comment.
Inconsistency: connect exposed in public createServerRpc but is internal in all other SDKs
This PR adds connect to the public createServerRpc() function, but in every other SDK this method is explicitly kept internal:
- Go: Lives on
InternalServerRpc.Connect()with the doc comment "Internal: Connect is part of the SDK's internal handshake/plumbing; external callers should not use it." - Python: Lives on
_InternalServerRpc.connect()(underscore-prefixed, private class) with:meta private:. - .NET:
ServerRpc.ConnectAsync()is markedinternal.
The connect RPC is already available via the correct internal helper in this file at createInternalServerRpc() (marked @internal). Adding it to the public createServerRpc() object leaks an implementation detail that consumers shouldn't call directly.
Suggestion: Remove the connect entry from createServerRpc(). It's already exposed through createInternalServerRpc() for internal SDK use.
// Remove these two lines from createServerRpc():
connect: async (params: ConnectRequest): Promise<ConnectResult> =>
connection.sendRequest("connect", params),|
|
||
| // Set COPILOT_HOME if configured | ||
| if c.options.CopilotHome != "" { | ||
| c.process.Env = append(c.process.Env, "COPILOT_HOME="+c.options.CopilotHome) |
There was a problem hiding this comment.
Bug: COPILOT_HOME is set twice in the same environment array
This PR adds a second block to set COPILOT_HOME, but the pre-existing code at line 1496 already handles it via setEnvValue:
// Already present (line ~1494-1497):
if c.options.CopilotHome != "" {
c.process.Env = setEnvValue(c.process.Env, "COPILOT_HOME", c.options.CopilotHome)
}
// This PR adds (redundant + incorrect):
// Set COPILOT_HOME if configured
if c.options.CopilotHome != "" {
c.process.Env = append(c.process.Env, "COPILOT_HOME="+c.options.CopilotHome) // ← duplicate!
}setEnvValue replaces an existing entry if it's already in the slice; append just adds another entry. The result is COPILOT_HOME appearing twice in the spawned process's environment. On most systems the first value wins, so the second assignment has no effect — but it's still incorrect and confusing.
Suggestion: Remove the newly added block entirely. The existing setEnvValue call at line 1496 is sufficient and was already there before this PR.
Add a `remote` option to CopilotClientOptions in all 4 language SDKs (Node, Python, Go, .NET) that passes `--remote` to the CLI process, enabling Mission Control integration for GitHub web and mobile access. Also adds `session.rpc.remote.enable()` and `session.rpc.remote.disable()` RPC methods for on-demand per-session toggling, providing parity with the CLI's `/remote on` and `/remote off` commands. Changes: - Node: remote option in CopilotClientOptions, generated RPC types - Python: remote field on SubprocessConfig, manual RPC types - Go: Remote field on ClientOptions, manual RPC types - .NET: Remote property on CopilotClientOptions + clone, generated RPC - Docs: new docs/features/remote-sessions.md with all-language examples - Updated docs/features/index.md and docs/index.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
option not in published SDK yet, fragments lack standalone context) - Fix C# permission handler: use Kind=Approved pattern - Fix Go permission handler: use correct 2-param signature Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
force-pushed
the
feat/remote-sessions
branch
from
9847c1e to
c3c877c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1192 · ● 824.1K
| session: self.session, | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Cross-SDK consistency gap — Rust ClientOptions is missing the remote flag
The Rust SDK correctly adds the session.rpc.remote.enable() / .disable() RPC methods here, but it is missing the client-level remote: bool option that all four other SDKs received in this PR:
| SDK | Client-level option | CLI flag added |
|---|---|---|
| Node.js | CopilotClientOptions.remote?: boolean |
✅ |
| Python | SubprocessConfig.remote: bool |
✅ |
| Go | ClientOptions.Remote bool |
✅ |
| .NET | CopilotClientOptions.Remote bool |
✅ |
| Rust | (not added) | ❌ |
Without the remote field in ClientOptions, Rust users cannot pass --remote to the spawned CLI process at startup (the "always-on" mode). They can work around it today via ClientOptions.extra_args, but that's not discoverable and breaks the API symmetry.
To bring the Rust SDK into full parity, rust/src/lib.rs needs:
- A
pub remote: boolfield onClientOptions(nearsession_idle_timeout_seconds, ~line 350) - A
falsedefault in theDefaultimpl (~line 632) - A
pub fn with_remote(mut self, remote: bool) -> Selfbuilder method - A
"remote"entry in theDebugimpl (~line 398) - The
--remoteflag appended in the CLI spawn path (near lines 1249/1273 wheresession_idle_timeout_argsandextra_argsare consumed)
Example implementation:
/// Enable remote session support (Mission Control integration).
/// When `true`, sessions in a GitHub repository working directory are
/// accessible from GitHub web and mobile.
/// Passes `--remote` to the spawned CLI process.
pub remote: bool,// in the spawn path, after session_idle_timeout_args:
if options.remote {
cmd.arg("--remote");
}
Cross-SDK Consistency Review ✅ (one minor issue)This PR adds remote session support consistently across all five SDKs (Node.js, Python, Go, .NET, Rust). The feature is well-structured and the naming follows each language's conventions:
One issue found
These appear to be auto-generated leftovers. The Removing those three types from
|
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1192 · ● 2.1M
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct SessionRemoteEnableParams { | ||
| /// Target session identifier | ||
| pub session_id: SessionId, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct SessionRemoteEnableResult { | ||
| /// Whether remote steering is enabled | ||
| pub remote_steerable: bool, | ||
| /// Mission Control frontend URL for this session | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub url: Option<String>, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct SessionRemoteDisableParams { | ||
| /// Target session identifier | ||
| pub session_id: SessionId, | ||
| } |
There was a problem hiding this comment.
Orphaned public types / duplicate of RemoteEnableResult
Three public types are added here that are never referenced anywhere in the codebase:
SessionRemoteEnableParams— not used;rpc.rspasses the session ID via an inlineserde_json::json!()literal instead.SessionRemoteEnableResult— an exact duplicate ofRemoteEnableResult(line 1288 of this same file), which is the type actually returned bySessionRpcRemote::enable(). Having two identical public types with different names will confuse library consumers.SessionRemoteDisableParams— not used for the same reason asSessionRemoteEnableParams.
For comparison, the .NET SDK defines the equivalent request types as internal sealed class (not part of the public API), and Go uses map[string]any inline — neither exposes them publicly.
Suggestion: Remove SessionRemoteEnableParams, SessionRemoteEnableResult, and SessionRemoteDisableParams. If the params structs are ever needed in the future they can be added then, and the result type is already covered by RemoteEnableResult.
Summary
Adds remote session support (Mission Control integration) to all 4 language SDKs, providing parity with the CLI's
--remoteflag and/remote on//remote offcommands.Two complementary mechanisms
1. Client-level
remoteoption (always-on)Set
remote: trueonCopilotClientOptionsto pass--remoteto the CLI process. All sessions in a GitHub repo working directory automatically get a remote URL viasession.infoevents.2. Per-session
session.rpc.remote.enable()/.disable()(on-demand)Toggle remote mid-session — equivalent to
/remote onand/remote offin the CLI.Changes
Client option (all 4 SDKs)
types.ts,client.tsremote?: booleanclient.pyremote: bool = FalseonSubprocessConfigtypes.go,client.goRemote boolonClientOptionsTypes.cs,Client.csbool RemoteonCopilotClientOptions+ cloneRPC types (generated + manual)
session.rpc.remote.enable()/.disable()session.Rpc.Remote.EnableAsync()/.DisableAsync()session.RPC.Remote.Enable(ctx)/.Disable(ctx)session.rpc.remote.enable()/.disable()Documentation
docs/features/remote-sessions.md— covers both always-on and on-demand patterns with all-language examples, QR code library recommendationsdocs/features/index.mdanddocs/index.mdVerification
tsc --noEmit)go build ./...)dotnet build)Related
session.remote.enable/.disableRPC methods