Address review feedback: use generated RPC types, allow null copilotHome, strengthen test assertions by Copilot · Pull Request #151 · github/copilot-sdk-java

Copilot · 2026-05-04T21:05:32Z

Before the change?

CopilotClient.verifyProtocolVersion used a hand-rolled ConnectResult record and an untyped HashMap to call the connect RPC, risking drift with the schema and sending token: null when no token applies.
CopilotClientOptions.setCopilotHome rejected null with Objects.requireNonNull, inconsistent with the .NET and Node.js reference implementations which allow null to mean "use CLI default".
testParsePermissionCompletedEvent only asserted result was non-null after the field changed to Object, providing weak coverage.
The instructionDirectories wiring in SessionRequestBuilder had no active unit test coverage (E2E tests were the only coverage).

After the change?

Replaced the duplicated ConnectResult record and untyped Map with the generated ConnectParams and generated.rpc.ConnectResult types. Since ConnectParams uses @JsonInclude(NON_NULL), a null token is simply omitted from the wire payload.
setCopilotHome now accepts null (matching .NET/Node.js behavior). Javadoc documents that null means "don't set COPILOT_HOME on the spawned process, letting the CLI use its default (~/.copilot)".
testParsePermissionCompletedEvent now casts result to Map<String, Object> and asserts kind equals "approved", matching the pattern used in testParsePermissionRequestedEvent.
Added testBuildCreateRequestPropagatesInstructionDirectories and testBuildResumeRequestPropagatesInstructionDirectories in SessionRequestBuilderTest to verify propagation into wire requests.

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

… reference implementation Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

…ries Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

…tionDirectories 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>

Auto-committed by codegen-check workflow.

edburns · 2026-05-04T22:24:23Z

@copilot Fix the 3 remaining test compilation errors. Do NOT edit any file under src/generated/java/ — those are auto-generated and correct.

The errors (from the Codegen Check run):

src/test/java/com/github/copilot/sdk/SessionEventDeserializationTest.java line 2278 — calls .kind() on an object that no longer has that accessor. Check the generated record to find the correct accessor name or remove the assertion.
src/test/java/com/github/copilot/sdk/SessionEventHandlingTest.java line 867 — AssistantMessageEventData constructor now requires 12 parameters (added a new first String field). Update the test to pass the correct number of arguments (use null for the new fields).
src/test/java/com/github/copilot/sdk/generated/rpc/GeneratedRpcRecordsCoverageTest.java line 835 — SessionUsageGetMetricsResult constructor now requires 11 parameters (gained Map<String, UsageMetricsTokenDetail> and Map<String, UsageMetricsModelMetric> fields). Update the test to pass the correct number of arguments.

After fixing, run mvn verify to confirm BUILD SUCCESS, then run mvn spotless:apply and commit.

Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Copilot · 2026-05-04T22:31:28Z

@copilot Fix the 3 remaining test compilation errors. Do NOT edit any file under src/generated/java/ — those are auto-generated and correct.

The errors (from the Codegen Check run):
...

Fixed all 3 compilation errors in fce9527:

SessionEventDeserializationTest.java:2278 — Replaced .result().kind() with assertNotNull(event.getData().result()) since result is now typed as Object.
SessionEventHandlingTest.java:867 — Added 12th null parameter for the new messageId field in AssistantMessageEventData.
GeneratedRpcRecordsCoverageTest.java:835 — Added null for the new totalNanoAiu, tokenDetails, and modelMetrics fields in SessionUsageGetMetricsResult (now 11 params).

mvn verify passes with BUILD SUCCESS.

Copilot

Pull request overview

Ports several configuration and protocol features from the upstream reference implementation into the Java SDK, improving CLI process configuration, TCP security, and session instruction discovery while aligning tests/docs with updated schemas.

Changes:

Add copilotHome (sets COPILOT_HOME) and tcpConnectionToken (supports authenticated TCP via connect, with legacy ping fallback).
Add instructionDirectories support to both session create and resume requests.
Update docs/tests and regenerated schema types to match the newer reference implementation version.

Show a summary per file

File	Description
src/main/java/com/github/copilot/sdk/CopilotClient.java	Adds TCP connection token validation, auto-token generation for SDK-spawned TCP, and `connect`-based protocol handshake with fallback to `ping`.
src/main/java/com/github/copilot/sdk/CliServerManager.java	Passes `COPILOT_HOME` and `COPILOT_CONNECTION_TOKEN` environment variables to spawned CLI process.
src/main/java/com/github/copilot/sdk/SessionRequestBuilder.java	Wires `instructionDirectories` into `session.create` and `session.resume` request payloads.
src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java	Adds `copilotHome` and `tcpConnectionToken` options and cloning support.
src/main/java/com/github/copilot/sdk/json/SessionConfig.java	Adds `instructionDirectories` field, accessors, and cloning support.
src/main/java/com/github/copilot/sdk/json/ResumeSessionConfig.java	Adds `instructionDirectories` field, accessors, and cloning support.
src/main/java/com/github/copilot/sdk/json/CreateSessionRequest.java	Adds `instructionDirectories` JSON property to create-session wire request.
src/main/java/com/github/copilot/sdk/json/ResumeSessionRequest.java	Adds `instructionDirectories` JSON property to resume-session wire request.
src/test/java/com/github/copilot/sdk/CopilotClientTest.java	Adds tests for `tcpConnectionToken` validation and `copilotHome` option persistence.
src/test/java/com/github/copilot/sdk/ConfigCloneTest.java	Ensures new options/config fields clone correctly and list independence is preserved.
src/test/java/com/github/copilot/sdk/SessionEventHandlingTest.java	Updates event construction to match new generated `AssistantMessageEventData` signature.
src/test/java/com/github/copilot/sdk/SessionEventDeserializationTest.java	Adjusts permission-completed assertions after schema changed `result` to `Object`.
src/test/java/com/github/copilot/sdk/generated/rpc/GeneratedRpcRecordsCoverageTest.java	Updates generated RPC record coverage test for new `SessionUsageGetMetricsResult` fields/order.
src/test/java/com/github/copilot/sdk/SessionConfigE2ETest.java	Adds (currently disabled) E2E tests intended to validate `instructionDirectories` behavior.
src/site/markdown/setup.md	Documents TCP connection token usage and adds `copilotHome`/`tcpConnectionToken` to the options table.
src/site/markdown/advanced.md	Documents `instructionDirectories` usage for create/resume.
scripts/codegen/package.json	Bumps `@github/copilot` used for code generation inputs.
scripts/codegen/package-lock.json	Locks updated `@github/copilot` dependency graph.
pom.xml	Updates the tracked reference-implementation CLI version property.
.lastmerge	Updates the recorded reference-implementation commit hash.
src/generated/java/com/github/copilot/sdk/generated/UserMessageEvent.java	Regenerated: adds `parentAgentTaskId`.
src/generated/java/com/github/copilot/sdk/generated/ToolExecutionStartEvent.java	Regenerated: adds `turnId`.
src/generated/java/com/github/copilot/sdk/generated/ToolExecutionCompleteEvent.java	Regenerated: adds `turnId`.
src/generated/java/com/github/copilot/sdk/generated/ShutdownTokenDetail.java	Regenerated: new type for shutdown token details.
src/generated/java/com/github/copilot/sdk/generated/ShutdownModelMetricTokenDetail.java	Regenerated: new type for shutdown model token details.
src/generated/java/com/github/copilot/sdk/generated/ShutdownModelMetric.java	Regenerated: adds nano-AI units + token detail map.
src/generated/java/com/github/copilot/sdk/generated/SessionShutdownEvent.java	Regenerated: adds nano-AI units + token details.
src/generated/java/com/github/copilot/sdk/generated/SessionResumeEvent.java	Regenerated: adds `sessionWasActive` and `continuePendingWork`.
src/generated/java/com/github/copilot/sdk/generated/SessionEvent.java	Regenerated: registers new `assistant.message_start` subtype.
src/generated/java/com/github/copilot/sdk/generated/rpc/UsageMetricsTokenDetail.java	Regenerated: new type for token details.
src/generated/java/com/github/copilot/sdk/generated/rpc/UsageMetricsModelMetricTokenDetail.java	Regenerated: new type for per-model token details.
src/generated/java/com/github/copilot/sdk/generated/rpc/UsageMetricsModelMetric.java	Regenerated: adds nano-AI units + token detail map.
src/generated/java/com/github/copilot/sdk/generated/rpc/SessionUsageGetMetricsResult.java	Regenerated: adds nano-AI units + token details at session scope.
src/generated/java/com/github/copilot/sdk/generated/rpc/SessionSuspendParams.java	Regenerated: new params record for `session.suspend`.
src/generated/java/com/github/copilot/sdk/generated/rpc/SessionRpc.java	Regenerated: adds `session.suspend()` helper.
src/generated/java/com/github/copilot/sdk/generated/rpc/ServerRpc.java	Regenerated: adds `connect()` helper.
src/generated/java/com/github/copilot/sdk/generated/rpc/ConnectResult.java	Regenerated: new `connect` result schema.
src/generated/java/com/github/copilot/sdk/generated/rpc/ConnectParams.java	Regenerated: new `connect` params schema.
src/generated/java/com/github/copilot/sdk/generated/PermissionCompletedEvent.java	Regenerated: changes `result` type to `Object`.
src/generated/java/com/github/copilot/sdk/generated/McpOauthRequiredStaticClientConfig.java	Regenerated: adds optional `grantType`.
src/generated/java/com/github/copilot/sdk/generated/CompactionCompleteCompactionTokensUsedCopilotUsage.java	Regenerated: doc wording update for nano-AI units.
src/generated/java/com/github/copilot/sdk/generated/AssistantUsageCopilotUsage.java	Regenerated: doc wording update for nano-AI units.
src/generated/java/com/github/copilot/sdk/generated/AssistantMessageStartEvent.java	Regenerated: new `assistant.message_start` event type.
src/generated/java/com/github/copilot/sdk/generated/AssistantMessageEvent.java	Regenerated: adds `turnId`.

Copilot's findings

Files not reviewed (1)

scripts/codegen/package-lock.json: Language not supported

Files reviewed: 18/44 changed files
Comments generated: 6

edburns

@copilot The instructionDirectories test snapshots from github/copilot-sdk PR #1190 have been merged. Remove the @disabled annotations from both tests in SessionConfigE2ETest.java and run mvn verify to confirm they pass. The test harness in target/copilot-sdk/ may need to be refreshed first — run mvn generate-test-resources to re-clone it with the latest snapshots.

edburns · 2026-05-04T22:57:01Z

@copilot The instructionDirectories test snapshots from github/copilot-sdk PR #1190 have been merged. Remove the Disabled annotations from both tests in SessionConfigE2ETest.java and run mvn verify to confirm they pass. The test harness in target/copilot-sdk/ may need to be refreshed first — run mvn generate-test-resources to re-clone it with the latest snapshots.

…ange parsing Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/cad2855d-0ec3-43e1-acef-49e96848b902 Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Copilot · 2026-05-04T23:11:16Z

@copilot The instructionDirectories test snapshots from github/copilot-sdk PR #1190 have been merged. Remove the Disabled annotations from both tests in SessionConfigE2ETest.java and...

Done in 79accd3. Removed both @Disabled annotations, refreshed the test harness via mvn generate-test-resources, and fixed the getSystemMessage helper to correctly navigate the exchange structure (exchange.request.messages[] rather than exchange.messages[]). Both tests pass.

@copilot The instructionDirectories test snapshots from github/copilot-sdk PR #1190 have been merged. Remove the @disabled annotations from both tests in SessionConfigE2ETest.java and run mvn verify...

Done in 79accd3. Removed both @Disabled annotations, refreshed the test harness via mvn generate-test-resources, and fixed the getSystemMessage helper to correctly navigate the exchange structure (exchange.request.messages[] rather than exchange.messages[]). Both tests pass.

…llow null copilotHome, strengthen permission test assertion, add instructionDirectories unit tests Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/bf21fd78-4e30-40e7-8e6b-536294818f16 Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Initial plan

5620113

Copilot AI assigned Copilot and edburns May 4, 2026

Copilot started work on behalf of edburns May 4, 2026 21:05 View session

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

[reference-impl-sync] Reference Implementation sync: 15 new commits (2026-05-04) #150

Closed

Copilot AI and others added 4 commits May 4, 2026 21:20

Port copilotHome, tcpConnectionToken, and instructionDirectories from…

09425c2

… reference implementation Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Add tests for copilotHome, tcpConnectionToken, and instructionDirecto…

637b51b

…ries Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Update documentation for copilotHome, tcpConnectionToken, and instruc…

d542b0f

…tionDirectories Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Update .lastmerge to ced6613253a595769d2e77547f9e1caf6bef6438, sync p…

2423d8b

…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 15 new commits~~ Port copilotHome, tcpConnectionToken, and instructionDirectories from reference implementation May 4, 2026

Copilot finished work on behalf of edburns May 4, 2026 21:41

Copilot AI requested a review from edburns May 4, 2026 21:41

Regenerate codegen output

e9c17ba

Auto-committed by codegen-check workflow.

Copilot started work on behalf of edburns May 4, 2026 22:24 View session

Fix test compilation errors for updated generated record signatures

fce9527

Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Copilot finished work on behalf of edburns May 4, 2026 22:32

edburns marked this pull request as ready for review May 4, 2026 22:38

Copilot AI review requested due to automatic review settings May 4, 2026 22:38

Copilot started reviewing on behalf of edburns May 4, 2026 22:38 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

edburns approved these changes May 4, 2026

View reviewed changes

Copilot started work on behalf of edburns May 4, 2026 22:57 View session

Enable instructionDirectories E2E tests and fix getSystemMessage exch…

79accd3

…ange parsing Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/cad2855d-0ec3-43e1-acef-49e96848b902 Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Copilot AI requested a review from edburns May 4, 2026 23:12

Copilot finished work on behalf of edburns May 4, 2026 23:12

Copilot started work on behalf of edburns May 4, 2026 23:59 View session

Copilot AI changed the title ~~Port copilotHome, tcpConnectionToken, and instructionDirectories from reference implementation~~ Address review feedback: use generated RPC types, allow null copilotHome, strengthen test assertions May 5, 2026

Copilot finished work on behalf of edburns May 5, 2026 00:11

edburns approved these changes May 5, 2026

View reviewed changes

edburns merged commit 923f3ec into main May 5, 2026
13 of 14 checks passed

edburns deleted the copilot/reference-implementation-sync branch May 5, 2026 00:33

edburns mentioned this pull request May 5, 2026

Merge reference implementation SDK changes (2026-05-04) #154

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address review feedback: use generated RPC types, allow null copilotHome, strengthen test assertions#151

Address review feedback: use generated RPC types, allow null copilotHome, strengthen test assertions#151
edburns merged 9 commits intomainfrom
copilot/reference-implementation-sync

Copilot AI commented May 4, 2026 •

edited

Loading

Uh oh!

edburns commented May 4, 2026

Uh oh!

Copilot AI commented May 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

edburns left a comment

Uh oh!

edburns commented May 4, 2026

Uh oh!

Copilot AI commented May 4, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented May 4, 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!

edburns commented May 4, 2026

Uh oh!

Copilot AI commented May 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

edburns left a comment

Choose a reason for hiding this comment

Uh oh!

edburns commented May 4, 2026

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented May 4, 2026 •

edited

Loading

Copilot AI commented May 4, 2026 •

edited

Loading