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)))
Add a first-class option to CopilotClientOptions in all four SDKs (Node/TS, Python, Go, .NET) that sets the COPILOT_HOME environment variable on the spawned CLI process, allowing users to control where the CLI stores session state, config, and other data files. This addresses environments with restricted write access (e.g., M365 B2 service where only specific directories like D:\data are writable). - Node/TS: copilotHome?: string - Python: copilot_home: str | None - Go: CopilotHome string (also used as fallback for embedded CLI cache) - .NET: CopilotHome: string? The explicit option takes priority over any COPILOT_HOME set in the raw env option. The option is ignored when connecting to an external server via cliUrl. Closes github/copilot-sdk-partners#29 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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. |
Cross-SDK Consistency ReviewThe remote session feature is added consistently to all four SDKs — the However, I found two issues in this PR: 🐛 Bug — Go:
|
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.
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