From f15fab25f530c623c3318997b005f1128c205ff1 Mon Sep 17 00:00:00 2001 From: Lutz Roeder Date: Thu, 13 Aug 2026 11:28:31 -0700 Subject: [PATCH 1/3] Add built-in plugin directory support --- docs/features/plugin-directories.md | 40 +++++ dotnet/src/Client.cs | 43 ++++++ dotnet/src/Types.cs | 8 + dotnet/test/Unit/CloneTests.cs | 3 + dotnet/test/Unit/GitHubTelemetryTests.cs | 66 ++++++++ go/client.go | 18 +++ go/client_test.go | 141 ++++++++++++++++++ go/types.go | 5 + .../com/github/copilot/CopilotClient.java | 28 +++- .../copilot/rpc/CopilotClientOptions.java | 44 +++++- .../copilot/BuiltinPluginDirectoriesTest.java | 128 ++++++++++++++++ nodejs/src/client.ts | 24 ++- nodejs/src/types.ts | 7 + nodejs/test/client.test.ts | 47 +++++- python/copilot/client.py | 22 +++ python/test_client.py | 45 ++++++ rust/src/lib.rs | 55 +++++++ rust/tests/builtin_plugin_directories_test.rs | 135 +++++++++++++++++ 18 files changed, 849 insertions(+), 10 deletions(-) create mode 100644 java/src/test/java/com/github/copilot/BuiltinPluginDirectoriesTest.java create mode 100644 rust/tests/builtin_plugin_directories_test.rs diff --git a/docs/features/plugin-directories.md b/docs/features/plugin-directories.md index ccd95df95..b2d349398 100644 --- a/docs/features/plugin-directories.md +++ b/docs/features/plugin-directories.md @@ -240,6 +240,46 @@ let client = Client::start( > The example above uses an stdio runtime connection — the default when the SDK bundles the CLI. If you connect to an external runtime via a URL (`forUri` / `ForUri`), pass `--plugin-dir` to the long-running CLI server when you start it; the SDK does not forward `--plugin-dir` to runtimes it didn't spawn. +## Trusted host-bundled plugin directories + +Applications that ship their own trusted plugins can register them as a client startup option. The SDK sends the complete ordered set after connecting and verifying the protocol, before `start` returns or any session can be created. Paths must be absolute; leaving the option unset or empty makes no RPC call. + +
+Node.js / TypeScript + + +```typescript +import { CopilotClient } from "@github/copilot-sdk"; + +async function main() { + const client = new CopilotClient({ + builtinPluginDirectories: [ + "/opt/my-app/copilot-plugins/core", + "/opt/my-app/copilot-plugins/github", + ], + }); + await client.start(); +} + +main(); +``` + + +
+ +The equivalent option in each SDK is: + +| SDK | Startup option | +|---|---| +| Node.js / TypeScript | `builtinPluginDirectories: string[]` | +| Python | `builtin_plugin_directories=[...]` | +| Go | `BuiltinPluginDirectories: []string{...}` | +| .NET | `BuiltinPluginDirectories = [...]` | +| Java | `.setBuiltinPluginDirectories(List.of(Path.of(...)))` | +| Rust | `.with_builtin_plugin_directories([...])` | + +This is a trust boundary for plugins bundled and controlled by the host application. It is distinct from `--plugin-dir`, which is a CLI process launch argument for explicitly loading ordinary plugin directories. The startup option also works when connecting to an existing runtime because it is sent over JSON-RPC rather than forwarded as a process argument. + ## What a plugin can contribute Loading a plugin directory makes its extensions visible to every session created by the client. The runtime merges plugin-provided extensions with anything you register inline: diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 8116be28e..fa3615dc4 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -76,6 +76,7 @@ public sealed partial class CopilotClient : IDisposable, IAsyncDisposable private readonly ILogger _logger; private readonly int? _optionsPort; private readonly string? _optionsHost; + private readonly string[] _builtinPluginDirectories; private readonly Func>>? _onListModels; private readonly List _lifecycleHandlers = []; @@ -138,6 +139,17 @@ public CopilotClient(CopilotClientOptions? options = null) { _options = options ?? new(); _connection = _options.Connection ?? ResolveDefaultConnection(_options); + _builtinPluginDirectories = _options.BuiltinPluginDirectories?.ToArray() ?? []; + foreach (var path in _builtinPluginDirectories) + { + if (!IsFullyQualifiedPath(path)) + { + throw new ArgumentException( + $"{nameof(CopilotClientOptions)}.{nameof(CopilotClientOptions.BuiltinPluginDirectories)} " + + $"must contain only absolute paths: {path}", + nameof(options)); + } + } switch (_connection) { @@ -317,6 +329,26 @@ private static Uri ParseRuntimeUrl(string url) return new Uri(url); } + private static bool IsFullyQualifiedPath(string path) + { + if (string.IsNullOrEmpty(path) || !Path.IsPathRooted(path)) + { + return false; + } +#if NETSTANDARD2_0 + if (Path.DirectorySeparatorChar != '\\') + { + return true; + } + + bool IsSeparator(char value) => value == '\\' || value == '/'; + return (path.Length >= 3 && path[1] == ':' && IsSeparator(path[2])) + || (path.Length >= 2 && IsSeparator(path[0]) && IsSeparator(path[1])); +#else + return Path.IsPathFullyQualified(path); +#endif + } + /// /// Starts the Copilot client and connects to the server. /// @@ -423,6 +455,13 @@ async Task StartCoreAsync(CancellationToken ct) "CopilotClient.StartAsync protocol verification complete. Elapsed={Elapsed}", startTimestamp); + if (_builtinPluginDirectories.Length > 0) + { + var request = new BuiltinPluginDirectoriesRequest(_builtinPluginDirectories); + await InvokeRpcAsync( + connection.Rpc, "plugins.builtin.set", [request], null, ct); + } + var sessionFsTimestamp = Stopwatch.GetTimestamp(); await ConfigureSessionFsAsync(ct); if (_options.SessionFs is not null) @@ -2946,6 +2985,9 @@ internal record ConnectHandshakeRequest( string? Token, [property: JsonPropertyName("enableGitHubTelemetryForwarding")] bool? EnableGitHubTelemetryForwarding = null); + internal record BuiltinPluginDirectoriesRequest( + string[] Paths); + internal record SetForegroundSessionRequest( string SessionId); @@ -2981,6 +3023,7 @@ internal record HooksInvokeResponse( [JsonSerializable(typeof(GetSessionMetadataRequest))] [JsonSerializable(typeof(GetSessionMetadataResponse))] [JsonSerializable(typeof(ConnectHandshakeRequest))] + [JsonSerializable(typeof(BuiltinPluginDirectoriesRequest))] [JsonSerializable(typeof(McpOAuthTokenStorageMode))] [JsonSerializable(typeof(EmbeddingCacheStorageMode))] [JsonSerializable(typeof(ModelCapabilitiesOverride))] diff --git a/dotnet/src/Types.cs b/dotnet/src/Types.cs index 59f892140..c0810b387 100644 --- a/dotnet/src/Types.cs +++ b/dotnet/src/Types.cs @@ -309,6 +309,7 @@ private CopilotClientOptions(CopilotClientOptions? other) Connection = other.Connection; WorkingDirectory = other.WorkingDirectory; BaseDirectory = other.BaseDirectory; + BuiltinPluginDirectories = other.BuiltinPluginDirectories is null ? null : [.. other.BuiltinPluginDirectories]; Environment = other.Environment; GitHubToken = other.GitHubToken; Logger = other.Logger; @@ -358,6 +359,13 @@ private CopilotClientOptions(CopilotClientOptions? other) /// public string? BaseDirectory { get; set; } + /// + /// Absolute paths to trusted plugin directories bundled by the host. + /// When non-empty, the complete set is registered with the runtime during + /// startup before sessions can be created. + /// + public IList? BuiltinPluginDirectories { get; set; } + /// /// Log level for the Copilot runtime. Use the well-known values on /// (, diff --git a/dotnet/test/Unit/CloneTests.cs b/dotnet/test/Unit/CloneTests.cs index 844eca135..4bacdfe33 100644 --- a/dotnet/test/Unit/CloneTests.cs +++ b/dotnet/test/Unit/CloneTests.cs @@ -20,6 +20,7 @@ public void CopilotClientOptions_Clone_CopiesAllProperties() GitHubToken = "ghp_test", UseLoggedInUser = false, BaseDirectory = "/custom/copilot/home", + BuiltinPluginDirectories = ["/plugins/core", "/plugins/github"], EnableRemoteSessions = true, SessionIdleTimeoutSeconds = 600, }; @@ -33,6 +34,8 @@ public void CopilotClientOptions_Clone_CopiesAllProperties() Assert.Equal(original.GitHubToken, clone.GitHubToken); Assert.Equal(original.UseLoggedInUser, clone.UseLoggedInUser); Assert.Equal(original.BaseDirectory, clone.BaseDirectory); + Assert.Equal(original.BuiltinPluginDirectories, clone.BuiltinPluginDirectories); + Assert.NotSame(original.BuiltinPluginDirectories, clone.BuiltinPluginDirectories); Assert.Equal(original.EnableRemoteSessions, clone.EnableRemoteSessions); Assert.Equal(original.SessionIdleTimeoutSeconds, clone.SessionIdleTimeoutSeconds); } diff --git a/dotnet/test/Unit/GitHubTelemetryTests.cs b/dotnet/test/Unit/GitHubTelemetryTests.cs index 24e633387..5919e4d92 100644 --- a/dotnet/test/Unit/GitHubTelemetryTests.cs +++ b/dotnet/test/Unit/GitHubTelemetryTests.cs @@ -17,6 +17,60 @@ namespace GitHub.Copilot.Test.Unit; public sealed class GitHubTelemetryTests { + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task BuiltinPluginDirectories_Default_Or_Empty_Does_Not_Call_Rpc(bool useEmpty) + { + await using var server = await FakeTelemetryServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions + { + Connection = RuntimeConnection.ForUri(server.Url), + BuiltinPluginDirectories = useEmpty ? [] : null, + }); + + await client.StartAsync(); + + Assert.Equal(0, server.BuiltinPluginSetCount); + } + + [Fact] + public async Task BuiltinPluginDirectories_Are_Set_Once_Before_Start_Completes() + { + var paths = new[] + { + Path.GetFullPath(Path.Combine("plugins", "core")), + Path.GetFullPath(Path.Combine("plugins", "github")), + }; + await using var server = await FakeTelemetryServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions + { + Connection = RuntimeConnection.ForUri(server.Url), + BuiltinPluginDirectories = paths, + }); + + await client.StartAsync(); + + Assert.Equal(1, server.BuiltinPluginSetCount); + var payload = server.LastBuiltinPluginParams + ?? throw new InvalidOperationException("plugins.builtin.set was not captured."); + Assert.Collection( + payload.GetProperty("paths").EnumerateArray(), + value => Assert.Equal(paths[0], value.GetString()), + value => Assert.Equal(paths[1], value.GetString())); + } + + [Fact] + public void BuiltinPluginDirectories_Reject_Relative_Paths() + { + var exception = Assert.Throws(() => new CopilotClient(new CopilotClientOptions + { + BuiltinPluginDirectories = ["plugins/core"], + })); + + Assert.Contains("absolute paths", exception.Message); + } + [Fact] public async Task CreateSession_Opts_Into_Forwarding_When_Handler_Provided() { @@ -266,6 +320,10 @@ public string Url public JsonElement? LastConnectParams { get; private set; } + public JsonElement? LastBuiltinPluginParams { get; private set; } + + public int BuiltinPluginSetCount { get; private set; } + public static Task StartAsync() { var listener = new TcpListener(IPAddress.Loopback, 0); @@ -347,6 +405,7 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel object? result = method switch { "connect" => CaptureConnect(request), + "plugins.builtin.set" => CaptureBuiltinPluginDirectories(request), "session.create" => CaptureCreate(request), "session.resume" => CaptureResume(request), "session.send" => new Dictionary { ["messageId"] = "message-1" }, @@ -375,6 +434,13 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel }; } + private Dictionary CaptureBuiltinPluginDirectories(JsonElement request) + { + BuiltinPluginSetCount++; + LastBuiltinPluginParams = request.TryGetProperty("params", out var p) ? p.Clone() : null; + return new Dictionary(); + } + private Dictionary CaptureCreate(JsonElement request) { LastCreateParams = request.TryGetProperty("params", out var p) ? p.Clone() : null; diff --git a/go/client.go b/go/client.go index 7d9b9e6bf..f36133672 100644 --- a/go/client.go +++ b/go/client.go @@ -38,6 +38,7 @@ import ( "net" "os" "os/exec" + "path/filepath" "regexp" "strconv" "strings" @@ -225,6 +226,12 @@ func NewClient(options *ClientOptions) *Client { if options != nil { opts = *options } + for _, path := range opts.BuiltinPluginDirectories { + if !filepath.IsAbs(path) { + panic(fmt.Sprintf("BuiltinPluginDirectories must contain only absolute paths: %s", path)) + } + } + opts.BuiltinPluginDirectories = append([]string(nil), opts.BuiltinPluginDirectories...) // Resolve the connection. An explicit connection always wins; otherwise // honor the same process/environment override as the other SDKs. @@ -453,6 +460,17 @@ func (c *Client) Start(ctx context.Context) error { return errors.Join(err, killErr) } + if len(c.options.BuiltinPluginDirectories) > 0 { + if _, err := c.client.Request(ctx, "plugins.builtin.set", map[string]any{ + "paths": c.options.BuiltinPluginDirectories, + }); err != nil { + c.client.Stop() + killErr := c.killProcess() + c.state = stateError + return errors.Join(err, killErr) + } + } + // If a session filesystem provider was configured, register it. if c.options.SessionFS != nil { req := &rpc.SessionFSSetProviderRequest{ diff --git a/go/client_test.go b/go/client_test.go index 8152fb469..0980f393c 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -139,6 +139,147 @@ func TestClient_URLParsing(t *testing.T) { }) } +func TestClient_BuiltinPluginDirectories(t *testing.T) { + t.Run("default and empty do not call RPC", func(t *testing.T) { + for _, paths := range [][]string{nil, []string{}} { + t.Run(fmt.Sprintf("len=%d", len(paths)), func(t *testing.T) { + url, requests, cleanup := newStartupRPCServer(t) + defer cleanup() + + client := NewClient(&ClientOptions{ + Connection: URIConnection{URL: url}, + BuiltinPluginDirectories: paths, + }) + if err := client.Start(t.Context()); err != nil { + t.Fatalf("Start failed: %v", err) + } + defer client.ForceStop() + + if got := countMethod(requests(), "plugins.builtin.set"); got != 0 { + t.Fatalf("plugins.builtin.set call count = %d, want 0", got) + } + }) + } + }) + + t.Run("configured paths call RPC once", func(t *testing.T) { + url, requests, cleanup := newStartupRPCServer(t) + defer cleanup() + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd failed: %v", err) + } + paths := []string{ + filepath.Join(cwd, "plugins", "core"), + filepath.Join(cwd, "plugins", "github"), + } + + client := NewClient(&ClientOptions{ + Connection: URIConnection{URL: url}, + BuiltinPluginDirectories: paths, + }) + if err := client.Start(t.Context()); err != nil { + t.Fatalf("Start failed: %v", err) + } + defer client.ForceStop() + + var calls []startupRPCRequest + for _, request := range requests() { + if request.Method == "plugins.builtin.set" { + calls = append(calls, request) + } + } + if len(calls) != 1 { + t.Fatalf("plugins.builtin.set call count = %d, want 1", len(calls)) + } + var payload struct { + Paths []string `json:"paths"` + } + if err := json.Unmarshal(calls[0].Params, &payload); err != nil { + t.Fatalf("decode plugins.builtin.set params: %v", err) + } + if !reflect.DeepEqual(payload.Paths, paths) { + t.Fatalf("paths = %v, want %v", payload.Paths, paths) + } + }) + + t.Run("relative path panics", func(t *testing.T) { + defer func() { + if recovered := recover(); recovered == nil { + t.Fatal("expected NewClient to panic") + } + }() + NewClient(&ClientOptions{BuiltinPluginDirectories: []string{"plugins/core"}}) + }) +} + +type startupRPCRequest struct { + Method string + Params json.RawMessage +} + +func newStartupRPCServer(t *testing.T) (string, func() []startupRPCRequest, func()) { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + + var mux sync.Mutex + var requests []startupRPCRequest + serverReady := make(chan *jsonrpc2.Client, 1) + go func() { + conn, acceptErr := listener.Accept() + if acceptErr != nil { + return + } + server := jsonrpc2.NewClient(conn, conn) + record := func(method string, params json.RawMessage) { + mux.Lock() + requests = append(requests, startupRPCRequest{ + Method: method, + Params: append(json.RawMessage(nil), params...), + }) + mux.Unlock() + } + server.SetRequestHandler("connect", func(params json.RawMessage) (json.RawMessage, *jsonrpc2.Error) { + record("connect", params) + return []byte(`{"ok":true,"protocolVersion":3,"version":"test"}`), nil + }) + server.SetRequestHandler("plugins.builtin.set", func(params json.RawMessage) (json.RawMessage, *jsonrpc2.Error) { + record("plugins.builtin.set", params) + return []byte(`{}`), nil + }) + server.Start() + serverReady <- server + }() + + snapshot := func() []startupRPCRequest { + mux.Lock() + defer mux.Unlock() + return append([]startupRPCRequest(nil), requests...) + } + cleanup := func() { + listener.Close() + select { + case server := <-serverReady: + server.Stop() + case <-time.After(time.Second): + } + } + return listener.Addr().String(), snapshot, cleanup +} + +func countMethod(requests []startupRPCRequest, method string) int { + count := 0 + for _, request := range requests { + if request.Method == method { + count++ + } + } + return count +} + func TestClient_StopRequestsRuntimeShutdownForOwnedProcess(t *testing.T) { rpcClient, server, shutdownCalled := newRuntimeShutdownRpcPair(t) client := &Client{ diff --git a/go/types.go b/go/types.go index 8690695f2..c0b34586d 100644 --- a/go/types.go +++ b/go/types.go @@ -129,6 +129,11 @@ type ClientOptions struct { // location. // Ignored when connecting to an existing runtime via [URIConnection]. BaseDirectory string + // BuiltinPluginDirectories contains absolute paths to trusted plugin + // directories bundled by the host. When non-empty, Start replaces the + // runtime's complete trusted built-in plugin directory set before sessions + // can be created. + BuiltinPluginDirectories []string // LogLevel for the runtime. When empty (the default), the runtime // uses its own default level; the SDK does not pass --log-level. // Recognized values: "none", "error", "warning", "info", "debug", "all". diff --git a/java/src/main/java/com/github/copilot/CopilotClient.java b/java/src/main/java/com/github/copilot/CopilotClient.java index 44878b87e..864c5ff1b 100644 --- a/java/src/main/java/com/github/copilot/CopilotClient.java +++ b/java/src/main/java/com/github/copilot/CopilotClient.java @@ -227,10 +227,9 @@ private CompletableFuture startCore() { private Connection startCoreBody() { Process process = null; + JsonRpcClient rpc = null; long startNanos = System.nanoTime(); try { - JsonRpcClient rpc; - if (optionsHost != null && optionsPort != null) { // External server (TCP) rpc = serverManager.connectToServer(null, optionsHost, optionsPort); @@ -245,11 +244,12 @@ private Connection startCoreBody() { LoggingHelpers.logTiming(LOG, Level.FINE, "CopilotClient.start transport setup complete. Elapsed={Elapsed}", startNanos); - Connection connection = new Connection(rpc, process, new ServerRpc(rpc::invoke)); + JsonRpcClient connectedRpc = rpc; + Connection connection = new Connection(connectedRpc, process, new ServerRpc(connectedRpc::invoke)); // Register handlers for server-to-client calls RpcHandlerDispatcher dispatcher = new RpcHandlerDispatcher(sessions, lifecycleManager::dispatch, executor); - dispatcher.registerHandlers(rpc); + dispatcher.registerHandlers(connectedRpc); // Register the LLM inference request handler when configured. com.github.copilot.CopilotRequestHandler requestHandler = this.options.getRequestHandler(); @@ -257,7 +257,7 @@ private Connection startCoreBody() { if (hasLlmInference) { LlmInferenceAdapter llmAdapter = new LlmInferenceAdapter(requestHandler, () -> connection.serverRpc().llmInference, executor); - llmAdapter.registerHandlers(rpc); + llmAdapter.registerHandlers(connectedRpc); } // Register the GitHub telemetry forwarding handler when configured. @@ -265,7 +265,7 @@ private Connection startCoreBody() { .getOnGitHubTelemetry(); if (onGitHubTelemetry != null) { GitHubTelemetryAdapter telemetryAdapter = new GitHubTelemetryAdapter(onGitHubTelemetry); - telemetryAdapter.registerHandlers(rpc); + telemetryAdapter.registerHandlers(connectedRpc); } // Verify protocol version @@ -273,6 +273,15 @@ private Connection startCoreBody() { LoggingHelpers.logTiming(LOG, Level.FINE, "CopilotClient.start protocol verification complete. Elapsed={Elapsed}", startNanos); + var builtinPluginDirectories = options.getBuiltinPluginDirectories(); + if (builtinPluginDirectories != null && !builtinPluginDirectories.isEmpty()) { + var paths = new ArrayList(builtinPluginDirectories.size()); + for (var path : builtinPluginDirectories) { + paths.add(path.toString()); + } + connection.rpc.invoke("plugins.builtin.set", Map.of("paths", paths), Void.class).join(); + } + // Register as the runtime's LLM inference provider once connected. if (hasLlmInference) { connection.serverRpc().llmInference.setProvider().join(); @@ -289,6 +298,13 @@ private Connection startCoreBody() { if (process != null) { cleanupCliProcess(process, true); } + if (rpc != null) { + try { + rpc.close(); + } catch (Exception closeError) { + LOG.log(Level.FINE, "Error closing RPC after failed startup", closeError); + } + } String stderr = serverManager.getStderrOutput(); if (!stderr.isEmpty()) { throw new CompletionException(new IOException( diff --git a/java/src/main/java/com/github/copilot/rpc/CopilotClientOptions.java b/java/src/main/java/com/github/copilot/rpc/CopilotClientOptions.java index 0d4494d73..800c6e5d5 100644 --- a/java/src/main/java/com/github/copilot/rpc/CopilotClientOptions.java +++ b/java/src/main/java/com/github/copilot/rpc/CopilotClientOptions.java @@ -4,11 +4,15 @@ package com.github.copilot.rpc; +import java.nio.file.Path; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.OptionalInt; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.function.Function; @@ -19,8 +23,6 @@ import com.github.copilot.CopilotExperimental; import com.github.copilot.CopilotRequestHandler; import com.github.copilot.generated.rpc.GitHubTelemetryNotification; -import java.util.Optional; -import java.util.OptionalInt; /** * Configuration options for creating a @@ -48,6 +50,7 @@ public class CopilotClientOptions { @Deprecated private boolean autoRestart; private boolean autoStart = true; + private List builtinPluginDirectories; private String[] cliArgs; private String cliPath; private String cliUrl; @@ -119,6 +122,40 @@ public CopilotClientOptions setAutoStart(boolean autoStart) { return this; } + /** + * Gets the trusted plugin directories bundled by the host. + * + * @return a copy of the configured absolute paths, or {@code null} + */ + public List getBuiltinPluginDirectories() { + return builtinPluginDirectories != null ? new ArrayList<>(builtinPluginDirectories) : null; + } + + /** + * Sets trusted plugin directories bundled by the host. Every path must be + * absolute. When non-empty, the complete set is registered during startup + * before sessions can be created. + * + * @param paths + * absolute plugin directory paths, or {@code null}/empty to disable + * @return this options instance for method chaining + */ + public CopilotClientOptions setBuiltinPluginDirectories(List paths) { + if (paths == null || paths.isEmpty()) { + this.builtinPluginDirectories = null; + return this; + } + for (Path path : paths) { + Objects.requireNonNull(path, "builtin plugin directory path must not be null"); + if (!path.isAbsolute()) { + throw new IllegalArgumentException( + "BuiltinPluginDirectories must contain only absolute paths: " + path); + } + } + this.builtinPluginDirectories = new ArrayList<>(paths); + return this; + } + /** * Gets the extra CLI arguments. *

@@ -751,6 +788,9 @@ public CopilotClientOptions clone() { CopilotClientOptions copy = new CopilotClientOptions(); copy.autoRestart = this.autoRestart; copy.autoStart = this.autoStart; + copy.builtinPluginDirectories = this.builtinPluginDirectories != null + ? new ArrayList<>(this.builtinPluginDirectories) + : null; copy.cliArgs = this.cliArgs != null ? this.cliArgs.clone() : null; copy.cliPath = this.cliPath; copy.cliUrl = this.cliUrl; diff --git a/java/src/test/java/com/github/copilot/BuiltinPluginDirectoriesTest.java b/java/src/test/java/com/github/copilot/BuiltinPluginDirectoriesTest.java new file mode 100644 index 000000000..fa353e627 --- /dev/null +++ b/java/src/test/java/com/github/copilot/BuiltinPluginDirectoriesTest.java @@ -0,0 +1,128 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; +import java.net.ServerSocket; +import java.net.Socket; +import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.databind.JsonNode; +import com.github.copilot.rpc.CopilotClientOptions; + +class BuiltinPluginDirectoriesTest { + + @Test + void defaultAndEmptyDoNotCallRpc() throws Exception { + assertDoesNotCallRpc(new CopilotClientOptions()); + assertDoesNotCallRpc(new CopilotClientOptions().setBuiltinPluginDirectories(List.of())); + } + + @Test + void configuredDirectoriesCallRpcOnceBeforeStartCompletes() throws Exception { + var paths = List.of(Path.of("").toAbsolutePath().resolve("plugins/core"), + Path.of("").toAbsolutePath().resolve("plugins/github")); + + try (var server = new FakeRuntimeServer(); + var client = new CopilotClient( + new CopilotClientOptions().setCliUrl(server.url()).setBuiltinPluginDirectories(paths))) { + client.start().get(15, TimeUnit.SECONDS); + + assertEquals(1, server.builtinSetCount()); + JsonNode params = server.awaitBuiltinParams(); + assertEquals(paths.get(0).toString(), params.path("paths").get(0).asText()); + assertEquals(paths.get(1).toString(), params.path("paths").get(1).asText()); + } + } + + @Test + void relativeDirectoryIsRejected() { + assertThrows(IllegalArgumentException.class, + () -> new CopilotClientOptions().setBuiltinPluginDirectories(List.of(Path.of("plugins/core")))); + } + + private static void assertDoesNotCallRpc(CopilotClientOptions options) throws Exception { + try (var server = new FakeRuntimeServer(); var client = new CopilotClient(options.setCliUrl(server.url()))) { + client.start().get(15, TimeUnit.SECONDS); + assertEquals(0, server.builtinSetCount()); + } + } + + private static final class FakeRuntimeServer implements AutoCloseable { + + private final ServerSocket serverSocket; + private final Thread acceptThread; + private final CompletableFuture ready = new CompletableFuture<>(); + private final CompletableFuture builtinParams = new CompletableFuture<>(); + private final AtomicInteger builtinSetCount = new AtomicInteger(); + + FakeRuntimeServer() throws IOException { + serverSocket = new ServerSocket(0); + acceptThread = new Thread(this::acceptLoop, "builtin-plugin-runtime"); + acceptThread.setDaemon(true); + acceptThread.start(); + } + + String url() { + return "127.0.0.1:" + serverSocket.getLocalPort(); + } + + int builtinSetCount() { + return builtinSetCount.get(); + } + + JsonNode awaitBuiltinParams() throws Exception { + return builtinParams.get(15, TimeUnit.SECONDS); + } + + private void acceptLoop() { + try { + Socket socket = serverSocket.accept(); + JsonRpcClient server = JsonRpcClient.fromSocket(socket); + server.registerMethodHandler("connect", (id, params) -> respond(server, id, + Map.of("ok", true, "protocolVersion", 3, "version", "test"))); + server.registerMethodHandler("plugins.builtin.set", (id, params) -> { + builtinSetCount.incrementAndGet(); + builtinParams.complete(params); + respond(server, id, Map.of()); + }); + ready.complete(server); + } catch (IOException e) { + ready.completeExceptionally(e); + builtinParams.completeExceptionally(e); + } + } + + private static void respond(JsonRpcClient server, String id, Object result) { + if (id == null) { + return; + } + try { + server.sendResponse(id, result); + } catch (IOException e) { + // Connection teardown can race the response during test cleanup. + } + } + + @Override + public void close() throws Exception { + JsonRpcClient server = ready.getNow(null); + if (server != null) { + server.close(); + } + serverSocket.close(); + } + } +} diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 1182b4106..30095186e 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -16,7 +16,7 @@ import { randomUUID } from "node:crypto"; import { existsSync } from "node:fs"; import { createRequire } from "node:module"; import { Socket } from "node:net"; -import { dirname, join } from "node:path"; +import { dirname, isAbsolute, join } from "node:path"; import { fileURLToPath } from "node:url"; import { createMessageConnection, @@ -522,6 +522,7 @@ export class CopilotClient { /** Connection-level session filesystem config, set via constructor option. */ private sessionFsConfig: SessionFsConfig | null = null; private requestHandler: CopilotRequestHandler | null = null; + private builtinPluginDirectories: string[] = []; private onGitHubTelemetry?: (notification: GitHubTelemetryNotification) => void | Promise; private clientGlobalHandlers: import("./generated/rpc.js").ClientGlobalApiHandlers = {}; @@ -682,6 +683,16 @@ export class CopilotClient { if (options.sessionFs) { this.validateSessionFsConfig(options.sessionFs); } + if (options.builtinPluginDirectories) { + for (const path of options.builtinPluginDirectories) { + if (!isAbsolute(path)) { + throw new Error( + `builtinPluginDirectories must contain only absolute paths: ${path}` + ); + } + } + this.builtinPluginDirectories = [...options.builtinPluginDirectories]; + } // Pre-parse the URI host/port and mark as external if applicable. if (conn.kind === "uri") { @@ -894,6 +905,17 @@ export class CopilotClient { // Verify protocol version compatibility await this.verifyProtocolVersion(); + if (this.builtinPluginDirectories.length > 0) { + try { + await this.connection!.sendRequest("plugins.builtin.set", { + paths: this.builtinPluginDirectories, + }); + } catch (error) { + await this.forceStop(); + throw error; + } + } + // If a session filesystem provider was configured, register it if (this.sessionFsConfig) { await this.connection!.sendRequest("sessionFs.setProvider", { diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 2567d7d31..3a5131b30 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -304,6 +304,13 @@ export interface CopilotClientOptions { */ baseDirectory?: string; + /** + * Absolute paths to trusted plugin directories bundled by the host. + * When non-empty, the complete set is registered with the runtime during + * startup before any sessions can be created. + */ + builtinPluginDirectories?: readonly string[]; + /** * Log level for the Copilot runtime. When omitted, the runtime uses its * own default (currently `"info"`). diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 254c126ed..49ec169c4 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -3,7 +3,7 @@ import { EventEmitter } from "node:events"; import { PassThrough } from "stream"; import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, resolve } from "node:path"; import { describe, expect, it, onTestFinished, vi } from "vitest"; import { approveAll, @@ -57,6 +57,51 @@ describe("approveAll", () => { }); describe("CopilotClient", () => { + async function startWithMockConnection( + builtinPluginDirectories?: readonly string[] + ): Promise> { + const client = new CopilotClient({ + connection: RuntimeConnection.forUri("localhost:1234"), + builtinPluginDirectories, + }); + const sendRequest = vi.fn(async () => ({})); + vi.spyOn(client as any, "connectToServer").mockImplementation(async () => { + (client as any).connection = { sendRequest }; + }); + vi.spyOn(client as any, "verifyProtocolVersion").mockResolvedValue(undefined); + + await client.start(); + return sendRequest; + } + + it.each([undefined, []])( + "does not configure built-in plugin directories when unset or empty", + async (builtinPluginDirectories) => { + const sendRequest = await startWithMockConnection(builtinPluginDirectories); + + expect(sendRequest).not.toHaveBeenCalledWith("plugins.builtin.set", expect.anything()); + } + ); + + it("configures built-in plugin directories before start completes", async () => { + const paths = [resolve("plugins/core"), resolve("plugins/github")]; + + const sendRequest = await startWithMockConnection(paths); + + expect(sendRequest).toHaveBeenCalledTimes(1); + expect(sendRequest).toHaveBeenCalledWith("plugins.builtin.set", { paths }); + }); + + it("rejects relative built-in plugin directories", () => { + expect( + () => + new CopilotClient({ + connection: RuntimeConnection.forUri("localhost:1234"), + builtinPluginDirectories: ["plugins/core"], + }) + ).toThrow(/builtinPluginDirectories.*absolute paths.*plugins\/core/); + }); + it("disposes the stdio connection when child stdin emits an error", async () => { const client = new CopilotClient(); onTestFinished(() => client.forceStop()); diff --git a/python/copilot/client.py b/python/copilot/client.py index 415f44ef5..6cdd765c3 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -655,6 +655,7 @@ class _CopilotClientOptions: env: dict[str, str] | None = None github_token: str | None = None base_directory: str | None = None + builtin_plugin_directories: tuple[str, ...] = () use_logged_in_user: bool | None = None telemetry: TelemetryConfig | None = None session_fs: SessionFsConfig | None = None @@ -1427,6 +1428,7 @@ def __init__( env: dict[str, str] | None = None, github_token: str | None = None, base_directory: str | None = None, + builtin_plugin_directories: Sequence[str] | None = None, use_logged_in_user: bool | None = None, telemetry: TelemetryConfig | None = None, session_fs: SessionFsConfig | None = None, @@ -1462,6 +1464,9 @@ def __init__( config, etc.). Sets the ``COPILOT_HOME`` environment variable on the spawned runtime. When ``None``, the runtime defaults to ``~/.copilot``. + builtin_plugin_directories: Absolute paths to trusted plugin + directories bundled by the host. When non-empty, the complete + set is registered during startup before sessions can be created. use_logged_in_user: Use the logged-in user for authentication. ``None`` (default) resolves to ``True`` unless ``github_token`` is set. @@ -1509,6 +1514,7 @@ def __init__( env=env, github_token=github_token, base_directory=base_directory, + builtin_plugin_directories=tuple(builtin_plugin_directories or ()), use_logged_in_user=use_logged_in_user, telemetry=telemetry, session_fs=session_fs, @@ -1525,6 +1531,11 @@ def __init__( else _resolve_default_connection(os.environ) ) _validate_environment_options(options, connection) + for path in options.builtin_plugin_directories: + if not os.path.isabs(path): + raise ValueError( + f"builtin_plugin_directories must contain only absolute paths: {path}" + ) _require_storage_for_empty_mode( mode=options.mode, base_directory=options.base_directory, @@ -1807,6 +1818,17 @@ async def start(self) -> None: start_time, ) + if self._options.builtin_plugin_directories: + assert self._client is not None + try: + await self._client.request( + "plugins.builtin.set", + {"paths": list(self._options.builtin_plugin_directories)}, + ) + except Exception: + await self.force_stop() + raise + if self._session_fs_config: session_fs_start = time.perf_counter() await self._set_session_fs_provider() diff --git a/python/test_client.py b/python/test_client.py index 893b82af1..cf4bdf192 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -6,6 +6,7 @@ import asyncio import inspect +import os from datetime import UTC, datetime from tempfile import TemporaryDirectory from unittest.mock import AsyncMock, Mock, patch @@ -57,6 +58,50 @@ def test_inprocess_connection_has_no_child_process_options(): assert not hasattr(connection, "args") +class TestBuiltinPluginDirectories: + @staticmethod + async def _start_client(paths=None): + client = CopilotClient( + connection=RuntimeConnection.for_uri("localhost:1234"), + builtin_plugin_directories=paths, + ) + client._connect_to_server = AsyncMock() + client._verify_protocol_version = AsyncMock() + client._client = Mock() + client._client.request = AsyncMock(return_value={}) + + await client.start() + return client + + @pytest.mark.asyncio + @pytest.mark.parametrize("paths", [None, []]) + async def test_default_or_empty_does_not_call_rpc(self, paths): + client = await self._start_client(paths) + + client._client.request.assert_not_awaited() + + @pytest.mark.asyncio + async def test_configured_paths_call_rpc_once_before_start_completes(self): + paths = [ + os.path.abspath("plugins/core"), + os.path.abspath("plugins/github"), + ] + + client = await self._start_client(paths) + + client._client.request.assert_awaited_once_with( + "plugins.builtin.set", + {"paths": paths}, + ) + + def test_relative_path_is_rejected(self): + with pytest.raises(ValueError, match="builtin_plugin_directories.*absolute paths"): + CopilotClient( + connection=RuntimeConnection.for_uri("localhost:1234"), + builtin_plugin_directories=["plugins/core"], + ) + + class TestClientShutdown: @pytest.mark.asyncio async def test_stop_requests_runtime_shutdown_for_owned_process(self): diff --git a/rust/src/lib.rs b/rust/src/lib.rs index cafa3c596..9e3041fec 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -254,6 +254,11 @@ pub struct ClientOptions { pub env_remove: Vec, /// Extra flags for child-process transports. pub extra_args: Vec, + /// Absolute paths to trusted plugin directories bundled by the host. + /// + /// When non-empty, [`Client::start`] replaces the runtime's complete + /// trusted built-in plugin directory set before sessions can be created. + pub builtin_plugin_directories: Vec, /// Transport mode used to communicate with the CLI server. pub transport: Transport, /// GitHub token for authentication. When set, the SDK passes the token @@ -368,6 +373,10 @@ impl std::fmt::Debug for ClientOptions { .field("env", &self.env) .field("env_remove", &self.env_remove) .field("extra_args", &self.extra_args) + .field( + "builtin_plugin_directories", + &self.builtin_plugin_directories, + ) .field("transport", &self.transport) .field( "github_token", @@ -632,6 +641,7 @@ impl Default for ClientOptions { env: Vec::new(), env_remove: Vec::new(), extra_args: Vec::new(), + builtin_plugin_directories: Vec::new(), transport: Transport::default(), github_token: None, use_logged_in_user: None, @@ -724,6 +734,19 @@ impl ClientOptions { self } + /// Set trusted plugin directories bundled by the host. + /// + /// Every path must be absolute; invalid paths are rejected by + /// [`Client::start`]. + pub fn with_builtin_plugin_directories(mut self, paths: I) -> Self + where + I: IntoIterator, + P: Into, + { + self.builtin_plugin_directories = paths.into_iter().map(Into::into).collect(); + self + } + /// Transport mode used to communicate with the CLI server. See [`Transport`]. pub fn with_transport(mut self, transport: Transport) -> Self { self.transport = transport; @@ -1071,6 +1094,30 @@ impl Client { if let Some(cfg) = &options.session_fs { validate_session_fs_config(cfg)?; } + let builtin_plugin_directories = options + .builtin_plugin_directories + .iter() + .map(|path| { + if !path.is_absolute() { + return Err(Error::with_message( + ErrorKind::InvalidConfig, + format!( + "builtin_plugin_directories must contain only absolute paths: {}", + path.display() + ), + )); + } + path.to_str().map(str::to_owned).ok_or_else(|| { + Error::with_message( + ErrorKind::InvalidConfig, + format!( + "builtin_plugin_directories must contain valid UTF-8 paths: {}", + path.display() + ), + ) + }) + }) + .collect::>>()?; // Auth options only make sense when the SDK spawns the CLI; with an // external server, the server manages its own auth. if matches!(options.transport, Transport::External { .. }) { @@ -1333,6 +1380,14 @@ impl Client { elapsed_ms = start_time.elapsed().as_millis(), "Client::start protocol verification complete" ); + if !builtin_plugin_directories.is_empty() { + client + .call( + "plugins.builtin.set", + Some(serde_json::json!({ "paths": builtin_plugin_directories })), + ) + .await?; + } if let Some(cfg) = session_fs_config { let session_fs_start = Instant::now(); let capabilities = cfg.capabilities.as_ref().map(|c| { diff --git a/rust/tests/builtin_plugin_directories_test.rs b/rust/tests/builtin_plugin_directories_test.rs new file mode 100644 index 000000000..f1310f9b0 --- /dev/null +++ b/rust/tests/builtin_plugin_directories_test.rs @@ -0,0 +1,135 @@ +#![allow(clippy::unwrap_used)] + +use std::path::PathBuf; + +use github_copilot_sdk::{CliProgram, Client, ClientOptions, ErrorKind, Transport}; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; +use tokio::net::TcpListener; + +async fn read_framed(reader: &mut (impl AsyncRead + Unpin)) -> serde_json::Value { + let mut header = String::new(); + loop { + let mut byte = [0u8; 1]; + reader.read_exact(&mut byte).await.unwrap(); + header.push(byte[0] as char); + if header.ends_with("\r\n\r\n") { + break; + } + } + let length = header + .trim() + .strip_prefix("Content-Length: ") + .unwrap() + .parse() + .unwrap(); + let mut body = vec![0; length]; + reader.read_exact(&mut body).await.unwrap(); + serde_json::from_slice(&body).unwrap() +} + +async fn write_result( + writer: &mut (impl AsyncWrite + Unpin), + request: &serde_json::Value, + result: serde_json::Value, +) { + let body = serde_json::to_vec(&serde_json::json!({ + "jsonrpc": "2.0", + "id": request["id"], + "result": result, + })) + .unwrap(); + writer + .write_all(format!("Content-Length: {}\r\n\r\n", body.len()).as_bytes()) + .await + .unwrap(); + writer.write_all(&body).await.unwrap(); + writer.flush().await.unwrap(); +} + +async fn run_start(paths: Option>) -> Vec { + let listener = TcpListener::bind(("127.0.0.1", 0)).await.unwrap(); + let address = listener.local_addr().unwrap(); + let expect_builtin = paths.as_ref().is_some_and(|paths| !paths.is_empty()); + + let server = tokio::spawn(async move { + let (stream, _) = listener.accept().await.unwrap(); + let (mut reader, mut writer) = tokio::io::split(stream); + let mut requests = Vec::new(); + + let connect = read_framed(&mut reader).await; + write_result( + &mut writer, + &connect, + serde_json::json!({ "ok": true, "protocolVersion": 3, "version": "test" }), + ) + .await; + requests.push(connect); + + if expect_builtin { + let builtin = read_framed(&mut reader).await; + write_result(&mut writer, &builtin, serde_json::json!({})).await; + requests.push(builtin); + } + requests + }); + + let mut options = ClientOptions::new() + .with_program(CliProgram::Path(std::env::current_exe().unwrap())) + .with_transport(Transport::External { + host: address.ip().to_string(), + port: address.port(), + connection_token: None, + }); + if let Some(paths) = paths { + options = options.with_builtin_plugin_directories(paths); + } + let client = Client::start(options).await.unwrap(); + let requests = server.await.unwrap(); + client.force_stop(); + requests +} + +#[tokio::test] +async fn default_and_empty_do_not_call_rpc() { + for paths in [None, Some(Vec::new())] { + let requests = run_start(paths).await; + assert_eq!(requests.len(), 1); + assert_eq!(requests[0]["method"], "connect"); + } +} + +#[tokio::test] +async fn configured_directories_call_rpc_once_before_start_completes() { + let cwd = std::env::current_dir().unwrap(); + let paths = vec![cwd.join("plugins/core"), cwd.join("plugins/github")]; + + let requests = run_start(Some(paths.clone())).await; + + assert_eq!(requests.len(), 2); + assert_eq!(requests[0]["method"], "connect"); + assert_eq!(requests[1]["method"], "plugins.builtin.set"); + assert_eq!( + requests[1]["params"], + serde_json::json!({ + "paths": paths + .iter() + .map(|path| path.to_str().unwrap()) + .collect::>() + }) + ); +} + +#[tokio::test] +async fn relative_directory_is_rejected() { + let options = ClientOptions::new() + .with_program(CliProgram::Path(std::env::current_exe().unwrap())) + .with_builtin_plugin_directories(["plugins/core"]); + + let error = match Client::start(options).await { + Ok(_) => panic!("relative path unexpectedly accepted"), + Err(error) => error, + }; + + assert_eq!(error.kind(), &ErrorKind::InvalidConfig); + assert!(error.to_string().contains("absolute paths")); +} From 8101299b9ce80c6690864f8452eebcb19dd9b419 Mon Sep 17 00:00:00 2001 From: Lutz Roeder Date: Thu, 13 Aug 2026 14:06:36 -0700 Subject: [PATCH 2/3] Address built-in plugin directory feedback Use a plain code fence for the single-language docs example and clear Go connection state when built-in plugin registration fails so reconnect can start cleanly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd609bcf-9f69-4d40-960b-dbd12e90158a --- docs/features/plugin-directories.md | 5 -- go/client.go | 4 ++ go/client_test.go | 105 +++++++++++++++++++++------- 3 files changed, 83 insertions(+), 31 deletions(-) diff --git a/docs/features/plugin-directories.md b/docs/features/plugin-directories.md index b2d349398..4af11d9a7 100644 --- a/docs/features/plugin-directories.md +++ b/docs/features/plugin-directories.md @@ -244,9 +244,6 @@ let client = Client::start( Applications that ship their own trusted plugins can register them as a client startup option. The SDK sends the complete ordered set after connecting and verifying the protocol, before `start` returns or any session can be created. Paths must be absolute; leaving the option unset or empty makes no RPC call. -

-Node.js / TypeScript - ```typescript import { CopilotClient } from "@github/copilot-sdk"; @@ -265,8 +262,6 @@ main(); ``` -
- The equivalent option in each SDK is: | SDK | Startup option | diff --git a/go/client.go b/go/client.go index f36133672..fb02897f9 100644 --- a/go/client.go +++ b/go/client.go @@ -465,6 +465,10 @@ func (c *Client) Start(ctx context.Context) error { "paths": c.options.BuiltinPluginDirectories, }); err != nil { c.client.Stop() + c.client = nil + c.conn = nil + c.RPC = nil + c.internalRPC = nil killErr := c.killProcess() c.state = stateError return errors.Join(err, killErr) diff --git a/go/client_test.go b/go/client_test.go index 0980f393c..f21442679 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -211,6 +211,40 @@ func TestClient_BuiltinPluginDirectories(t *testing.T) { }() NewClient(&ClientOptions{BuiltinPluginDirectories: []string{"plugins/core"}}) }) + + t.Run("startup RPC failure clears transport for reconnect", func(t *testing.T) { + url, _, cleanup := newStartupRPCServerWithBuiltinFailure(t, true) + defer cleanup() + cwd, err := os.Getwd() + if err != nil { + t.Fatalf("Getwd failed: %v", err) + } + client := NewClient(&ClientOptions{ + Connection: URIConnection{URL: url}, + BuiltinPluginDirectories: []string{filepath.Join(cwd, "plugins", "core")}, + }) + + if err := client.Start(t.Context()); err == nil { + t.Fatal("Start unexpectedly succeeded") + } + if client.client != nil { + t.Fatal("client transport was not cleared after startup RPC failure") + } + if client.conn != nil { + t.Fatal("connection was not cleared after startup RPC failure") + } + if client.RPC != nil { + t.Fatal("typed RPC client was not cleared after startup RPC failure") + } + if client.internalRPC != nil { + t.Fatal("internal RPC client was not cleared after startup RPC failure") + } + + if err := client.Start(t.Context()); err != nil { + t.Fatalf("second Start failed: %v", err) + } + defer client.ForceStop() + }) } type startupRPCRequest struct { @@ -219,6 +253,10 @@ type startupRPCRequest struct { } func newStartupRPCServer(t *testing.T) (string, func() []startupRPCRequest, func()) { + return newStartupRPCServerWithBuiltinFailure(t, false) +} + +func newStartupRPCServerWithBuiltinFailure(t *testing.T, failFirstBuiltin bool) (string, func() []startupRPCRequest, func()) { t.Helper() listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -227,31 +265,41 @@ func newStartupRPCServer(t *testing.T) (string, func() []startupRPCRequest, func var mux sync.Mutex var requests []startupRPCRequest - serverReady := make(chan *jsonrpc2.Client, 1) + serverReady := make(chan *jsonrpc2.Client, 8) + var builtinSetCount int go func() { - conn, acceptErr := listener.Accept() - if acceptErr != nil { - return - } - server := jsonrpc2.NewClient(conn, conn) - record := func(method string, params json.RawMessage) { - mux.Lock() - requests = append(requests, startupRPCRequest{ - Method: method, - Params: append(json.RawMessage(nil), params...), + for { + conn, acceptErr := listener.Accept() + if acceptErr != nil { + return + } + server := jsonrpc2.NewClient(conn, conn) + record := func(method string, params json.RawMessage) { + mux.Lock() + requests = append(requests, startupRPCRequest{ + Method: method, + Params: append(json.RawMessage(nil), params...), + }) + mux.Unlock() + } + server.SetRequestHandler("connect", func(params json.RawMessage) (json.RawMessage, *jsonrpc2.Error) { + record("connect", params) + return []byte(`{"ok":true,"protocolVersion":3,"version":"test"}`), nil + }) + server.SetRequestHandler("plugins.builtin.set", func(params json.RawMessage) (json.RawMessage, *jsonrpc2.Error) { + record("plugins.builtin.set", params) + mux.Lock() + builtinSetCount++ + shouldFail := failFirstBuiltin && builtinSetCount == 1 + mux.Unlock() + if shouldFail { + return nil, &jsonrpc2.Error{Code: -32000, Message: "builtin registration failed"} + } + return []byte(`{}`), nil }) - mux.Unlock() + server.Start() + serverReady <- server } - server.SetRequestHandler("connect", func(params json.RawMessage) (json.RawMessage, *jsonrpc2.Error) { - record("connect", params) - return []byte(`{"ok":true,"protocolVersion":3,"version":"test"}`), nil - }) - server.SetRequestHandler("plugins.builtin.set", func(params json.RawMessage) (json.RawMessage, *jsonrpc2.Error) { - record("plugins.builtin.set", params) - return []byte(`{}`), nil - }) - server.Start() - serverReady <- server }() snapshot := func() []startupRPCRequest { @@ -261,10 +309,15 @@ func newStartupRPCServer(t *testing.T) (string, func() []startupRPCRequest, func } cleanup := func() { listener.Close() - select { - case server := <-serverReady: - server.Stop() - case <-time.After(time.Second): + for { + select { + case server := <-serverReady: + server.Stop() + case <-time.After(time.Second): + return + default: + return + } } } return listener.Addr().String(), snapshot, cleanup From d7127df985792fbae033e5c07c085f96bc1a4750 Mon Sep 17 00:00:00 2001 From: Lutz Roeder Date: Thu, 13 Aug 2026 15:06:11 -0700 Subject: [PATCH 3/3] Address .NET code scanning comments Make the built-in plugin path validation filter explicit and avoid Path.Combine in the test paths flagged by code scanning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cd609bcf-9f69-4d40-960b-dbd12e90158a --- dotnet/src/Client.cs | 13 +++++-------- dotnet/test/Unit/GitHubTelemetryTests.cs | 4 ++-- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index fa3615dc4..58c1074c0 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -140,15 +140,12 @@ public CopilotClient(CopilotClientOptions? options = null) _options = options ?? new(); _connection = _options.Connection ?? ResolveDefaultConnection(_options); _builtinPluginDirectories = _options.BuiltinPluginDirectories?.ToArray() ?? []; - foreach (var path in _builtinPluginDirectories) + foreach (var path in _builtinPluginDirectories.Where(path => !IsFullyQualifiedPath(path))) { - if (!IsFullyQualifiedPath(path)) - { - throw new ArgumentException( - $"{nameof(CopilotClientOptions)}.{nameof(CopilotClientOptions.BuiltinPluginDirectories)} " + - $"must contain only absolute paths: {path}", - nameof(options)); - } + throw new ArgumentException( + $"{nameof(CopilotClientOptions)}.{nameof(CopilotClientOptions.BuiltinPluginDirectories)} " + + $"must contain only absolute paths: {path}", + nameof(options)); } switch (_connection) diff --git a/dotnet/test/Unit/GitHubTelemetryTests.cs b/dotnet/test/Unit/GitHubTelemetryTests.cs index 5919e4d92..a4a241e38 100644 --- a/dotnet/test/Unit/GitHubTelemetryTests.cs +++ b/dotnet/test/Unit/GitHubTelemetryTests.cs @@ -39,8 +39,8 @@ public async Task BuiltinPluginDirectories_Are_Set_Once_Before_Start_Completes() { var paths = new[] { - Path.GetFullPath(Path.Combine("plugins", "core")), - Path.GetFullPath(Path.Combine("plugins", "github")), + Path.GetFullPath(Path.Join("plugins", "core")), + Path.GetFullPath(Path.Join("plugins", "github")), }; await using var server = await FakeTelemetryServer.StartAsync(); await using var client = new CopilotClient(new CopilotClientOptions