diff --git a/.github/actions/java-test-report/action.yml b/.github/actions/java-test-report/action.yml index d5f6d2709..7d610e6b6 100644 --- a/.github/actions/java-test-report/action.yml +++ b/.github/actions/java-test-report/action.yml @@ -4,7 +4,7 @@ inputs: report-path: description: "Path to the test report XML files (glob pattern)" required: false - default: "java/target/surefire-reports*/TEST-*.xml" + default: "java/target/{surefire-reports*,failsafe-reports}/TEST-*.xml" jacoco-path: description: "Path to the JaCoCo XML report" required: false @@ -17,13 +17,17 @@ inputs: description: "Name for the check run" required: false default: "Java SDK Test Results" + title: + description: "Title for the test report summary" + required: false + default: "Copilot Java SDK :: Test Results" runs: using: "composite" steps: - name: Generate Test Summary shell: bash run: | - echo "## 🧪 Copilot Java SDK :: Test Results" >> $GITHUB_STEP_SUMMARY + echo "## 🧪 ${{ inputs.title }}" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY if ls ${{ inputs.report-path }} 1>/dev/null 2>&1; then diff --git a/.github/workflows/docs-validation.yml b/.github/workflows/docs-validation.yml index e5fe01d91..4f53b71e4 100644 --- a/.github/workflows/docs-validation.yml +++ b/.github/workflows/docs-validation.yml @@ -143,7 +143,7 @@ jobs: - uses: actions/setup-java@v4 with: distribution: 'microsoft' - java-version: '17' + java-version: '25' cache: 'maven' - name: Install SDK to local repo diff --git a/.github/workflows/java-publish-maven.yml b/.github/workflows/java-publish-maven.yml index 2f150f1b1..20b9b2054 100644 --- a/.github/workflows/java-publish-maven.yml +++ b/.github/workflows/java-publish-maven.yml @@ -54,10 +54,10 @@ jobs: - uses: ./.github/actions/setup-copilot - - name: Set up JDK 17 + - name: Set up JDK 25 uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5 with: - java-version: "17" + java-version: "25" distribution: "microsoft" cache: "maven" server-id: central diff --git a/.github/workflows/java-publish-snapshot.yml b/.github/workflows/java-publish-snapshot.yml index 7bc231c73..8c957627f 100644 --- a/.github/workflows/java-publish-snapshot.yml +++ b/.github/workflows/java-publish-snapshot.yml @@ -30,10 +30,10 @@ jobs: - uses: ./.github/actions/setup-copilot - - name: Set up JDK 17 + - name: Set up JDK 25 uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5 with: - java-version: "17" + java-version: "25" distribution: "microsoft" cache: "maven" server-id: central diff --git a/.github/workflows/java-sdk-tests.yml b/.github/workflows/java-sdk-tests.yml index 5e9b504fd..ef8fcc05e 100644 --- a/.github/workflows/java-sdk-tests.yml +++ b/.github/workflows/java-sdk-tests.yml @@ -37,10 +37,14 @@ permissions: jobs: java-sdk: - name: "Java SDK Tests" + name: "Java SDK Tests (JDK ${{ matrix.test-jdk }})" if: github.event.repository.fork == false runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + test-jdk: ["25", "17"] defaults: run: shell: bash @@ -52,7 +56,7 @@ jobs: - uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5 with: - java-version: "17" + java-version: "25" distribution: "microsoft" cache: "maven" @@ -60,14 +64,18 @@ jobs: with: node-version: 22 - - uses: ./.github/actions/setup-copilot - id: setup-copilot + - name: Build SDK and set up test harness + run: mvn test-compile jar:jar + + - name: Verify Javadoc generation + if: matrix.test-jdk == '25' + run: mvn javadoc:javadoc -q - - name: Install test harness dependencies - working-directory: ./test/harness - run: npm ci --ignore-scripts + - name: Verify CLI works + run: node target/copilot-sdk/nodejs/node_modules/@github/copilot/index.js --version - name: Run spotless check + if: matrix.test-jdk == '25' run: | mvn spotless:check if [ $? -ne 0 ]; then @@ -76,18 +84,30 @@ jobs: fi echo "✅ spotless:check passed" - - name: Verify Javadoc generation - run: mvn compile javadoc:javadoc -q + - name: Run Java SDK tests (JDK 25) + if: matrix.test-jdk == '25' + env: + CI: "true" + run: mvn verify -Dskip.test.harness=true - - name: Run Java SDK tests + - name: Switch to JDK 17 + if: matrix.test-jdk == '17' + uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5 + with: + java-version: "17" + distribution: "microsoft" + + - name: Run Java SDK tests (JDK 17, no recompilation) + if: matrix.test-jdk == '17' env: CI: "true" - COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }} - COPILOT_CLI_PATH: ${{ steps.setup-copilot.outputs.cli-path }} - run: mvn verify + run: | + echo "Running tests against JDK 25-built classes using JDK 17 runtime..." + java -version + mvn jacoco:prepare-agent@wire-up-coverage-instrumentation antrun:run@print-test-jdk-banner surefire:test failsafe:integration-test failsafe:verify jacoco:report@build-coverage-report-from-tests -Denforcer.skip=true - name: Upload test results for site generation - if: success() && github.ref == 'refs/heads/main' + if: success() && github.ref == 'refs/heads/main' && matrix.test-jdk == '25' uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: test-results-for-site @@ -98,12 +118,12 @@ jobs: retention-days: 1 - name: Generate JaCoCo badge - if: success() && github.ref == 'refs/heads/main' + if: success() && github.ref == 'refs/heads/main' && matrix.test-jdk == '25' working-directory: . run: .github/scripts/generate-java-coverage-badge.sh java/target/site/jacoco-coverage/jacoco.csv .github/badges - name: Create PR for JaCoCo badge update - if: success() && github.ref == 'refs/heads/main' + if: success() && github.ref == 'refs/heads/main' && matrix.test-jdk == '25' uses: peter-evans/create-pull-request@5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v7 with: commit-message: "Update Java JaCoCo coverage badge" @@ -116,12 +136,14 @@ jobs: - name: Generate Test Report Summary if: always() uses: ./.github/actions/java-test-report + with: + title: "Copilot Java SDK :: Test Results JDK ${{ matrix.test-jdk }}" - name: Upload test results on failure if: failure() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - name: java-test-results + name: java-test-results-jdk-${{ matrix.test-jdk }} path: | java/target/surefire-reports/ java/target/surefire-reports-isolated/ diff --git a/.vscode/settings.json b/.vscode/settings.json index d0d8465c3..c4ae9c761 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -25,5 +25,6 @@ "[go]": { "editor.defaultFormatter": "golang.go" }, - "java.configuration.updateBuildConfiguration": "automatic" + "java.configuration.updateBuildConfiguration": "automatic", + "java.compile.nullAnalysis.mode": "automatic" } diff --git a/java/README.md b/java/README.md index 61e59c8d6..46c01635a 100644 --- a/java/README.md +++ b/java/README.md @@ -19,10 +19,10 @@ Java SDK for programmatic control of GitHub Copilot CLI, enabling you to build A ## Installation -### Requirements +### Runtime requirements -- Java 17 or later. **JDK 25 recommended**. Selecting JDK 25 enables the use of virtual threads, as shown in the [Quick Start](#quick-start). -- GitHub Copilot CLI 1.0.17 or later installed and in `PATH` (or provide custom `cliPath`) +- Java 17 or later. **JDK 25 recommended**. The distributed jar is a multi-release jar (MR-JAR) and is compiled on JDK 25 with `maven.compiler.release` set to 17. This means, when run on JDK 25 and later, the SDK automatically uses virtual threads for its default internal executor. +- GitHub Copilot CLI 1.0.55-5. or later installed and in `PATH` (or provide custom `cliPath`) ### Maven @@ -30,10 +30,16 @@ Java SDK for programmatic control of GitHub Copilot CLI, enabling you to build A com.github copilot-sdk-java - 1.0.0-beta-java.4 + 1.0.0-beta-10-java.0 ``` +### Gradle + +```groovy +implementation 'com.github:copilot-sdk-java:1.0.0-beta-10-java.0' + + #### Snapshot Builds Snapshot builds of the next development version are published to Maven Central Snapshots. To use them, add the repository and update the dependency version in your `pom.xml`: @@ -50,14 +56,14 @@ Snapshot builds of the next development version are published to Maven Central S com.github copilot-sdk-java - 1.0.0-beta-java.5-SNAPSHOT + 1.0.0-beta-10-java.0-SNAPSHOT ``` ### Gradle ```groovy -implementation 'com.github:copilot-sdk-java:1.0.0-beta-java.4' +implementation 'com.github:copilot-sdk-java:1.0.0-beta-10-java.0-SNAPSHOT' ``` ## Quick Start @@ -66,23 +72,16 @@ implementation 'com.github:copilot-sdk-java:1.0.0-beta-java.4' import com.github.copilot.CopilotClient; import com.github.copilot.generated.AssistantMessageEvent; import com.github.copilot.generated.SessionUsageInfoEvent; -import com.github.copilot.rpc.CopilotClientOptions; import com.github.copilot.rpc.MessageOptions; import com.github.copilot.rpc.PermissionHandler; import com.github.copilot.rpc.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()) { // 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)) { + try (var client = new CopilotClient()) { client.start().get(); // Create a session @@ -160,6 +159,8 @@ This SDK tracks the official [Copilot SDK](https://github.com/github/copilot-sdk ### Development Setup +Requires JDK 25 or later for development. + ```bash # Clone the repository git clone https://github.com/github/copilot-sdk.git @@ -168,8 +169,12 @@ cd copilot-sdk/java # Enable git hooks for code formatting git config core.hooksPath .githooks -# Build and test +# Build and test with JDK 25 mvn clean verify + +# Set your paths for JDK 17 +# Run the JDK 25 built jar with JDK 17 JVM for tests. Do not re-compile the jar. +mvn jacoco:prepare-agent@wire-up-coverage-instrumentation antrun:run@print-test-jdk-banner surefire:test failsafe:integration-test failsafe:verify jacoco:report@build-coverage-report-from-tests -Denforcer.skip=true ``` The tests require the official [copilot-sdk](https://github.com/github/copilot-sdk) test harness, which is automatically cloned during build. @@ -190,6 +195,3 @@ See [SECURITY.md](SECURITY.md) for reporting security vulnerabilities. MIT — see [LICENSE](LICENSE) for details. -## Acknowledgement - -- Initially developed with Copilot and [Bruno Borges](https://www.linkedin.com/in/brunocborges/). diff --git a/java/pom.xml b/java/pom.xml index a21ae9013..e5d4592ee 100644 --- a/java/pom.xml +++ b/java/pom.xml @@ -7,7 +7,7 @@ com.github copilot-sdk-java - 1.0.0-beta-java.5-SNAPSHOT + 1.0.0-beta-10-java.0-SNAPSHOT jar GitHub Copilot SDK :: Java @@ -245,6 +245,18 @@ + + print-test-jdk-banner + process-test-classes + + run + + + + + + + @@ -307,6 +319,33 @@ + + + org.apache.maven.plugins + maven-failsafe-plugin + 3.5.5 + + + + integration-test + verify + + + + + + ${project.build.directory} + ${project.build.finalName} + ${project.build.testOutputDirectory} + + + org.apache.maven.plugins maven-surefire-plugin @@ -447,6 +486,10 @@ ${project.build.directory}/jacoco-test-results/sdk-tests.exec ${project.reporting.outputDirectory}/jacoco-coverage + + + META-INF/versions/**/*.class + @@ -489,6 +532,44 @@ true + + org.apache.maven.plugins + maven-enforcer-plugin + 3.5.0 + + + enforce-jdk25 + + enforce + + + + + [25,) + JDK 25+ is required to build the Multi-Release JAR with the virtual-thread overlay. + + + + + + verify-multi-release-overlay + verify + + enforce + + + + + + ${project.build.outputDirectory}/META-INF/versions/25/com/github/copilot/InternalExecutorProvider.class + + Multi-Release JAR overlay missing: META-INF/versions/25/com/github/copilot/InternalExecutorProvider.class was not compiled. Ensure the build runs on JDK 25+. + + + + + + @@ -507,6 +588,88 @@ -XX:+EnableDynamicAgentLoading + + java25-multi-release + + [25,) + + + + + org.apache.maven.plugins + maven-compiler-plugin + + + compile-java25 + compile + + compile + + + 25 + false + + ${project.basedir}/src/main/java25 + + true + + + + + + org.apache.maven.plugins + maven-jar-plugin + + + + true + + + + + + + org.apache.maven.plugins + maven-antrun-plugin + + + verify-java25-overlay + package + + run + + + + + + + + + +JDK 25 multi-release overlay class is missing from the packaged JAR. +Expected entry: META-INF/versions/25/com/github/copilot/InternalExecutorProvider.class +JAR: ${project.build.directory}/${project.build.finalName}.jar + +This usually means the 'java25-multi-release' Maven profile did not activate +(e.g. the build is running on a JDK older than 25) or maven-compiler-plugin +did not produce the multi-release output. Re-build on JDK 25+ and verify the +'compile-java25' execution ran during the 'compile' phase. + + + + + + + + + skip-test-harness diff --git a/java/src/main/java/com/github/copilot/CopilotClient.java b/java/src/main/java/com/github/copilot/CopilotClient.java index 471a4bcd7..36a3c78d1 100644 --- a/java/src/main/java/com/github/copilot/CopilotClient.java +++ b/java/src/main/java/com/github/copilot/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.ExecutorService; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.logging.Level; @@ -80,7 +81,24 @@ public final class CopilotClient implements AutoCloseable { */ public static final int AUTOCLOSEABLE_TIMEOUT_SECONDS = 10; private static final int FORCE_KILL_TIMEOUT_SECONDS = 10; + + /** + * One-shot dispatcher used to run the owned-executor shutdown off any caller + * thread that might itself belong to that executor (e.g. the + * {@link #forceStop()} continuation, which is chained off async work scheduled + * on the internal executor). Spawning a fresh daemon thread guarantees + * {@link java.util.concurrent.ExecutorService#awaitTermination(long, TimeUnit)} + * is never called from inside the very executor it is waiting on. + */ + private static final Executor SHUTDOWN_DISPATCHER = runnable -> { + Thread t = new Thread(runnable, "copilot-client-shutdown"); + t.setDaemon(true); + t.start(); + }; + private final CopilotClientOptions options; + private final Executor executor; + private final boolean executorCanBeShutdown; private final CliServerManager serverManager; private final LifecycleEventManager lifecycleManager = new LifecycleEventManager(); private final Map sessions = new ConcurrentHashMap<>(); @@ -168,6 +186,10 @@ public CopilotClient(CopilotClientOptions options) { this.optionsPort = null; } + InternalExecutorProvider executorProvider = new InternalExecutorProvider(this.options.getExecutor()); + this.executor = executorProvider.get(); + this.executorCanBeShutdown = executorProvider.canBeShutdown(); + this.serverManager = new CliServerManager(this.options); this.serverManager.setConnectionToken(this.effectiveConnectionToken); } @@ -191,11 +213,8 @@ public CompletableFuture start() { private CompletableFuture startCore() { LOG.fine("Starting Copilot client"); - Executor exec = options.getExecutor(); try { - return exec != null - ? CompletableFuture.supplyAsync(this::startCoreBody, exec) - : CompletableFuture.supplyAsync(this::startCoreBody); + return CompletableFuture.supplyAsync(this::startCoreBody, executor); } catch (RejectedExecutionException e) { return CompletableFuture.failedFuture(e); } @@ -224,8 +243,7 @@ private Connection startCoreBody() { Connection connection = new Connection(rpc, process, new ServerRpc(rpc::invoke)); // Register handlers for server-to-client calls - RpcHandlerDispatcher dispatcher = new RpcHandlerDispatcher(sessions, lifecycleManager::dispatch, - options.getExecutor()); + RpcHandlerDispatcher dispatcher = new RpcHandlerDispatcher(sessions, lifecycleManager::dispatch, executor); dispatcher.registerHandlers(rpc); // Verify protocol version @@ -323,7 +341,6 @@ private static boolean isUnsupportedConnectMethod(JsonRpcException ex) { */ public CompletableFuture stop() { var closeFutures = new ArrayList>(); - Executor exec = options.getExecutor(); for (CopilotSession session : new ArrayList<>(sessions.values())) { Runnable closeTask = () -> { @@ -335,9 +352,7 @@ public CompletableFuture stop() { }; CompletableFuture future; try { - future = exec != null - ? CompletableFuture.runAsync(closeTask, exec) - : CompletableFuture.runAsync(closeTask); + future = CompletableFuture.runAsync(closeTask, executor); } catch (RejectedExecutionException e) { LOG.log(Level.WARNING, "Executor rejected session close task; closing inline", e); closeTask.run(); @@ -359,7 +374,12 @@ public CompletableFuture stop() { public CompletableFuture forceStop() { disposed = true; sessions.clear(); - return cleanupConnection(); + // Dispatch the blocking shutdownOwnedExecutor() on a dedicated thread: + // cleanupConnection() is chained off async work running on the owned + // executor, so a plain whenComplete(...) here could land the awaitTermination + // call on one of the very threads it is waiting to drain, forcing the full + // AUTOCLOSEABLE_TIMEOUT_SECONDS timeout followed by shutdownNow(). + return cleanupConnection().whenCompleteAsync((ignored, error) -> shutdownOwnedExecutor(), SHUTDOWN_DISPATCHER); } private CompletableFuture cleanupConnection() { @@ -470,9 +490,7 @@ public CompletableFuture createSession(SessionConfig config) { java.util.function.Function initializeSession = sid -> { long setupNanos = System.nanoTime(); var s = new CopilotSession(sid, connection.rpc); - if (options.getExecutor() != null) { - s.setExecutor(options.getExecutor()); - } + s.setExecutor(executor); SessionRequestBuilder.configureSession(s, config); if (extracted.transformCallbacks() != null) { s.registerTransformCallbacks(extracted.transformCallbacks()); @@ -599,9 +617,7 @@ public CompletableFuture resumeSession(String sessionId, ResumeS // Register the session before the RPC call to avoid missing early events. long setupNanos = System.nanoTime(); var session = new CopilotSession(sessionId, connection.rpc); - if (options.getExecutor() != null) { - session.setExecutor(options.getExecutor()); - } + session.setExecutor(executor); SessionRequestBuilder.configureSession(session, config); sessions.put(sessionId, session); LoggingHelpers.logTiming(LOG, Level.FINE, @@ -1148,6 +1164,44 @@ public void close() { stop().get(AUTOCLOSEABLE_TIMEOUT_SECONDS, TimeUnit.SECONDS); } catch (Exception e) { LOG.log(Level.FINE, "Error during close", e); + } finally { + shutdownOwnedExecutor(); + } + } + + private void shutdownOwnedExecutor() { + if (!executorCanBeShutdown) { + return; + } + + ExecutorService serviceToShutdown = executor instanceof ExecutorService es ? es : null; + if (serviceToShutdown == null) { + LOG.log(Level.FINE, "Executor is not an ExecutorService; skipping shutdown"); + return; + } + + // Short-circuit when the owned executor is already shut down. close() and + // forceStop() can each call this method (e.g. forceStop() invoked before a + // subsequent close() in user code), and re-entering shutdown() + + // awaitTermination() + // is redundant. Logging at FINE aids diagnostics without spamming normal + // output. + if (serviceToShutdown.isShutdown()) { + LOG.log(Level.FINE, "Owned executor was already shut down; skipping redundant shutdown call."); + return; + } + + serviceToShutdown.shutdown(); + try { + if (!serviceToShutdown.awaitTermination(AUTOCLOSEABLE_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { + LOG.log(Level.FINE, "Owned executor did not terminate within {0} seconds; forcing shutdown.", + AUTOCLOSEABLE_TIMEOUT_SECONDS); + serviceToShutdown.shutdownNow(); + } + } catch (InterruptedException e) { + serviceToShutdown.shutdownNow(); + Thread.currentThread().interrupt(); + LOG.log(Level.FINE, "Interrupted while waiting for owned executor to terminate", e); } } diff --git a/java/src/main/java/com/github/copilot/InternalExecutorProvider.java b/java/src/main/java/com/github/copilot/InternalExecutorProvider.java new file mode 100644 index 000000000..284965513 --- /dev/null +++ b/java/src/main/java/com/github/copilot/InternalExecutorProvider.java @@ -0,0 +1,60 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot; + +import java.util.concurrent.Executor; +import java.util.concurrent.ForkJoinPool; + +/** + * Resolves the {@link Executor} used by {@link CopilotClient} for internal + * asynchronous work. + * + *

+ * This is the baseline (JDK 17+) implementation. When no + * user-provided executor is supplied, it falls back to + * {@link ForkJoinPool#commonPool()}, which is shared with the rest of the JVM + * and therefore never owned by the SDK. + * + *

+ * Multi-release JAR contract. This class has a sibling variant + * at {@code src/main/java25/com/github/copilot/InternalExecutorProvider.java} + * that is compiled with {@code --release 25} into {@code META-INF/versions/25/} + * and selected automatically by the JVM on JDK 25+. Any change to the + * package-private surface of this class + * ({@link #InternalExecutorProvider(Executor) constructor}, {@link #get()}, + * {@link #canBeShutdown()}) must be mirrored in both source + * trees. The two implementations must remain behaviourally + * interchangeable from the caller's perspective; only the default-executor + * strategy and ownership semantics differ. + * + * @implNote Maintainers: when editing this file, also edit + * {@code src/main/java25/com/github/copilot/InternalExecutorProvider.java}. + * The packaged JAR is verified at build time (see the + * {@code java25-multi-release} profile in {@code pom.xml}) to ensure + * the JDK 25 overlay is present. + */ +final class InternalExecutorProvider { + + private final Executor executor; + + InternalExecutorProvider(Executor userProvided) { + if (userProvided != null) { + this.executor = userProvided; + } else { + this.executor = ForkJoinPool.commonPool(); + } + } + + Executor get() { + return executor; + } + + boolean canBeShutdown() { + // Since we are using ForkJoinPool.commonPool() or user provided only, + // we should not attempt to shut it down + return false; + } + +} 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 69464aa72..941467059 100644 --- a/java/src/main/java/com/github/copilot/rpc/CopilotClientOptions.java +++ b/java/src/main/java/com/github/copilot/rpc/CopilotClientOptions.java @@ -288,9 +288,11 @@ public CopilotClientOptions setEnvironment(Map environment) { /** * Gets the executor used for internal asynchronous operations. + *

+ * Returns {@code null} if no executor has been explicitly set, indicating that + * the SDK should use its default executor strategy. * - * @return the executor, or {@code null} to use the default - * {@code ForkJoinPool.commonPool()} + * @return the executor, or {@code null} if using SDK defaults */ public Executor getExecutor() { return executor; @@ -300,15 +302,18 @@ public Executor getExecutor() { * 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. + * {@code CompletableFuture} combinators. 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. + * The SDK will not shut down a user-provided executor. If you pass a custom + * {@code ExecutorService}, you remain responsible for shutting it down. + *

+ * If not set (or set to {@code null}), the SDK uses its default executor: + * virtual threads on JDK 25+, {@code ForkJoinPool.commonPool()} on older JDKs. * * @param executor - * the executor to use, or {@code null} for the default + * the executor to use, or {@code null} for SDK defaults * @return this options instance for fluent chaining */ public CopilotClientOptions setExecutor(Executor executor) { diff --git a/java/src/main/java25/com/github/copilot/InternalExecutorProvider.java b/java/src/main/java25/com/github/copilot/InternalExecutorProvider.java new file mode 100644 index 000000000..10878bb0c --- /dev/null +++ b/java/src/main/java25/com/github/copilot/InternalExecutorProvider.java @@ -0,0 +1,65 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot; + +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.ForkJoinPool; + +/** + * Resolves the {@link Executor} used by {@link CopilotClient} for internal + * asynchronous work. + * + *

This is the JDK 25+ multi-release variant. It is + * compiled with {@code --release 25} into + * {@code META-INF/versions/25/com/github/copilot/InternalExecutorProvider.class} + * inside the packaged JAR and is automatically loaded in preference to the + * baseline class when the JVM runtime feature version is 25 or greater. + * When no user-provided executor is supplied, it creates an SDK-owned + * {@link Executors#newVirtualThreadPerTaskExecutor() virtual-thread executor} + * that is shut down by {@link CopilotClient#close()}. + * + *

Multi-release JAR contract. This class is the + * JDK 25 sibling of the baseline implementation at + * {@code src/main/java/com/github/copilot/InternalExecutorProvider.java}. + * The package-private surface of both classes + * ({@link #InternalExecutorProvider(Executor) constructor}, + * {@link #get()}, {@link #canBeShutdown()}) must be kept in + * lock-step; only the default-executor strategy and ownership + * semantics differ. + * + * @implNote + * Maintainers: when editing this file, also edit + * {@code src/main/java/com/github/copilot/InternalExecutorProvider.java}. + * The packaged JAR is verified at build time (see the + * {@code java25-multi-release} profile in {@code pom.xml}) to ensure this + * overlay class is present. + */ +final class InternalExecutorProvider { + + private final Executor executor; + private final boolean owned; + + InternalExecutorProvider(Executor userProvided) { + if (userProvided != null) { + this.executor = userProvided; + this.owned = false; + } else { + this.executor = Executors.newVirtualThreadPerTaskExecutor(); + this.owned = true; + } + } + + Executor get() { + return executor; + } + + boolean canBeShutdown() { + // We can only shut down the executor if we created it (i.e., if it's owned) + // such as when using Executors.newVirtualThreadPerTaskExecutor(), + // which creates an executor that we are responsible for shutting down. + return owned; + } +} diff --git a/java/src/test/java/com/github/copilot/InternalExecutorProviderIT.java b/java/src/test/java/com/github/copilot/InternalExecutorProviderIT.java new file mode 100644 index 000000000..1cc6b482e --- /dev/null +++ b/java/src/test/java/com/github/copilot/InternalExecutorProviderIT.java @@ -0,0 +1,108 @@ +/*--------------------------------------------------------------------------------------------- + * 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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.Test; + +/** + * Failsafe integration test that asserts the multi-release behaviour of + * {@link InternalExecutorProvider} against the actually packaged JAR. + *

+ * Runs after {@code package}, when {@code target/${finalName}.jar} exists with + * its real {@code Multi-Release: true} manifest and (on JDK 25+ builds) the + * {@code META-INF/versions/25/} override produced by {@code maven-jar-plugin}. + *

+ * The test spawns a child JVM with the packaged JAR plus {@code test-classes} + * on the classpath, runs {@link InternalExecutorProviderProbe}, and asserts + * that the executor selected for the current runtime matches expectations. + */ +class InternalExecutorProviderIT { + + @Test + void packagedJarSelectsExecutorPerRuntimeVersion() throws Exception { + Path packagedJar = locatePackagedJar(); + Path testClasses = locateTestClassesDir(); + String javaBin = locateJavaBinary(); + + String classpath = packagedJar.toString() + File.pathSeparator + testClasses.toString(); + Process process = new ProcessBuilder(javaBin, "-cp", classpath, + "com.github.copilot.InternalExecutorProviderProbe").redirectErrorStream(true).start(); + + String output; + try { + output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8); + assertTrue(process.waitFor(30, TimeUnit.SECONDS), "Probe JVM did not exit within 30s. Output:\n" + output); + } finally { + if (process.isAlive()) { + process.destroyForcibly(); + } + } + + assertEquals(0, process.exitValue(), "Probe exited non-zero. Output:\n" + output); + + Map kv = parseKeyValues(output); + String featureRaw = kv.get("feature"); + assertNotNull(featureRaw, "Probe did not report 'feature'. Output:\n" + output); + int feature = Integer.parseInt(featureRaw); + + boolean expectOwnedVirtual = feature >= 25; + assertEquals(String.valueOf(expectOwnedVirtual), kv.get("canBeShutdown"), + "canBeShutdown mismatch for JDK feature=" + feature + ". Output:\n" + output); + assertEquals(String.valueOf(expectOwnedVirtual), kv.get("virtual"), + "virtual mismatch for JDK feature=" + feature + ". Output:\n" + output); + } + + private static Path locatePackagedJar() { + String buildDir = System.getProperty("project.build.directory"); + String finalName = System.getProperty("project.build.finalName"); + assertNotNull(buildDir, "System property 'project.build.directory' must be set by failsafe"); + assertNotNull(finalName, "System property 'project.build.finalName' must be set by failsafe"); + Path jar = Path.of(buildDir, finalName + ".jar"); + assertTrue(Files.isRegularFile(jar), "Packaged JAR must exist: " + jar); + return jar; + } + + private static Path locateTestClassesDir() { + String testOutput = System.getProperty("project.build.testOutputDirectory"); + assertNotNull(testOutput, "System property 'project.build.testOutputDirectory' must be set by failsafe"); + Path dir = Path.of(testOutput); + assertTrue(Files.isDirectory(dir), "test-classes dir must exist: " + dir); + return dir; + } + + private static String locateJavaBinary() { + Path javaHome = Path.of(System.getProperty("java.home")); + Path candidate = javaHome.resolve("bin").resolve(isWindows() ? "java.exe" : "java"); + assertTrue(Files.isExecutable(candidate), "java binary must be executable: " + candidate); + return candidate.toString(); + } + + private static boolean isWindows() { + return System.getProperty("os.name", "").toLowerCase().contains("win"); + } + + private static Map parseKeyValues(String output) { + Map map = new HashMap<>(); + for (String line : output.split("\\R")) { + int eq = line.indexOf('='); + if (eq > 0) { + map.put(line.substring(0, eq).trim(), line.substring(eq + 1).trim()); + } + } + return map; + } +} diff --git a/java/src/test/java/com/github/copilot/InternalExecutorProviderProbe.java b/java/src/test/java/com/github/copilot/InternalExecutorProviderProbe.java new file mode 100644 index 000000000..85d12f14f --- /dev/null +++ b/java/src/test/java/com/github/copilot/InternalExecutorProviderProbe.java @@ -0,0 +1,74 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot; + +import java.lang.reflect.Method; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Diagnostic main launched as a separate JVM by + * {@code InternalExecutorProviderIT} to inspect the multi-release behaviour of + * {@link InternalExecutorProvider} against the actually packaged JAR. + *

+ * Lives in the same package as {@link InternalExecutorProvider} so it can use + * its package-private API directly, without reflection. + *

+ * Output format (key=value, one per line): + * + *

+ *   feature=<JDK feature version>
+ *   canBeShutdown=<true|false>
+ *   virtual=<true|false>
+ * 
+ */ +final class InternalExecutorProviderProbe { + + private InternalExecutorProviderProbe() { + } + + public static void main(String[] args) throws Exception { + InternalExecutorProvider provider = new InternalExecutorProvider(null); + Executor executor = provider.get(); + boolean canBeShutdown = provider.canBeShutdown(); + + AtomicBoolean virtual = new AtomicBoolean(); + CountDownLatch latch = new CountDownLatch(1); + executor.execute(() -> { + try { + virtual.set(isCurrentThreadVirtual()); + } finally { + latch.countDown(); + } + }); + + try { + if (!latch.await(5, TimeUnit.SECONDS)) { + System.out.println("error=task-timeout"); + System.exit(2); + } + } finally { + if (executor instanceof ExecutorService es) { + es.shutdownNow(); + } + } + + System.out.println("feature=" + Runtime.version().feature()); + System.out.println("canBeShutdown=" + canBeShutdown); + System.out.println("virtual=" + virtual.get()); + } + + private static boolean isCurrentThreadVirtual() { + try { + Method isVirtual = Thread.class.getMethod("isVirtual"); + return (Boolean) isVirtual.invoke(Thread.currentThread()); + } catch (ReflectiveOperationException e) { + return false; + } + } +} diff --git a/java/src/test/java/com/github/copilot/InternalExecutorProviderTest.java b/java/src/test/java/com/github/copilot/InternalExecutorProviderTest.java new file mode 100644 index 000000000..f1d854cb5 --- /dev/null +++ b/java/src/test/java/com/github/copilot/InternalExecutorProviderTest.java @@ -0,0 +1,55 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertSame; + +import java.lang.reflect.Modifier; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.ForkJoinPool; + +import org.junit.jupiter.api.Test; + +import com.github.copilot.rpc.CopilotClientOptions; + +class InternalExecutorProviderTest { + + @Test + void baseProviderReturnsCommonPool() { + Executor executor = new InternalExecutorProvider(null).get(); + + assertSame(ForkJoinPool.commonPool(), executor); + } + + @Test + void userProvidedExecutorIsNotOwned() { + Executor executor = ForkJoinPool.commonPool(); + + assertFalse(new InternalExecutorProvider(executor).canBeShutdown()); + } + + @Test + void providerIsPackagePrivate() { + assertFalse(Modifier.isPublic(InternalExecutorProvider.class.getModifiers())); + } + + @Test + void clientDoesNotShutDownUserProvidedExecutor() { + ExecutorService executor = Executors.newSingleThreadExecutor(); + try { + try (var client = new CopilotClient(new CopilotClientOptions().setAutoStart(false).setExecutor(executor))) { + assertNotNull(client); + } + + assertFalse(executor.isShutdown()); + } finally { + executor.shutdownNow(); + } + } +} diff --git a/scripts/docs-validation/package.json b/scripts/docs-validation/package.json index b802f0ba3..e2f40b8ee 100644 --- a/scripts/docs-validation/package.json +++ b/scripts/docs-validation/package.json @@ -6,11 +6,11 @@ "scripts": { "extract": "tsx extract.ts", "validate": "tsx validate.ts", - "validate:ts": "tsx validate.ts --lang typescript", - "validate:py": "tsx validate.ts --lang python", - "validate:go": "tsx validate.ts --lang go", - "validate:cs": "tsx validate.ts --lang csharp", - "validate:java": "tsx validate.ts --lang java" + "validate:ts": "tsx validate.ts --lang=typescript", + "validate:py": "tsx validate.ts --lang=python", + "validate:go": "tsx validate.ts --lang=go", + "validate:cs": "tsx validate.ts --lang=csharp", + "validate:java": "tsx validate.ts --lang=java" }, "dependencies": { "glob": "^11.0.0",