Reference implementation sync: 4 new commits (2026-04-25)#122
Conversation
…ut, defaultAgent.excludedTools, permission kind values Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/34e41e1a-798d-4058-87fa-d6e929b2f783 Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
edburns
left a comment
There was a problem hiding this comment.
Test run https://github.com/github/copilot-sdk-java/actions/runs/24935116719 had failures. Please analyze the complete job logs to determine the cause of the failures and push additional commits to resolve them.
Use mvn clean verify as the signal for correctness. All tests must pass with no failures and no errors.
If you find you want to change any code in src/generated/java in order to make the tests pass, YOU MUST NOT DO THAT. Changes to those classes may only be affected by altering the java.ts and re-generating the classes as shown in the workflow update-copilot-dependency.yml.
Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/6e6133f4-9ebc-4ee5-b0cd-a8bccfb8e03b Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
Fixed in commit Root cause: The Copilot CLI v1.0.32 was updated to require the new permission kind values introduced in commit Changes made:
All 143 unit tests pass locally. |
|
@copilot I looked at the failures for Actions run https://github.com/github/copilot-sdk-java/actions/runs/24948129268/ . They were all of type "error": It is possible such errors are ephemeral. First, test this hypothesis by simply running another iteration of "Build & Test". If that fails again, with the same set of errors, perform a deeper analysis and push additional commits as necessary. |
…t/tool mismatch, session destroy before resume Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/1cce7322-6672-4e54-be77-b026e33610d7 Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
The failures were not ephemeral — all four were caused by real bugs in the test code (commit
|
There was a problem hiding this comment.
Pull request overview
Ports recent github/copilot-sdk protocol/config updates into the Java SDK to restore parity with CLI v1.0.32+, including per-session GitHub auth, default-agent tool hiding, and session idle timeout support, plus E2E test harness fixes.
Changes:
- Add per-session GitHub authentication (
gitHubToken) and wire it through create/resume session requests. - Add
defaultAgent.excludedToolssupport viaDefaultAgentConfigand request builder plumbing. - Update permission result kind wire values (with deprecated aliases) and adjust core/session logic + tests; add CLI
--session-idle-timeoutoption and related tests/helpers.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/github/copilot/sdk/json/SessionConfig.java | Adds defaultAgent and gitHubToken session configuration fields and clones them. |
| src/main/java/com/github/copilot/sdk/json/ResumeSessionConfig.java | Adds defaultAgent and gitHubToken for resumed sessions and clones them. |
| src/main/java/com/github/copilot/sdk/json/CreateSessionRequest.java | Adds defaultAgent + gitHubToken to the JSON-RPC create request DTO. |
| src/main/java/com/github/copilot/sdk/json/ResumeSessionRequest.java | Adds defaultAgent + gitHubToken to the JSON-RPC resume request DTO. |
| src/main/java/com/github/copilot/sdk/json/DefaultAgentConfig.java | New type supporting excludedTools for default-agent tool visibility. |
| src/main/java/com/github/copilot/sdk/json/PermissionRequestResultKind.java | Updates well-known permission kind wire values and keeps deprecated aliases. |
| src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java | Adds sessionIdleTimeoutSeconds option and clones it. |
| src/main/java/com/github/copilot/sdk/SessionRequestBuilder.java | Wires defaultAgent and gitHubToken into create/resume request construction. |
| src/main/java/com/github/copilot/sdk/CopilotSession.java | Updates permission-handling fallback kind to user-not-available. |
| src/main/java/com/github/copilot/sdk/CliServerManager.java | Passes --session-idle-timeout <seconds> when configured (> 0). |
| src/test/java/com/github/copilot/sdk/ToolsTest.java | Updates permission test to use new PermissionRequestResultKind constants. |
| src/test/java/com/github/copilot/sdk/PermissionsTest.java | Updates permission handler tests to new kind constants. |
| src/test/java/com/github/copilot/sdk/ExecutorWiringTest.java | Updates permission handler wiring test to new kind constants. |
| src/test/java/com/github/copilot/sdk/SessionHandlerTest.java | Updates assertions for new denied fallback wire value. |
| src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java | Updates assertions for new denied fallback wire value. |
| src/test/java/com/github/copilot/sdk/PermissionRequestResultKindTest.java | Updates kind value expectations + deprecated-alias behavior checks. |
| src/test/java/com/github/copilot/sdk/SessionRequestBuilderTest.java | Adds tests for defaultAgent and gitHubToken mapping into requests. |
| src/test/java/com/github/copilot/sdk/ConfigCloneTest.java | Adds clone coverage for new config fields/options. |
| src/test/java/com/github/copilot/sdk/CopilotClientTest.java | Adds basic option getter/setter tests for idle timeout. |
| src/test/java/com/github/copilot/sdk/CliServerManagerTest.java | Adds coverage to exercise idle-timeout CLI arg path. |
| src/test/java/com/github/copilot/sdk/E2ETestContext.java | Adds createClient(options), initializeProxy(), and per-token user config helper. |
| src/test/java/com/github/copilot/sdk/CapiProxy.java | Adds proxy endpoint helper to register per-token Copilot user responses. |
| src/test/java/com/github/copilot/sdk/PerSessionAuthTest.java | New E2E coverage for per-session auth identity resolution/isolation. |
| src/test/java/com/github/copilot/sdk/McpAndAgentsTest.java | Adds/adjusts default-agent excluded-tools tests and resume behavior. |
| src/test/java/com/github/copilot/sdk/CopilotSessionTest.java | Adds E2E coverage for creating session with defaultAgent.excludedTools. |
| .lastmerge | Updates tracked reference implementation commit. |
Copilot's findings
- Files reviewed: 26/26 changed files
- Comments generated: 1
| * if the request is interrupted | ||
| */ | ||
| public void setCopilotUserByToken(String token, String login, String copilotPlan, String apiUrl, | ||
| String telemetryUrl, String analyticsTrackingId) throws IOException, InterruptedException { |
There was a problem hiding this comment.
E2ETestContext#setCopilotUserByToken() does not call ensureProxyAlive() before sending the request. If the proxy process crashes/restarts between tests (or between initializeProxy() and this call), this can hit a dead proxy URL and introduce flakiness. Consider calling ensureProxyAlive() at the start of this method (similar to configureForTest()/initializeProxy()) so it always targets a live proxy instance.
| String telemetryUrl, String analyticsTrackingId) throws IOException, InterruptedException { | |
| String telemetryUrl, String analyticsTrackingId) throws IOException, InterruptedException { | |
| ensureProxyAlive(); |
There was a problem hiding this comment.
Applied the suggestion in c9241f1. Also updated the PermissionHandler Javadoc example (which used the old stale string literals "user-denied"/"user-approved") to use the new PermissionRequestResultKind.REJECTED/APPROVED constants, and fixed the advanced.md permission table to show the current wire values ("approve-once", "reject", "user-not-available", "no-result") replacing the old deprecated-alias rows. The README quick start and jbang example both already use PermissionHandler.APPROVE_ALL and needed no changes.
… advanced.md table Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/a4ec83e7-4677-4d1f-a323-5de8dbfa2ce2 Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
Ports 4 commits from the official
github/copilot-sdkto the Java SDK (922959f→dd2dcbc).Before the change?
defaultAgent.excludedTools).PermissionRequestResultKindused old wire values ("approved","denied-interactively-by-user","denied-no-approval-rule-and-could-not-request-from-user") that are no longer accepted by CLI v1.0.32.CopilotSession.javaerror-fallback paths for permission handling hardcoded the old denied string, causing all permission-related E2E tests to time out.PerSessionAuthTestfailed with HTTP 500 because the proxy's internal state was never initialized before the/copilot_internal/userendpoint was called.McpAndAgentsTest.testShouldHideExcludedToolsFromDefaultAgenttimed out because the test sent"What is 2+2?"while the snapshot expected"Do you have access to a tool called secret_tool?", and the wrong tool ("view"instead of"secret_tool") was excluded.McpAndAgentsTest.testShouldAcceptDefaultAgentConfigurationOnSessionResumefailed with "Session not found" becausesession.close()was called beforeresumeSession(), which permanently destroyed the server-side session viasession.destroyRPC.E2ETestContext.setCopilotUserByToken()did not callensureProxyAlive(), risking flakiness if the proxy process crashed between tests.PermissionHandlerJavadoc example used stale string literals ("user-denied","user-approved") that are no longer valid wire values.advanced.mdpermission table listed old/deprecated constants with incorrect wire values.After the change?
Per-session GitHub authentication (
dd2dcbc)New
gitHubTokenfield onSessionConfig/ResumeSessionConfig. When set, the runtime resolves the token into a full GitHub identity (content exclusion, model routing, quota) scoped to that session only:PermissionRequestResultKindvalues updated to match the new CLI v1.0.32 protocol:APPROVED"approved"APPROVED"approve-once"DENIED_INTERACTIVELY_BY_USER"denied-interactively-by-user"REJECTED"reject"DENIED_BY_RULES/DENIED_COULD_NOT_REQUEST_FROM_USERUSER_NOT_AVAILABLE"user-not-available"Old constants are preserved as
@Deprecatedaliases. All permission handler code — includingCopilotSession.javafallback paths and all test files (PermissionsTest,ToolsTest,ExecutorWiringTest,RpcHandlerDispatcherTest,SessionHandlerTest) — updated to use the new values.Per-agent tool visibility (
b1b0df5)New
DefaultAgentConfigclass lets you hide tools from the built-in agent while keeping them accessible to custom sub-agents:Configurable session idle timeout (
b4ef955)New
sessionIdleTimeoutSecondsoption onCopilotClientOptions. When > 0, passes--session-idle-timeout <seconds>to the CLI process:SessionFs structured error contract (
a3e273c)No Java changes required — affects only generated code and
.NET-specificSessionFsProviderrename; Java SDK has no SessionFs implementation yet.Updates
.lastmergeUpdated to
dd2dcbc439256acfb9feb2cff07c0b9c820091b8.Test and documentation fixes
E2ETestContext.setCopilotUserByToken(): Now callsensureProxyAlive()before delegating to the proxy, ensuring the method always targets a live proxy instance and eliminating a potential source of flakiness.E2ETestContext: AddedinitializeProxy()method that posts to the proxy's/configendpoint with no snapshot file, initializing its internal state so that endpoints like/copilot_internal/user(used for per-session auth resolution) are handled correctly without requiring an AI completion snapshot.PerSessionAuthTest: CallsinitializeProxy()before registering per-token user configs, ensuring the proxy is ready before the CLI attempts to resolve the per-session GitHub token.McpAndAgentsTest.testShouldHideExcludedToolsFromDefaultAgent: Corrected the prompt to"Do you have access to a tool called secret_tool? Answer yes or no."(matching the snapshot), registered asecret_toolToolDefinition, and changedexcludedToolsto["secret_tool"].McpAndAgentsTest.testShouldAcceptDefaultAgentConfigurationOnSessionResume: Removed the prematuresession.close()call beforeresumeSession().close()invokessession.destroyRPC which removes the server-side session; the session handle is now simply abandoned so the CLI keeps the session alive for the resume.PermissionHandlerJavadoc: Updated example to usePermissionRequestResultKind.REJECTEDandPermissionRequestResultKind.APPROVEDconstants instead of the stale and now-incorrect string literals"user-denied"/"user-approved".advanced.mdpermission table: Replaced deprecated-alias rows with the four current constants and their correct wire values:APPROVED→"approve-once",REJECTED→"reject",USER_NOT_AVAILABLE→"user-not-available",NO_RESULT→"no-result". The README quick start andjbang-example.javaboth already usePermissionHandler.APPROVE_ALLand required no changes.Pull request checklist
mvn spotless:applyhas been run to format the codemvn clean verifypasses locallyDoes this introduce a breaking change?