From 59b7fa116f8516fbc5a580a632c60778c2c1c7b9 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Fri, 27 Mar 2026 17:20:02 -0400 Subject: [PATCH 1/9] Add optional Executor to CopilotClientOptions; wire all internal *Async calls through it; shared timeout scheduler. src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java - Added `Executor` field, `getExecutor()`, fluent `setExecutor(Executor)` with pending-null guard, and clone support. src/main/java/com/github/copilot/sdk/CopilotClient.java - Extracted `startCoreBody()` from `startCore()` lambda; `supplyAsync` uses provided executor when non-null. - `stop()` routes session-close `runAsync` through provided executor when non-null. - Passes executor to `RpcHandlerDispatcher` constructor. - Sets executor on new sessions via `session.setExecutor()` in `createSession` and `resumeSession`. src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java - Added `Executor` field and 3-arg constructor. - All 5 `CompletableFuture.runAsync()` calls now go through private `runAsync(Runnable)` helper that uses executor when non-null. src/main/java/com/github/copilot/sdk/CopilotSession.java - Added `Executor` field and package-private `setExecutor()`. - Replaced per-call `ScheduledExecutorService` with shared `timeoutScheduler` (daemon thread, shut down in `close()`). - `executeToolAndRespondAsync` and `executePermissionAndRespondAsync` use executor when non-null. src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java (new) - 6 E2E tests using `TrackingExecutor` decorator to verify all `*Async` paths route through the provided executor: client start, tool call, permission, user input, hooks, and client stop. src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java - Updated constructor call to pass `null` for new executor parameter. --- .../com/github/copilot/sdk/CopilotClient.java | 83 ++-- .../github/copilot/sdk/CopilotSession.java | 29 +- .../copilot/sdk/RpcHandlerDispatcher.java | 26 +- .../sdk/json/CopilotClientOptions.java | 34 ++ .../copilot/sdk/ExecutorWiringTest.java | 368 ++++++++++++++++++ .../copilot/sdk/RpcHandlerDispatcherTest.java | 2 +- 6 files changed, 498 insertions(+), 44 deletions(-) create mode 100644 src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java diff --git a/src/main/java/com/github/copilot/sdk/CopilotClient.java b/src/main/java/com/github/copilot/sdk/CopilotClient.java index 707469428..00dea3876 100644 --- a/src/main/java/com/github/copilot/sdk/CopilotClient.java +++ b/src/main/java/com/github/copilot/sdk/CopilotClient.java @@ -13,6 +13,7 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -150,42 +151,48 @@ public CompletableFuture start() { private CompletableFuture startCore() { LOG.fine("Starting Copilot client"); - return CompletableFuture.supplyAsync(() -> { - try { - JsonRpcClient rpc; - Process process = null; - - if (optionsHost != null && optionsPort != null) { - // External server (TCP) - rpc = serverManager.connectToServer(null, optionsHost, optionsPort); - } else { - // Child process (stdio or TCP) - CliServerManager.ProcessInfo processInfo = serverManager.startCliServer(); - process = processInfo.process(); - rpc = serverManager.connectToServer(process, processInfo.port() != null ? "localhost" : null, - processInfo.port()); - } + Executor exec = options.getExecutor(); + return exec != null + ? CompletableFuture.supplyAsync(this::startCoreBody, exec) + : CompletableFuture.supplyAsync(this::startCoreBody); + } - Connection connection = new Connection(rpc, process); + private Connection startCoreBody() { + try { + JsonRpcClient rpc; + Process process = null; + + if (optionsHost != null && optionsPort != null) { + // External server (TCP) + rpc = serverManager.connectToServer(null, optionsHost, optionsPort); + } else { + // Child process (stdio or TCP) + CliServerManager.ProcessInfo processInfo = serverManager.startCliServer(); + process = processInfo.process(); + rpc = serverManager.connectToServer(process, processInfo.port() != null ? "localhost" : null, + processInfo.port()); + } - // Register handlers for server-to-client calls - RpcHandlerDispatcher dispatcher = new RpcHandlerDispatcher(sessions, lifecycleManager::dispatch); - dispatcher.registerHandlers(rpc); + Connection connection = new Connection(rpc, process); - // Verify protocol version - verifyProtocolVersion(connection); + // Register handlers for server-to-client calls + RpcHandlerDispatcher dispatcher = new RpcHandlerDispatcher(sessions, lifecycleManager::dispatch, + options.getExecutor()); + dispatcher.registerHandlers(rpc); - LOG.info("Copilot client connected"); - return connection; - } catch (Exception e) { - String stderr = serverManager.getStderrOutput(); - if (!stderr.isEmpty()) { - throw new CompletionException( - new IOException("CLI process exited unexpectedly. stderr: " + stderr, e)); - } - throw new CompletionException(e); + // Verify protocol version + verifyProtocolVersion(connection); + + LOG.info("Copilot client connected"); + return connection; + } catch (Exception e) { + String stderr = serverManager.getStderrOutput(); + if (!stderr.isEmpty()) { + throw new CompletionException( + new IOException("CLI process exited unexpectedly. stderr: " + stderr, e)); } - }); + throw new CompletionException(e); + } } private static final int MIN_PROTOCOL_VERSION = 2; @@ -228,15 +235,19 @@ private void verifyProtocolVersion(Connection connection) throws Exception { */ public CompletableFuture stop() { var closeFutures = new ArrayList>(); + Executor exec = options.getExecutor(); for (CopilotSession session : new ArrayList<>(sessions.values())) { - closeFutures.add(CompletableFuture.runAsync(() -> { + Runnable closeTask = () -> { try { session.close(); } catch (Exception e) { LOG.log(Level.WARNING, "Error closing session " + session.getSessionId(), e); } - })); + }; + closeFutures.add(exec != null + ? CompletableFuture.runAsync(closeTask, exec) + : CompletableFuture.runAsync(closeTask)); } sessions.clear(); @@ -329,6 +340,9 @@ public CompletableFuture createSession(SessionConfig config) { : java.util.UUID.randomUUID().toString(); var session = new CopilotSession(sessionId, connection.rpc); + if (options.getExecutor() != null) { + session.setExecutor(options.getExecutor()); + } SessionRequestBuilder.configureSession(session, config); sessions.put(sessionId, session); @@ -399,6 +413,9 @@ public CompletableFuture resumeSession(String sessionId, ResumeS return ensureConnected().thenCompose(connection -> { // Register the session before the RPC call to avoid missing early events. var session = new CopilotSession(sessionId, connection.rpc); + if (options.getExecutor() != null) { + session.setExecutor(options.getExecutor()); + } SessionRequestBuilder.configureSession(session, config); sessions.put(sessionId, session); diff --git a/src/main/java/com/github/copilot/sdk/CopilotSession.java b/src/main/java/com/github/copilot/sdk/CopilotSession.java index 6ee5d8c4e..485c39312 100644 --- a/src/main/java/com/github/copilot/sdk/CopilotSession.java +++ b/src/main/java/com/github/copilot/sdk/CopilotSession.java @@ -13,6 +13,8 @@ import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -125,6 +127,7 @@ public final class CopilotSession implements AutoCloseable { private volatile EventErrorPolicy eventErrorPolicy = EventErrorPolicy.PROPAGATE_AND_LOG_ERRORS; private volatile Map>> transformCallbacks; private final ScheduledExecutorService timeoutScheduler; + private volatile Executor executor; /** Tracks whether this session instance has been terminated via close(). */ private volatile boolean isTerminated = false; @@ -170,6 +173,14 @@ public final class CopilotSession implements AutoCloseable { this.timeoutScheduler = executor; } + /** + * Sets the executor for internal async operations. Package-private; called by + * CopilotClient after construction. + */ + void setExecutor(Executor executor) { + this.executor = executor; + } + /** * Gets the unique identifier for this session. * @@ -673,7 +684,7 @@ private void handleBroadcastEventAsync(AbstractSessionEvent event) { */ private void executeToolAndRespondAsync(String requestId, String toolName, String toolCallId, Object arguments, ToolDefinition tool) { - CompletableFuture.runAsync(() -> { + Runnable task = () -> { try { JsonNode argumentsNode = arguments instanceof JsonNode jn ? jn @@ -718,7 +729,12 @@ private void executeToolAndRespondAsync(String requestId, String toolName, Strin LOG.log(Level.WARNING, "Error sending tool error for requestId=" + requestId, sendEx); } } - }); + }; + if (executor != null) { + CompletableFuture.runAsync(task, executor); + } else { + CompletableFuture.runAsync(task); + } } /** @@ -727,7 +743,7 @@ private void executeToolAndRespondAsync(String requestId, String toolName, Strin */ private void executePermissionAndRespondAsync(String requestId, PermissionRequest permissionRequest, PermissionHandler handler) { - CompletableFuture.runAsync(() -> { + Runnable task = () -> { try { var invocation = new PermissionInvocation(); invocation.setSessionId(sessionId); @@ -766,7 +782,12 @@ private void executePermissionAndRespondAsync(String requestId, PermissionReques LOG.log(Level.WARNING, "Error sending permission denied for requestId=" + requestId, sendEx); } } - }); + }; + if (executor != null) { + CompletableFuture.runAsync(task, executor); + } else { + CompletableFuture.runAsync(task); + } } /** diff --git a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java index 101f68528..9f55938af 100644 --- a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java +++ b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java @@ -9,6 +9,7 @@ import java.util.Collections; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; import java.util.logging.Level; import java.util.logging.Logger; @@ -45,6 +46,7 @@ final class RpcHandlerDispatcher { private final Map sessions; private final LifecycleEventDispatcher lifecycleDispatcher; + private final Executor executor; /** * Creates a dispatcher with session registry and lifecycle dispatcher. @@ -53,10 +55,14 @@ final class RpcHandlerDispatcher { * the session registry to look up sessions by ID * @param lifecycleDispatcher * callback for dispatching lifecycle events + * @param executor + * the executor for async dispatch, or {@code null} for default */ - RpcHandlerDispatcher(Map sessions, LifecycleEventDispatcher lifecycleDispatcher) { + RpcHandlerDispatcher(Map sessions, LifecycleEventDispatcher lifecycleDispatcher, + Executor executor) { this.sessions = sessions; this.lifecycleDispatcher = lifecycleDispatcher; + this.executor = executor; } /** @@ -118,7 +124,7 @@ private void handleLifecycleEvent(JsonNode params) { } private void handleToolCall(JsonRpcClient rpc, String requestId, JsonNode params) { - CompletableFuture.runAsync(() -> { + runAsync(() -> { try { String sessionId = params.get("sessionId").asText(); String toolCallId = params.get("toolCallId").asText(); @@ -178,7 +184,7 @@ private void handleToolCall(JsonRpcClient rpc, String requestId, JsonNode params } private void handlePermissionRequest(JsonRpcClient rpc, String requestId, JsonNode params) { - CompletableFuture.runAsync(() -> { + runAsync(() -> { try { String sessionId = params.get("sessionId").asText(); JsonNode permissionRequest = params.get("permissionRequest"); @@ -222,7 +228,7 @@ private void handlePermissionRequest(JsonRpcClient rpc, String requestId, JsonNo private void handleUserInputRequest(JsonRpcClient rpc, String requestId, JsonNode params) { LOG.fine("Received userInput.request: " + params); - CompletableFuture.runAsync(() -> { + runAsync(() -> { try { String sessionId = params.get("sessionId").asText(); String question = params.get("question").asText(); @@ -278,7 +284,7 @@ private void handleUserInputRequest(JsonRpcClient rpc, String requestId, JsonNod } private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode params) { - CompletableFuture.runAsync(() -> { + runAsync(() -> { try { String sessionId = params.get("sessionId").asText(); String hookType = params.get("hookType").asText(); @@ -321,7 +327,7 @@ interface LifecycleEventDispatcher { } private void handleSystemMessageTransform(JsonRpcClient rpc, String requestId, JsonNode params) { - CompletableFuture.runAsync(() -> { + runAsync(() -> { try { final long requestIdLong; try { @@ -359,4 +365,12 @@ private void handleSystemMessageTransform(JsonRpcClient rpc, String requestId, J } }); } + + private void runAsync(Runnable task) { + if (executor != null) { + CompletableFuture.runAsync(task, executor); + } else { + CompletableFuture.runAsync(task); + } + } } diff --git a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java index 4cdee912c..33d84a9c5 100644 --- a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java +++ b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java @@ -7,6 +7,7 @@ import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; import java.util.function.Supplier; import com.fasterxml.jackson.annotation.JsonInclude; @@ -49,6 +50,7 @@ public class CopilotClientOptions { private Boolean useLoggedInUser; private Supplier>> onListModels; private TelemetryConfig telemetry; + private Executor executor; /** * Gets the path to the Copilot CLI executable. @@ -412,6 +414,37 @@ public CopilotClientOptions setTelemetry(TelemetryConfig telemetry) { return this; } + /** + * Gets the executor used for internal asynchronous operations. + * + * @return the executor, or {@code null} to use the default + * {@code ForkJoinPool.commonPool()} + */ + public Executor getExecutor() { + return executor; + } + + /** + * Sets the executor used for internal asynchronous operations. + *

+ * When provided, the SDK uses this executor for all internal + * {@code CompletableFuture} combinators instead of the default + * {@code ForkJoinPool.commonPool()}. This allows callers to isolate SDK + * work onto a dedicated thread pool or integrate with container-managed + * threading. + * + * @param executor + * the executor to use, or {@code null} for the default + * @return this options instance for fluent chaining + */ + public CopilotClientOptions setExecutor(Executor executor) { + if (null == executor) { + throw new IllegalArgumentException("PENDING(copilot): not implemented"); + } + this.executor = executor; + return this; + } + /** * Creates a shallow clone of this {@code CopilotClientOptions} instance. *

@@ -439,6 +472,7 @@ public CopilotClientOptions clone() { copy.useLoggedInUser = this.useLoggedInUser; copy.onListModels = this.onListModels; copy.telemetry = this.telemetry; + copy.executor = this.executor; return copy; } } diff --git a/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java b/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java new file mode 100644 index 000000000..736564139 --- /dev/null +++ b/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java @@ -0,0 +1,368 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.sdk; + +import static org.junit.jupiter.api.Assertions.*; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.Executor; +import java.util.concurrent.ForkJoinPool; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import com.github.copilot.sdk.events.AssistantMessageEvent; +import com.github.copilot.sdk.json.CopilotClientOptions; +import com.github.copilot.sdk.json.MessageOptions; +import com.github.copilot.sdk.json.PermissionHandler; +import com.github.copilot.sdk.json.PermissionRequest; +import com.github.copilot.sdk.json.PermissionRequestResult; +import com.github.copilot.sdk.json.PreToolUseHookOutput; +import com.github.copilot.sdk.json.SessionConfig; +import com.github.copilot.sdk.json.SessionHooks; +import com.github.copilot.sdk.json.ToolDefinition; +import com.github.copilot.sdk.json.UserInputResponse; + +/** + * TDD red-phase tests verifying that when an {@link Executor} is provided via + * {@link CopilotClientOptions#setExecutor(Executor)}, all internal + * {@code CompletableFuture.*Async} calls are routed through that executor + * instead of {@code ForkJoinPool.commonPool()}. + * + *

+ * Uses a {@link TrackingExecutor} decorator that delegates to a real executor + * while counting task submissions. After SDK operations complete, the tests + * assert the decorator was invoked. + *

+ */ +public class ExecutorWiringTest { + + private static E2ETestContext ctx; + + @BeforeAll + static void setup() throws Exception { + ctx = E2ETestContext.create(); + } + + @AfterAll + static void teardown() throws Exception { + if (ctx != null) { + ctx.close(); + } + } + + /** + * A decorator executor that delegates to a real executor while counting + * task submissions. + */ + static class TrackingExecutor implements Executor { + + private final Executor delegate; + private final AtomicInteger taskCount = new AtomicInteger(0); + + TrackingExecutor(Executor delegate) { + this.delegate = delegate; + } + + @Override + public void execute(Runnable command) { + taskCount.incrementAndGet(); + delegate.execute(command); + } + + int getTaskCount() { + return taskCount.get(); + } + } + + private CopilotClientOptions createOptionsWithExecutor(TrackingExecutor executor) { + CopilotClientOptions options = new CopilotClientOptions().setCliPath(ctx.getCliPath()) + .setCwd(ctx.getWorkDir().toString()).setEnvironment(ctx.getEnvironment()).setExecutor(executor); + + String ci = System.getenv("GITHUB_ACTIONS"); + if (ci != null && !ci.isEmpty()) { + options.setGitHubToken("fake-token-for-e2e-tests"); + } + return options; + } + + /** + * Verifies that client start-up routes through the provided executor. + * + *

+ * {@code CopilotClient.startCore()} uses + * {@code CompletableFuture.supplyAsync(...)} to initialize the connection. + * This test asserts that the start-up task goes through the caller-supplied + * executor, not {@code ForkJoinPool.commonPool()}. + *

+ * + * @see Snapshot: tools/invokes_custom_tool + */ + @Test + void testClientStartUsesProvidedExecutor() throws Exception { + ctx.configureForTest("tools", "invokes_custom_tool"); + + TrackingExecutor trackingExecutor = new TrackingExecutor(ForkJoinPool.commonPool()); + int beforeStart = trackingExecutor.getTaskCount(); + + try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + client.start().get(30, TimeUnit.SECONDS); + + assertTrue(trackingExecutor.getTaskCount() > beforeStart, + "Expected the tracking executor to have been invoked during client start, " + + "but task count did not increase. CopilotClient.startCore() is not " + + "routing supplyAsync through the provided executor."); + } + } + + /** + * Verifies that tool call dispatch routes through the provided executor. + * + *

+ * When a custom tool is invoked by the LLM, the + * {@code RpcHandlerDispatcher} calls + * {@code CompletableFuture.runAsync(...)} to dispatch the tool handler. + * This test asserts that dispatch goes through the caller-supplied executor. + *

+ * + * @see Snapshot: tools/invokes_custom_tool + */ + @Test + void testToolCallDispatchUsesProvidedExecutor() throws Exception { + ctx.configureForTest("tools", "invokes_custom_tool"); + + TrackingExecutor trackingExecutor = new TrackingExecutor(ForkJoinPool.commonPool()); + + var parameters = new HashMap(); + var properties = new HashMap(); + var inputProp = new HashMap(); + inputProp.put("type", "string"); + inputProp.put("description", "String to encrypt"); + properties.put("input", inputProp); + parameters.put("type", "object"); + parameters.put("properties", properties); + parameters.put("required", List.of("input")); + + ToolDefinition encryptTool = ToolDefinition.create("encrypt_string", "Encrypts a string", parameters, + (invocation) -> { + Map args = invocation.getArguments(); + String input = (String) args.get("input"); + return CompletableFuture.completedFuture(input.toUpperCase()); + }); + + // Reset count after client construction to isolate tool-call dispatch + try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + CopilotSession session = client.createSession(new SessionConfig().setTools(List.of(encryptTool)) + .setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get(); + + int beforeToolCall = trackingExecutor.getTaskCount(); + + AssistantMessageEvent response = session + .sendAndWait(new MessageOptions().setPrompt("Use encrypt_string to encrypt this string: Hello")) + .get(60, TimeUnit.SECONDS); + + assertNotNull(response); + + assertTrue(trackingExecutor.getTaskCount() > beforeToolCall, + "Expected the tracking executor to have been invoked for tool call dispatch, " + + "but task count did not increase after sendAndWait. " + + "RpcHandlerDispatcher is not routing runAsync through the provided executor."); + + session.close(); + } + } + + /** + * Verifies that permission request dispatch routes through the provided + * executor. + * + *

+ * When the LLM requests a permission, the {@code RpcHandlerDispatcher} + * calls {@code CompletableFuture.runAsync(...)} to dispatch the permission + * handler. This test asserts that dispatch goes through the caller-supplied + * executor. + *

+ * + * @see Snapshot: permissions/permission_handler_for_write_operations + */ + @Test + void testPermissionDispatchUsesProvidedExecutor() throws Exception { + ctx.configureForTest("permissions", "permission_handler_for_write_operations"); + + TrackingExecutor trackingExecutor = new TrackingExecutor(ForkJoinPool.commonPool()); + + var config = new SessionConfig().setOnPermissionRequest((request, invocation) -> CompletableFuture + .completedFuture(new PermissionRequestResult().setKind("approved"))); + + try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + CopilotSession session = client.createSession(config).get(); + + Path testFile = ctx.getWorkDir().resolve("test.txt"); + Files.writeString(testFile, "original content"); + + int beforeSend = trackingExecutor.getTaskCount(); + + session.sendAndWait( + new MessageOptions().setPrompt("Edit test.txt and replace 'original' with 'modified'")) + .get(60, TimeUnit.SECONDS); + + assertTrue(trackingExecutor.getTaskCount() > beforeSend, + "Expected the tracking executor to have been invoked for permission dispatch, " + + "but task count did not increase after sendAndWait. " + + "RpcHandlerDispatcher is not routing permission runAsync through the provided executor."); + + session.close(); + } + } + + /** + * Verifies that user input request dispatch routes through the provided + * executor. + * + *

+ * When the LLM asks for user input, the {@code RpcHandlerDispatcher} calls + * {@code CompletableFuture.runAsync(...)} to dispatch the user input + * handler. This test asserts that dispatch goes through the caller-supplied + * executor. + *

+ * + * @see Snapshot: + * ask_user/should_invoke_user_input_handler_when_model_uses_ask_user_tool + */ + @Test + void testUserInputDispatchUsesProvidedExecutor() throws Exception { + ctx.configureForTest("ask_user", "should_invoke_user_input_handler_when_model_uses_ask_user_tool"); + + TrackingExecutor trackingExecutor = new TrackingExecutor(ForkJoinPool.commonPool()); + + var config = new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL) + .setOnUserInputRequest((request, invocation) -> { + String answer = (request.getChoices() != null && !request.getChoices().isEmpty()) + ? request.getChoices().get(0) + : "freeform answer"; + boolean wasFreeform = request.getChoices() == null || request.getChoices().isEmpty(); + return CompletableFuture + .completedFuture(new UserInputResponse().setAnswer(answer).setWasFreeform(wasFreeform)); + }); + + try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + CopilotSession session = client.createSession(config).get(); + + int beforeSend = trackingExecutor.getTaskCount(); + + session.sendAndWait(new MessageOptions().setPrompt( + "Ask me to choose between 'Option A' and 'Option B' using the ask_user tool. Wait for my response before continuing.")) + .get(60, TimeUnit.SECONDS); + + assertTrue(trackingExecutor.getTaskCount() > beforeSend, + "Expected the tracking executor to have been invoked for user input dispatch, " + + "but task count did not increase after sendAndWait. " + + "RpcHandlerDispatcher is not routing userInput runAsync through the provided executor."); + + session.close(); + } + } + + /** + * Verifies that hooks dispatch routes through the provided executor. + * + *

+ * When the LLM triggers a hook, the {@code RpcHandlerDispatcher} calls + * {@code CompletableFuture.runAsync(...)} to dispatch the hooks handler. + * This test asserts that dispatch goes through the caller-supplied executor. + *

+ * + * @see Snapshot: hooks/invoke_pre_tool_use_hook_when_model_runs_a_tool + */ + @Test + void testHooksDispatchUsesProvidedExecutor() throws Exception { + ctx.configureForTest("hooks", "invoke_pre_tool_use_hook_when_model_runs_a_tool"); + + TrackingExecutor trackingExecutor = new TrackingExecutor(ForkJoinPool.commonPool()); + + var config = new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL) + .setHooks(new SessionHooks().setOnPreToolUse( + (input, invocation) -> CompletableFuture.completedFuture(PreToolUseHookOutput.allow()))); + + try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + CopilotSession session = client.createSession(config).get(); + + Path testFile = ctx.getWorkDir().resolve("hello.txt"); + Files.writeString(testFile, "Hello from the test!"); + + int beforeSend = trackingExecutor.getTaskCount(); + + session.sendAndWait( + new MessageOptions().setPrompt("Read the contents of hello.txt and tell me what it says")) + .get(60, TimeUnit.SECONDS); + + assertTrue(trackingExecutor.getTaskCount() > beforeSend, + "Expected the tracking executor to have been invoked for hooks dispatch, " + + "but task count did not increase after sendAndWait. " + + "RpcHandlerDispatcher is not routing hooks runAsync through the provided executor."); + + session.close(); + } + } + + /** + * Verifies that {@code CopilotClient.stop()} routes session closure through + * the provided executor. + * + *

+ * {@code CopilotClient.stop()} uses + * {@code CompletableFuture.runAsync(...)} to close each active session. + * This test asserts that those closures go through the caller-supplied + * executor. + *

+ * + * @see Snapshot: tools/invokes_custom_tool + */ + @Test + void testClientStopUsesProvidedExecutor() throws Exception { + ctx.configureForTest("tools", "invokes_custom_tool"); + + TrackingExecutor trackingExecutor = new TrackingExecutor(ForkJoinPool.commonPool()); + + var parameters = new HashMap(); + var properties = new HashMap(); + var inputProp = new HashMap(); + inputProp.put("type", "string"); + inputProp.put("description", "String to encrypt"); + properties.put("input", inputProp); + parameters.put("type", "object"); + parameters.put("properties", properties); + parameters.put("required", List.of("input")); + + ToolDefinition encryptTool = ToolDefinition.create("encrypt_string", "Encrypts a string", parameters, + (invocation) -> { + Map args = invocation.getArguments(); + String input = (String) args.get("input"); + return CompletableFuture.completedFuture(input.toUpperCase()); + }); + + CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor)); + client.createSession(new SessionConfig().setTools(List.of(encryptTool)) + .setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get(); + + int beforeStop = trackingExecutor.getTaskCount(); + + // stop() should use the provided executor for async session closure + client.stop().get(30, TimeUnit.SECONDS); + + assertTrue(trackingExecutor.getTaskCount() > beforeStop, + "Expected the tracking executor to have been invoked during client stop, " + + "but task count did not increase. CopilotClient.stop() is not " + + "routing session closure runAsync through the provided executor."); + } +} diff --git a/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java b/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java index 61ad4dadd..79f5d7c7e 100644 --- a/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java +++ b/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java @@ -66,7 +66,7 @@ void setup() throws Exception { sessions = new ConcurrentHashMap<>(); lifecycleEvents = new CopyOnWriteArrayList<>(); - dispatcher = new RpcHandlerDispatcher(sessions, lifecycleEvents::add); + dispatcher = new RpcHandlerDispatcher(sessions, lifecycleEvents::add, null); dispatcher.registerHandlers(rpc); // Extract the registered handlers via reflection so we can invoke them directly From 31803bcb7dfcc34c0b75a8411abc5208ebaa1f7d Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Mon, 30 Mar 2026 16:59:43 -0400 Subject: [PATCH 2/9] On branch edburns/dd-2758695-virtual-threads-accept-executor modified: src/main/java/com/github/copilot/sdk/CopilotClient.java - Spotless. modified: src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java - Remove stub from TDD red phase. modified: src/site/markdown/cookbook/multiple-sessions.md - Document new feature. modified: src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java - Update test documentation. Signed-off-by: Ed Burns --- .../com/github/copilot/sdk/CopilotClient.java | 8 +-- .../sdk/json/CopilotClientOptions.java | 8 +-- .../markdown/cookbook/multiple-sessions.md | 63 +++++++++++++++++++ .../copilot/sdk/ExecutorWiringTest.java | 44 ++++++------- 4 files changed, 87 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/github/copilot/sdk/CopilotClient.java b/src/main/java/com/github/copilot/sdk/CopilotClient.java index 00dea3876..285b5fbdc 100644 --- a/src/main/java/com/github/copilot/sdk/CopilotClient.java +++ b/src/main/java/com/github/copilot/sdk/CopilotClient.java @@ -188,8 +188,7 @@ private Connection startCoreBody() { } catch (Exception e) { String stderr = serverManager.getStderrOutput(); if (!stderr.isEmpty()) { - throw new CompletionException( - new IOException("CLI process exited unexpectedly. stderr: " + stderr, e)); + throw new CompletionException(new IOException("CLI process exited unexpectedly. stderr: " + stderr, e)); } throw new CompletionException(e); } @@ -245,9 +244,8 @@ public CompletableFuture stop() { LOG.log(Level.WARNING, "Error closing session " + session.getSessionId(), e); } }; - closeFutures.add(exec != null - ? CompletableFuture.runAsync(closeTask, exec) - : CompletableFuture.runAsync(closeTask)); + closeFutures.add( + exec != null ? CompletableFuture.runAsync(closeTask, exec) : CompletableFuture.runAsync(closeTask)); } sessions.clear(); diff --git a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java index 33d84a9c5..af8667864 100644 --- a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java +++ b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java @@ -429,18 +429,14 @@ public Executor getExecutor() { *

* When provided, the SDK uses this executor for all internal * {@code CompletableFuture} combinators instead of the default - * {@code ForkJoinPool.commonPool()}. This allows callers to isolate SDK - * work onto a dedicated thread pool or integrate with container-managed - * threading. + * {@code ForkJoinPool.commonPool()}. This allows callers to isolate SDK work + * onto a dedicated thread pool or integrate with container-managed threading. * * @param executor * the executor to use, or {@code null} for the default * @return this options instance for fluent chaining */ public CopilotClientOptions setExecutor(Executor executor) { - if (null == executor) { - throw new IllegalArgumentException("PENDING(copilot): not implemented"); - } this.executor = executor; return this; } diff --git a/src/site/markdown/cookbook/multiple-sessions.md b/src/site/markdown/cookbook/multiple-sessions.md index 94a83acae..0121bfc4c 100644 --- a/src/site/markdown/cookbook/multiple-sessions.md +++ b/src/site/markdown/cookbook/multiple-sessions.md @@ -164,6 +164,69 @@ public class ParallelSessions { } ``` +## Providing a custom Executor for parallel sessions + +By default, `CompletableFuture` operations run on `ForkJoinPool.commonPool()`, +which has limited parallelism (typically `Runtime.availableProcessors() - 1` +threads). When multiple sessions block waiting for CLI responses, those threads +are unavailable for other work—a condition known as *pool starvation*. + +Use `CopilotClientOptions.setExecutor(Executor)` to supply a dedicated thread +pool so that SDK work does not compete with the rest of your application for +common-pool threads: + +```java +//DEPS com.github:copilot-sdk-java:${project.version} +import com.github.copilot.sdk.CopilotClient; +import com.github.copilot.sdk.json.CopilotClientOptions; +import com.github.copilot.sdk.json.SessionConfig; +import com.github.copilot.sdk.json.MessageOptions; +import com.github.copilot.sdk.json.PermissionHandler; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +public class ParallelSessionsWithExecutor { + public static void main(String[] args) throws Exception { + ExecutorService pool = Executors.newFixedThreadPool(4); + try { + var options = new CopilotClientOptions().setExecutor(pool); + try (CopilotClient client = new CopilotClient(options)) { + client.start().get(); + + var s1 = client.createSession(new SessionConfig() + .setOnPermissionRequest(PermissionHandler.APPROVE_ALL) + .setModel("gpt-5")).get(); + var s2 = client.createSession(new SessionConfig() + .setOnPermissionRequest(PermissionHandler.APPROVE_ALL) + .setModel("gpt-5")).get(); + var s3 = client.createSession(new SessionConfig() + .setOnPermissionRequest(PermissionHandler.APPROVE_ALL) + .setModel("claude-sonnet-4.5")).get(); + + CompletableFuture.allOf( + s1.sendAndWait(new MessageOptions().setPrompt("Question 1")), + s2.sendAndWait(new MessageOptions().setPrompt("Question 2")), + s3.sendAndWait(new MessageOptions().setPrompt("Question 3")) + ).get(); + + s1.close(); + s2.close(); + s3.close(); + } + } finally { + pool.shutdown(); + } + } +} +``` + +Passing `null` (or omitting `setExecutor` entirely) keeps the default +`ForkJoinPool.commonPool()` behaviour. The executor is used for all internal +`CompletableFuture.runAsync` / `supplyAsync` calls—including client start/stop, +tool-call dispatch, permission dispatch, user-input dispatch, and hooks. + ## Use cases - **Multi-user applications**: One session per user diff --git a/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java b/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java index 736564139..db5b1aaf4 100644 --- a/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java +++ b/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java @@ -25,7 +25,6 @@ import com.github.copilot.sdk.json.CopilotClientOptions; import com.github.copilot.sdk.json.MessageOptions; import com.github.copilot.sdk.json.PermissionHandler; -import com.github.copilot.sdk.json.PermissionRequest; import com.github.copilot.sdk.json.PermissionRequestResult; import com.github.copilot.sdk.json.PreToolUseHookOutput; import com.github.copilot.sdk.json.SessionConfig; @@ -62,8 +61,8 @@ static void teardown() throws Exception { } /** - * A decorator executor that delegates to a real executor while counting - * task submissions. + * A decorator executor that delegates to a real executor while counting task + * submissions. */ static class TrackingExecutor implements Executor { @@ -101,8 +100,8 @@ private CopilotClientOptions createOptionsWithExecutor(TrackingExecutor executor * *

* {@code CopilotClient.startCore()} uses - * {@code CompletableFuture.supplyAsync(...)} to initialize the connection. - * This test asserts that the start-up task goes through the caller-supplied + * {@code CompletableFuture.supplyAsync(...)} to initialize the connection. This + * test asserts that the start-up task goes through the caller-supplied * executor, not {@code ForkJoinPool.commonPool()}. *

* @@ -129,9 +128,8 @@ void testClientStartUsesProvidedExecutor() throws Exception { * Verifies that tool call dispatch routes through the provided executor. * *

- * When a custom tool is invoked by the LLM, the - * {@code RpcHandlerDispatcher} calls - * {@code CompletableFuture.runAsync(...)} to dispatch the tool handler. + * When a custom tool is invoked by the LLM, the {@code RpcHandlerDispatcher} + * calls {@code CompletableFuture.runAsync(...)} to dispatch the tool handler. * This test asserts that dispatch goes through the caller-supplied executor. *

* @@ -187,10 +185,9 @@ void testToolCallDispatchUsesProvidedExecutor() throws Exception { * executor. * *

- * When the LLM requests a permission, the {@code RpcHandlerDispatcher} - * calls {@code CompletableFuture.runAsync(...)} to dispatch the permission - * handler. This test asserts that dispatch goes through the caller-supplied - * executor. + * When the LLM requests a permission, the {@code RpcHandlerDispatcher} calls + * {@code CompletableFuture.runAsync(...)} to dispatch the permission handler. + * This test asserts that dispatch goes through the caller-supplied executor. *

* * @see Snapshot: permissions/permission_handler_for_write_operations @@ -212,8 +209,7 @@ void testPermissionDispatchUsesProvidedExecutor() throws Exception { int beforeSend = trackingExecutor.getTaskCount(); - session.sendAndWait( - new MessageOptions().setPrompt("Edit test.txt and replace 'original' with 'modified'")) + session.sendAndWait(new MessageOptions().setPrompt("Edit test.txt and replace 'original' with 'modified'")) .get(60, TimeUnit.SECONDS); assertTrue(trackingExecutor.getTaskCount() > beforeSend, @@ -231,9 +227,8 @@ void testPermissionDispatchUsesProvidedExecutor() throws Exception { * *

* When the LLM asks for user input, the {@code RpcHandlerDispatcher} calls - * {@code CompletableFuture.runAsync(...)} to dispatch the user input - * handler. This test asserts that dispatch goes through the caller-supplied - * executor. + * {@code CompletableFuture.runAsync(...)} to dispatch the user input handler. + * This test asserts that dispatch goes through the caller-supplied executor. *

* * @see Snapshot: @@ -278,8 +273,8 @@ void testUserInputDispatchUsesProvidedExecutor() throws Exception { * *

* When the LLM triggers a hook, the {@code RpcHandlerDispatcher} calls - * {@code CompletableFuture.runAsync(...)} to dispatch the hooks handler. - * This test asserts that dispatch goes through the caller-supplied executor. + * {@code CompletableFuture.runAsync(...)} to dispatch the hooks handler. This + * test asserts that dispatch goes through the caller-supplied executor. *

* * @see Snapshot: hooks/invoke_pre_tool_use_hook_when_model_runs_a_tool @@ -316,14 +311,13 @@ void testHooksDispatchUsesProvidedExecutor() throws Exception { } /** - * Verifies that {@code CopilotClient.stop()} routes session closure through - * the provided executor. + * Verifies that {@code CopilotClient.stop()} routes session closure through the + * provided executor. * *

- * {@code CopilotClient.stop()} uses - * {@code CompletableFuture.runAsync(...)} to close each active session. - * This test asserts that those closures go through the caller-supplied - * executor. + * {@code CopilotClient.stop()} uses {@code CompletableFuture.runAsync(...)} to + * close each active session. This test asserts that those closures go through + * the caller-supplied executor. *

* * @see Snapshot: tools/invokes_custom_tool From e62485739e973ce899cbf57396a43ad4cfc47be0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 30 Mar 2026 21:19:56 +0000 Subject: [PATCH 3/9] Remove "TDD red-phase" from ExecutorWiringTest Javadoc Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/93199b25-7c90-4c45-9540-527396b8990c Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --- src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java b/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java index db5b1aaf4..15904504a 100644 --- a/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java +++ b/src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java @@ -33,7 +33,7 @@ import com.github.copilot.sdk.json.UserInputResponse; /** - * TDD red-phase tests verifying that when an {@link Executor} is provided via + * Tests verifying that when an {@link Executor} is provided via * {@link CopilotClientOptions#setExecutor(Executor)}, all internal * {@code CompletableFuture.*Async} calls are routed through that executor * instead of {@code ForkJoinPool.commonPool()}. From e975d6cf420547ecbc89ce1ae6a7cdf518e6e330 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 30 Mar 2026 21:53:47 +0000 Subject: [PATCH 4/9] Handle RejectedExecutionException in all async submission sites Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/63b9b09f-f1f4-44d3-8e34-ad01e355cc6a Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --- .../com/github/copilot/sdk/CopilotClient.java | 24 +++++++++++++---- .../github/copilot/sdk/CopilotSession.java | 27 +++++++++++++------ .../copilot/sdk/RpcHandlerDispatcher.java | 14 +++++++--- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/github/copilot/sdk/CopilotClient.java b/src/main/java/com/github/copilot/sdk/CopilotClient.java index 285b5fbdc..e2790f6a3 100644 --- a/src/main/java/com/github/copilot/sdk/CopilotClient.java +++ b/src/main/java/com/github/copilot/sdk/CopilotClient.java @@ -14,6 +14,7 @@ import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; @@ -152,9 +153,13 @@ private CompletableFuture startCore() { LOG.fine("Starting Copilot client"); Executor exec = options.getExecutor(); - return exec != null - ? CompletableFuture.supplyAsync(this::startCoreBody, exec) - : CompletableFuture.supplyAsync(this::startCoreBody); + try { + return exec != null + ? CompletableFuture.supplyAsync(this::startCoreBody, exec) + : CompletableFuture.supplyAsync(this::startCoreBody); + } catch (RejectedExecutionException e) { + return CompletableFuture.failedFuture(e); + } } private Connection startCoreBody() { @@ -244,8 +249,17 @@ public CompletableFuture stop() { LOG.log(Level.WARNING, "Error closing session " + session.getSessionId(), e); } }; - closeFutures.add( - exec != null ? CompletableFuture.runAsync(closeTask, exec) : CompletableFuture.runAsync(closeTask)); + CompletableFuture future; + try { + future = exec != null + ? CompletableFuture.runAsync(closeTask, exec) + : CompletableFuture.runAsync(closeTask); + } catch (RejectedExecutionException e) { + LOG.log(Level.WARNING, "Executor rejected session close task; closing inline", e); + closeTask.run(); + future = CompletableFuture.completedFuture(null); + } + closeFutures.add(future); } sessions.clear(); diff --git a/src/main/java/com/github/copilot/sdk/CopilotSession.java b/src/main/java/com/github/copilot/sdk/CopilotSession.java index 485c39312..23b54ed2d 100644 --- a/src/main/java/com/github/copilot/sdk/CopilotSession.java +++ b/src/main/java/com/github/copilot/sdk/CopilotSession.java @@ -730,10 +730,15 @@ private void executeToolAndRespondAsync(String requestId, String toolName, Strin } } }; - if (executor != null) { - CompletableFuture.runAsync(task, executor); - } else { - CompletableFuture.runAsync(task); + try { + if (executor != null) { + CompletableFuture.runAsync(task, executor); + } else { + CompletableFuture.runAsync(task); + } + } catch (RejectedExecutionException e) { + LOG.log(Level.WARNING, "Executor rejected tool task for requestId=" + requestId + "; running inline", e); + task.run(); } } @@ -783,10 +788,16 @@ private void executePermissionAndRespondAsync(String requestId, PermissionReques } } }; - if (executor != null) { - CompletableFuture.runAsync(task, executor); - } else { - CompletableFuture.runAsync(task); + try { + if (executor != null) { + CompletableFuture.runAsync(task, executor); + } else { + CompletableFuture.runAsync(task); + } + } catch (RejectedExecutionException e) { + LOG.log(Level.WARNING, "Executor rejected permission task for requestId=" + requestId + "; running inline", + e); + task.run(); } } diff --git a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java index 9f55938af..f1d488105 100644 --- a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java +++ b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java @@ -10,6 +10,7 @@ import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; +import java.util.concurrent.RejectedExecutionException; import java.util.logging.Level; import java.util.logging.Logger; @@ -367,10 +368,15 @@ private void handleSystemMessageTransform(JsonRpcClient rpc, String requestId, J } private void runAsync(Runnable task) { - if (executor != null) { - CompletableFuture.runAsync(task, executor); - } else { - CompletableFuture.runAsync(task); + try { + if (executor != null) { + CompletableFuture.runAsync(task, executor); + } else { + CompletableFuture.runAsync(task); + } + } catch (RejectedExecutionException e) { + LOG.log(Level.WARNING, "Executor rejected handler task; running inline", e); + task.run(); } } } From 63b6d31bd8d6bc516fcfe5f44144728fe1053dd2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 30 Mar 2026 21:56:13 +0000 Subject: [PATCH 5/9] Align RejectedExecutionException log message formatting in CopilotSession Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/63b9b09f-f1f4-44d3-8e34-ad01e355cc6a Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --- src/main/java/com/github/copilot/sdk/CopilotSession.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/com/github/copilot/sdk/CopilotSession.java b/src/main/java/com/github/copilot/sdk/CopilotSession.java index 23b54ed2d..523da1114 100644 --- a/src/main/java/com/github/copilot/sdk/CopilotSession.java +++ b/src/main/java/com/github/copilot/sdk/CopilotSession.java @@ -795,8 +795,7 @@ private void executePermissionAndRespondAsync(String requestId, PermissionReques CompletableFuture.runAsync(task); } } catch (RejectedExecutionException e) { - LOG.log(Level.WARNING, "Executor rejected permission task for requestId=" + requestId + "; running inline", - e); + LOG.log(Level.WARNING, "Executor rejected perm task for requestId=" + requestId + "; running inline", e); task.run(); } } From 7728b4f65bc396eddcd55a776ef27730c0a1d9b4 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Mon, 30 Mar 2026 19:17:00 -0400 Subject: [PATCH 6/9] On branch edburns/dd-2758695-virtual-threads-accept-executor modified: README.md - Use the "uncomment these three lines to get Virtual Threads" approach modified: src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java - Cleanup. Sorting. Signed-off-by: Ed Burns --- README.md | 9 +- .../sdk/json/CopilotClientOptions.java | 342 +++++++++--------- 2 files changed, 179 insertions(+), 172 deletions(-) diff --git a/README.md b/README.md index 0084eb417..84f0d6d81 100644 --- a/README.md +++ b/README.md @@ -69,16 +69,23 @@ implementation 'com.github:copilot-sdk-java:0.2.1-java.0' import com.github.copilot.sdk.CopilotClient; import com.github.copilot.sdk.events.AssistantMessageEvent; import com.github.copilot.sdk.events.SessionUsageInfoEvent; +import com.github.copilot.sdk.json.CopilotClientOptions; import com.github.copilot.sdk.json.MessageOptions; import com.github.copilot.sdk.json.PermissionHandler; import com.github.copilot.sdk.json.SessionConfig; +import java.util.concurrent.Executors; + public class CopilotSDK { public static void main(String[] args) throws Exception { var lastMessage = new String[]{null}; // Create and start client - try (var client = new CopilotClient()) { + try (var client = new CopilotClient()) { // JDK 25+: comment out this line + // JDK 25+: uncomment the following 3 lines for virtual thread support + // var options = new CopilotClientOptions() + // .setExecutor(Executors.newVirtualThreadPerTaskExecutor()); + // try (var client = new CopilotClient(options)) { client.start().get(); // Create a session diff --git a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java index af8667864..3ba9fb0a0 100644 --- a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java +++ b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java @@ -35,133 +35,115 @@ @JsonInclude(JsonInclude.Include.NON_NULL) public class CopilotClientOptions { - private String cliPath; - private String[] cliArgs; - private String cwd; - private int port; - private boolean useStdio = true; - private String cliUrl; - private String logLevel = "info"; - private boolean autoStart = true; @Deprecated private boolean autoRestart; + private boolean autoStart = true; + private String[] cliArgs; + private String cliPath; + private String cliUrl; + private String cwd; private Map environment; + private Executor executor; private String gitHubToken; - private Boolean useLoggedInUser; + private String logLevel = "info"; private Supplier>> onListModels; + private int port; private TelemetryConfig telemetry; - private Executor executor; - - /** - * Gets the path to the Copilot CLI executable. - * - * @return the CLI path, or {@code null} to use "copilot" from PATH - */ - public String getCliPath() { - return cliPath; - } - - /** - * Sets the path to the Copilot CLI executable. - * - * @param cliPath - * the path to the CLI executable, or {@code null} to use "copilot" - * from PATH - * @return this options instance for method chaining - */ - public CopilotClientOptions setCliPath(String cliPath) { - this.cliPath = cliPath; - return this; - } + private Boolean useLoggedInUser; + private boolean useStdio = true; /** - * Gets the extra CLI arguments. + * Returns whether the client should automatically restart the server on crash. * - * @return the extra arguments to pass to the CLI + * @return the auto-restart flag value (no longer has any effect) + * @deprecated This option has no effect and will be removed in a future + * release. */ - public String[] getCliArgs() { - return cliArgs; + @Deprecated + public boolean isAutoRestart() { + return autoRestart; } /** - * Sets extra arguments to pass to the CLI process. - *

- * These arguments are prepended before SDK-managed flags. + * Sets whether the client should automatically restart the CLI server if it + * crashes unexpectedly. * - * @param cliArgs - * the extra arguments to pass + * @param autoRestart + * ignored — this option no longer has any effect * @return this options instance for method chaining + * @deprecated This option has no effect and will be removed in a future + * release. */ - public CopilotClientOptions setCliArgs(String[] cliArgs) { - this.cliArgs = cliArgs; + @Deprecated + public CopilotClientOptions setAutoRestart(boolean autoRestart) { + this.autoRestart = autoRestart; return this; } /** - * Gets the working directory for the CLI process. + * Returns whether the client should automatically start the server. * - * @return the working directory path + * @return {@code true} to auto-start (default), {@code false} for manual start */ - public String getCwd() { - return cwd; + public boolean isAutoStart() { + return autoStart; } /** - * Sets the working directory for the CLI process. + * Sets whether the client should automatically start the CLI server when the + * first request is made. * - * @param cwd - * the working directory path + * @param autoStart + * {@code true} to auto-start, {@code false} for manual start * @return this options instance for method chaining */ - public CopilotClientOptions setCwd(String cwd) { - this.cwd = cwd; + public CopilotClientOptions setAutoStart(boolean autoStart) { + this.autoStart = autoStart; return this; } /** - * Gets the TCP port for the CLI server. + * Gets the extra CLI arguments. * - * @return the port number, or 0 for a random port + * @return the extra arguments to pass to the CLI */ - public int getPort() { - return port; + public String[] getCliArgs() { + return cliArgs; } /** - * Sets the TCP port for the CLI server to listen on. + * Sets extra arguments to pass to the CLI process. *

- * This is only used when {@link #isUseStdio()} is {@code false}. + * These arguments are prepended before SDK-managed flags. * - * @param port - * the port number, or 0 for a random port + * @param cliArgs + * the extra arguments to pass * @return this options instance for method chaining */ - public CopilotClientOptions setPort(int port) { - this.port = port; + public CopilotClientOptions setCliArgs(String[] cliArgs) { + this.cliArgs = cliArgs; return this; } /** - * Returns whether to use stdio transport instead of TCP. + * Gets the path to the Copilot CLI executable. * - * @return {@code true} to use stdio (default), {@code false} to use TCP + * @return the CLI path, or {@code null} to use "copilot" from PATH */ - public boolean isUseStdio() { - return useStdio; + public String getCliPath() { + return cliPath; } /** - * Sets whether to use stdio transport instead of TCP. - *

- * Stdio transport is more efficient and is the default. TCP transport can be - * useful for debugging or connecting to remote servers. + * Sets the path to the Copilot CLI executable. * - * @param useStdio - * {@code true} to use stdio, {@code false} to use TCP + * @param cliPath + * the path to the CLI executable, or {@code null} to use "copilot" + * from PATH * @return this options instance for method chaining */ - public CopilotClientOptions setUseStdio(boolean useStdio) { - this.useStdio = useStdio; + public CopilotClientOptions setCliPath(String cliPath) { + this.cliPath = cliPath; return this; } @@ -193,98 +175,73 @@ public CopilotClientOptions setCliUrl(String cliUrl) { } /** - * Gets the log level for the CLI process. - * - * @return the log level (default: "info") - */ - public String getLogLevel() { - return logLevel; - } - - /** - * Sets the log level for the CLI process. - *

- * Valid levels include: "error", "warn", "info", "debug", "trace". - * - * @param logLevel - * the log level - * @return this options instance for method chaining - */ - public CopilotClientOptions setLogLevel(String logLevel) { - this.logLevel = logLevel; - return this; - } - - /** - * Returns whether the client should automatically start the server. + * Gets the working directory for the CLI process. * - * @return {@code true} to auto-start (default), {@code false} for manual start + * @return the working directory path */ - public boolean isAutoStart() { - return autoStart; + public String getCwd() { + return cwd; } /** - * Sets whether the client should automatically start the CLI server when the - * first request is made. + * Sets the working directory for the CLI process. * - * @param autoStart - * {@code true} to auto-start, {@code false} for manual start + * @param cwd + * the working directory path * @return this options instance for method chaining */ - public CopilotClientOptions setAutoStart(boolean autoStart) { - this.autoStart = autoStart; + public CopilotClientOptions setCwd(String cwd) { + this.cwd = cwd; return this; } /** - * Returns whether the client should automatically restart the server on crash. + * Gets the environment variables for the CLI process. * - * @return the auto-restart flag value (no longer has any effect) - * @deprecated This option has no effect and will be removed in a future - * release. + * @return the environment variables map */ - @Deprecated - public boolean isAutoRestart() { - return autoRestart; + public Map getEnvironment() { + return environment; } /** - * Sets whether the client should automatically restart the CLI server if it - * crashes unexpectedly. + * Sets environment variables to pass to the CLI process. + *

+ * When set, these environment variables replace the inherited environment. * - * @param autoRestart - * ignored — this option no longer has any effect + * @param environment + * the environment variables map * @return this options instance for method chaining - * @deprecated This option has no effect and will be removed in a future - * release. */ - @Deprecated - public CopilotClientOptions setAutoRestart(boolean autoRestart) { - this.autoRestart = autoRestart; + public CopilotClientOptions setEnvironment(Map environment) { + this.environment = environment; return this; } /** - * Gets the environment variables for the CLI process. + * Gets the executor used for internal asynchronous operations. * - * @return the environment variables map + * @return the executor, or {@code null} to use the default + * {@code ForkJoinPool.commonPool()} */ - public Map getEnvironment() { - return environment; + public Executor getExecutor() { + return executor; } /** - * Sets environment variables to pass to the CLI process. + * Sets the executor used for internal asynchronous operations. *

- * When set, these environment variables replace the inherited environment. + * When provided, the SDK uses this executor for all internal + * {@code CompletableFuture} combinators instead of the default + * {@code ForkJoinPool.commonPool()}. This allows callers to isolate SDK work + * onto a dedicated thread pool or integrate with container-managed threading. * - * @param environment - * the environment variables map - * @return this options instance for method chaining + * @param executor + * the executor to use, or {@code null} for the default + * @return this options instance for fluent chaining */ - public CopilotClientOptions setEnvironment(Map environment) { - this.environment = environment; + public CopilotClientOptions setExecutor(Executor executor) { + this.executor = executor; return this; } @@ -338,28 +295,25 @@ public CopilotClientOptions setGithubToken(String githubToken) { } /** - * Returns whether to use the logged-in user for authentication. + * Gets the log level for the CLI process. * - * @return {@code true} to use logged-in user auth, {@code false} to use only - * explicit tokens, or {@code null} to use default behavior + * @return the log level (default: "info") */ - public Boolean getUseLoggedInUser() { - return useLoggedInUser; + public String getLogLevel() { + return logLevel; } /** - * Sets whether to use the logged-in user for authentication. + * Sets the log level for the CLI process. *

- * When true, the CLI server will attempt to use stored OAuth tokens or gh CLI - * auth. When false, only explicit tokens (gitHubToken or environment variables) - * are used. Default: true (but defaults to false when gitHubToken is provided). + * Valid levels include: "error", "warn", "info", "debug", "trace". * - * @param useLoggedInUser - * {@code true} to use logged-in user auth, {@code false} otherwise + * @param logLevel + * the log level * @return this options instance for method chaining */ - public CopilotClientOptions setUseLoggedInUser(Boolean useLoggedInUser) { - this.useLoggedInUser = useLoggedInUser; + public CopilotClientOptions setLogLevel(String logLevel) { + this.logLevel = logLevel; return this; } @@ -388,6 +342,29 @@ public CopilotClientOptions setOnListModels(Supplier + * This is only used when {@link #isUseStdio()} is {@code false}. + * + * @param port + * the port number, or 0 for a random port + * @return this options instance for method chaining + */ + public CopilotClientOptions setPort(int port) { + this.port = port; + return this; + } + /** * Gets the OpenTelemetry configuration for the CLI server. * @@ -415,29 +392,52 @@ public CopilotClientOptions setTelemetry(TelemetryConfig telemetry) { } /** - * Gets the executor used for internal asynchronous operations. + * Returns whether to use the logged-in user for authentication. * - * @return the executor, or {@code null} to use the default - * {@code ForkJoinPool.commonPool()} + * @return {@code true} to use logged-in user auth, {@code false} to use only + * explicit tokens, or {@code null} to use default behavior */ - public Executor getExecutor() { - return executor; + public Boolean getUseLoggedInUser() { + return useLoggedInUser; } /** - * Sets the executor used for internal asynchronous operations. + * Sets whether to use the logged-in user for authentication. *

- * When provided, the SDK uses this executor for all internal - * {@code CompletableFuture} combinators instead of the default - * {@code ForkJoinPool.commonPool()}. This allows callers to isolate SDK work - * onto a dedicated thread pool or integrate with container-managed threading. + * When true, the CLI server will attempt to use stored OAuth tokens or gh CLI + * auth. When false, only explicit tokens (gitHubToken or environment variables) + * are used. Default: true (but defaults to false when gitHubToken is provided). * - * @param executor - * the executor to use, or {@code null} for the default - * @return this options instance for fluent chaining + * @param useLoggedInUser + * {@code true} to use logged-in user auth, {@code false} otherwise + * @return this options instance for method chaining */ - public CopilotClientOptions setExecutor(Executor executor) { - this.executor = executor; + public CopilotClientOptions setUseLoggedInUser(Boolean useLoggedInUser) { + this.useLoggedInUser = useLoggedInUser; + return this; + } + + /** + * Returns whether to use stdio transport instead of TCP. + * + * @return {@code true} to use stdio (default), {@code false} to use TCP + */ + public boolean isUseStdio() { + return useStdio; + } + + /** + * Sets whether to use stdio transport instead of TCP. + *

+ * Stdio transport is more efficient and is the default. TCP transport can be + * useful for debugging or connecting to remote servers. + * + * @param useStdio + * {@code true} to use stdio, {@code false} to use TCP + * @return this options instance for method chaining + */ + public CopilotClientOptions setUseStdio(boolean useStdio) { + this.useStdio = useStdio; return this; } @@ -454,21 +454,21 @@ public CopilotClientOptions setExecutor(Executor executor) { @Override public CopilotClientOptions clone() { CopilotClientOptions copy = new CopilotClientOptions(); - copy.cliPath = this.cliPath; + copy.autoRestart = this.autoRestart; + copy.autoStart = this.autoStart; copy.cliArgs = this.cliArgs != null ? this.cliArgs.clone() : null; - copy.cwd = this.cwd; - copy.port = this.port; - copy.useStdio = this.useStdio; + copy.cliPath = this.cliPath; copy.cliUrl = this.cliUrl; - copy.logLevel = this.logLevel; - copy.autoStart = this.autoStart; - copy.autoRestart = this.autoRestart; + copy.cwd = this.cwd; copy.environment = this.environment != null ? new java.util.HashMap<>(this.environment) : null; + copy.executor = this.executor; copy.gitHubToken = this.gitHubToken; - copy.useLoggedInUser = this.useLoggedInUser; + copy.logLevel = this.logLevel; copy.onListModels = this.onListModels; + copy.port = this.port; copy.telemetry = this.telemetry; - copy.executor = this.executor; + copy.useLoggedInUser = this.useLoggedInUser; + copy.useStdio = this.useStdio; return copy; } } From 1721ffc78104f7988ef8e85adb282f09e2c72f04 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 31 Mar 2026 16:42:11 +0000 Subject: [PATCH 7/9] Add null checks to all reference-type setters in CopilotClientOptions Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/019cc9a8-a29a-49a5-a7ac-aa573931dfb8 Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --- .../sdk/json/CopilotClientOptions.java | 32 +++++++++---------- .../copilot/sdk/CliServerManagerTest.java | 4 +-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java index 3ba9fb0a0..ecba8ce82 100644 --- a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java +++ b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.Executor; import java.util.function.Supplier; @@ -121,7 +122,7 @@ public String[] getCliArgs() { * @return this options instance for method chaining */ public CopilotClientOptions setCliArgs(String[] cliArgs) { - this.cliArgs = cliArgs; + this.cliArgs = Objects.requireNonNull(cliArgs, "cliArgs must not be null"); return this; } @@ -138,12 +139,11 @@ public String getCliPath() { * Sets the path to the Copilot CLI executable. * * @param cliPath - * the path to the CLI executable, or {@code null} to use "copilot" - * from PATH + * the path to the CLI executable * @return this options instance for method chaining */ public CopilotClientOptions setCliPath(String cliPath) { - this.cliPath = cliPath; + this.cliPath = Objects.requireNonNull(cliPath, "cliPath must not be null"); return this; } @@ -170,7 +170,7 @@ public String getCliUrl() { * @return this options instance for method chaining */ public CopilotClientOptions setCliUrl(String cliUrl) { - this.cliUrl = cliUrl; + this.cliUrl = Objects.requireNonNull(cliUrl, "cliUrl must not be null"); return this; } @@ -191,7 +191,7 @@ public String getCwd() { * @return this options instance for method chaining */ public CopilotClientOptions setCwd(String cwd) { - this.cwd = cwd; + this.cwd = Objects.requireNonNull(cwd, "cwd must not be null"); return this; } @@ -214,7 +214,7 @@ public Map getEnvironment() { * @return this options instance for method chaining */ public CopilotClientOptions setEnvironment(Map environment) { - this.environment = environment; + this.environment = Objects.requireNonNull(environment, "environment must not be null"); return this; } @@ -237,11 +237,11 @@ public Executor getExecutor() { * onto a dedicated thread pool or integrate with container-managed threading. * * @param executor - * the executor to use, or {@code null} for the default + * the executor to use * @return this options instance for fluent chaining */ public CopilotClientOptions setExecutor(Executor executor) { - this.executor = executor; + this.executor = Objects.requireNonNull(executor, "executor must not be null"); return this; } @@ -265,7 +265,7 @@ public String getGitHubToken() { * @return this options instance for method chaining */ public CopilotClientOptions setGitHubToken(String gitHubToken) { - this.gitHubToken = gitHubToken; + this.gitHubToken = Objects.requireNonNull(gitHubToken, "gitHubToken must not be null"); return this; } @@ -290,7 +290,7 @@ public String getGithubToken() { */ @Deprecated public CopilotClientOptions setGithubToken(String githubToken) { - this.gitHubToken = githubToken; + this.gitHubToken = Objects.requireNonNull(githubToken, "githubToken must not be null"); return this; } @@ -313,7 +313,7 @@ public String getLogLevel() { * @return this options instance for method chaining */ public CopilotClientOptions setLogLevel(String logLevel) { - this.logLevel = logLevel; + this.logLevel = Objects.requireNonNull(logLevel, "logLevel must not be null"); return this; } @@ -338,7 +338,7 @@ public Supplier>> getOnListModels() { * @return this options instance for method chaining */ public CopilotClientOptions setOnListModels(Supplier>> onListModels) { - this.onListModels = onListModels; + this.onListModels = Objects.requireNonNull(onListModels, "onListModels must not be null"); return this; } @@ -378,8 +378,8 @@ public TelemetryConfig getTelemetry() { /** * Sets the OpenTelemetry configuration for the CLI server. *

- * When set to a non-{@code null} value, the CLI server is started with - * OpenTelemetry instrumentation enabled using the provided settings. + * When set, the CLI server is started with OpenTelemetry instrumentation + * enabled using the provided settings. * * @param telemetry * the telemetry configuration @@ -387,7 +387,7 @@ public TelemetryConfig getTelemetry() { * @since 1.2.0 */ public CopilotClientOptions setTelemetry(TelemetryConfig telemetry) { - this.telemetry = telemetry; + this.telemetry = Objects.requireNonNull(telemetry, "telemetry must not be null"); return this; } diff --git a/src/test/java/com/github/copilot/sdk/CliServerManagerTest.java b/src/test/java/com/github/copilot/sdk/CliServerManagerTest.java index 86d6be875..f17201583 100644 --- a/src/test/java/com/github/copilot/sdk/CliServerManagerTest.java +++ b/src/test/java/com/github/copilot/sdk/CliServerManagerTest.java @@ -199,8 +199,8 @@ void startCliServerWithGitHubTokenAndNoExplicitUseLoggedInUser() throws Exceptio @Test void startCliServerWithNullCliPath() throws Exception { - // Test the null cliPath branch (defaults to "copilot") - var options = new CopilotClientOptions().setCliPath(null).setUseStdio(true); + // Test the default cliPath branch (defaults to "copilot" when not set) + var options = new CopilotClientOptions().setUseStdio(true); var manager = new CliServerManager(options); // "copilot" likely doesn't exist in the test env — that's fine From 11aa5d65e300f50e510d6f484e72754d00d873e0 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Tue, 31 Mar 2026 14:10:44 -0400 Subject: [PATCH 8/9] On branch edburns/dd-2758695-virtual-threads-accept-executor modified: src/main/java/com/github/copilot/sdk/CopilotSession.java modified: src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java - Spotless apply. - Sync javadoc to behavior. --- .../github/copilot/sdk/CopilotSession.java | 1 - .../sdk/json/CopilotClientOptions.java | 73 ++++++++++++++----- 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/github/copilot/sdk/CopilotSession.java b/src/main/java/com/github/copilot/sdk/CopilotSession.java index 523da1114..844737fc2 100644 --- a/src/main/java/com/github/copilot/sdk/CopilotSession.java +++ b/src/main/java/com/github/copilot/sdk/CopilotSession.java @@ -14,7 +14,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; -import java.util.concurrent.Executors; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; diff --git a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java index ecba8ce82..9470cd05f 100644 --- a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java +++ b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java @@ -4,6 +4,8 @@ package com.github.copilot.sdk.json; +import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -105,24 +107,35 @@ public CopilotClientOptions setAutoStart(boolean autoStart) { /** * Gets the extra CLI arguments. + *

+ * Returns a shallow copy of the internal array, or {@code null} if no arguments + * have been set. * - * @return the extra arguments to pass to the CLI + * @return a copy of the extra arguments, or {@code null} */ public String[] getCliArgs() { - return cliArgs; + return cliArgs != null ? Arrays.copyOf(cliArgs, cliArgs.length) : null; } /** * Sets extra arguments to pass to the CLI process. *

- * These arguments are prepended before SDK-managed flags. + * These arguments are prepended before SDK-managed flags. A shallow copy of the + * provided array is stored. If {@code null} or empty, the existing arguments + * are cleared. * * @param cliArgs - * the extra arguments to pass + * the extra arguments to pass, or {@code null}/empty to clear * @return this options instance for method chaining */ public CopilotClientOptions setCliArgs(String[] cliArgs) { - this.cliArgs = Objects.requireNonNull(cliArgs, "cliArgs must not be null"); + if (cliArgs == null || cliArgs.length == 0) { + if (this.cliArgs != null) { + this.cliArgs = new String[0]; + } + } else { + this.cliArgs = Arrays.copyOf(cliArgs, cliArgs.length); + } return this; } @@ -166,8 +179,11 @@ public String getCliUrl() { * {@link #setUseStdio(boolean)} and {@link #setCliPath(String)}. * * @param cliUrl - * the CLI server URL to connect to + * the CLI server URL to connect to (must not be {@code null} or + * empty) * @return this options instance for method chaining + * @throws IllegalArgumentException + * if {@code cliUrl} is {@code null} or empty */ public CopilotClientOptions setCliUrl(String cliUrl) { this.cliUrl = Objects.requireNonNull(cliUrl, "cliUrl must not be null"); @@ -187,8 +203,10 @@ public String getCwd() { * Sets the working directory for the CLI process. * * @param cwd - * the working directory path + * the working directory path (must not be {@code null} or empty) * @return this options instance for method chaining + * @throws IllegalArgumentException + * if {@code cwd} is {@code null} or empty */ public CopilotClientOptions setCwd(String cwd) { this.cwd = Objects.requireNonNull(cwd, "cwd must not be null"); @@ -197,24 +215,35 @@ public CopilotClientOptions setCwd(String cwd) { /** * Gets the environment variables for the CLI process. + *

+ * Returns a shallow copy of the internal map, or {@code null} if no environment + * has been set. * - * @return the environment variables map + * @return a copy of the environment variables map, or {@code null} */ public Map getEnvironment() { - return environment; + return environment != null ? new HashMap<>(environment) : null; } /** * Sets environment variables to pass to the CLI process. *

- * When set, these environment variables replace the inherited environment. + * When set, these environment variables replace the inherited environment. A + * shallow copy of the provided map is stored. If {@code null} or empty, the + * existing environment is cleared. * * @param environment - * the environment variables map + * the environment variables map, or {@code null}/empty to clear * @return this options instance for method chaining */ public CopilotClientOptions setEnvironment(Map environment) { - this.environment = Objects.requireNonNull(environment, "environment must not be null"); + if (environment == null || environment.isEmpty()) { + if (this.environment != null) { + this.environment.clear(); + } + } else { + this.environment = new HashMap<>(environment); + } return this; } @@ -261,8 +290,10 @@ public String getGitHubToken() { * variable. This takes priority over other authentication methods. * * @param gitHubToken - * the GitHub token + * the GitHub token (must not be {@code null} or empty) * @return this options instance for method chaining + * @throws IllegalArgumentException + * if {@code gitHubToken} is {@code null} or empty */ public CopilotClientOptions setGitHubToken(String gitHubToken) { this.gitHubToken = Objects.requireNonNull(gitHubToken, "gitHubToken must not be null"); @@ -309,8 +340,10 @@ public String getLogLevel() { * Valid levels include: "error", "warn", "info", "debug", "trace". * * @param logLevel - * the log level + * the log level (must not be {@code null} or empty) * @return this options instance for method chaining + * @throws IllegalArgumentException + * if {@code logLevel} is {@code null} or empty */ public CopilotClientOptions setLogLevel(String logLevel) { this.logLevel = Objects.requireNonNull(logLevel, "logLevel must not be null"); @@ -334,8 +367,11 @@ public Supplier>> getOnListModels() { * available from your custom provider. * * @param onListModels - * the handler that returns the list of available models + * the handler that returns the list of available models (must not be + * {@code null}) * @return this options instance for method chaining + * @throws IllegalArgumentException + * if {@code onListModels} is {@code null} */ public CopilotClientOptions setOnListModels(Supplier>> onListModels) { this.onListModels = Objects.requireNonNull(onListModels, "onListModels must not be null"); @@ -407,13 +443,16 @@ public Boolean getUseLoggedInUser() { * When true, the CLI server will attempt to use stored OAuth tokens or gh CLI * auth. When false, only explicit tokens (gitHubToken or environment variables) * are used. Default: true (but defaults to false when gitHubToken is provided). + *

+ * Passing {@code null} is equivalent to passing {@link Boolean#FALSE}. * * @param useLoggedInUser - * {@code true} to use logged-in user auth, {@code false} otherwise + * {@code true} to use logged-in user auth, {@code false} or + * {@code null} otherwise * @return this options instance for method chaining */ public CopilotClientOptions setUseLoggedInUser(Boolean useLoggedInUser) { - this.useLoggedInUser = useLoggedInUser; + this.useLoggedInUser = useLoggedInUser != null ? useLoggedInUser : Boolean.FALSE; return this; } From 4fdef27df41fba977ffff7bfb58ed1e296b07299 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Tue, 31 Mar 2026 14:31:33 -0400 Subject: [PATCH 9/9] On branch edburns/dd-2758695-virtual-threads-accept-executor modified: src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java - Correctly implement the semantic of "null argument to setExecutor means use the default executor." modified: src/test/java/com/github/copilot/sdk/ConfigCloneTest.java - Adjust test based on defensive copy changes. --- .../sdk/json/CopilotClientOptions.java | 7 +++++-- .../github/copilot/sdk/ConfigCloneTest.java | 19 ++++++++++++++----- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java index 9470cd05f..2e9a80456 100644 --- a/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java +++ b/src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java @@ -264,13 +264,16 @@ public Executor getExecutor() { * {@code CompletableFuture} combinators instead of the default * {@code ForkJoinPool.commonPool()}. This allows callers to isolate SDK work * onto a dedicated thread pool or integrate with container-managed threading. + *

+ * Passing {@code null} reverts to the default {@code ForkJoinPool.commonPool()} + * behavior. * * @param executor - * the executor to use + * the executor to use, or {@code null} for the default * @return this options instance for fluent chaining */ public CopilotClientOptions setExecutor(Executor executor) { - this.executor = Objects.requireNonNull(executor, "executor must not be null"); + this.executor = executor; return this; } diff --git a/src/test/java/com/github/copilot/sdk/ConfigCloneTest.java b/src/test/java/com/github/copilot/sdk/ConfigCloneTest.java index f3eceb4c2..bf4881d5c 100644 --- a/src/test/java/com/github/copilot/sdk/ConfigCloneTest.java +++ b/src/test/java/com/github/copilot/sdk/ConfigCloneTest.java @@ -49,10 +49,16 @@ void copilotClientOptionsArrayIndependence() { original.setCliArgs(args); CopilotClientOptions cloned = original.clone(); - cloned.getCliArgs()[0] = "--changed"; + // Mutate the source array after set — should not affect original or clone + args[0] = "--changed"; + + assertEquals("--flag1", original.getCliArgs()[0]); + assertEquals("--flag1", cloned.getCliArgs()[0]); + + // getCliArgs() returns a copy, so mutating it should not affect internals + original.getCliArgs()[0] = "--mutated"; assertEquals("--flag1", original.getCliArgs()[0]); - assertEquals("--changed", cloned.getCliArgs()[0]); } @Test @@ -64,12 +70,15 @@ void copilotClientOptionsEnvironmentIndependence() { CopilotClientOptions cloned = original.clone(); - // Mutate the original environment map to test independence + // Mutate the source map after set — should not affect original or clone env.put("KEY2", "value2"); - // The cloned config should be unaffected by mutations to the original map + assertEquals(1, original.getEnvironment().size()); assertEquals(1, cloned.getEnvironment().size()); - assertEquals(2, original.getEnvironment().size()); + + // getEnvironment() returns a copy, so mutating it should not affect internals + original.getEnvironment().put("KEY3", "value3"); + assertEquals(1, original.getEnvironment().size()); } @Test