Skip to content

Commit 9678a79

Browse files
patnikoCopilot
andcommitted
fix: address PR review feedback
- Add unit tests for copilotHome option in all 4 SDKs - Fix Python README: add missing 'Ignored when using ExternalServerConfig' caveat - Fix Go README: clarify that CopilotHome option controls CLI subprocess env, not embedded binary cache (use COPILOT_HOME env var or embeddedcli.Config.Dir for that) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f2719df commit 9678a79

6 files changed

Lines changed: 64 additions & 2 deletions

File tree

dotnet/test/Unit/CloneTests.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public void CopilotClientOptions_Clone_CopiesAllProperties()
2626
Environment = new Dictionary<string, string> { ["KEY"] = "value" },
2727
GitHubToken = "ghp_test",
2828
UseLoggedInUser = false,
29+
CopilotHome = "/custom/copilot/home",
2930
SessionIdleTimeoutSeconds = 600,
3031
};
3132

@@ -43,6 +44,7 @@ public void CopilotClientOptions_Clone_CopiesAllProperties()
4344
Assert.Equal(original.Environment, clone.Environment);
4445
Assert.Equal(original.GitHubToken, clone.GitHubToken);
4546
Assert.Equal(original.UseLoggedInUser, clone.UseLoggedInUser);
47+
Assert.Equal(original.CopilotHome, clone.CopilotHome);
4648
Assert.Equal(original.SessionIdleTimeoutSeconds, clone.SessionIdleTimeoutSeconds);
4749
}
4850

go/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ Event types: `SessionLifecycleCreated`, `SessionLifecycleDeleted`, `SessionLifec
133133
- `CLIPath` (string): Path to CLI executable (default: "copilot" or `COPILOT_CLI_PATH` env var)
134134
- `CLIUrl` (string): URL of existing CLI server (e.g., `"localhost:8080"`, `"http://127.0.0.1:9000"`, or just `"8080"`). When provided, the client will not spawn a CLI process.
135135
- `Cwd` (string): Working directory for CLI process
136-
- `CopilotHome` (string): Base directory for Copilot data (session state, config, etc.). Sets `COPILOT_HOME` on the spawned CLI process. When empty, the CLI defaults to `~/.copilot`. Also used as the default cache directory for the embedded CLI binary. Useful in restricted environments where only specific directories are writable. Ignored when using `CLIUrl`.
136+
- `CopilotHome` (string): Base directory for Copilot data (session state, config, etc.). Sets `COPILOT_HOME` on the spawned CLI process. When empty, the CLI defaults to `~/.copilot`. Useful in restricted environments where only specific directories are writable. Ignored when using `CLIUrl`. To also control where the embedded CLI binary is cached, set the `COPILOT_HOME` environment variable in the parent process or use `embeddedcli.Config.Dir`.
137137
- `Port` (int): Server port for TCP mode (default: 0 for random)
138138
- `UseStdio` (bool): Use stdio transport instead of TCP (default: true)
139139
- `LogLevel` (string): Log level (default: "info")

go/client_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,26 @@ func TestClient_AuthOptions(t *testing.T) {
344344
})
345345
}
346346

347+
func TestClient_CopilotHome(t *testing.T) {
348+
t.Run("should accept CopilotHome option", func(t *testing.T) {
349+
client := NewClient(&ClientOptions{
350+
CopilotHome: "/custom/copilot/home",
351+
})
352+
353+
if client.options.CopilotHome != "/custom/copilot/home" {
354+
t.Errorf("Expected CopilotHome to be '/custom/copilot/home', got %q", client.options.CopilotHome)
355+
}
356+
})
357+
358+
t.Run("should default CopilotHome to empty string", func(t *testing.T) {
359+
client := NewClient(&ClientOptions{})
360+
361+
if client.options.CopilotHome != "" {
362+
t.Errorf("Expected CopilotHome to be empty, got %q", client.options.CopilotHome)
363+
}
364+
})
365+
}
366+
347367
func TestClient_EnvOptions(t *testing.T) {
348368
t.Run("should store custom environment variables", func(t *testing.T) {
349369
client := NewClient(&ClientOptions{

nodejs/test/client.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,23 @@ describe("CopilotClient", () => {
662662
expect((client as any).options.useLoggedInUser).toBe(false);
663663
});
664664

665+
it("should accept copilotHome option", () => {
666+
const client = new CopilotClient({
667+
copilotHome: "/custom/copilot/home",
668+
logLevel: "error",
669+
});
670+
671+
expect((client as any).options.copilotHome).toBe("/custom/copilot/home");
672+
});
673+
674+
it("should leave copilotHome undefined when not provided", () => {
675+
const client = new CopilotClient({
676+
logLevel: "error",
677+
});
678+
679+
expect((client as any).options.copilotHome).toBeUndefined();
680+
});
681+
665682
it("should throw error when gitHubToken is used with cliUrl", () => {
666683
expect(() => {
667684
new CopilotClient({

python/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ CopilotClient(
153153
- `log_level` (str): Log level (default: "info")
154154
- `env` (dict | None): Environment variables for the CLI process
155155
- `github_token` (str | None): GitHub token for authentication. When provided, takes priority over other auth methods.
156-
- `copilot_home` (str | None): Base directory for Copilot data (session state, config, etc.). Sets `COPILOT_HOME` on the spawned CLI process. When `None`, the CLI defaults to `~/.copilot`. Useful in restricted environments where only specific directories are writable.
156+
- `copilot_home` (str | None): Base directory for Copilot data (session state, config, etc.). Sets `COPILOT_HOME` on the spawned CLI process. When `None`, the CLI defaults to `~/.copilot`. Useful in restricted environments where only specific directories are writable. Ignored when using `ExternalServerConfig`.
157157
- `use_logged_in_user` (bool | None): Whether to use logged-in user for authentication (default: True, but False when `github_token` is provided).
158158
- `telemetry` (dict | None): OpenTelemetry configuration for the CLI process. Providing this enables telemetry — no separate flag needed. See [Telemetry](#telemetry) below.
159159

python/test_client.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,29 @@ def test_default_session_idle_timeout_seconds_is_none(self):
222222
assert client._config.session_idle_timeout_seconds is None
223223

224224

225+
class TestCopilotHome:
226+
def test_accepts_copilot_home(self):
227+
client = CopilotClient(
228+
SubprocessConfig(
229+
cli_path=CLI_PATH,
230+
copilot_home="/custom/copilot/home",
231+
log_level="error",
232+
)
233+
)
234+
assert isinstance(client._config, SubprocessConfig)
235+
assert client._config.copilot_home == "/custom/copilot/home"
236+
237+
def test_default_copilot_home_is_none(self):
238+
client = CopilotClient(
239+
SubprocessConfig(
240+
cli_path=CLI_PATH,
241+
log_level="error",
242+
)
243+
)
244+
assert isinstance(client._config, SubprocessConfig)
245+
assert client._config.copilot_home is None
246+
247+
225248
class TestOverridesBuiltInTool:
226249
@pytest.mark.asyncio
227250
async def test_overrides_built_in_tool_sent_in_tool_definition(self):

0 commit comments

Comments
 (0)