From 42f6c89b34dadfbfe6fa2840a7e16c8d1b9173bc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Aug 2026 19:20:25 +0000 Subject: [PATCH 1/7] Initial plan From a7880ac4885221f8ed616c60571c9a947c58bac5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Aug 2026 20:24:54 +0000 Subject: [PATCH 2/7] Achieve InProcess/out-of-process Java SDK test parity Co-authored-by: edburns <75821+edburns@users.noreply.github.com> --- java/sdk/pom.xml | 13 +++--- .../github/copilot/ClientOptionsE2ETest.java | 5 ++ .../com/github/copilot/CopilotClientTest.java | 9 ++++ .../com/github/copilot/E2ETestContext.java | 46 ++++++++++++++++--- .../github/copilot/PerSessionAuthTest.java | 3 ++ .../github/copilot/RpcServerMiscE2ETest.java | 3 ++ .../com/github/copilot/SlashCommandsIT.java | 3 ++ .../github/copilot/e2e/RequireInProcess.java | 2 +- .../com/github/copilot/e2e/SkipInProcess.java | 2 +- 9 files changed, 72 insertions(+), 14 deletions(-) diff --git a/java/sdk/pom.xml b/java/sdk/pom.xml index 9bfe76a4b8..7f11023d63 100644 --- a/java/sdk/pom.xml +++ b/java/sdk/pom.xml @@ -265,7 +265,7 @@ alphabetical - ${testExecutionAgentArgs} ${surefire.jvm.args} + ${testExecutionAgentArgs} ${surefire.jvm.args} --add-opens com.github.copilot.java/com.github.copilot.e2e=ALL-UNNAMED false + 0 + none + + inprocess + org.apache.maven.plugins maven-failsafe-plugin - 1 + 0 none - - **/InProcessTransportIT.java - ${copilot.inprocess.cli.path} inprocess diff --git a/java/sdk/src/test/java/com/github/copilot/ClientOptionsE2ETest.java b/java/sdk/src/test/java/com/github/copilot/ClientOptionsE2ETest.java index 45056afdb4..f6ff46a144 100644 --- a/java/sdk/src/test/java/com/github/copilot/ClientOptionsE2ETest.java +++ b/java/sdk/src/test/java/com/github/copilot/ClientOptionsE2ETest.java @@ -15,6 +15,8 @@ import org.junit.jupiter.api.Test; +import com.github.copilot.e2e.SkipInProcess; + import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.github.copilot.generated.rpc.SessionLimitsConfig; @@ -29,6 +31,7 @@ class ClientOptionsE2ETest { private static final ObjectMapper MAPPER = new ObjectMapper(); @Test + @SkipInProcess("Exercises direct CLI argument and working-directory forwarding to a spawned stdio subprocess") void testShouldForwardAdvancedSessionCreationOptionsToTheCli() throws Exception { try (var fake = FakeStdioCli.create()) { var workDir = fake.path("create-work"); @@ -95,6 +98,7 @@ void testShouldForwardAdvancedSessionCreationOptionsToTheCli() throws Exception } @Test + @SkipInProcess("Exercises direct CLI argument and working-directory forwarding to a spawned stdio subprocess") void testShouldForwardSingularProviderConfigurationOnSessionCreation() throws Exception { try (var fake = FakeStdioCli.create()) { try (var client = fake.createClient()) { @@ -123,6 +127,7 @@ void testShouldForwardSingularProviderConfigurationOnSessionCreation() throws Ex } @Test + @SkipInProcess("Exercises direct CLI argument and working-directory forwarding to a spawned stdio subprocess") void testShouldForwardAdvancedSessionResumeOptionsToTheCli() throws Exception { try (var fake = FakeStdioCli.create()) { var workDir = fake.path("resume-work"); diff --git a/java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java b/java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java index 067571df13..6cfc985fd5 100644 --- a/java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java +++ b/java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java @@ -7,6 +7,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import com.github.copilot.e2e.SkipInProcess; + import com.github.copilot.rpc.CopilotClientOptions; import com.github.copilot.rpc.PermissionHandler; import com.github.copilot.rpc.PingResponse; @@ -119,6 +121,7 @@ void testClientConstruction() { } @Test + @SkipInProcess("Constructs the client with CliPath explicitly, which the in-process transport does not accept") void testClientConstructionWithOptions() { var options = new CopilotClientOptions().setCliPath("/path/to/cli").setLogLevel("debug").setAutoStart(false); @@ -149,6 +152,7 @@ void testCliUrlOnlyConstruction() { } @Test + @SkipInProcess("Validates CliPath and CliUrl subprocess transport conflicts, which are not applicable in in-process mode") void testCliUrlMutualExclusionWithCliPath() { var options = new CopilotClientOptions().setCliUrl("localhost:3000").setCliPath("/path/to/cli"); @@ -156,6 +160,7 @@ void testCliUrlMutualExclusionWithCliPath() { } @Test + @SkipInProcess("Verifies spawning and talking to a child stdio process instead of the in-process runtime") void testStartAndConnectUsingStdio() throws Exception { assertNotNull(cliPath, "Copilot CLI not found in PATH or COPILOT_CLI_PATH"); @@ -173,6 +178,7 @@ void testStartAndConnectUsingStdio() throws Exception { } @Test + @SkipInProcess("Asserts child-process startup stderr reporting for invalid CLI arguments") void testShouldReportErrorWithStderrWhenCliFailsToStart() throws Exception { assertNotNull(cliPath, "Copilot CLI not found in PATH or COPILOT_CLI_PATH"); @@ -190,6 +196,7 @@ void testShouldReportErrorWithStderrWhenCliFailsToStart() throws Exception { } @Test + @SkipInProcess("Verifies spawning and talking to a child TCP process instead of the in-process runtime") void testStartAndConnectUsingTcp() throws Exception { assertNotNull(cliPath, "Copilot CLI not found in PATH or COPILOT_CLI_PATH"); @@ -205,6 +212,7 @@ void testStartAndConnectUsingTcp() throws Exception { } @Test + @SkipInProcess("Exercises lifecycle of a spawned CLI process selected via CliPath") void testForceStopWithoutCleanup() throws Exception { assertNotNull(cliPath, "Copilot CLI not found in PATH or COPILOT_CLI_PATH"); @@ -497,6 +505,7 @@ void testForceStopWithNoConnectionCompletes() throws Exception { } @Test + @SkipInProcess("Exercises session shutdown after stopping a spawned CLI process selected via CliPath") void testCloseSessionAfterStoppingClientDoesNotThrow() throws Exception { assertNotNull(cliPath, "Copilot CLI not found in PATH or COPILOT_CLI_PATH"); diff --git a/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java b/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java index f524b33dab..f22392f548 100644 --- a/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java +++ b/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java @@ -18,7 +18,10 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import com.github.copilot.ffi.InProcessEnvGuard; import com.github.copilot.rpc.CopilotClientOptions; +import com.github.copilot.rpc.InProcessRuntimeConnection; +import com.github.copilot.rpc.RuntimeConnection; /** * E2E test context that manages the test environment including the CapiProxy, @@ -71,6 +74,7 @@ public class E2ETestContext implements AutoCloseable { private String proxyUrl; private final CapiProxy proxy; private final Path repoRoot; + private final List inProcessEnvGuards = new ArrayList<>(); private Path currentSnapshotFile; private E2ETestContext(String cliPath, Path homeDir, Path workDir, String proxyUrl, CapiProxy proxy, @@ -322,9 +326,8 @@ public Map getEnvironment() { * @return a new CopilotClient */ public CopilotClient createClient() { - CopilotClientOptions options = new CopilotClientOptions().setCliPath(cliPath).setCwd(workDir.toString()) - .setEnvironment(getEnvironment()).setGitHubToken(DEFAULT_GITHUB_TOKEN); - + CopilotClientOptions options = new CopilotClientOptions().setGitHubToken(DEFAULT_GITHUB_TOKEN); + applyContextOptions(options); return new CopilotClient(options); } @@ -338,6 +341,19 @@ public CopilotClient createClient() { * @return a new CopilotClient */ public CopilotClient createClient(CopilotClientOptions options) { + applyContextOptions(options); + if (options.getGitHubToken() == null) { + options.setGitHubToken(DEFAULT_GITHUB_TOKEN); + } + + return new CopilotClient(options); + } + + private void applyContextOptions(CopilotClientOptions options) { + if (isInProcessMode(options)) { + inProcessEnvGuards.add(new InProcessEnvGuard(buildInProcessEnvironment(options))); + return; + } if (options.getCliPath() == null) { options.setCliPath(cliPath); } @@ -347,11 +363,26 @@ public CopilotClient createClient(CopilotClientOptions options) { if (options.getEnvironment() == null || options.getEnvironment().isEmpty()) { options.setEnvironment(getEnvironment()); } - if (options.getGitHubToken() == null) { - options.setGitHubToken(DEFAULT_GITHUB_TOKEN); + } + + private boolean isInProcessMode(CopilotClientOptions options) { + RuntimeConnection connection = options.getConnection(); + if (connection instanceof InProcessRuntimeConnection) { + return true; } + String defaultConnection = System.getenv("COPILOT_SDK_DEFAULT_CONNECTION"); + return defaultConnection != null && "inprocess".equalsIgnoreCase(defaultConnection.trim()); + } - return new CopilotClient(options); + private Map buildInProcessEnvironment(CopilotClientOptions options) { + Map env = new HashMap<>(getEnvironment()); + Map optionEnvironment = options.getEnvironment(); + if (optionEnvironment != null && !optionEnvironment.isEmpty()) { + env.putAll(optionEnvironment); + options.setEnvironment(null); + } + env.put("COPILOT_CLI_PATH", cliPath); + return env; } /** @@ -428,6 +459,9 @@ public void initializeProxy() throws IOException, InterruptedException { @Override public void close() throws Exception { + for (int i = inProcessEnvGuards.size() - 1; i >= 0; i--) { + inProcessEnvGuards.get(i).close(); + } proxy.stop(); // Clean up temp directories (best effort) diff --git a/java/sdk/src/test/java/com/github/copilot/PerSessionAuthTest.java b/java/sdk/src/test/java/com/github/copilot/PerSessionAuthTest.java index 9e5cd1b324..974536300c 100644 --- a/java/sdk/src/test/java/com/github/copilot/PerSessionAuthTest.java +++ b/java/sdk/src/test/java/com/github/copilot/PerSessionAuthTest.java @@ -13,6 +13,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import com.github.copilot.e2e.SkipInProcess; + import com.github.copilot.generated.rpc.SessionGitHubAuthGetStatusResult; import com.github.copilot.rpc.CopilotClientOptions; import com.github.copilot.rpc.PermissionHandler; @@ -110,6 +112,7 @@ void shouldIsolateAuthBetweenSessions() throws Exception { } @Test + @SkipInProcess("Builds a client with per-client environment, cwd, and logged-in-user overrides that the shared in-process runtime cannot isolate") void shouldBeUnauthenticatedWithoutToken() throws Exception { Map env = new HashMap<>(ctx.getEnvironment()); env.put("COPILOT_DEBUG_GITHUB_API_URL", ctx.getProxyUrl()); diff --git a/java/sdk/src/test/java/com/github/copilot/RpcServerMiscE2ETest.java b/java/sdk/src/test/java/com/github/copilot/RpcServerMiscE2ETest.java index 1db801d841..21dfb9c08f 100644 --- a/java/sdk/src/test/java/com/github/copilot/RpcServerMiscE2ETest.java +++ b/java/sdk/src/test/java/com/github/copilot/RpcServerMiscE2ETest.java @@ -14,6 +14,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import com.github.copilot.e2e.SkipInProcess; + import com.github.copilot.generated.rpc.AccountAllUsers; import com.github.copilot.generated.rpc.AccountLoginParams; import com.github.copilot.generated.rpc.AccountLogoutParams; @@ -70,6 +72,7 @@ void testShouldGetSetAndClearUserSettings() throws Exception { } @Test + @SkipInProcess("Builds a client with per-client environment and cwd overrides to test account login/logout flows") void testShouldLoginListGetCurrentAuthAndLogoutAccount() throws Exception { ctx.configureForTest("rpc_server_misc", "should_login_list_getcurrentauth_and_logout_account"); var token = "java-account-token"; diff --git a/java/sdk/src/test/java/com/github/copilot/SlashCommandsIT.java b/java/sdk/src/test/java/com/github/copilot/SlashCommandsIT.java index 634c0bad9b..5dec064644 100644 --- a/java/sdk/src/test/java/com/github/copilot/SlashCommandsIT.java +++ b/java/sdk/src/test/java/com/github/copilot/SlashCommandsIT.java @@ -22,6 +22,8 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import com.github.copilot.e2e.SkipInProcess; + import com.github.copilot.generated.rpc.SessionCommandsListResult; import com.github.copilot.generated.rpc.SessionCommandsInvokeParams; import com.github.copilot.generated.rpc.SlashCommandAgentPromptResult; @@ -41,6 +43,7 @@ * Requires the CLI to be installed and the user to be signed in. Uses * {@link TestUtil#findCliPath()} so the test harness binary is found in CI. */ +@SkipInProcess("Requires a live signed-in CLI subprocess and logged-in-user transport behavior rather than the replayed in-process harness") class SlashCommandsIT { private static CopilotClient client; diff --git a/java/sdk/src/test/java/com/github/copilot/e2e/RequireInProcess.java b/java/sdk/src/test/java/com/github/copilot/e2e/RequireInProcess.java index ef85f261ae..12de4e5b73 100644 --- a/java/sdk/src/test/java/com/github/copilot/e2e/RequireInProcess.java +++ b/java/sdk/src/test/java/com/github/copilot/e2e/RequireInProcess.java @@ -44,7 +44,7 @@ /** * JUnit 5 execution condition backing {@link RequireInProcess}. */ - final class Condition implements org.junit.jupiter.api.extension.ExecutionCondition { + public static final class Condition implements org.junit.jupiter.api.extension.ExecutionCondition { private static final String DEFAULT_CONNECTION_ENV_VAR = "COPILOT_SDK_DEFAULT_CONNECTION"; diff --git a/java/sdk/src/test/java/com/github/copilot/e2e/SkipInProcess.java b/java/sdk/src/test/java/com/github/copilot/e2e/SkipInProcess.java index b1a27ad025..3f626e133f 100644 --- a/java/sdk/src/test/java/com/github/copilot/e2e/SkipInProcess.java +++ b/java/sdk/src/test/java/com/github/copilot/e2e/SkipInProcess.java @@ -44,7 +44,7 @@ /** * JUnit 5 execution condition backing {@link SkipInProcess}. */ - final class Condition implements org.junit.jupiter.api.extension.ExecutionCondition { + public static final class Condition implements org.junit.jupiter.api.extension.ExecutionCondition { private static final String DEFAULT_CONNECTION_ENV_VAR = "COPILOT_SDK_DEFAULT_CONNECTION"; From f36371ac769752424a9b84e409559404758247db Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Wed, 5 Aug 2026 21:44:14 +0000 Subject: [PATCH 3/7] Address Copilot review findings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- java/sdk/pom.xml | 4 ++-- .../com/github/copilot/CopilotClient.java | 9 ++++++++ .../com/github/copilot/CopilotClientTest.java | 7 ++++++ .../com/github/copilot/E2ETestContext.java | 22 ++++++++++++++----- .../github/copilot/ExecutorWiringTest.java | 15 ++++++------- .../com/github/copilot/MetadataApiTest.java | 4 ++++ .../github/copilot/ffi/InProcessEnvGuard.java | 7 +++++- 7 files changed, 51 insertions(+), 17 deletions(-) diff --git a/java/sdk/pom.xml b/java/sdk/pom.xml index 7f11023d63..12cb42a3ee 100644 --- a/java/sdk/pom.xml +++ b/java/sdk/pom.xml @@ -617,7 +617,7 @@ did not produce the multi-release output. Re-build on JDK 25+ and verify the maven-surefire-plugin false - 0 + 1 none inprocess @@ -628,7 +628,7 @@ did not produce the multi-release output. Re-build on JDK 25+ and verify the org.apache.maven.plugins maven-failsafe-plugin - 0 + 1 none ${copilot.inprocess.cli.path} diff --git a/java/sdk/src/main/java/com/github/copilot/CopilotClient.java b/java/sdk/src/main/java/com/github/copilot/CopilotClient.java index d53be740cf..f420585be7 100644 --- a/java/sdk/src/main/java/com/github/copilot/CopilotClient.java +++ b/java/sdk/src/main/java/com/github/copilot/CopilotClient.java @@ -123,6 +123,7 @@ public final class CopilotClient implements AutoCloseable { private final Integer optionsPort; private final RuntimeConnection runtimeConnection; private final String effectiveConnectionToken; + private final Runnable closeHook; private volatile List modelsCache; private final Object modelsCacheLock = new Object(); @@ -142,7 +143,12 @@ public CopilotClient() { * if mutually exclusive options are provided */ public CopilotClient(CopilotClientOptions options) { + this(options, null); + } + + CopilotClient(CopilotClientOptions options, Runnable closeHook) { this.options = options != null ? options : new CopilotClientOptions(); + this.closeHook = closeHook; // Resolve the transport: an explicit RuntimeConnection wins; otherwise the // COPILOT_SDK_DEFAULT_CONNECTION env var, or the individual transport options. @@ -1663,6 +1669,9 @@ public void close() { LOG.log(Level.FINE, "Error during close", e); } finally { shutdownOwnedExecutor(); + if (closeHook != null) { + closeHook.run(); + } } } diff --git a/java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java b/java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java index 6cfc985fd5..7fad694558 100644 --- a/java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java +++ b/java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java @@ -131,6 +131,7 @@ void testClientConstructionWithOptions() { } @Test + @SkipInProcess("Validates external CLI URL transport options") void testCliUrlAutoCorrectsUseStdio() { var options = new CopilotClientOptions().setCliUrl("localhost:3000").setUseStdio(true); @@ -141,6 +142,7 @@ void testCliUrlAutoCorrectsUseStdio() { } @Test + @SkipInProcess("Validates external CLI URL transport options") void testCliUrlOnlyConstruction() { var options = new CopilotClientOptions().setCliUrl("localhost:4321"); @@ -289,6 +291,7 @@ void testTcpConnectionTokenWithUseStdioThrows() { } @Test + @SkipInProcess("Validates TCP transport options") void testTcpConnectionTokenAcceptedInTcpMode() { var options = new CopilotClientOptions().setUseStdio(false).setTcpConnectionToken("my-token"); @@ -409,6 +412,7 @@ void testOnLifecycleMultipleHandlers() throws Exception { // ===== getState() coverage ===== @Test + @SkipInProcess("Validates subprocess CLI path failure handling") void testGetStateErrorAfterFailedStart() throws Exception { // Use a non-existent CLI path to trigger a startup failure var options = new CopilotClientOptions().setCliPath("/nonexistent/path/to/cli").setAutoStart(false); @@ -429,6 +433,7 @@ void testGetStateErrorAfterFailedStart() throws Exception { } @Test + @SkipInProcess("Validates subprocess CLI path failure handling") void testGetStateConnectingDuringStart() throws Exception { // Use a non-existent CLI path; the future won't complete immediately var options = new CopilotClientOptions().setCliPath("/nonexistent/path/to/cli").setAutoStart(false); @@ -469,6 +474,7 @@ void testCloseIsIdempotent() { } @Test + @SkipInProcess("Validates subprocess CLI path failure handling") void testCloseAfterFailedStart() throws Exception { var options = new CopilotClientOptions().setCliPath("/nonexistent/path/to/cli").setAutoStart(false); var client = new CopilotClient(options); @@ -528,6 +534,7 @@ void testCloseSessionAfterStoppingClientDoesNotThrow() throws Exception { // ===== start() idempotency ===== @Test + @SkipInProcess("Validates subprocess CLI path failure handling") void testStartIsIdempotentSingleConnectionAttempt() throws Exception { var options = new CopilotClientOptions().setCliPath("/nonexistent/path/to/cli").setAutoStart(false); diff --git a/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java b/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java index f22392f548..f040607353 100644 --- a/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java +++ b/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java @@ -327,8 +327,7 @@ public Map getEnvironment() { */ public CopilotClient createClient() { CopilotClientOptions options = new CopilotClientOptions().setGitHubToken(DEFAULT_GITHUB_TOKEN); - applyContextOptions(options); - return new CopilotClient(options); + return createClient(options); } /** @@ -341,7 +340,10 @@ public CopilotClient createClient() { * @return a new CopilotClient */ public CopilotClient createClient(CopilotClientOptions options) { - applyContextOptions(options); + CopilotClient client = applyContextOptions(options); + if (client != null) { + return client; + } if (options.getGitHubToken() == null) { options.setGitHubToken(DEFAULT_GITHUB_TOKEN); } @@ -349,10 +351,17 @@ public CopilotClient createClient(CopilotClientOptions options) { return new CopilotClient(options); } - private void applyContextOptions(CopilotClientOptions options) { + private CopilotClient applyContextOptions(CopilotClientOptions options) { if (isInProcessMode(options)) { - inProcessEnvGuards.add(new InProcessEnvGuard(buildInProcessEnvironment(options))); - return; + InProcessEnvGuard guard = new InProcessEnvGuard(buildInProcessEnvironment(options)); + inProcessEnvGuards.add(guard); + try { + options.setEnvironment(null); + return new CopilotClient(options, guard::close); + } catch (RuntimeException e) { + guard.close(); + throw e; + } } if (options.getCliPath() == null) { options.setCliPath(cliPath); @@ -363,6 +372,7 @@ private void applyContextOptions(CopilotClientOptions options) { if (options.getEnvironment() == null || options.getEnvironment().isEmpty()) { options.setEnvironment(getEnvironment()); } + return null; } private boolean isInProcessMode(CopilotClientOptions options) { diff --git a/java/sdk/src/test/java/com/github/copilot/ExecutorWiringTest.java b/java/sdk/src/test/java/com/github/copilot/ExecutorWiringTest.java index 78764db0fb..a8319475c6 100644 --- a/java/sdk/src/test/java/com/github/copilot/ExecutorWiringTest.java +++ b/java/sdk/src/test/java/com/github/copilot/ExecutorWiringTest.java @@ -86,8 +86,7 @@ int getTaskCount() { } private CopilotClientOptions createOptionsWithExecutor(TrackingExecutor executor) { - CopilotClientOptions options = new CopilotClientOptions().setCliPath(ctx.getCliPath()) - .setCwd(ctx.getWorkDir().toString()).setEnvironment(ctx.getEnvironment()).setExecutor(executor) + CopilotClientOptions options = new CopilotClientOptions().setExecutor(executor) .setGitHubToken("fake-token-for-e2e-tests"); return options; } @@ -111,7 +110,7 @@ void testClientStartUsesProvidedExecutor() throws Exception { TrackingExecutor trackingExecutor = new TrackingExecutor(ForkJoinPool.commonPool()); int beforeStart = trackingExecutor.getTaskCount(); - try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + try (CopilotClient client = ctx.createClient(createOptionsWithExecutor(trackingExecutor))) { client.start().get(30, TimeUnit.SECONDS); assertTrue(trackingExecutor.getTaskCount() > beforeStart, @@ -156,7 +155,7 @@ void testToolCallDispatchUsesProvidedExecutor() throws Exception { }); // Reset count after client construction to isolate tool-call dispatch - try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + try (CopilotClient client = ctx.createClient(createOptionsWithExecutor(trackingExecutor))) { CopilotSession session = client.createSession(new SessionConfig().setTools(List.of(encryptTool)) .setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get(); @@ -198,7 +197,7 @@ void testPermissionDispatchUsesProvidedExecutor() throws Exception { var config = new SessionConfig().setOnPermissionRequest((request, invocation) -> CompletableFuture .completedFuture(new PermissionRequestResult().setKind(PermissionRequestResultKind.APPROVED))); - try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + try (CopilotClient client = ctx.createClient(createOptionsWithExecutor(trackingExecutor))) { CopilotSession session = client.createSession(config).get(); Path testFile = ctx.getWorkDir().resolve("test.txt"); @@ -247,7 +246,7 @@ void testUserInputDispatchUsesProvidedExecutor() throws Exception { .completedFuture(new UserInputResponse().setAnswer(answer).setWasFreeform(wasFreeform)); }); - try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + try (CopilotClient client = ctx.createClient(createOptionsWithExecutor(trackingExecutor))) { CopilotSession session = client.createSession(config).get(); int beforeSend = trackingExecutor.getTaskCount(); @@ -286,7 +285,7 @@ void testHooksDispatchUsesProvidedExecutor() throws Exception { .setHooks(new SessionHooks().setOnPreToolUse( (input, invocation) -> CompletableFuture.completedFuture(PreToolUseHookOutput.allow()))); - try (CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor))) { + try (CopilotClient client = ctx.createClient(createOptionsWithExecutor(trackingExecutor))) { CopilotSession session = client.createSession(config).get(); Path testFile = ctx.getWorkDir().resolve("hello.txt"); @@ -342,7 +341,7 @@ void testClientStopUsesProvidedExecutor() throws Exception { return CompletableFuture.completedFuture(input.toUpperCase()); }); - CopilotClient client = new CopilotClient(createOptionsWithExecutor(trackingExecutor)); + CopilotClient client = ctx.createClient(createOptionsWithExecutor(trackingExecutor)); client.createSession(new SessionConfig().setTools(List.of(encryptTool)) .setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get(); diff --git a/java/sdk/src/test/java/com/github/copilot/MetadataApiTest.java b/java/sdk/src/test/java/com/github/copilot/MetadataApiTest.java index ec3b9ea707..4f06176d93 100644 --- a/java/sdk/src/test/java/com/github/copilot/MetadataApiTest.java +++ b/java/sdk/src/test/java/com/github/copilot/MetadataApiTest.java @@ -10,6 +10,7 @@ import com.github.copilot.generated.rpc.ModelBillingTokenPrices; import com.github.copilot.generated.rpc.ModelBillingTokenPricesLongContext; import com.github.copilot.rpc.*; +import com.github.copilot.e2e.SkipInProcess; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -271,6 +272,7 @@ void testGetModelsResponseDeserialization() throws Exception { // ===== Integration Tests (require CLI) ===== @Test + @SkipInProcess("Uses explicit CLI stdio transport") void testGetStatus() throws Exception { assertNotNull(cliPath, "Copilot CLI not found in PATH or COPILOT_CLI_PATH"); @@ -287,6 +289,7 @@ void testGetStatus() throws Exception { } @Test + @SkipInProcess("Uses explicit CLI stdio transport") void testGetAuthStatus() throws Exception { assertNotNull(cliPath, "Copilot CLI not found in PATH or COPILOT_CLI_PATH"); @@ -302,6 +305,7 @@ void testGetAuthStatus() throws Exception { } @Test + @SkipInProcess("Uses explicit CLI stdio transport") void testListModels() throws Exception { assertNotNull(cliPath, "Copilot CLI not found in PATH or COPILOT_CLI_PATH"); diff --git a/java/sdk/src/test/java/com/github/copilot/ffi/InProcessEnvGuard.java b/java/sdk/src/test/java/com/github/copilot/ffi/InProcessEnvGuard.java index 12062ada6a..43df713715 100644 --- a/java/sdk/src/test/java/com/github/copilot/ffi/InProcessEnvGuard.java +++ b/java/sdk/src/test/java/com/github/copilot/ffi/InProcessEnvGuard.java @@ -83,6 +83,7 @@ private interface LibcEnv extends Library { * name -> previous value ({@code null} means the variable was not set before). */ private final List> saved = new ArrayList<>(); + private boolean closed; /** * Applies {@code applyEnv} to the native process environment block, saving the @@ -116,7 +117,11 @@ private void apply(String name, String value) { * before construction. */ @Override - public void close() { + public synchronized void close() { + if (closed) { + return; + } + closed = true; List> reversed = new ArrayList<>(saved); Collections.reverse(reversed); for (Map.Entry entry : reversed) { From f22b4afb41a9fda074cf5912aca2e5c6d9d35787 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Fri, 7 Aug 2026 01:03:04 +0000 Subject: [PATCH 4/7] Satisfy necessary-and-sufficient runtime artifact invariant Bundle both runtime.node and the copilot CLI executable in the classifier JAR. The Rust embedded_host.rs spawns the CLI as a child process via host_start argv[0], so runtime.node alone is insufficient. Changes: - fetch-native.mjs: also extract package/copilot from npm tarball - NativeRuntimeLoader: add resolveEntrypoint() that finds copilot CLI alongside the resolved runtime.node; extractToCache also extracts CLI - CopilotClient: simplify resolveInProcessEntrypoint() to use only the bundled artifact (no COPILOT_CLI_PATH, no PATH search) - E2ETestContext: remove COPILOT_CLI_PATH from InProcess env (unused now) - Add diagnostic spike and yolo prompt for context Previously ExecutorWiringTest failed with FileNotFoundException and AskUserTest hung indefinitely. Both now pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d7a38160-a65e-4d0c-9087-4f28da2a51b8 --- ...ed-cli-runtime-ignorance-reduction-plan.md | 4 +- ...isfy-necessary-and-sufficient-invariant.md | 118 +++++++ .../README.md | 92 +++++ .../dependency-reduced-pom.xml | 66 ++++ .../pom.xml | 96 +++++ .../copilot/spike/CopilotRuntimeLibrary.java | 57 +++ .../copilot/spike/RealRuntimeSpikeMain.java | 328 ++++++++++++++++++ java/copilot-native/scripts/fetch-native.mjs | 17 +- .../com/github/copilot/CopilotClient.java | 21 +- .../copilot/ffi/NativeRuntimeLoader.java | 74 ++++ .../com/github/copilot/E2ETestContext.java | 1 - 11 files changed, 854 insertions(+), 20 deletions(-) create mode 100644 1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-satisfy-necessary-and-sufficient-invariant.md create mode 100644 1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/README.md create mode 100644 1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/dependency-reduced-pom.xml create mode 100644 1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/pom.xml create mode 100644 1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/src/main/java/com/github/copilot/spike/CopilotRuntimeLibrary.java create mode 100644 1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/src/main/java/com/github/copilot/spike/RealRuntimeSpikeMain.java diff --git a/1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md b/1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md index 66996196ac..011b70e233 100644 --- a/1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md +++ b/1917-java-embed-rust-cli-runtime-remove-before-merge/1917-embed-cli-runtime-ignorance-reduction-plan.md @@ -135,7 +135,9 @@ The .NET PR uses MSBuild targets to copy `runtime.node` from `runtimes//nat The `package.json`-as-dependency-manifest approach was ruled out by experiment: `npm install` returns `EBADPLATFORM` for cross-platform packages, and `npm install --force` disables all npm safety checks. `npm pack` downloads the tarball without any platform check and does not require `--force`. -Long-term target shape: the `copilot-native` module's `generate-resources` phase runs `npm pack @github/copilot-@${project.version}` for each supported platform. This produces `.tgz` tarballs, which are then extracted with `tar` to stage the `runtime.node` binary at `target/native-staging//native//runtime.node`. The version comes from `${project.version}` — the SDK and npm package versions are identical, so no separate version property is needed. +Long-term target shape: the `copilot-native` module's `generate-resources` phase runs `npm pack @github/copilot-@${project.version}` for each supported platform. This produces `.tgz` tarballs, which are then extracted with `tar` to stage **both** the `runtime.node` shared library and the `copilot` CLI executable at `target/native-staging//native//`. The version comes from `${project.version}` — the SDK and npm package versions are identical, so no separate version property is needed. + +**Necessary-and-sufficient runtime artifact invariant:** The classifier JAR must contain both `native//runtime.node` (the cdylib loaded via JNA) **and** `native//copilot` (the CLI executable passed as `argv[0]` to `copilot_runtime_host_start`). The Rust `embedded_host.rs` spawns the CLI as a child process to service TypeScript method bodies not yet ported to Rust. Without the CLI executable, `host_start` fails — the classifier JAR is not self-sufficient. Both artifacts ship together in the same `@github/copilot-` npm package; both must be extracted and bundled. This matches the .NET SDK, which bundles the CLI binary and cdylib together under `runtimes//native/`. When the TypeScript migration completes and `embedded_host.rs` no longer spawns a child process, the CLI executable can be removed from the classifier JAR. Temporary invariant (`linux-x64` only for now): perform this only for `linux-x64` on Ubuntu 24.04 in this phase; all other platform packaging is deferred to a later phase. diff --git a/1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-satisfy-necessary-and-sufficient-invariant.md b/1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-satisfy-necessary-and-sufficient-invariant.md new file mode 100644 index 0000000000..eef2373eb9 --- /dev/null +++ b/1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-satisfy-necessary-and-sufficient-invariant.md @@ -0,0 +1,118 @@ +# Prompt: Satisfy the necessary-and-sufficient runtime artifact invariant + +## Goal + +When `cd java && mvn clean verify -Pinprocess` is invoked, all tests pass cleanly. Currently the InProcess tests hang or fail because `copilot_runtime_host_start` cannot find the copilot CLI executable to spawn as a child process. + +## Branch + +Work on branch `edburns/review-copilot-pr-2272` in `copilot-sdk-01`. The current HEAD is `f36371ac`. + +## Background + +The InProcess transport loads `runtime.node` (a Rust cdylib) via JNA and calls `copilot_runtime_host_start(argv_json, env_json)`. Internally, the Rust code in `embedded_host.rs` uses `argv[0]` from `argv_json` as the program in `Command::new(program)` to spawn a child process — the copilot CLI binary that services TypeScript method bodies not yet ported to Rust. + +Today the classifier JAR (`copilot-sdk-java-runtime-*-linux-x64.jar`) only contains `native/linux-x64/runtime.node`. The copilot CLI executable is **not** included, even though it ships in the same `@github/copilot-linux-x64` npm tarball at path `package/copilot`. + +The `resolveInProcessEntrypoint()` method in `CopilotClient.java` tries to find the copilot CLI via `COPILOT_CLI_PATH` env, `options.getCliPath()`, or PATH — all independent of where `runtime.node` was resolved. This is wrong. The copilot CLI must come from the **same** package as `runtime.node` to avoid version skew. + +## The necessary-and-sufficient runtime artifact invariant + +The classifier JAR must contain **both**: +- `native//runtime.node` — the cdylib loaded via JNA +- `native//copilot` — the CLI executable passed as `argv[0]` to `host_start` + +These two files must come from the same `@github/copilot-` npm package version. This matches how the .NET SDK bundles both under `runtimes//native/`. + +## Changes required + +### 1. `java/copilot-native/scripts/fetch-native.mjs` — also extract the copilot binary + +Currently the script extracts only `package/prebuilds//runtime.node` from the npm tarball. It must **also** extract `package/copilot` and stage it at `target/native-staging//native//copilot`. + +After extraction, set the executable permission on the copilot binary (`chmod +x` or `fs.chmodSync(..., 0o755)`). + +The tarball paths are: +- `package/prebuilds//runtime.node` → `//native//runtime.node` (already done) +- `package/copilot` → `//native//copilot` (NEW) + +On Windows the binary is named `copilot.exe` and lives at `package/copilot.exe` in the tarball. + +### 2. `java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java` — add method to resolve the copilot CLI from the same location as runtime.node + +Add a new public method `resolveEntrypoint()` that returns the path to the copilot CLI executable. The logic is: + +1. Call `resolve()` to get the path to `runtime.node` (e.g. `~/.copilot/runtime-cache//linux-x64/runtime.node`) +2. Look for `copilot` (or `copilot.exe` on Windows) in the **same directory** as the resolved `runtime.node` +3. If found and is a regular file, return it +4. If not found, throw `IOException` with a clear message + +When `resolve()` extracts `runtime.node` from the classpath to the cache directory, it must **also** extract `native//copilot` to the same cache directory. Update the extraction logic in `resolve()` (or `resolveFromClasspathOrBundledCli`) to extract the copilot binary alongside `runtime.node`. The copilot binary is a classpath resource at `native//copilot`. + +After extraction, ensure the copilot binary has executable permission (`Files.setPosixFilePermissions` or similar, guarded for non-POSIX systems). + +### 3. `java/sdk/src/main/java/com/github/copilot/CopilotClient.java` — simplify `resolveInProcessEntrypoint()` + +Replace the current three-step independent resolution with: + +```java +private static String resolveInProcessEntrypoint(CopilotClientOptions options) throws IOException { + return NativeRuntimeLoader.resolveEntrypoint().toString(); +} +``` + +The copilot CLI is always derived from the same location as `runtime.node`. There are no other loading mechanisms. No `COPILOT_CLI_PATH` check. No `options.getCliPath()` check. No PATH search. The InProcess entrypoint comes from the bundled classifier JAR, period. + +If the user has NOT configured `RuntimeConnection.forInProcess()`, this method is never called — the SDK falls back to the existing subprocess transport via `CliServerManager`, which uses `COPILOT_CLI_PATH` / PATH as before. That fallback path is unchanged. + +### 4. `java/sdk/src/test/java/com/github/copilot/E2ETestContext.java` — simplify InProcess test setup + +In `applyContextOptions()`, the InProcess branch currently creates an `InProcessEnvGuard` that sets `COPILOT_CLI_PATH` in the native env. This is no longer needed because `resolveInProcessEntrypoint` no longer reads `COPILOT_CLI_PATH`. + +The `InProcessEnvGuard` is still needed for other env vars (`COPILOT_API_URL`, `GITHUB_TOKEN`, etc.) that the Rust runtime reads from the process environment. But `COPILOT_CLI_PATH` should be removed from `buildInProcessEnvironment()`. + +In `buildInProcessEnvironment()`, remove the line: +```java +env.put("COPILOT_CLI_PATH", cliPath); +``` + +### 5. Verify the classifier JAR contents + +After `mvn clean package -pl copilot-native`, the classifier JAR at `java/copilot-native/target/copilot-sdk-java-runtime-*-linux-x64.jar` must contain: +``` +native/linux-x64/runtime.node +native/linux-x64/copilot +native/linux-x64/platform.properties +``` + +### 6. Verify tests pass + +Run `cd java && mvn clean verify -Pinprocess` and confirm all tests pass. The `-Pinprocess` profile activates the `copilot-native` module build and sets `COPILOT_SDK_DEFAULT_CONNECTION=inprocess` for the E2E tests. + +## What NOT to change + +- Do NOT change the subprocess transport path (`CliServerManager`, `TcpRuntimeConnection`, `StdioRuntimeConnection`). Those paths continue to use `COPILOT_CLI_PATH` / PATH / `options.getCliPath()` as before. +- Do NOT add `COPILOT_CLI_PATH` as a resolution mechanism for the InProcess entrypoint. InProcess uses only the bundled artifact. +- Do NOT change the `RuntimeConnection.forInProcess()` API or `InProcessRuntimeConnection` class. +- Do NOT change the Rust code in `copilot-agent-runtime`. +- Do NOT change any code outside the `java/` directory except this prompt file. + +## Key file locations + +| File | Purpose | +|------|---------| +| `java/copilot-native/scripts/fetch-native.mjs` | Downloads and extracts native binaries from npm | +| `java/copilot-native/pom.xml` | Builds the classifier JAR | +| `java/sdk/src/main/java/com/github/copilot/CopilotClient.java` | `resolveInProcessEntrypoint()` at ~line 481 | +| `java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java` | `resolve()` and `findRuntimeOnPath()` | +| `java/sdk/src/main/java/com/github/copilot/ffi/FfiRuntimeHost.java` | `start()` calls `buildArgvJson(entrypointPath, ...)` | +| `java/sdk/src/test/java/com/github/copilot/E2ETestContext.java` | `buildInProcessEnvironment()` and `applyContextOptions()` | +| `java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java` | Unit tests for NativeRuntimeLoader | + +## Verification command + +```bash +cd java && mvn clean verify -Pinprocess +``` + +All tests must pass. No hangs. No timeouts. diff --git a/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/README.md b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/README.md new file mode 100644 index 0000000000..e8da02f39e --- /dev/null +++ b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/README.md @@ -0,0 +1,92 @@ +# Spike — Validate Callback Flow with Real runtime.node + +**Question:** Why does `copilot_runtime_host_start` hang (or return 0) when +called from the Java SDK's InProcess transport during E2E tests? + +**Context:** In the shepherd-task run for PR #2272 (issue #2271), CCA's +60-minute session budget expired while validating its work. The Phase 2 local +validation also hung on `AskUserTest.testShouldReceiveChoicesInUserInputRequest` +— the `main` thread was blocked on `CompletableFuture.get()` waiting for the +`FfiRuntimeHost` to start, while the `copilot-ffi-host-start` thread was stuck +inside the JNA native call to `copilot_runtime_host_start`. + +The Rust side (`embedded_host.rs`) has **zero logging**, and the Java side +(`FfiRuntimeHost.runHostStartOnBlockingThread`) calls `future.get()` with **no +timeout**. This spike isolates the exact native call with full instrumentation. + +## What this spike does + +1. **Downloads the real `runtime.node`** using the same `fetch-native.mjs` from + the `copilot-native` module (pinned version from `nodejs/package-lock.json`). +2. **Loads `runtime.node` via JNA** and calls the real C ABI entry points. +3. **Adds diagnostic logging** with timestamps and thread IDs before/after every + native call. +4. **Adds a 60-second timeout** on `host_start` (the Rust side has a 30 s + `READY_TIMEOUT` internally, so 60 s gives headroom). +5. **Dumps relevant threads** on timeout to diagnose where the hang occurs. + +## Prerequisites + +- JDK 17+ +- Maven 3.9+ +- Node.js and npm (for fetching `runtime.node`) +- Must be run from within the `copilot-sdk` monorepo (needs + `nodejs/package-lock.json` for version pinning) + +## Build + +```sh +cd spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node +mvn clean package +``` + +This downloads `runtime.node` during `generate-resources` and produces an +executable uber-jar. + +## Run + +```sh +java -jar target/real-runtime-callback-spike-0.1.0.jar +``` + +Or with an explicit runtime.node path: + +```sh +java -jar target/real-runtime-callback-spike-0.1.0.jar /path/to/runtime.node +``` + +## Expected outcomes + +### Happy path (host_start succeeds) +``` +[STEP 3] host_start completed in ms, serverHandle= +[STEP 4] connection_open returned connHandle= +``` + +### Timeout (host_start hangs) +``` +[STEP 3] TIMEOUT after 60000 ms waiting for host_start! +--- Thread dump (relevant threads) --- +``` +The thread dump will show where `copilot_runtime_host_start` is stuck: +- If `spike-host-start` is in JNA's `invokeInt` → the native call hasn't returned +- This maps to the Rust `embedded_host::start()` function which: + 1. Spawns a child via `spawn_and_serve_background(command)` + 2. Waits on a `Condvar` for up to 30 s (`READY_TIMEOUT`) + 3. The child must call `notify_ready(server_id)` to unblock + +### Failure (host_start returns 0) +``` +[STEP 3] host_start returned 0 (failure) +``` +This means the Rust side explicitly returned 0. Possible causes: +- argv_json parse failure +- Child process spawn failure +- 30 s readiness timeout elapsed (child never called `notify_ready`) + +## Relationship to spike-3-4 + +This spike is structurally based on spike-3-4 (JNA callback threading) but +replaces the toy Rust DLL with the real `runtime.node` binary. The callback +instrumentation pattern (AtomicInteger tracking, thread-ID logging) is carried +over from spike-3-4. diff --git a/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/dependency-reduced-pom.xml b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/dependency-reduced-pom.xml new file mode 100644 index 0000000000..ee5eb8550b --- /dev/null +++ b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/dependency-reduced-pom.xml @@ -0,0 +1,66 @@ + + + 4.0.0 + com.github.copilot.spike + real-runtime-callback-spike + Spike — Validate Callback Flow with Real runtime.node + 0.1.0 + Minimal program to debug the InProcess FFI flow against the real + runtime.node binary. Calls copilot_runtime_host_start, connection_open, + and connection_close with full diagnostic logging and timeouts. + + + + org.codehaus.mojo + exec-maven-plugin + 3.5.0 + + + fetch-native-linux-x64 + generate-resources + + exec + + + node + + ${copilot.sdk.root}/java/copilot-native/scripts/fetch-native.mjs + ${copilot.sdk.root} + ${copilot.native.staging} + ${copilot.native.classifier} + + + + + + + maven-shade-plugin + 3.6.0 + + + package + + shade + + + + + com.github.copilot.spike.RealRuntimeSpikeMain + + + + + + + + + + ${project.build.directory}/native-staging + 17 + ${project.basedir}/../.. + 17 + UTF-8 + 5.19.1 + linux-x64 + + diff --git a/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/pom.xml b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/pom.xml new file mode 100644 index 0000000000..aba201a1f0 --- /dev/null +++ b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/pom.xml @@ -0,0 +1,96 @@ + + + 4.0.0 + + com.github.copilot.spike + real-runtime-callback-spike + 0.1.0 + jar + + Spike — Validate Callback Flow with Real runtime.node + + Minimal program to debug the InProcess FFI flow against the real + runtime.node binary. Calls copilot_runtime_host_start, connection_open, + and connection_close with full diagnostic logging and timeouts. + + + + 17 + 17 + UTF-8 + 5.19.1 + + ${project.basedir}/../.. + linux-x64 + ${project.build.directory}/native-staging + + + + + net.java.dev.jna + jna + ${jna.version} + + + + + + + + org.codehaus.mojo + exec-maven-plugin + 3.5.0 + + + fetch-native-linux-x64 + generate-resources + + exec + + + node + + ${copilot.sdk.root}/java/copilot-native/scripts/fetch-native.mjs + ${copilot.sdk.root} + ${copilot.native.staging} + ${copilot.native.classifier} + + + + + + + + + org.apache.maven.plugins + maven-shade-plugin + 3.6.0 + + + package + + shade + + + + + com.github.copilot.spike.RealRuntimeSpikeMain + + + + + + + + + diff --git a/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/src/main/java/com/github/copilot/spike/CopilotRuntimeLibrary.java b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/src/main/java/com/github/copilot/spike/CopilotRuntimeLibrary.java new file mode 100644 index 0000000000..622962d711 --- /dev/null +++ b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/src/main/java/com/github/copilot/spike/CopilotRuntimeLibrary.java @@ -0,0 +1,57 @@ +package com.github.copilot.spike; + +import com.sun.jna.Callback; +import com.sun.jna.Library; +import com.sun.jna.Pointer; + +/** + * JNA interface mapping the real {@code runtime.node} C ABI exports. + * + *

Function signatures match {@code cabi.rs} in copilot-agent-runtime. + */ +public interface CopilotRuntimeLibrary extends Library { + + /** + * {@code copilot_runtime_host_start} — spawns the embedded Node child and + * blocks until it reports readiness (up to ~30 s on the Rust side). + * + * @return server handle ({@code 0} on failure or timeout) + */ + int copilot_runtime_host_start(byte[] argvJson, int argvJsonLen, + byte[] envJson, int envJsonLen); + + /** + * {@code copilot_runtime_host_shutdown} — tears down the embedded host. + * Returns {@code byte} (not boolean) because the Rust ABI exports a + * one-byte bool. + */ + byte copilot_runtime_host_shutdown(int serverId); + + /** + * {@code copilot_runtime_connection_open} — opens a bidirectional + * connection and registers the outbound callback. + */ + int copilot_runtime_connection_open(int serverId, OutboundCallback callback, + Pointer userData, byte[] extSource, + int extSourceLen, byte[] extName, + int extNameLen, byte[] connToken, + int connTokenLen); + + /** + * {@code copilot_runtime_connection_write} — writes a JSON-RPC frame. + */ + byte copilot_runtime_connection_write(int connectionId, byte[] data, + int dataLen); + + /** + * {@code copilot_runtime_connection_close} — closes a connection. + */ + byte copilot_runtime_connection_close(int connectionId); + + /** + * Outbound callback: Rust → Java data delivery on a native thread. + */ + interface OutboundCallback extends Callback { + void invoke(Pointer userData, Pointer data, int len); + } +} diff --git a/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/src/main/java/com/github/copilot/spike/RealRuntimeSpikeMain.java b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/src/main/java/com/github/copilot/spike/RealRuntimeSpikeMain.java new file mode 100644 index 0000000000..89a8e30651 --- /dev/null +++ b/1917-java-embed-rust-cli-runtime-remove-before-merge/spike-post-agentic-01-test-parity-validate-callback-flow-with-real-runtime_node/src/main/java/com/github/copilot/spike/RealRuntimeSpikeMain.java @@ -0,0 +1,328 @@ +package com.github.copilot.spike; + +import com.sun.jna.Native; +import com.sun.jna.Pointer; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.ConsoleHandler; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.logging.SimpleFormatter; + +/** + * Spike — validate the InProcess callback flow against the real runtime.node. + * + *

This program isolates and instruments the exact code path that hangs in the + * SDK's E2E tests: + *

    + *
  1. Load {@code runtime.node} via JNA
    2. + *
  2. Call {@code copilot_runtime_host_start} with a bounded timeout
    3. + *
  3. If successful, call {@code copilot_runtime_connection_open} with a + * diagnostic callback
    4. + *
  4. Clean up: {@code connection_close} → {@code host_shutdown}
    5. + *
+ * + *

Usage: + * 

+ * # Build:
+ * mvn clean package -q
+ *
+ * # Run (runtime.node path from the native-staging directory):
+ * java -jar target/real-runtime-callback-spike-0.1.0.jar \
+ *     target/native-staging/linux-x64/native/linux-x64/runtime.node
+ *
+ * # Or supply the path to any runtime.node on disk:
+ * java -jar target/real-runtime-callback-spike-0.1.0.jar /path/to/runtime.node
+ *
+ */ +public class RealRuntimeSpikeMain { + + private static final Logger LOG = Logger.getLogger(RealRuntimeSpikeMain.class.getName()); + + /** Timeout for host_start (the Rust side has a 30 s READY_TIMEOUT internally). */ + private static final int HOST_START_TIMEOUT_SECONDS = 60; + + public static void main(String[] args) throws Exception { + configureLogging(); + + // --- Resolve runtime.node path (the shared library loaded via JNA) --- + String runtimePath; + if (args.length > 0) { + runtimePath = args[0]; + } else { + // Default: look in the native-staging directory populated by the POM + runtimePath = "target/native-staging/linux-x64/native/linux-x64/runtime.node"; + } + + // --- Resolve copilot CLI path (the executable spawned as a child by host_start) --- + // runtime.node is a shared library (cdylib), NOT an executable. + // host_start's argv must point to the copilot CLI binary, which gets + // --embedded-host and connects back via napi-oop. + String copilotCliPath; + if (args.length > 1) { + copilotCliPath = args[1]; + } else { + // Default: look for the copilot binary next to runtime.node or in + // the npm package location + Path runtimeDir = Path.of(runtimePath).toAbsolutePath().normalize().getParent(); + Path candidate = runtimeDir.resolve("copilot"); + if (!Files.exists(candidate)) { + // Try the npm package layout: nodejs/node_modules/@github/copilot-linux-x64/copilot + // relative to the monorepo root + Path repoRoot = Path.of("../..").toAbsolutePath().normalize(); + candidate = repoRoot.resolve("nodejs/node_modules/@github/copilot-linux-x64/copilot"); + } + copilotCliPath = candidate.toString(); + } + + Path runtimeFile = Path.of(runtimePath).toAbsolutePath().normalize(); + Path copilotCliFile = Path.of(copilotCliPath).toAbsolutePath().normalize(); + LOG.info("=== Spike: Validate Callback Flow with Real runtime.node ==="); + LOG.info("runtime.node path (JNA library): " + runtimeFile); + LOG.info(" File exists: " + Files.exists(runtimeFile)); + if (Files.exists(runtimeFile)) { + LOG.info(" File size: " + Files.size(runtimeFile) + " bytes"); + } + LOG.info("copilot CLI path (host_start argv[0]): " + copilotCliFile); + LOG.info(" File exists: " + Files.exists(copilotCliFile)); + if (Files.exists(copilotCliFile)) { + LOG.info(" File size: " + Files.size(copilotCliFile) + " bytes"); + LOG.info(" Executable: " + Files.isExecutable(copilotCliFile)); + } + LOG.info("Main thread: " + Thread.currentThread().getName() + + " (id=" + Thread.currentThread().threadId() + ")"); + LOG.info("java.version: " + System.getProperty("java.version")); + LOG.info("os.name: " + System.getProperty("os.name")); + LOG.info("os.arch: " + System.getProperty("os.arch")); + + if (!Files.exists(runtimeFile)) { + LOG.severe("runtime.node not found at " + runtimeFile); + LOG.severe("Run 'mvn generate-resources' first, or pass the path as an argument."); + System.exit(1); + } + if (!Files.exists(copilotCliFile)) { + LOG.severe("copilot CLI not found at " + copilotCliFile); + LOG.severe("Ensure 'npm ci' has been run in the nodejs/ directory, or pass the path as the second argument."); + System.exit(1); + } + + // --- Load the real runtime.node via JNA --- + LOG.info("[STEP 1] Loading runtime.node via JNA..."); + long loadStart = System.nanoTime(); + CopilotRuntimeLibrary lib; + try { + lib = Native.load(runtimeFile.toString(), CopilotRuntimeLibrary.class); + } catch (UnsatisfiedLinkError e) { + LOG.severe("[STEP 1] FAILED to load runtime.node: " + e.getMessage()); + e.printStackTrace(); + System.exit(1); + return; + } + long loadElapsed = (System.nanoTime() - loadStart) / 1_000_000; + LOG.info("[STEP 1] runtime.node loaded successfully in " + loadElapsed + " ms"); + + // --- Build argv_json (same as FfiRuntimeHost.buildArgvJson) --- + // argv[0] is the copilot CLI executable, NOT runtime.node. + // The Rust embedded_host::start() does Command::new(argv[0]) to spawn + // the child that connects back via napi-oop. + String entrypoint = copilotCliFile.toString(); + String argvJson = "[\"" + escapeJson(entrypoint) + "\"," + + "\"--embedded-host\"," + + "\"--no-auto-update\"," + + "\"--log-level\",\"info\"," + + "\"--no-auto-login\"]"; + byte[] argvBytes = argvJson.getBytes(StandardCharsets.UTF_8); + LOG.info("[STEP 2] argv_json (" + argvBytes.length + " bytes): " + argvJson); + + // --- Build env_json (minimal: just disable keytar) --- + String envJson = "{\"COPILOT_DISABLE_KEYTAR\":\"1\"}"; + byte[] envBytes = envJson.getBytes(StandardCharsets.UTF_8); + LOG.info("[STEP 2] env_json (" + envBytes.length + " bytes): " + envJson); + + // --- Call host_start on a separate thread with timeout --- + LOG.info("[STEP 3] Calling copilot_runtime_host_start on background thread..."); + LOG.info("[STEP 3] Timeout: " + HOST_START_TIMEOUT_SECONDS + " s" + + " (Rust READY_TIMEOUT is 30 s internally)"); + long hostStartTime = System.nanoTime(); + + ExecutorService executor = Executors.newSingleThreadExecutor(r -> { + Thread t = new Thread(r, "spike-host-start"); + t.setDaemon(true); + return t; + }); + + Future hostStartFuture = executor.submit(() -> { + LOG.info("[host-start-thread] Thread started: " + Thread.currentThread().getName() + + " (id=" + Thread.currentThread().threadId() + ")"); + LOG.info("[host-start-thread] Calling copilot_runtime_host_start NOW..."); + long callStart = System.nanoTime(); + int result = lib.copilot_runtime_host_start(argvBytes, argvBytes.length, + envBytes, envBytes.length); + long callElapsed = (System.nanoTime() - callStart) / 1_000_000; + LOG.info("[host-start-thread] copilot_runtime_host_start returned: " + + result + " (elapsed: " + callElapsed + " ms)"); + return result; + }); + + int serverHandle; + try { + serverHandle = hostStartFuture.get(HOST_START_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } catch (TimeoutException e) { + long elapsed = (System.nanoTime() - hostStartTime) / 1_000_000; + LOG.severe("[STEP 3] TIMEOUT after " + elapsed + " ms waiting for host_start!"); + LOG.severe("[STEP 3] The Rust side has a 30 s READY_TIMEOUT. Possible causes:"); + LOG.severe(" - spawn_and_serve_background failed to spawn the child"); + LOG.severe(" - Child spawned but never called notify_ready"); + LOG.severe(" - Child spawned but crashed before connecting back"); + LOG.severe(" - The napi-oop socket handshake is hanging"); + LOG.severe("[STEP 3] Cancelling future and dumping threads..."); + hostStartFuture.cancel(true); + dumpRelevantThreads(); + executor.shutdownNow(); + System.exit(2); + return; + } + + long hostStartElapsed = (System.nanoTime() - hostStartTime) / 1_000_000; + LOG.info("[STEP 3] host_start completed in " + hostStartElapsed + " ms, serverHandle=" + serverHandle); + + if (serverHandle == 0) { + LOG.severe("[STEP 3] host_start returned 0 (failure). Possible causes:"); + LOG.severe(" - argv_json parsing failed on the Rust side"); + LOG.severe(" - Child process spawn failed"); + LOG.severe(" - Child timed out during readiness handshake (30 s Rust READY_TIMEOUT)"); + dumpRelevantThreads(); + executor.shutdownNow(); + System.exit(3); + return; + } + + // --- connection_open with diagnostic callback --- + LOG.info("[STEP 4] Calling copilot_runtime_connection_open..."); + AtomicInteger callbackCount = new AtomicInteger(0); + AtomicInteger activeCallbacks = new AtomicInteger(0); + + // CRITICAL: hold as strong reference to prevent GC + CopilotRuntimeLibrary.OutboundCallback callback = (Pointer userData, Pointer data, int len) -> { + int active = activeCallbacks.incrementAndGet(); + int count = callbackCount.incrementAndGet(); + String threadName = Thread.currentThread().getName(); + long threadId = Thread.currentThread().threadId(); + try { + byte[] bytes = data.getByteArray(0, Math.min(len, 4096)); + String preview = new String(bytes, StandardCharsets.UTF_8); + if (preview.length() > 200) { + preview = preview.substring(0, 200) + "..."; + } + LOG.info("[callback #" + count + "] thread='" + threadName + "' (id=" + threadId + + "), active=" + active + ", len=" + len + + ", preview: " + preview); + } catch (Exception e) { + LOG.warning("[callback #" + count + "] Error reading data: " + e.getMessage()); + } finally { + activeCallbacks.decrementAndGet(); + } + }; + + long connStart = System.nanoTime(); + int connHandle = lib.copilot_runtime_connection_open( + serverHandle, callback, Pointer.NULL, + null, 0, null, 0, null, 0); + long connElapsed = (System.nanoTime() - connStart) / 1_000_000; + LOG.info("[STEP 4] connection_open returned connHandle=" + connHandle + + " (elapsed: " + connElapsed + " ms)"); + + if (connHandle == 0) { + LOG.severe("[STEP 4] connection_open returned 0 (failure)."); + LOG.info("[STEP 5] Shutting down host..."); + lib.copilot_runtime_host_shutdown(serverHandle); + executor.shutdownNow(); + System.exit(4); + return; + } + + // --- Wait briefly for any initial callbacks --- + LOG.info("[STEP 4.1] Waiting 5 s for any initial outbound callbacks..."); + Thread.sleep(5000); + LOG.info("[STEP 4.1] Callbacks received so far: " + callbackCount.get()); + + // --- Send a minimal JSON-RPC initialize request --- + String initRequest = "Content-Length: 80\r\n\r\n" + + "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"initialize\"," + + "\"params\":{\"processId\":1}}"; + byte[] initBytes = initRequest.getBytes(StandardCharsets.UTF_8); + LOG.info("[STEP 5] Sending initialize request (" + initBytes.length + " bytes)..."); + byte writeResult = lib.copilot_runtime_connection_write(connHandle, initBytes, initBytes.length); + LOG.info("[STEP 5] connection_write returned: " + writeResult); + + // Wait for response callbacks + LOG.info("[STEP 5.1] Waiting 5 s for response callbacks..."); + Thread.sleep(5000); + LOG.info("[STEP 5.1] Total callbacks received: " + callbackCount.get()); + + // --- Cleanup --- + LOG.info("[STEP 6] Cleaning up..."); + + LOG.info("[STEP 6.1] connection_close(connHandle=" + connHandle + ")..."); + byte closeResult = lib.copilot_runtime_connection_close(connHandle); + LOG.info("[STEP 6.1] connection_close returned: " + closeResult); + + LOG.info("[STEP 6.2] host_shutdown(serverHandle=" + serverHandle + ")..."); + byte shutdownResult = lib.copilot_runtime_host_shutdown(serverHandle); + LOG.info("[STEP 6.2] host_shutdown returned: " + shutdownResult); + + executor.shutdownNow(); + + LOG.info("=== Spike complete ==="); + LOG.info("Summary:"); + LOG.info(" runtime.node loaded: YES (" + loadElapsed + " ms)"); + LOG.info(" host_start result: " + serverHandle + " (" + hostStartElapsed + " ms)"); + LOG.info(" connection_open result: " + connHandle + " (" + connElapsed + " ms)"); + LOG.info(" Total callbacks: " + callbackCount.get()); + LOG.info(" write result: " + writeResult); + LOG.info(" close result: " + closeResult); + LOG.info(" shutdown result: " + shutdownResult); + } + + private static void dumpRelevantThreads() { + LOG.info("--- Thread dump (relevant threads) ---"); + Thread.getAllStackTraces().forEach((thread, stack) -> { + String name = thread.getName(); + if (name.contains("spike") || name.contains("copilot") || name.contains("ffi") + || name.contains("napi") || name.contains("host") || name.contains("main")) { + StringBuilder sb = new StringBuilder(); + sb.append(" Thread '").append(name).append("' (id=").append(thread.threadId()) + .append(", state=").append(thread.getState()).append(")\n"); + for (StackTraceElement ste : stack) { + sb.append(" at ").append(ste).append("\n"); + } + LOG.info(sb.toString()); + } + }); + LOG.info("--- End thread dump ---"); + } + + private static String escapeJson(String s) { + return s.replace("\\", "\\\\").replace("\"", "\\\""); + } + + private static void configureLogging() { + Logger root = Logger.getLogger(""); + root.setLevel(Level.ALL); + for (var handler : root.getHandlers()) { + root.removeHandler(handler); + } + ConsoleHandler ch = new ConsoleHandler(); + ch.setLevel(Level.ALL); + ch.setFormatter(new SimpleFormatter()); + root.addHandler(ch); + } +} diff --git a/java/copilot-native/scripts/fetch-native.mjs b/java/copilot-native/scripts/fetch-native.mjs index 18449badfb..8e00dbfc4d 100644 --- a/java/copilot-native/scripts/fetch-native.mjs +++ b/java/copilot-native/scripts/fetch-native.mjs @@ -13,7 +13,9 @@ * 3. Verify the downloaded tarball against the `integrity` value. * 4. Extract `package/prebuilds//runtime.node` to * `//native//runtime.node`. - * 5. Write `//native//platform.properties`. + * 5. Extract `package/copilot` (or `package/copilot.exe` on Windows) to + * `//native//copilot`. + * 6. Write `//native//platform.properties`. * * Usage: node fetch-native.mjs */ @@ -89,6 +91,19 @@ console.log(`Integrity verified (${integrity.slice(0, 20)}...).`); const memberPath = `package/prebuilds/${classifier}/runtime.node`; execFileSync('tar', ['-xzf', tarballPath, '-C', outDir, memberPath], { stdio: 'inherit' }); fs.renameSync(path.join(outDir, memberPath), runtimePath); + +// Extract the copilot CLI executable (necessary-and-sufficient runtime artifact invariant: +// host_start needs both runtime.node and the copilot CLI from the same package version). +const isWindows = classifier.startsWith('win32'); +const cliTarballMember = isWindows ? 'package/copilot.exe' : 'package/copilot'; +const cliFilename = isWindows ? 'copilot.exe' : 'copilot'; +const cliPath = path.join(resourceDir, cliFilename); +execFileSync('tar', ['-xzf', tarballPath, '-C', outDir, cliTarballMember], { stdio: 'inherit' }); +fs.renameSync(path.join(outDir, cliTarballMember), cliPath); +if (!isWindows) { + fs.chmodSync(cliPath, 0o755); +} + fs.rmSync(path.join(outDir, 'package'), { recursive: true, force: true }); fs.rmSync(tarballPath, { force: true }); diff --git a/java/sdk/src/main/java/com/github/copilot/CopilotClient.java b/java/sdk/src/main/java/com/github/copilot/CopilotClient.java index f420585be7..9aad53ee49 100644 --- a/java/sdk/src/main/java/com/github/copilot/CopilotClient.java +++ b/java/sdk/src/main/java/com/github/copilot/CopilotClient.java @@ -474,25 +474,12 @@ private static InProcessTransport openInProcessTransport(CopilotClientOptions op } /** - * Resolves the runtime entrypoint handed to the in-process host. Callers do not - * configure this: the bundled runtime is used unless an explicit override is - * present in the environment. + * Resolves the runtime entrypoint handed to the in-process host. The copilot + * CLI executable is resolved from the same bundled location as + * {@code runtime.node} — no environment variables or PATH search. */ private static String resolveInProcessEntrypoint(CopilotClientOptions options) throws IOException { - String envPath = System.getenv(NativeRuntimeLoader.COPILOT_CLI_PATH_ENV); - if (envPath != null && !envPath.isBlank()) { - return envPath; - } - String cliPath = options.getCliPath(); - if (cliPath != null && !cliPath.isBlank()) { - return cliPath; - } - String discovered = NativeRuntimeLoader.findRuntimeOnPath(); - if (discovered != null) { - return discovered; - } - throw new IOException("The in-process runtime could not be located. Add the runtime artifact for this" - + " platform to the classpath, or use a child-process connection."); + return NativeRuntimeLoader.resolveEntrypoint().toString(); } private static void closeRuntimeHost(AutoCloseable host) { diff --git a/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java index edc58284eb..797c8e9aed 100644 --- a/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java +++ b/java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java @@ -37,6 +37,8 @@ public final class NativeRuntimeLoader { static final String RUNTIME_FILENAME = "runtime.node"; + static final String CLI_FILENAME = "copilot"; + static final String CLI_FILENAME_WINDOWS = "copilot.exe"; /** Environment variable that overrides where the runtime is loaded from. */ public static final String COPILOT_CLI_PATH_ENV = "COPILOT_CLI_PATH"; static final String VERSION_RESOURCE = "copilot-runtime.properties"; @@ -117,6 +119,33 @@ public static Path resolve() throws IOException { return resolve(null, findRuntimeOnPath(), cacheBase, loader, classifier, version); } + /** + * Resolves the copilot CLI executable from the same location as the bundled + * {@code runtime.node}. The CLI is used as {@code argv[0]} in + * {@code copilot_runtime_host_start} — the Rust runtime spawns it as a child + * process. + * + *

+ * This method calls {@link #resolve()} to locate {@code runtime.node}, then + * looks for the {@code copilot} executable in the same directory. Both + * artifacts are extracted from the classifier JAR together. + * + * @return absolute path to the {@code copilot} CLI executable + * @throws IOException + * if the CLI executable cannot be located + */ + public static Path resolveEntrypoint() throws IOException { + Path runtimePath = resolve(); + Path parent = runtimePath.getParent(); + String cliName = isWindows() ? CLI_FILENAME_WINDOWS : CLI_FILENAME; + Path cliPath = parent.resolve(cliName); + if (Files.isRegularFile(cliPath) && Files.size(cliPath) > 0) { + return cliPath; + } + throw new IOException("Copilot CLI executable not found at " + cliPath + + " — the classifier JAR must contain both runtime.node and the copilot binary"); + } + /** * Reads the SDK version from the filtered {@code copilot-runtime.properties} * resource. @@ -259,6 +288,7 @@ static Path extractToCache(Path cacheBase, ClassLoader loader, String classifier // Step 1 — fast path: return an existing valid cache entry. if (isValidCachedFile(cached)) { + extractCliToCache(cacheDir, loader, classifier, publisher); return cached; } @@ -281,9 +311,53 @@ static Path extractToCache(Path cacheBase, ClassLoader loader, String classifier tryDelete(temp); } + // Step 5 — also extract the copilot CLI executable alongside runtime.node. + extractCliToCache(cacheDir, loader, classifier, publisher); + return cached; } + /** + * Extracts the copilot CLI executable from the classpath to the same cache + * directory as {@code runtime.node}. Idempotent — skips extraction if already + * present and valid. + */ + static void extractCliToCache(Path cacheDir, ClassLoader loader, String classifier, AtomicPublisher publisher) + throws IOException { + String cliName = isWindows() ? CLI_FILENAME_WINDOWS : CLI_FILENAME; + String cliResourcePath = "native/" + classifier + "/" + cliName; + Path cachedCli = cacheDir.resolve(cliName); + + if (isValidCachedFile(cachedCli)) { + return; + } + + URL cliResource = loader.getResource(cliResourcePath); + if (cliResource == null) { + // CLI not on classpath — this is allowed for the COPILOT_CLI_PATH fallback + // path but will fail later in resolveEntrypoint() if InProcess is selected. + return; + } + + Files.createDirectories(cacheDir); + Path temp = Files.createTempFile(cacheDir, "cli-tmp-", ""); + try { + copyResourceToTemp(cliResource, cliResourcePath, temp); + publisher.publish(temp, cachedCli); + } finally { + tryDelete(temp); + } + + // Set executable permission on non-Windows systems. + if (!isWindows()) { + try { + cachedCli.toFile().setExecutable(true, false); + } catch (SecurityException ignored) { + // Best-effort; the file may already be executable from the temp copy. + } + } + } + /** * Tries source 2 (classpath extraction) first and falls back to source 3 * (bundled-CLI sibling) only when the classpath resource is absent. diff --git a/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java b/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java index f040607353..5ac4bde7c9 100644 --- a/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java +++ b/java/sdk/src/test/java/com/github/copilot/E2ETestContext.java @@ -391,7 +391,6 @@ private Map buildInProcessEnvironment(CopilotClientOptions optio env.putAll(optionEnvironment); options.setEnvironment(null); } - env.put("COPILOT_CLI_PATH", cliPath); return env; } From e4a1e0c2e293db21d4e1bac8f027fe933e344d33 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Fri, 7 Aug 2026 01:38:33 +0000 Subject: [PATCH 5/7] Fix InProcess test parity: respect explicit subprocess options over env var - resolveDefaultConnection: when cliUrl, cliPath, or port are explicitly set, fall back to subprocess transport even if COPILOT_SDK_DEFAULT_CONNECTION is 'inprocess'. Explicit options take precedence over the env var default. - validateEnvironmentOptions: check isEmpty() in addition to null, since setEnvironment(null) clears the map rather than nulling the field. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d7a38160-a65e-4d0c-9087-4f28da2a51b8 --- .../main/java/com/github/copilot/CopilotClient.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/java/sdk/src/main/java/com/github/copilot/CopilotClient.java b/java/sdk/src/main/java/com/github/copilot/CopilotClient.java index 9aad53ee49..8e23e666ab 100644 --- a/java/sdk/src/main/java/com/github/copilot/CopilotClient.java +++ b/java/sdk/src/main/java/com/github/copilot/CopilotClient.java @@ -259,6 +259,16 @@ private static RuntimeConnection resolveDefaultConnection(CopilotClientOptions o static RuntimeConnection resolveDefaultConnection(CopilotClientOptions options, String envValue) { if (envValue != null && !envValue.isEmpty()) { if ("inprocess".equalsIgnoreCase(envValue)) { + // Explicit subprocess options take precedence over the env var default. + if (options.getCliUrl() != null && !options.getCliUrl().isEmpty()) { + return inferConnectionFromOptions(options); + } + if (options.getCliPath() != null && !options.getCliPath().isEmpty()) { + return inferConnectionFromOptions(options); + } + if (options.getPort() != 0) { + return inferConnectionFromOptions(options); + } return RuntimeConnection.forInProcess(); } if (!"stdio".equalsIgnoreCase(envValue)) { @@ -390,7 +400,7 @@ private static void validateEnvironmentOptions(CopilotClientOptions options, Run return; } - rejectInProcessOption("Environment", options.getEnvironment() != null, + rejectInProcessOption("Environment", options.getEnvironment() != null && !options.getEnvironment().isEmpty(), "set the variables on the host process environment instead"); rejectInProcessOption("Telemetry", options.getTelemetry() != null, "configure telemetry through the host process environment instead"); From b78e74ef5f334bba7c2d740563ff81ad8b187eba Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Fri, 7 Aug 2026 01:45:10 +0000 Subject: [PATCH 6/7] GUTDODP --- ...ntic-01-test-parity-fix-remaining-tests.md | 125 ++++++++++++++++++ 1 file changed, 125 insertions(+) create mode 100644 1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests.md diff --git a/1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests.md b/1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests.md new file mode 100644 index 0000000000..e80087baab --- /dev/null +++ b/1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests.md @@ -0,0 +1,125 @@ +# Fix remaining InProcess test parity failures + +## Context + +Branch: `edburns/review-copilot-pr-2272` (local worktree at `copilot-sdk-01`) +Push target: `git push upstream HEAD:copilot/edburns1917-java-embed-rust-cli-runtime-post-agent` + +The `-Pinprocess` Maven profile sets `COPILOT_SDK_DEFAULT_CONNECTION=inprocess`, which forces all E2E tests to use the InProcess FFI transport instead of subprocess. Most tests now pass. 24 tests still fail in two categories. + +## Category 1: Tests that set `cwd` or `cliArgs` on options + +These tests go through `ctx.createClient(options)` → `E2ETestContext.applyContextOptions()`. The InProcess branch absorbs `environment` into `InProcessEnvGuard` and nulls it, but does NOT do the same for `cwd` or `cliArgs`. The `CopilotClient` constructor then calls `validateEnvironmentOptions()` which rejects non-null `cwd`/`cliArgs` for InProcess. + +**Fix:** In `E2ETestContext.applyContextOptions()`, when InProcess mode is detected, also null out `cwd` and `cliArgs` before constructing the client. For `cwd`, it's meaningless in InProcess (host process cwd is already set). For `cliArgs`, they're subprocess-specific flags. + +Location: `java/sdk/src/test/java/com/github/copilot/E2ETestContext.java` lines 354-376 + +Current InProcess branch in `applyContextOptions`: +```java +if (isInProcessMode(options)) { + InProcessEnvGuard guard = new InProcessEnvGuard(buildInProcessEnvironment(options)); + inProcessEnvGuards.add(guard); + try { + options.setEnvironment(null); + return new CopilotClient(options, guard::close); + } catch (RuntimeException e) { + guard.close(); + throw e; + } +} +``` + +Needs to also null `cwd` and `cliArgs`: +```java +options.setEnvironment(null); +options.setCwd(null); +options.setCliArgs(null); +``` + +Affected tests: `PerSessionAuthTest` (sets cwd+environment), possibly others. + +## Category 2: StreamingFidelityTest hang + +`StreamingFidelityTest.testShouldEmitStreamingDeltasWithReasoningEffortConfigured` hangs indefinitely in InProcess mode. The main thread is blocked on `CompletableFuture.get()` at line 258. The JSON-RPC reader thread is reading from `QueueInputStream` (the InProcess FFI receive stream) but never receives the expected response. + +This is a functional issue, not a validation issue. The replay proxy is running (CapiProxy thread is active), but the InProcess transport isn't completing the streaming interaction. + +Diagnosis approach: +1. Check if the test's replay snapshot exists and is correct for streaming +2. Check if `host_start` succeeds for this test (serverHandle != 0) +3. jstack showed the reader thread blocked in `QueueInputStream.read()` — no data arriving via the FFI callback +4. Possible causes: the replay proxy response format doesn't match what the InProcess runtime expects for streaming, or the connection isn't routing correctly through the replay proxy + +## Key architectural facts + +- `runtime.node` is loaded via JNA. `copilot` CLI binary is spawned as child by `host_start` via `argv[0]`. +- Both are now bundled in the classifier JAR at `native//runtime.node` and `native//copilot`. +- `NativeRuntimeLoader.resolve()` extracts both to `~/.copilot/runtime-cache///`. +- `NativeRuntimeLoader.resolveEntrypoint()` finds `copilot` alongside `runtime.node`. +- `CopilotClient.resolveInProcessEntrypoint()` simply calls `NativeRuntimeLoader.resolveEntrypoint().toString()`. +- `InProcessEnvGuard` uses JNA `libc.setenv()` to mutate the native process env (not visible to `System.getenv()`). +- The replay proxy (CapiProxy) runs as a Node.js subprocess serving YAML snapshot responses. + +## CopilotClientOptions.setEnvironment(null) quirk + +`setEnvironment(null)` does NOT set the field to null — it calls `this.environment.clear()`, leaving an empty HashMap. `getEnvironment()` then returns a non-null empty map. The validation now checks `!isEmpty()` too (already fixed). + +Similarly, check if `setCwd(null)` / `setCliArgs(null)` have similar behavior. If `setCwd(null)` doesn't actually null the field, the validation might still fire. + +## Validation in CopilotClient constructor + +```java +private static void validateEnvironmentOptions(CopilotClientOptions options, RuntimeConnection connection) { + if (!(connection instanceof InProcessRuntimeConnection)) return; + rejectInProcessOption("Environment", options.getEnvironment() != null && !options.getEnvironment().isEmpty(), ...); + rejectInProcessOption("Telemetry", options.getTelemetry() != null, ...); + rejectInProcessOption("Cwd", options.getCwd() != null, ...); + rejectInProcessOption("CliArgs", options.getCliArgs() != null && options.getCliArgs().length > 0, ...); +} +``` + +## resolveDefaultConnection precedence (already fixed) + +When `COPILOT_SDK_DEFAULT_CONNECTION=inprocess` but `cliUrl`/`cliPath`/`port` are explicitly set, the explicit options win and subprocess transport is used. Tests like `McpAuthInterestRegistrationTest` that create `new CopilotClient(options.setCliUrl(...))` directly now correctly bypass InProcess. + +## Full list of 24 failing test methods + +``` +ByokBearerTokenProviderE2ETest (3 methods) +CopilotRequestCancelErrorE2ETest (2) +CopilotRequestHandlerE2ETest (2) +CopilotRequestSessionIdE2ETest (1) +GitHubTelemetryTest (2) +McpAuthInterestRegistrationTest (3) +ModeHandlersTest (2) +PerSessionAuthTest (3) +ProviderEndpointE2ETest (2) +RpcServerE2ETest (1 - testShouldAddSecretFilterValues — NOW PASSES) +SessionConfigE2ETest (2) +StreamingFidelityTest (1 - hangs) +SubagentHooksE2ETest (1) +``` + +## Commands + +```bash +# Run all tests with InProcess +cd java && mvn clean verify -Pinprocess + +# Run specific failing tests +COPILOT_SDK_DEFAULT_CONNECTION=inprocess mvn test -pl sdk -Dtest="PerSessionAuthTest,StreamingFidelityTest" -DfailIfNoTests=false + +# Format before commit +mvn spotless:apply + +# Push +git push upstream HEAD:copilot/edburns1917-java-embed-rust-cli-runtime-post-agent +``` + +## Java env bootstrap (required before any mvn/java command) +```bash +export JAVA_HOME="/usr/lib/jvm/msopenjdk-25-amd64" +export M2_HOME="${HOME}/Downloads/apache-maven-3.9.8" +export PATH="${M2_HOME}/bin:${JAVA_HOME}/bin:${PATH}" +``` From 9fe28a8aeb3b183f6ed176a32c464bf57e901b60 Mon Sep 17 00:00:00 2001 From: Ed Burns Date: Fri, 7 Aug 2026 17:29:06 +0000 Subject: [PATCH 7/7] Fix Java in-process test lifecycle and parity Prevent the Java in-process test profile from corrupting Surefire control streams or poisoning later tests through the runtime's process-global LLM provider registration. Preserve explicit subprocess and TCP transport choices, sanitize process-only options before constructing in-process clients, and run request-handler tests over their required isolated stdio runtime. Fix the remaining test-contract issues by making fake socket RPC handler registration atomic with reader startup, honoring the configured CLI entrypoint when runtime.node is in a prebuilds directory, and isolating the streaming model-cache scenario. Remove in-process skip annotations from tests that already exercise an explicit subprocess transport. The complete `mvn clean verify -Pinprocess` run now finishes successfully without hangs, transport timeouts, provider-ownership failures, or Surefire stream corruption. File-by-file manifest: - `java/sdk/pom.xml`: use Surefire's TCP fork channel for unit and integration tests so native runtime output cannot corrupt Maven's process-pipe protocol. - `java/sdk/src/main/java/com/github/copilot/CopilotClient.java`: preserve explicitly selected TCP options when the default connection environment requests in-process transport. - `java/sdk/src/main/java/com/github/copilot/JsonRpcClient.java`: add a socket construction hook that registers handlers before the reader thread starts. - `java/sdk/src/main/java/com/github/copilot/ffi/NativeRuntimeLoader.java`: resolve the configured Copilot executable separately from runtime.node when the native library uses the package's prebuilds layout. - `java/sdk/src/main/java/com/github/copilot/rpc/CopilotClientOptions.java`: allow `setCwd(null)` to clear a previously configured working directory. - `java/sdk/src/test/java/com/github/copilot/ClientOptionsE2ETest.java`: run explicit fake-stdio option forwarding tests under the in-process profile. - `java/sdk/src/test/java/com/github/copilot/ConfigCloneTest.java`: cover clearing a configured working directory. - `java/sdk/src/test/java/com/github/copilot/CopilotClientTest.java`: remove obsolete in-process skips from explicit subprocess and TCP lifecycle tests. - `java/sdk/src/test/java/com/github/copilot/CopilotClientTransportTest.java`: test explicit transport precedence, in-process option sanitization, and TCP token selection under the profile default. - `java/sdk/src/test/java/com/github/copilot/CopilotRequestTestSupport.java`: explicitly select stdio for request-handler tests that register the process-global LLM inference provider. - `java/sdk/src/test/java/com/github/copilot/E2ETestContext.java`: honor explicit transports, route request-handler clients to subprocess isolation, and clear environment, cwd, and CLI arguments before in-process client construction. - `java/sdk/src/test/java/com/github/copilot/GitHubTelemetryTest.java`: register fake runtime RPC handlers before socket message processing begins. - `java/sdk/src/test/java/com/github/copilot/MetadataApiTest.java`: run explicit stdio metadata tests instead of skipping them under the profile. - `java/sdk/src/test/java/com/github/copilot/PerSessionAuthTest.java`: run the explicit subprocess unauthenticated case under the profile. - `java/sdk/src/test/java/com/github/copilot/RpcServerMiscE2ETest.java`: run the explicit subprocess account lifecycle case under the profile. - `java/sdk/src/test/java/com/github/copilot/StreamingFidelityTest.java`: give the gpt-5.4 reasoning/streaming scenario an isolated proxy and runtime model cache. - `java/sdk/src/test/java/com/github/copilot/ffi/FfiRuntimeHostTest.java`: cover failed connection-open cleanup followed by successful sequential startup. - `java/sdk/src/test/java/com/github/copilot/ffi/NativeRuntimeLoaderTest.java`: cover resolving a configured CLI beside a prebuilds runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e03c4e94-97b0-41ad-9f4e-c01633dc0bf7 --- ...c-01-test-parity-fix-remaining-tests-01.md | 172 ++++++++++++++++++ java/sdk/pom.xml | 2 + .../com/github/copilot/CopilotClient.java | 3 + .../com/github/copilot/JsonRpcClient.java | 13 ++ .../copilot/ffi/NativeRuntimeLoader.java | 14 +- .../copilot/rpc/CopilotClientOptions.java | 6 +- .../github/copilot/ClientOptionsE2ETest.java | 5 - .../com/github/copilot/ConfigCloneTest.java | 9 + .../com/github/copilot/CopilotClientTest.java | 16 -- .../copilot/CopilotClientTransportTest.java | 32 ++-- .../copilot/CopilotRequestTestSupport.java | 3 +- .../com/github/copilot/E2ETestContext.java | 10 +- .../github/copilot/GitHubTelemetryTest.java | 33 ++-- .../com/github/copilot/MetadataApiTest.java | 4 - .../github/copilot/PerSessionAuthTest.java | 3 - .../github/copilot/RpcServerMiscE2ETest.java | 3 - .../github/copilot/StreamingFidelityTest.java | 46 ++--- .../copilot/ffi/FfiRuntimeHostTest.java | 45 +++++ .../copilot/ffi/NativeRuntimeLoaderTest.java | 10 + 19 files changed, 339 insertions(+), 90 deletions(-) create mode 100644 1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests-01.md diff --git a/1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests-01.md b/1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests-01.md new file mode 100644 index 0000000000..f4c9bc3ad0 --- /dev/null +++ b/1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests-01.md @@ -0,0 +1,172 @@ +# Prompt: make the Java InProcess test run clean + +You are working in `/home/edburns/workareas/copilot-sdk-01`, branch +`edburns/review-copilot-pr-2272`. Read these files first: + +- `1917-java-embed-rust-cli-runtime-remove-before-merge/post-agentic-01-test-parity-fix-remaining-tests.md` +- `java/20260807-0145-job-logs.txt` +- the current git diff and the Java test/runtime/harness sources + +The target command is: + +```bash +cd java +mvn clean verify -Pinprocess +``` + +Make the implementation and test changes necessary for a genuinely clean, +non-hanging run. Do not solve this by broadly skipping tests, increasing +timeouts, weakening assertions, or hiding errors. Preserve the negative-test +assertions; expected negative cases may be logged, but they must not be +reported as test errors. + +## What the interrupted log establishes + +The run was interrupted after more than an hour; it has no `BUILD SUCCESS`. +There are 88 errors in 20 suites. The failures are highly clustered: + +- `std/in stream corrupted` appears during `AskUserTest`. +- `ByokBearerTokenProviderE2ETest` has the expected fake 404 in one negative + case, but the other two tests fail because + `llmInference.setProvider` says “Another client is already the LLM inference + provider.” +- The same provider-ownership error breaks + `CopilotRequestCancelErrorE2ETest`, `CopilotRequestHandlerE2ETest`, + `SessionConfigE2ETest`, and other provider/handler tests. +- `CompactionTest`, `CopilotSessionTest`, `ErrorHandlingTest`, + `EventFidelityTest`, `ExecutorWiringTest`, `HooksTest`, `McpAndAgentsTest`, + `ModeHandlersTest`, `MultiProviderRegistryE2ETest`, `PermissionsTest`, + `PreMcpToolCallHookTest`, `RpcSessionStateExtrasE2ETest`, + `SessionConfigE2ETest`, and `SessionEventsE2ETest` contain repeated + approximately 60-second `sendAndWait`/future timeouts. +- `GitHubTelemetryTest` fails immediately because an InProcess connection + receives `Method not found: connect` and `Method not found: ping`; determine + whether this test must explicitly use the subprocess/socket transport or + whether the InProcess endpoint is missing required handlers. +- `RpcServerE2ETest` has a 30-second RPC timeout and + `RpcSessionStateExtrasE2ETest` has a 60-second timeout. +- `PerSessionAuthTest` has one skipped test and a negative 401 “Bad + credentials” trace. The test itself is not an error. +- `ClientOptionsE2ETest` skips all three tests. Other suites also report + intentional-looking skips: `CopilotClientTest` (14), + `CopilotClientTransportTest` (4), `MetadataApiTest` (3), + `RpcServerMiscE2ETest` (1), and `CompactionTest` (1). +- Many stack traces in `CreateSessionReKeyEntryTest`, `JsonRpcClientTest`, + `LifecycleEventManagerTest`, `RpcHandlerDispatcherTest`, and + `SessionHandlerTest` are deliberately generated negative-test traces and + are followed by passing summaries. Do not misclassify them as failures. + +## Priority 1: stop stream corruption and fix InProcess ownership/lifecycle + +Investigate `std/in stream corrupted` first. Trace every process and stream +created by the InProcess FFI path, `host_start`, the bundled `copilot` +entrypoint, `NativeRuntimeLoader`, `InProcessRuntimeConnection`, `CapiProxy`, +and Surefire. Identify which native/child process is writing bytes to the +Surefire-controlled stdout/stdin protocol. Ensure child stdout/stderr are +consumed or redirected in the same way as the supported transport and that +the FFI receive/send streams are not closed or reused by another client. +Do not merely suppress Surefire output. + +Then fix the “Another client is already the LLM inference provider” root +cause. Determine whether clients, native hosts, provider registrations, or +`InProcessEnvGuard` instances survive test teardown. Verify the close path on +both successful and failed `start()`, failed `createSession()`, and failed +requests. Ensure a failed startup cannot leave a provider registered and that +each test context closes its client/proxy/runtime deterministically. If the +InProcess runtime is process-global, serialize or otherwise coordinate provider +ownership rather than allowing overlapping providers. Add focused regression +coverage for failed-start cleanup and sequential client startup. + +The earlier context notes that `E2ETestContext.applyContextOptions()` must +clear InProcess-incompatible `cwd` and `cliArgs` in addition to `environment`. +Implement that carefully, and verify the actual setter semantics: +`setEnvironment(null)` clears to an empty map, while `setCwd(null)` and +`setCliArgs(null)` must be checked rather than assumed. Add or update tests so +the options are truly absent according to constructor validation. + +## Priority 2: isolate and repair the common timeout + +After Priority 1, run small, serial selectors, not the full suite: + +```bash +cd java +COPILOT_SDK_DEFAULT_CONNECTION=inprocess mvn test -pl sdk \ + -Dtest="AskUserTest,ByokBearerTokenProviderE2ETest,CopilotSessionTest" \ + -DfailIfNoTests=false +``` + +Use a bounded shell timeout while debugging so a regression cannot consume an +hour. For any remaining timeout, capture a thread dump and inspect the +corresponding Surefire report plus replay-proxy output. Follow one request +from Java JSON-RPC send, through the FFI callback/`QueueInputStream`, into the +replay proxy, and back to the Java reader. Confirm that: + +1. `host_start` returns a valid handle and the child `copilot` entrypoint is + reachable. +2. The request reaches the proxy with the expected snapshot. +3. Every response/event is framed correctly and enqueued to the receive + stream. +4. stream completion/EOF and client close wake blocked readers. +5. callbacks do not depend on a thread or executor that has already shut down. + +Use `StreamingFidelityTest.testShouldEmitStreamingDeltasWithReasoningEffortConfigured` +as the minimal streaming reproducer, but also test one ordinary +`CopilotSessionTest` request. Do not patch each timed-out suite individually; +the repeated 60-second failures indicate a shared transport or lifecycle +defect. Once the common path works, rerun representative handler, hook, +permission, event, session-config, MCP, and RPC-server selectors and only +then the complete profile. + +`GitHubTelemetryTest` is a separate transport-contract issue: inspect its +test setup and the supported connection mode. If it intentionally uses a +minimal fake RPC peer that only supports telemetry, make it explicitly select +that transport so the global InProcess profile cannot route it to a runtime +without `connect`/`ping`. If InProcess is intended, implement the missing +protocol surface and add focused coverage. + +## Priority 3: remove unjustified skips + +Audit every skipped test in the log and the associated assumptions. For each: + +- make it run under InProcess when the behavior is transport-independent; +- explicitly force subprocess/socket transport when the test is specifically + validating subprocess-only options or protocol behavior; or +- change the test setup so the same public behavior is exercised through + InProcess. + +Do not add a profile-wide exclusion and do not convert skipped tests to +passing assertions. In particular, investigate all three +`ClientOptionsE2ETest` skips, the `PerSessionAuthTest` skip, and the skips in +`CopilotClientTest`, `CopilotClientTransportTest`, `MetadataApiTest`, +`RpcServerMiscE2ETest`, and `CompactionTest`. The final profile run should +have zero skips unless a test is demonstrably impossible on the platform and +the repository’s existing policy explicitly permits it; document any +remaining exception in the test source. + +## Priority 4: make expected negative output intentional + +Do not alter assertions for negative tests. After all tests pass, reduce noisy +expected stack-trace logging only where the repository’s logging conventions +support it: distinguish expected test-triggered failures from unexpected +transport failures, and avoid logging full stack traces at warning/error for +the expected path if that can be done without hiding real failures. The +`fake byok endpoint`, `401 Bad credentials`, `session.resume` not-found, +handler exceptions, malformed JSON, socket-close, and re-key traces must +remain asserted and diagnosable. + +## Validation and completion criteria + +Use the repository’s normal Java bootstrap and Maven logging conventions. +Format Java changes with `mvn spotless:apply` from `java`. Run focused tests +after each root-cause fix, then: + +```bash +cd java +mvn clean verify -Pinprocess +``` + +The task is complete only when this command terminates normally with +`BUILD SUCCESS`, all test suites report zero failures and zero errors, no +test hangs or 60-second transport timeouts occur, no Surefire stream +corruption occurs, and the skip count is zero or each explicitly justified +platform exception is documented and approved by the existing test policy. diff --git a/java/sdk/pom.xml b/java/sdk/pom.xml index 12cb42a3ee..1afcf9c9be 100644 --- a/java/sdk/pom.xml +++ b/java/sdk/pom.xml @@ -243,6 +243,7 @@ + ${project.build.directory} ${project.build.finalName} @@ -264,6 +265,7 @@ maven-surefire-plugin alphabetical + ${testExecutionAgentArgs} ${surefire.jvm.args} --add-opens com.github.copilot.java/com.github.copilot.e2e=ALL-UNNAMED