Accept optional Executor on CopilotClientOptions to mitigate pool starvation

[edburns]

Before the change?

• All internal CompletableFuture.*Async calls used ForkJoinPool.commonPool(), which could become starved when multiple sessions blocked concurrently.
• CopilotClientOptions had no way for callers to supply a custom thread pool.
• RejectedExecutionException thrown synchronously by any async submission site would propagate to callers unexpectedly, potentially leaving JSON-RPC requests unanswered or skipping connection cleanup.
• All reference-type setters in CopilotClientOptions silently accepted null, making the API error-prone.

After the change?

• CopilotClientOptions has a new Executor field with fluent setExecutor(Executor) / getExecutor() accessors. When not set, the default ForkJoinPool.commonPool() behaviour is preserved.
• Clone support for the new field is included.
• The executor is wired through CopilotClient start/stop, RpcHandlerDispatcher (all 5 runAsync call sites via a private helper), and CopilotSession (executeToolAndRespondAsync / executePermissionAndRespondAsync).
• All async submission sites now catch RejectedExecutionException and handle it context-sensitively:
  • CopilotClient.startCore() — returns CompletableFuture.failedFuture(e) so rejection surfaces through the returned future instead of throwing synchronously.
  • CopilotClient.stop() — falls back to closing each session inline so cleanup always completes even if the executor has been shut down.
  • RpcHandlerDispatcher.runAsync() — falls back to running the handler inline on the notification thread so the JSON-RPC request is never left unanswered.
  • CopilotSession.executeToolAndRespondAsync() and executePermissionAndRespondAsync() — fall back to running the task inline so pending tool/permission calls always receive a reply.
• All reference-type setters in CopilotClientOptions now throw NullPointerException via Objects.requireNonNull when null is passed, making the API stricter and safer toward GA. Callers wishing to use the default (unset) value for an optional field should simply not call the setter. Affected setters: setCliArgs, setCliPath, setCliUrl, setCwd, setEnvironment, setExecutor, setGitHubToken, setGithubToken (deprecated), setLogLevel, setOnListModels, setTelemetry.
• ExecutorWiringTest (6 tests) verifies the executor is used for client start, tool-call dispatch, permission dispatch, user-input dispatch, hooks dispatch, and client stop.
• The multiple-sessions cookbook gains a new "Providing a custom Executor for parallel sessions" section with a pool-starvation explanation and a complete JBang example.
• RpcHandlerDispatcherTest updated for the new 3-arg constructor (passes null).
• CliServerManagerTest updated: the test that previously relied on setCliPath(null) to exercise the default-to-"copilot" code path now uses a default CopilotClientOptions() without calling setCliPath.

Pull request checklist

• Tests for the changes have been added (for bug fixes / features)
• Docs have been reviewed and added / updated if needed (for bug fixes / features)
• mvn spotless:apply has been run to format the code
• mvn clean verify passes locally

Does this introduce a breaking change?

• Yes
• No

---

[Copilot]

Pull request overview

Adds support for supplying a caller-provided Executor to route the SDK’s internal CompletableFuture.*Async work off ForkJoinPool.commonPool(), reducing starvation risk when running multiple blocking sessions concurrently.

Changes:

• Introduces CopilotClientOptions.getExecutor() / setExecutor(Executor) (nullable) and includes it in clone().
• Wires the executor through CopilotClient start/stop paths, RpcHandlerDispatcher, and CopilotSession tool/permission response execution.
• Adds E2E coverage (ExecutorWiringTest) and documents the new option in the multiple-sessions cookbook.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java	Adds nullable Executor option and clones it.
src/main/java/com/github/copilot/sdk/CopilotClient.java	Uses provided executor for startCore() and stop() async work; passes executor into dispatcher and sessions.
src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java	Adds executor-aware async dispatch helper for server-to-client handler execution.
src/main/java/com/github/copilot/sdk/CopilotSession.java	Routes broadcast tool/permission response tasks through provided executor when present.
src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java	New E2E tests asserting executor usage across key async dispatch paths.
src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java	Updates dispatcher construction for new constructor signature.
src/site/markdown/cookbook/multiple-sessions.md	Documents pool starvation and how to provide a custom executor.

Comments suppressed due to low confidence (1)

src/main/java/com/github/copilot/sdk/CopilotSession.java:789

• Same as tool execution: submitting this permission-response task via CompletableFuture.runAsync(task, executor) can throw RejectedExecutionException synchronously if the provided executor rejects tasks. Consider handling submission failure to ensure the pending permission request is either answered (e.g., denied) or otherwise completed deterministically.

        if (executor != null) {
            CompletableFuture.runAsync(task, executor);
        } else {
            CompletableFuture.runAsync(task);
        }

[brunoborges]

@edburns should we throw an IAE if the user passes null?

[edburns]

@brunoborges wrote:

should we throw an IAE if the user passes null?

We don't do that for other setters in CopilotClientOptions. I might say "yes" we should do that, but I'd like to understand why we would not do it for all other options before I fully commit to "yes" in the case of Executor.

[brunoborges]

@brunoborges wrote:

should we throw an IAE if the user passes null?

We don't do that for other setters in CopilotClientOptions. I might say "yes" we should do that, but I'd like to understand why we would not do it for all other options before I fully commit to "yes" in the case of Executor.

There was no deep thinking before. Making null-safe now leads us to a more stable and strict API, and towards GA.

What I mean is that this is perhaps now the moment to make the API safer.

[edburns]

@brunoborges wrote:

There was no deep thinking before. Making null-safe now leads us to a more stable and strict API, and towards GA.

What I mean is that this is perhaps now the moment to make the API safer.

Of course. I'll take this as consent to add the null checks throughout, at least in the CopilotClientOptions class.

I'll take the remainder of the null checks as a separate issue.

[Copilot]

@brunoborges wrote:

There was no deep thinking before. Making null-safe now leads us to a more stable and strict API, and towards GA.
...

Done in commit 9098d5a. All reference-type setters in CopilotClientOptions now call Objects.requireNonNull and throw NullPointerException for null arguments: setCliArgs, setCliPath, setCliUrl, setCwd, setEnvironment, setExecutor, setGitHubToken, setGithubToken (deprecated), setLogLevel, setOnListModels, and setTelemetry. Javadoc "or null to…" wording removed from affected setters.

The one test that explicitly called setCliPath(null) (CliServerManagerTest.startCliServerWithNullCliPath) was updated to use a default CopilotClientOptions() without calling setCliPath — which is the correct migration pattern: simply don't call the setter to get the default (null) field value.