Merge reference implementation SDK changes (2026-05-18)#212
Conversation
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.
…ummary field The generated SessionModelSwitchToParams record gained a new reasoningSummary field, changing the constructor from 4 to 5 parameters. Updated both call sites in CopilotSession to pass null for the new reasoningSummary parameter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Commit pushed:
|
Code Review: PR #212 (Reference Impl Sync — 14 new commits)APPROVAL — CopilotSession.javaThe two // Before: (sessionId, model, reasoningEffort, capabilities)
// After: (sessionId, model, reasoningEffort, reasoningSummary, capabilities)APPROVAL — CloudSessionOptions + CloudSessionRepository (new files)Clean DTOs following established patterns: APPROVAL — CreateSessionRequest + SessionConfig + SessionRequestBuilderStandard wiring of the new
APPROVAL — CustomAgentConfig (model field)Clean addition with good Javadoc explaining fallback behavior. WARNING — Hook Input Records (breaking positional API)
// Previously valid, now compile error:
new SessionStartHookInput(timestamp, cwd, source, initialPrompt)
// Must become:
new SessionStartHookInput(sessionId, timestamp, cwd, source, initialPrompt)Mitigating factors: These records are deserialized from CLI JSON internally — users shouldn't construct them directly. The existing test ( Verdict: Acceptable for a minor version bump but should be noted in CHANGELOG. APPROVAL — PostToolUseHookInput + PreToolUseHookInput (sessionId)These are classes (not records), so adding APPROVAL — Documentation (advanced.md, hooks.md)
SUGGESTION — Missing TestsNo new test coverage for:
These would ideally have unit tests verifying serialization round-trip. However, E2E coverage depends on reference-impl snapshots, so this is acceptable to defer. Summary
Recommendation: Merge-ready. The one warning is a documented acceptable trade-off for reference-impl parity. Consider adding a CHANGELOG note about the hook input record signature change. |
… from generated type changes SessionRequestBuilderTest.java (+48): - Add testBuildCreateRequestPropagatesCloudSessionOptions: verifies CloudSessionOptions (repository name/ID) are wired through SessionRequestBuilder into the create request JSON. - Add testBuildCreateRequestOmitsCloudWhenNull: verifies cloud field is omitted when not set. - Add testCloudSessionOptionsSerializesCorrectly: verifies Jackson serialization round-trip of CloudSessionOptions with repository fields. SessionHandlerTest.java (+48): - Add testHookInputSessionIdDeserializedForSessionStart: verifies sessionId is deserialized from JSON for session.start hook inputs. - Add testHookInputSessionIdDeserializedForSessionEnd: same for session.end hook inputs. - Add testHookInputSessionIdDeserializedForUserPromptSubmitted: same for user_prompt_submitted hook inputs. DataObjectCoverageTest.java (+58): - Add assertNull(input.getSessionId()) to preToolUseHookInputGetters and postToolUseHookInputGetters to verify sessionId getter exists and defaults to null. - Add preToolUseHookInputSessionIdRoundTrip: verifies setSessionId/getSessionId round-trip. - Add postToolUseHookInputSessionIdRoundTrip: same for PostToolUseHookInput. - Add customAgentConfigModelGetterAndSetter: verifies model getter/setter on CustomAgentConfig. - Add customAgentConfigModelFluentChaining: verifies fluent setter returns same instance. - Add customAgentConfigModelSerializationRoundTrip: verifies Jackson serialization of model. - Add customAgentConfigModelOmittedWhenNull: verifies model omitted from JSON when null. RpcWrappersTest.java (+1, -1): - Fix SessionModelSwitchToParams constructor call: add 5th null argument for new reasoningSummary parameter added by codegen. SessionEventHandlingTest.java (+2, -2): - Fix SessionStartEventData construction: add 12th null argument for new detachedFromSpawningParentSessionId field added by codegen. GeneratedRpcApiCoverageTest.java (+3, -2): - Fix SessionFsSetProviderParams constructor: add 4th null argument for new capabilities field. - Add assertNull(params.capabilities()) to verify the new field accessor. GeneratedRpcRecordsCoverageTest.java (+11, -10): - Fix SessionModelSwitchToParams: add 5th null arg, add assertNull for reasoningSummary. - Fix SkillDefinition: use SkillSource.PROJECT enum instead of raw "project" string. - Fix DiscoveredMcpServer: use McpServerSource.USER enum instead of DiscoveredMcpServerSource.USER. - Fix ModelPolicyRecord: use ModelPolicyState.ENABLED enum instead of raw "active" string. - Fix ModelPolicyRecord dateAdded: use OffsetDateTime.parse() instead of raw string.
… from generated type changes SessionRequestBuilderTest.java (+48): - Add testBuildCreateRequestPropagatesCloudSessionOptions: verifies CloudSessionOptions (repository name/ID) are wired through SessionRequestBuilder into the create request JSON. - Add testBuildCreateRequestOmitsCloudWhenNull: verifies cloud field is omitted when not set. - Add testCloudSessionOptionsSerializesCorrectly: verifies Jackson serialization round-trip of CloudSessionOptions with repository fields. SessionHandlerTest.java (+48): - Add testHookInputSessionIdDeserializedForSessionStart: verifies sessionId is deserialized from JSON for session.start hook inputs. - Add testHookInputSessionIdDeserializedForSessionEnd: same for session.end hook inputs. - Add testHookInputSessionIdDeserializedForUserPromptSubmitted: same for user_prompt_submitted hook inputs. DataObjectCoverageTest.java (+58): - Add assertNull(input.getSessionId()) to preToolUseHookInputGetters and postToolUseHookInputGetters to verify sessionId getter exists and defaults to null. - Add preToolUseHookInputSessionIdRoundTrip: verifies setSessionId/getSessionId round-trip. - Add postToolUseHookInputSessionIdRoundTrip: same for PostToolUseHookInput. - Add customAgentConfigModelGetterAndSetter: verifies model getter/setter on CustomAgentConfig. - Add customAgentConfigModelFluentChaining: verifies fluent setter returns same instance. - Add customAgentConfigModelSerializationRoundTrip: verifies Jackson serialization of model. - Add customAgentConfigModelOmittedWhenNull: verifies model omitted from JSON when null. RpcWrappersTest.java (+1, -1): - Fix SessionModelSwitchToParams constructor call: add 5th null argument for new reasoningSummary parameter added by codegen. SessionEventHandlingTest.java (+2, -2): - Fix SessionStartEventData construction: add 12th null argument for new detachedFromSpawningParentSessionId field added by codegen. GeneratedRpcApiCoverageTest.java (+3, -2): - Fix SessionFsSetProviderParams constructor: add 4th null argument for new capabilities field. - Add assertNull(params.capabilities()) to verify the new field accessor. GeneratedRpcRecordsCoverageTest.java (+11, -10): - Fix SessionModelSwitchToParams: add 5th null arg, add assertNull for reasoningSummary. - Fix SkillDefinition: use SkillSource.PROJECT enum instead of raw "project" string. - Fix DiscoveredMcpServer: use McpServerSource.USER enum instead of DiscoveredMcpServerSource.USER. - Fix ModelPolicyRecord: use ModelPolicyState.ENABLED enum instead of raw "active" string. - Fix ModelPolicyRecord dateAdded: use OffsetDateTime.parse() instead of raw string.
edburns
left a comment
There was a problem hiding this comment.
@copilot I added some tests, and there are test failures.
Look at the job run https://github.com/github/copilot-sdk-java/actions/runs/26061898666 to discern the failures and fix them, pushing additional commits to the topic branch for this work copilot/reference-impl-sync-14-new-commits.
- GeneratedEventTypesCoverageTest: use valid ExitPlanModeAction values (exit_only/interactive/autopilot instead of approve/edit/reject), use McpServerStatus instead of McpServersLoadedServerStatus, use SkillSource.PROJECT enum instead of string "project" - SessionEventDeserializationTest: same ExitPlanModeAction fix - ModeHandlersTest: compare ExitPlanModeAction.INTERACTIVE enum instead of string "interactive" for selectedAction() Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
Fixed in commit
|
Resolves #207
Before the change?
CustomAgentConfighad no way to specify a model for an agentPreToolUseHookInput,PostToolUseHookInput,SessionStartHookInput,SessionEndHookInput,UserPromptSubmittedHookInput) did not expose the session ID, breaking sub-agent hook propagationAfter the change?
modelfield onCustomAgentConfig— optional model identifier (e.g."claude-haiku-4.5") that the runtime uses for the agent, falling back to the parent session model if unavailablesessionIdon all hook inputs — enables hooks to identify which session (including sub-agent sessions) triggered themCloudSessionOptionsandCloudSessionRepositoryclasses, wired throughSessionConfig.setCloud()andCreateSessionRequest.lastmergeupdated tof6c1adf8,@github/copilotsynced to^1.0.49-1Not ported (intentionally): .NET-specific changes (
CopilotToolhelper, MSBuild targets, netstandard targets, C# argument validation), test snapshot reorganization, Python/Go-only doc fixes. See PR body checklist for full breakdown.Pull request checklist
mvn spotless:applyhas been run to format the codemvn clean verifypasses locallyDoes this introduce a breaking change?