Merge reference implementation SDK changes (2026-05-05) and address review feedback by Copilot · Pull Request #157 · github/copilot-sdk-java

Copilot · 2026-05-05T00:54:16Z

Before the change?

Process cleanup used force-kill behavior that could swallow interrupts and block indefinitely while waiting for process termination
Connect method fallback only matched the JSON-RPC METHOD_NOT_FOUND error code, missing servers that return "Unhandled method connect"
CLI-exit stderr formatting created a bidirectional dependency between CopilotClient and CliServerManager
The parallel tools E2E test only proved both tools were called, not that execution overlapped
One permission test comment described non-blocking send() behavior while using sendAndWait(...)

After the change?

Process cleanup hardening: force-kill now uses a bounded waitFor(timeout, unit), logs timeout cases, and preserves thread interrupt status when interrupted
Connect fallback: isUnsupportedConnectMethod() matches both error code -32601 and "Unhandled method connect" message
Error formatting/layering: CLI-exit message formatting is centralized in CliServerManager and used by CopilotClient, removing the reverse class dependency
Stderr reader: awaitStderrReader() joins the reader thread before reading the buffer in port announcement, ensuring complete error context
Parallel tool verification: the E2E test now uses latches/overlap tracking to assert handlers execute in overlapping time, not just that both were invoked
Permission test clarity: updated the slow-handler test comment to match sendAndWait(...) future usage
New E2E tests: EventFidelityTest, ToolResultsTest, plus extensions to StreamingFidelityTest, ToolsTest, and PermissionsTest covering parallel tools, rejected/denied result types, setApproveAll short-circuit, reasoning effort with streaming, and slow permission handlers

Ports source changes from c063458 (Expand SDK E2E runtime coverage), with follow-up fixes from review feedback.

Pull request checklist

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

Does this introduce a breaking change?

Yes
No

…ack, and E2E test coverage Source changes from c063458 (Expand SDK E2E runtime coverage): - Add waitFor() after destroyForcibly() in process cleanup - Expand connect method fallback to match 'Unhandled method connect' message - Extract formatCliExitedMessage helper for consistent error formatting - Wait for stderr reader before throwing in port announcement New E2E tests ported: - EventFidelityTest: assistant.usage and session.usage_info event tests - ToolResultsTest: rejected and denied resultType handling - StreamingFidelityTest: streaming disabled on resume, reasoning effort - ToolsTest: parallel tools, availableTools/excludedTools combined - PermissionsTest: noResult kind, setApproveAll, slow handler Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

…om.xml CLI version, and update scripts/codegen @github/copilot version Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Copilot

Pull request overview

Ports updates from the upstream reference implementation to improve Copilot CLI process lifecycle robustness, broaden legacy server compatibility during connection setup, standardize CLI stderr error formatting, and expand E2E runtime coverage.

Changes:

Improve CLI process shutdown and stderr-context error reporting (including waiting for stderr reader before throwing on missing port announcement).
Expand connect fallback detection for legacy servers.
Add/extend E2E tests covering event fidelity, tool result types, streaming resume behavior, permission edge-cases, and tool availability/exclusion behavior.

Show a summary per file

File	Description
`src/main/java/com/github/copilot/sdk/CopilotClient.java`	Standardizes stderr-inclusive error formatting, expands legacy `connect` fallback detection, and changes process teardown to wait for termination.
`src/main/java/com/github/copilot/sdk/CliServerManager.java`	Tracks stderr reader thread and waits briefly for it to finish before reading stderr on early process exit; reuses shared error formatting.
`src/test/java/com/github/copilot/sdk/ToolsTest.java`	Adds E2E coverage for multi-tool usage in a single turn and combined `availableTools` + `excludedTools` behavior.
`src/test/java/com/github/copilot/sdk/ToolResultsTest.java`	Adds E2E tests validating tool executions reported as failed for `rejected`/`denied` tool result types.
`src/test/java/com/github/copilot/sdk/StreamingFidelityTest.java`	Extends streaming tests to cover resume-with-streaming-disabled and streaming with `reasoningEffort`.
`src/test/java/com/github/copilot/sdk/PermissionsTest.java`	Extends permission tests to cover NO_RESULT handling, approve-all short-circuiting, and slow permission handlers.
`src/test/java/com/github/copilot/sdk/EventFidelityTest.java`	Adds E2E tests asserting presence of key usage-related events after model calls.
`.lastmerge`	Updates the recorded upstream reference-implementation merge base.

Copilot's findings

Files reviewed: 8/8 changed files
Comments generated: 5

Copilot · 2026-05-05T18:47:49Z

                    if (connection.process.isAlive()) {
-                        connection.process.destroyForcibly();
+                        connection.process.destroyForcibly().waitFor();
                    }


Addressed in ee55571 (refined in a9c88d7): cleanupConnection() now catches InterruptedException separately, restores the interrupt flag, and logs at FINE level.

Copilot · 2026-05-05T18:47:54Z

                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(
+                            CopilotClient.formatCliExitedMessage("CLI process exited unexpectedly.", stderr));
                }


Addressed in ee55571: moved CLI exit message formatting into CliServerManager and updated CopilotClient to call it, removing the reverse dependency.

Copilot · 2026-05-05T18:47:57Z

+    /**
+     * Verifies that the model can call multiple custom tools in parallel within a
+     * single turn.
+     *
+     * @see Snapshot:
+     *      tools/should_execute_multiple_custom_tools_in_parallel_single_turn
+     */
+    @Test
+    void testShouldExecuteMultipleCustomToolsInParallelSingleTurn() throws Exception {
+        ctx.configureForTest("tools", "should_execute_multiple_custom_tools_in_parallel_single_turn");
+
+        var toolACalled = new CompletableFuture<String>();
+        var toolBCalled = new CompletableFuture<String>();
+
+        Map<String, Object> cityParams = Map.of("type", "object", "properties",
+                Map.of("city", Map.of("type", "string", "description", "City name")), "required", List.of("city"));
+        Map<String, Object> countryParams = Map.of("type", "object", "properties",
+                Map.of("country", Map.of("type", "string", "description", "Country name")), "required",
+                List.of("country"));
+
+        ToolDefinition lookupCity = ToolDefinition.create("lookup_city", "Looks up city information", cityParams,
+                (invocation) -> {
+                    String city = (String) invocation.getArguments().get("city");
+                    toolACalled.complete(city);
+                    return CompletableFuture.completedFuture("CITY_" + city.toUpperCase());
+                });
+
+        ToolDefinition lookupCountry = ToolDefinition.create("lookup_country", "Looks up country information",
+                countryParams, (invocation) -> {
+                    String country = (String) invocation.getArguments().get("country");
+                    toolBCalled.complete(country);
+                    return CompletableFuture.completedFuture("COUNTRY_" + country.toUpperCase());
+                });
+
+        try (CopilotClient client = ctx.createClient()) {
+            CopilotSession session = client.createSession(new SessionConfig()
+                    .setTools(List.of(lookupCity, lookupCountry)).setOnPermissionRequest(PermissionHandler.APPROVE_ALL))
+                    .get();
+
+            AssistantMessageEvent response = session.sendAndWait(new MessageOptions().setPrompt(
+                    "Use lookup_city with 'Paris' and lookup_country with 'France' at the same time, then combine both results in your reply."))
+                    .get(60, TimeUnit.SECONDS);
+
+            // Both tools should have been called
+            assertEquals("Paris", toolACalled.get(10, TimeUnit.SECONDS));
+            assertEquals("France", toolBCalled.get(10, TimeUnit.SECONDS));
+


Addressed in ee55571: the test now uses latches/overlap tracking to require both handlers to start and overlap before release, so it validates parallel execution behavior.

Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/38b90523-b5be-4dbd-9d33-36361110399b Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Initial plan

7b80d72

Copilot AI assigned Copilot and edburns May 5, 2026

Copilot started work on behalf of edburns May 5, 2026 00:54 View session

Copilot AI linked an issue May 5, 2026 that may be closed by this pull request

[reference-impl-sync] Reference Implementation sync: 1 new commits (2026-05-05) #156

Closed

Copilot AI and others added 2 commits May 5, 2026 01:19

Update .lastmerge to c063458ecc3d606766f04cf203b11b08de672cc8, sync p…

0a2157c

…om.xml CLI version, and update scripts/codegen @github/copilot version Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Sync Reference Implementation with new commits from Copilot SDK~~ Merge reference implementation SDK changes (2026-05-05) May 5, 2026

Copilot finished work on behalf of edburns May 5, 2026 01:23

Copilot AI requested a review from edburns May 5, 2026 01:23

edburns marked this pull request as ready for review May 5, 2026 18:17

Copilot AI review requested due to automatic review settings May 5, 2026 18:17

Copilot started reviewing on behalf of edburns May 5, 2026 18:18 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

Copilot started work on behalf of edburns May 5, 2026 18:25 View session

Copilot AI and others added 2 commits May 5, 2026 18:43

Address PR review feedback on cleanup and tests

ee55571

Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/38b90523-b5be-4dbd-9d33-36361110399b Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Refine timeout constants from validation feedback

a9c88d7

Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/38b90523-b5be-4dbd-9d33-36361110399b Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Copilot AI changed the title ~~Merge reference implementation SDK changes (2026-05-05)~~ Merge reference implementation SDK changes (2026-05-05) and address review feedback May 5, 2026

Copilot finished work on behalf of edburns May 5, 2026 18:49

edburns approved these changes May 5, 2026

View reviewed changes

edburns merged commit 21a8f06 into main May 5, 2026
18 of 19 checks passed

edburns deleted the copilot/reference-impl-sync-expand-sdk-e2e-coverage branch May 5, 2026 19:26

This was referenced May 5, 2026

Update JaCoCo coverage badge #159

Closed

[MAINT]: Investigate and resolved failed test run after merge of #157 #160

Closed

Copilot AI mentioned this pull request May 5, 2026

Fix flaky parallel tools test by removing non-deterministic synchronization #161

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

Merge reference implementation SDK changes (2026-05-05) and address review feedback#157
edburns merged 5 commits intomainfrom
copilot/reference-impl-sync-expand-sdk-e2e-coverage

Copilot AI commented May 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 5, 2026

Uh oh!

Uh oh!

Copilot AI May 5, 2026

Uh oh!

Copilot AI May 5, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before the change?

After the change?

Pull request checklist

Does this introduce a breaking change?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Copilot AI May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented May 5, 2026 •

edited

Loading