Skip to content

Commit 21a8f06

Browse files
authored
Merge pull request #157 from github/copilot/reference-impl-sync-expand-sdk-e2e-coverage
Merge reference implementation SDK changes (2026-05-05) and address review feedback
2 parents 8454969 + a9c88d7 commit 21a8f06

8 files changed

Lines changed: 651 additions & 11 deletions

File tree

.lastmerge

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
ced6613253a595769d2e77547f9e1caf6bef6438
1+
c063458ecc3d606766f04cf203b11b08de672cc8

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@
3030
final class CliServerManager {
3131

3232
private static final Logger LOG = Logger.getLogger(CliServerManager.class.getName());
33+
private static final int STDERR_READER_JOIN_TIMEOUT_MS = 5000;
3334

3435
private final CopilotClientOptions options;
3536
private final StringBuilder stderrBuffer = new StringBuilder();
37+
private volatile Thread stderrThread;
3638
private String connectionToken;
3739

3840
CliServerManager(CopilotClientOptions options) {
@@ -199,7 +201,7 @@ JsonRpcClient connectToServer(Process process, String tcpHost, Integer tcpPort)
199201
}
200202

201203
private void startStderrReader(Process process) {
202-
var stderrThread = new Thread(() -> {
204+
var thread = new Thread(() -> {
203205
try (BufferedReader reader = new BufferedReader(
204206
new InputStreamReader(process.getErrorStream(), StandardCharsets.UTF_8))) {
205207
String line;
@@ -213,8 +215,9 @@ private void startStderrReader(Process process) {
213215
LOG.log(Level.FINE, "Error reading stderr", e);
214216
}
215217
}, "cli-stderr-reader");
216-
stderrThread.setDaemon(true);
217-
stderrThread.start();
218+
thread.setDaemon(true);
219+
thread.start();
220+
this.stderrThread = thread;
218221
}
219222

220223
private Integer waitForPortAnnouncement(Process process) throws IOException {
@@ -226,11 +229,9 @@ private Integer waitForPortAnnouncement(Process process) throws IOException {
226229
while (System.currentTimeMillis() < deadline) {
227230
String line = reader.readLine();
228231
if (line == null) {
232+
awaitStderrReader();
229233
String stderr = getStderrOutput();
230-
if (!stderr.isEmpty()) {
231-
throw new IOException("CLI process exited unexpectedly. stderr: " + stderr);
232-
}
233-
throw new IOException("CLI process exited unexpectedly");
234+
throw new IOException(formatCliExitedMessage("CLI process exited unexpectedly.", stderr));
234235
}
235236

236237
Matcher matcher = portPattern.matcher(line);
@@ -250,12 +251,30 @@ String getStderrOutput() {
250251
}
251252
}
252253

254+
private void awaitStderrReader() {
255+
Thread t = this.stderrThread;
256+
if (t != null) {
257+
try {
258+
t.join(STDERR_READER_JOIN_TIMEOUT_MS);
259+
} catch (InterruptedException e) {
260+
Thread.currentThread().interrupt();
261+
}
262+
}
263+
}
264+
253265
private void clearStderrBuffer() {
254266
synchronized (stderrBuffer) {
255267
stderrBuffer.setLength(0);
256268
}
257269
}
258270

271+
static String formatCliExitedMessage(String message, String stderrOutput) {
272+
if (stderrOutput == null || stderrOutput.isEmpty()) {
273+
return message;
274+
}
275+
return message + "\nstderr: " + stderrOutput;
276+
}
277+
259278
private List<String> resolveCliCommand(String cliPath, List<String> args) {
260279
boolean isJsFile = cliPath.toLowerCase().endsWith(".js");
261280

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ public final class CopilotClient implements AutoCloseable {
7676
* shutdown via {@link #stop()}.
7777
*/
7878
public static final int AUTOCLOSEABLE_TIMEOUT_SECONDS = 10;
79+
private static final int FORCE_KILL_TIMEOUT_SECONDS = 10;
7980
private final CopilotClientOptions options;
8081
private final CliServerManager serverManager;
8182
private final LifecycleEventManager lifecycleManager = new LifecycleEventManager();
@@ -216,7 +217,8 @@ private Connection startCoreBody() {
216217
} catch (Exception e) {
217218
String stderr = serverManager.getStderrOutput();
218219
if (!stderr.isEmpty()) {
219-
throw new CompletionException(new IOException("CLI process exited unexpectedly. stderr: " + stderr, e));
220+
throw new CompletionException(new IOException(
221+
CliServerManager.formatCliExitedMessage("CLI process exited unexpectedly.", stderr), e));
220222
}
221223
throw new CompletionException(e);
222224
}
@@ -244,7 +246,7 @@ private void verifyProtocolVersion(Connection connection) throws Exception {
244246
while (cause instanceof java.util.concurrent.ExecutionException || cause instanceof CompletionException) {
245247
cause = cause.getCause();
246248
}
247-
if (cause instanceof JsonRpcException rpcEx && rpcEx.getCode() == METHOD_NOT_FOUND_ERROR_CODE) {
249+
if (cause instanceof JsonRpcException rpcEx && isUnsupportedConnectMethod(rpcEx)) {
248250
// Legacy server without 'connect'; fall back to 'ping'.
249251
// A token, if any, is silently dropped — the legacy server can't enforce one.
250252
var params = new HashMap<String, Object>();
@@ -270,6 +272,10 @@ private void verifyProtocolVersion(Connection connection) throws Exception {
270272
}
271273
}
272274

275+
private static boolean isUnsupportedConnectMethod(JsonRpcException ex) {
276+
return ex.getCode() == METHOD_NOT_FOUND_ERROR_CODE || "Unhandled method connect".equals(ex.getMessage());
277+
}
278+
273279
/**
274280
* Disconnects from the Copilot server and closes all active sessions.
275281
* <p>
@@ -348,8 +354,14 @@ private CompletableFuture<Void> cleanupConnection() {
348354
if (connection.process != null) {
349355
try {
350356
if (connection.process.isAlive()) {
351-
connection.process.destroyForcibly();
357+
Process destroyedProcess = connection.process.destroyForcibly();
358+
if (!destroyedProcess.waitFor(FORCE_KILL_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
359+
LOG.fine("Process did not terminate within force kill timeout");
360+
}
352361
}
362+
} catch (InterruptedException e) {
363+
Thread.currentThread().interrupt();
364+
LOG.log(Level.FINE, "Interrupted while killing process", e);
353365
} catch (Exception e) {
354366
LOG.log(Level.FINE, "Error killing process", e);
355367
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
*--------------------------------------------------------------------------------------------*/
4+
5+
package com.github.copilot.sdk;
6+
7+
import static org.junit.jupiter.api.Assertions.*;
8+
9+
import java.util.ArrayList;
10+
import java.util.List;
11+
import java.util.concurrent.TimeUnit;
12+
13+
import org.junit.jupiter.api.AfterAll;
14+
import org.junit.jupiter.api.BeforeAll;
15+
import org.junit.jupiter.api.Test;
16+
17+
import com.github.copilot.sdk.generated.AssistantUsageEvent;
18+
import com.github.copilot.sdk.generated.SessionEvent;
19+
import com.github.copilot.sdk.generated.SessionUsageInfoEvent;
20+
import com.github.copilot.sdk.json.MessageOptions;
21+
import com.github.copilot.sdk.json.PermissionHandler;
22+
import com.github.copilot.sdk.json.SessionConfig;
23+
24+
/**
25+
* E2E tests for event fidelity — verifying the shape, ordering, and presence of
26+
* key events emitted from the runtime.
27+
*
28+
* <p>
29+
* Snapshots are stored in {@code test/snapshots/event_fidelity/}.
30+
* </p>
31+
*/
32+
public class EventFidelityTest {
33+
34+
private static E2ETestContext ctx;
35+
36+
@BeforeAll
37+
static void setup() throws Exception {
38+
ctx = E2ETestContext.create();
39+
}
40+
41+
@AfterAll
42+
static void teardown() throws Exception {
43+
if (ctx != null) {
44+
ctx.close();
45+
}
46+
}
47+
48+
/**
49+
* Verifies that an {@code assistant.usage} event is emitted after the model
50+
* processes a prompt.
51+
*
52+
* @see Snapshot:
53+
* event_fidelity/should_emit_assistant_usage_event_after_model_call
54+
*/
55+
@Test
56+
void testShouldEmitAssistantUsageEventAfterModelCall() throws Exception {
57+
ctx.configureForTest("event_fidelity", "should_emit_assistant_usage_event_after_model_call");
58+
59+
try (CopilotClient client = ctx.createClient()) {
60+
CopilotSession session = client
61+
.createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get();
62+
63+
List<SessionEvent> events = new ArrayList<>();
64+
session.on(events::add);
65+
66+
session.sendAndWait(new MessageOptions().setPrompt("What is 5+5? Reply with just the number.")).get(60,
67+
TimeUnit.SECONDS);
68+
69+
List<AssistantUsageEvent> usageEvents = events.stream().filter(e -> e instanceof AssistantUsageEvent)
70+
.map(e -> (AssistantUsageEvent) e).toList();
71+
72+
assertFalse(usageEvents.isEmpty(), "Should have received an assistant.usage event after model call");
73+
74+
AssistantUsageEvent lastUsage = usageEvents.get(usageEvents.size() - 1);
75+
assertNotNull(lastUsage.getData().model(), "Usage event should have a model field");
76+
assertFalse(lastUsage.getData().model().isEmpty(), "Model field should not be empty");
77+
78+
session.close();
79+
}
80+
}
81+
82+
/**
83+
* Verifies that a {@code session.usage_info} event is emitted after the model
84+
* processes a prompt.
85+
*
86+
* @see Snapshot:
87+
* event_fidelity/should_emit_session_usage_info_event_after_model_call
88+
*/
89+
@Test
90+
void testShouldEmitSessionUsageInfoEventAfterModelCall() throws Exception {
91+
ctx.configureForTest("event_fidelity", "should_emit_session_usage_info_event_after_model_call");
92+
93+
try (CopilotClient client = ctx.createClient()) {
94+
CopilotSession session = client
95+
.createSession(new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get();
96+
97+
List<SessionEvent> events = new ArrayList<>();
98+
session.on(events::add);
99+
100+
session.sendAndWait(new MessageOptions().setPrompt("What is 5+5? Reply with just the number.")).get(60,
101+
TimeUnit.SECONDS);
102+
103+
List<SessionUsageInfoEvent> usageInfoEvents = events.stream()
104+
.filter(e -> e instanceof SessionUsageInfoEvent).map(e -> (SessionUsageInfoEvent) e).toList();
105+
106+
assertFalse(usageInfoEvents.isEmpty(), "Should have received a session.usage_info event after model call");
107+
108+
session.close();
109+
}
110+
}
111+
}

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

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,4 +363,115 @@ void testShouldDenyToolOperationsWhenHandlerExplicitlyDeniesAfterResume(TestInfo
363363
session2.close();
364364
}
365365
}
366+
367+
/**
368+
* Verifies that a permission handler returning {@code noResult} is handled
369+
* correctly — the handler is called, and the session can be aborted afterward.
370+
*
371+
* @see Snapshot: permissions/should_deny_permission_with_noresult_kind
372+
*/
373+
@Test
374+
void testShouldDenyPermissionWithNoResultKind() throws Exception {
375+
ctx.configureForTest("permissions", "should_deny_permission_with_noresult_kind");
376+
377+
var permissionCalled = new CompletableFuture<Boolean>();
378+
379+
try (CopilotClient client = ctx.createClient()) {
380+
CopilotSession session = client
381+
.createSession(new SessionConfig().setOnPermissionRequest((request, invocation) -> {
382+
permissionCalled.complete(true);
383+
return CompletableFuture.completedFuture(
384+
new PermissionRequestResult().setKind(PermissionRequestResultKind.NO_RESULT));
385+
})).get();
386+
387+
session.send(new MessageOptions().setPrompt("Run 'node --version'"));
388+
389+
assertTrue(permissionCalled.get(30, TimeUnit.SECONDS),
390+
"Expected the no-result permission handler to be called.");
391+
392+
session.abort().get(10, TimeUnit.SECONDS);
393+
session.close();
394+
}
395+
}
396+
397+
/**
398+
* Verifies that the runtime short-circuits the permission handler when
399+
* {@code session.permissions.setApproveAll(true)} has been called.
400+
*
401+
* @see Snapshot:
402+
* permissions/should_short_circuit_permission_handler_when_set_approve_all_enabled
403+
*/
404+
@Test
405+
void testShouldShortCircuitPermissionHandlerWhenSetApproveAllEnabled() throws Exception {
406+
ctx.configureForTest("permissions", "should_short_circuit_permission_handler_when_set_approve_all_enabled");
407+
408+
var handlerCallCount = new int[]{0};
409+
410+
try (CopilotClient client = ctx.createClient()) {
411+
CopilotSession session = client
412+
.createSession(new SessionConfig().setOnPermissionRequest((request, invocation) -> {
413+
handlerCallCount[0]++;
414+
return CompletableFuture.completedFuture(
415+
new PermissionRequestResult().setKind(PermissionRequestResultKind.APPROVED));
416+
})).get();
417+
418+
// Set approve-all so the runtime short-circuits
419+
var setResult = session.getRpc().permissions
420+
.setApproveAll(new com.github.copilot.sdk.generated.rpc.SessionPermissionsSetApproveAllParams(
421+
session.getSessionId(), true))
422+
.get(10, TimeUnit.SECONDS);
423+
assertTrue(setResult.success(), "setApproveAll should succeed");
424+
425+
AssistantMessageEvent response = session
426+
.sendAndWait(new MessageOptions().setPrompt("Run 'echo test' and tell me what happens"))
427+
.get(60, TimeUnit.SECONDS);
428+
assertNotNull(response);
429+
430+
// Handler should not have been called since runtime approves all
431+
assertEquals(0, handlerCallCount[0],
432+
"Permission handler should not be called when setApproveAll is enabled");
433+
434+
session.close();
435+
}
436+
}
437+
438+
/**
439+
* Verifies that the SDK correctly waits for a slow permission handler before
440+
* completing tool execution.
441+
*
442+
* @see Snapshot: permissions/should_wait_for_slow_permission_handler
443+
*/
444+
@Test
445+
void testShouldWaitForSlowPermissionHandler() throws Exception {
446+
ctx.configureForTest("permissions", "should_wait_for_slow_permission_handler");
447+
448+
var handlerEntered = new CompletableFuture<Void>();
449+
var releaseHandler = new CompletableFuture<Void>();
450+
451+
try (CopilotClient client = ctx.createClient()) {
452+
CopilotSession session = client
453+
.createSession(new SessionConfig().setOnPermissionRequest((request, invocation) -> {
454+
handlerEntered.complete(null);
455+
return releaseHandler.thenApply(
456+
v -> new PermissionRequestResult().setKind(PermissionRequestResultKind.APPROVED));
457+
})).get();
458+
459+
// Capture the sendAndWait future before awaiting it so we can interact with the
460+
// handler
461+
CompletableFuture<AssistantMessageEvent> responseFuture = session
462+
.sendAndWait(new MessageOptions().setPrompt("Run 'echo slow_handler_test'"));
463+
464+
// Wait for permission handler to be entered
465+
handlerEntered.get(30, TimeUnit.SECONDS);
466+
467+
// Release the handler
468+
releaseHandler.complete(null);
469+
470+
// Session should complete successfully
471+
AssistantMessageEvent message = responseFuture.get(60, TimeUnit.SECONDS);
472+
assertNotNull(message);
473+
474+
session.close();
475+
}
476+
}
366477
}

0 commit comments

Comments
 (0)