Skip to content

Merge reference implementation SDK changes (2026-05-05)#157

Draft
Copilot wants to merge 3 commits intomainfrom
copilot/reference-impl-sync-expand-sdk-e2e-coverage
Draft

Merge reference implementation SDK changes (2026-05-05)#157
Copilot wants to merge 3 commits intomainfrom
copilot/reference-impl-sync-expand-sdk-e2e-coverage

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 5, 2026

Resolves #157


Before the change?

  • Process cleanup called destroyForcibly() without waiting for the process tree to terminate
  • Connect method fallback only matched the JSON-RPC METHOD_NOT_FOUND error code, missing servers that return "Unhandled method connect"
  • Error messages with stderr context were formatted inline with inconsistent patterns
  • Port announcement could throw before stderr reader finished, losing error context

After the change?

  • Process cleanup: destroyForcibly().waitFor() ensures process tree is fully terminated before continuing
  • Connect fallback: isUnsupportedConnectMethod() matches both error code -32601 and "Unhandled method connect" message
  • Error formatting: Extracted formatCliExitedMessage() helper used consistently across CopilotClient and CliServerManager
  • Stderr reader: awaitStderrReader() joins the reader thread before reading the buffer in port announcement, ensuring complete error context
  • 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).

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

Copilot AI and others added 2 commits May 5, 2026 01:19
…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 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants