Merge reference implementation SDK changes (2026-05-05) and address review feedback

.lastmerge

@@ -1 +1 @@
-ced6613253a595769d2e77547f9e1caf6bef6438
+c063458ecc3d606766f04cf203b11b08de672cc8

src/main/java/com/github/copilot/sdk/CliServerManager.java

@@ -30,9 +30,11 @@
 final class CliServerManager {
 
     private static final Logger LOG = Logger.getLogger(CliServerManager.class.getName());
+    private static final int STDERR_READER_JOIN_TIMEOUT_MS = 5000;
 
     private final CopilotClientOptions options;
     private final StringBuilder stderrBuffer = new StringBuilder();
+    private volatile Thread stderrThread;
     private String connectionToken;
 
     CliServerManager(CopilotClientOptions options) {
@@ -199,7 +201,7 @@ JsonRpcClient connectToServer(Process process, String tcpHost, Integer tcpPort)
     }
 
     private void startStderrReader(Process process) {
-        var stderrThread = new Thread(() -> {
+        var thread = new Thread(() -> {
             try (BufferedReader reader = new BufferedReader(
                     new InputStreamReader(process.getErrorStream(), StandardCharsets.UTF_8))) {
                 String line;
@@ -213,8 +215,9 @@ private void startStderrReader(Process process) {
                 LOG.log(Level.FINE, "Error reading stderr", e);
             }
         }, "cli-stderr-reader");
-        stderrThread.setDaemon(true);
-        stderrThread.start();
+        thread.setDaemon(true);
+        thread.start();
+        this.stderrThread = thread;
     }
 
     private Integer waitForPortAnnouncement(Process process) throws IOException {
@@ -226,11 +229,9 @@ private Integer waitForPortAnnouncement(Process process) throws IOException {
             while (System.currentTimeMillis() < deadline) {
                 String line = reader.readLine();
                 if (line == null) {
+                    awaitStderrReader();
                     String stderr = getStderrOutput();
-                    if (!stderr.isEmpty()) {
-                        throw new IOException("CLI process exited unexpectedly. stderr: " + stderr);
-                    }
-                    throw new IOException("CLI process exited unexpectedly");
+                    throw new IOException(formatCliExitedMessage("CLI process exited unexpectedly.", stderr));
                 }
 
                 Matcher matcher = portPattern.matcher(line);
@@ -250,12 +251,30 @@ String getStderrOutput() {
         }
     }
 
+    private void awaitStderrReader() {
+        Thread t = this.stderrThread;
+        if (t != null) {
+            try {
+                t.join(STDERR_READER_JOIN_TIMEOUT_MS);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+            }
+        }
+    }
+
     private void clearStderrBuffer() {
         synchronized (stderrBuffer) {
             stderrBuffer.setLength(0);
         }
     }
 
+    static String formatCliExitedMessage(String message, String stderrOutput) {
+        if (stderrOutput == null || stderrOutput.isEmpty()) {
+            return message;
+        }
+        return message + "\nstderr: " + stderrOutput;
+    }
+
     private List<String> resolveCliCommand(String cliPath, List<String> args) {
         boolean isJsFile = cliPath.toLowerCase().endsWith(".js");
 

src/main/java/com/github/copilot/sdk/CopilotClient.java

@@ -76,6 +76,7 @@ public final class CopilotClient implements AutoCloseable {
      * shutdown via {@link #stop()}.
      */
     public static final int AUTOCLOSEABLE_TIMEOUT_SECONDS = 10;
+    private static final int FORCE_KILL_TIMEOUT_SECONDS = 10;
     private final CopilotClientOptions options;
     private final CliServerManager serverManager;
     private final LifecycleEventManager lifecycleManager = new LifecycleEventManager();
@@ -216,7 +217,8 @@ private Connection startCoreBody() {
         } catch (Exception e) {
             String stderr = serverManager.getStderrOutput();
             if (!stderr.isEmpty()) {
-                throw new CompletionException(new IOException("CLI process exited unexpectedly. stderr: " + stderr, e));
+                throw new CompletionException(new IOException(
+                        CliServerManager.formatCliExitedMessage("CLI process exited unexpectedly.", stderr), e));
             }
             throw new CompletionException(e);
         }
@@ -244,7 +246,7 @@ private void verifyProtocolVersion(Connection connection) throws Exception {
             while (cause instanceof java.util.concurrent.ExecutionException || cause instanceof CompletionException) {
                 cause = cause.getCause();
             }
-            if (cause instanceof JsonRpcException rpcEx && rpcEx.getCode() == METHOD_NOT_FOUND_ERROR_CODE) {
+            if (cause instanceof JsonRpcException rpcEx && isUnsupportedConnectMethod(rpcEx)) {
                 // Legacy server without 'connect'; fall back to 'ping'.
                 // A token, if any, is silently dropped — the legacy server can't enforce one.
                 var params = new HashMap<String, Object>();
@@ -270,6 +272,10 @@ private void verifyProtocolVersion(Connection connection) throws Exception {
         }
     }
 
+    private static boolean isUnsupportedConnectMethod(JsonRpcException ex) {
+        return ex.getCode() == METHOD_NOT_FOUND_ERROR_CODE || "Unhandled method connect".equals(ex.getMessage());
+    }
+
     /**
      * Disconnects from the Copilot server and closes all active sessions.
      * <p>
@@ -348,8 +354,14 @@ private CompletableFuture<Void> cleanupConnection() {
             if (connection.process != null) {
                 try {
                     if (connection.process.isAlive()) {
-                        connection.process.destroyForcibly();
+                        Process destroyedProcess = connection.process.destroyForcibly();
+                        if (!destroyedProcess.waitFor(FORCE_KILL_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
+                            LOG.fine("Process did not terminate within force kill timeout");
+                        }
                     }
+                } catch (InterruptedException e) {
+                    Thread.currentThread().interrupt();
+                    LOG.log(Level.FINE, "Interrupted while killing process", e);
                 } catch (Exception e) {
                     LOG.log(Level.FINE, "Error killing process", e);
                 }

src/test/java/com/github/copilot/sdk/EventFidelityTest.java

@@ -0,0 +1,111 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *--------------------------------------------------------------------------------------------*/
+
+package com.github.copilot.sdk;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import com.github.copilot.sdk.generated.AssistantUsageEvent;
+import com.github.copilot.sdk.generated.SessionEvent;
+import com.github.copilot.sdk.generated.SessionUsageInfoEvent;
+import com.github.copilot.sdk.json.MessageOptions;
+import com.github.copilot.sdk.json.PermissionHandler;
+import com.github.copilot.sdk.json.SessionConfig;
+
+/**
+ * E2E tests for event fidelity — verifying the shape, ordering, and presence of
+ * key events emitted from the runtime.
+ *
+ * <p>
+ * Snapshots are stored in {@code test/snapshots/event_fidelity/}.
+ * </p>
+ */
+public class EventFidelityTest {
+
+    private static E2ETestContext ctx;
+
+    @BeforeAll
+    static void setup() throws Exception {
+        ctx = E2ETestContext.create();
+    }
+
+    @AfterAll
+    static void teardown() throws Exception {
+        if (ctx != null) {
+            ctx.close();
+        }
+    }
+
+    /**
+     * Verifies that an {@code assistant.usage} event is emitted after the model
+     * processes a prompt.
+     *
+     * @see Snapshot:
+     *      event_fidelity/should_emit_assistant_usage_event_after_model_call
+     */
+    @Test
+    void testShouldEmitAssistantUsageEventAfterModelCall() throws Exception {
+        ctx.configureForTest("event_fidelity", "should_emit_assistant_usage_event_after_model_call");
+
+        try (CopilotClient client = ctx.createClient()) {
+            CopilotSession session = client
+                    .createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get();
+
+            List<SessionEvent> events = new ArrayList<>();
+            session.on(events::add);
+
+            session.sendAndWait(new MessageOptions().setPrompt("What is 5+5? Reply with just the number.")).get(60,
+                    TimeUnit.SECONDS);
+
+            List<AssistantUsageEvent> usageEvents = events.stream().filter(e -> e instanceof AssistantUsageEvent)
+                    .map(e -> (AssistantUsageEvent) e).toList();
+
+            assertFalse(usageEvents.isEmpty(), "Should have received an assistant.usage event after model call");
+
+            AssistantUsageEvent lastUsage = usageEvents.get(usageEvents.size() - 1);
+            assertNotNull(lastUsage.getData().model(), "Usage event should have a model field");
+            assertFalse(lastUsage.getData().model().isEmpty(), "Model field should not be empty");
+
+            session.close();
+        }
+    }
+
+    /**
+     * Verifies that a {@code session.usage_info} event is emitted after the model
+     * processes a prompt.
+     *
+     * @see Snapshot:
+     *      event_fidelity/should_emit_session_usage_info_event_after_model_call
+     */
+    @Test
+    void testShouldEmitSessionUsageInfoEventAfterModelCall() throws Exception {
+        ctx.configureForTest("event_fidelity", "should_emit_session_usage_info_event_after_model_call");
+
+        try (CopilotClient client = ctx.createClient()) {
+            CopilotSession session = client
+                    .createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get();
+
+            List<SessionEvent> events = new ArrayList<>();
+            session.on(events::add);
+
+            session.sendAndWait(new MessageOptions().setPrompt("What is 5+5? Reply with just the number.")).get(60,
+                    TimeUnit.SECONDS);
+
+            List<SessionUsageInfoEvent> usageInfoEvents = events.stream()
+                    .filter(e -> e instanceof SessionUsageInfoEvent).map(e -> (SessionUsageInfoEvent) e).toList();
+
+            assertFalse(usageInfoEvents.isEmpty(), "Should have received a session.usage_info event after model call");
+
+            session.close();
+        }
+    }
+}

src/test/java/com/github/copilot/sdk/PermissionsTest.java

@@ -363,4 +363,115 @@ void testShouldDenyToolOperationsWhenHandlerExplicitlyDeniesAfterResume(TestInfo
             session2.close();
         }
     }
+
+    /**
+     * Verifies that a permission handler returning {@code noResult} is handled
+     * correctly — the handler is called, and the session can be aborted afterward.
+     *
+     * @see Snapshot: permissions/should_deny_permission_with_noresult_kind
+     */
+    @Test
+    void testShouldDenyPermissionWithNoResultKind() throws Exception {
+        ctx.configureForTest("permissions", "should_deny_permission_with_noresult_kind");
+
+        var permissionCalled = new CompletableFuture<Boolean>();
+
+        try (CopilotClient client = ctx.createClient()) {
+            CopilotSession session = client
+                    .createSession(new SessionConfig().setOnPermissionRequest((request, invocation) -> {
+                        permissionCalled.complete(true);
+                        return CompletableFuture.completedFuture(
+                                new PermissionRequestResult().setKind(PermissionRequestResultKind.NO_RESULT));
+                    })).get();
+
+            session.send(new MessageOptions().setPrompt("Run 'node --version'"));
+
+            assertTrue(permissionCalled.get(30, TimeUnit.SECONDS),
+                    "Expected the no-result permission handler to be called.");
+
+            session.abort().get(10, TimeUnit.SECONDS);
+            session.close();
+        }
+    }
+
+    /**
+     * Verifies that the runtime short-circuits the permission handler when
+     * {@code session.permissions.setApproveAll(true)} has been called.
+     *
+     * @see Snapshot:
+     *      permissions/should_short_circuit_permission_handler_when_set_approve_all_enabled
+     */
+    @Test
+    void testShouldShortCircuitPermissionHandlerWhenSetApproveAllEnabled() throws Exception {
+        ctx.configureForTest("permissions", "should_short_circuit_permission_handler_when_set_approve_all_enabled");
+
+        var handlerCallCount = new int[]{0};
+
+        try (CopilotClient client = ctx.createClient()) {
+            CopilotSession session = client
+                    .createSession(new SessionConfig().setOnPermissionRequest((request, invocation) -> {
+                        handlerCallCount[0]++;
+                        return CompletableFuture.completedFuture(
+                                new PermissionRequestResult().setKind(PermissionRequestResultKind.APPROVED));
+                    })).get();
+
+            // Set approve-all so the runtime short-circuits
+            var setResult = session.getRpc().permissions
+                    .setApproveAll(new com.github.copilot.sdk.generated.rpc.SessionPermissionsSetApproveAllParams(
+                            session.getSessionId(), true))
+                    .get(10, TimeUnit.SECONDS);
+            assertTrue(setResult.success(), "setApproveAll should succeed");
+
+            AssistantMessageEvent response = session
+                    .sendAndWait(new MessageOptions().setPrompt("Run 'echo test' and tell me what happens"))
+                    .get(60, TimeUnit.SECONDS);
+            assertNotNull(response);
+
+            // Handler should not have been called since runtime approves all
+            assertEquals(0, handlerCallCount[0],
+                    "Permission handler should not be called when setApproveAll is enabled");
+
+            session.close();
+        }
+    }
+
+    /**
+     * Verifies that the SDK correctly waits for a slow permission handler before
+     * completing tool execution.
+     *
+     * @see Snapshot: permissions/should_wait_for_slow_permission_handler
+     */
+    @Test
+    void testShouldWaitForSlowPermissionHandler() throws Exception {
+        ctx.configureForTest("permissions", "should_wait_for_slow_permission_handler");
+
+        var handlerEntered = new CompletableFuture<Void>();
+        var releaseHandler = new CompletableFuture<Void>();
+
+        try (CopilotClient client = ctx.createClient()) {
+            CopilotSession session = client
+                    .createSession(new SessionConfig().setOnPermissionRequest((request, invocation) -> {
+                        handlerEntered.complete(null);
+                        return releaseHandler.thenApply(
+                                v -> new PermissionRequestResult().setKind(PermissionRequestResultKind.APPROVED));
+                    })).get();
+
+            // Capture the sendAndWait future before awaiting it so we can interact with the
+            // handler
+            CompletableFuture<AssistantMessageEvent> responseFuture = session
+                    .sendAndWait(new MessageOptions().setPrompt("Run 'echo slow_handler_test'"));
+
+            // Wait for permission handler to be entered
+            handlerEntered.get(30, TimeUnit.SECONDS);
+
+            // Release the handler
+            releaseHandler.complete(null);
+
+            // Session should complete successfully
+            AssistantMessageEvent message = responseFuture.get(60, TimeUnit.SECONDS);
+            assertNotNull(message);
+
+            session.close();
+        }
+    }
 }