Align the Kotlin SDK with agent-client-protocol pr 576 that flattens type and value at the top level#81
Conversation
type and value at the top leveltype and value at the top level
Align with agentclientprotocol/agent-client-protocol#576 by introducing a custom serializer that uses a "type" discriminator field for boolean config options while preserving backward compatibility for select/string values. - Boolean values serialize as {"type":"boolean","value":true} - String values serialize as {"value":"..."} (no type field, unchanged) - Unknown types fall back to UnknownValue for forward compatibility
368f631 to
0226c30
Compare
There was a problem hiding this comment.
Pull request overview
This PR aligns the Kotlin acp-model SDK’s SetSessionConfigOptionRequest JSON shape with agent-client-protocol#576 by introducing a custom serializer that flattens type/value to the top level (while keeping string/select values untyped for backward compatibility).
Changes:
- Bumped SDK version from
0.16.5to0.16.6. - Switched
SetSessionConfigOptionRequestto a customKSerializerthat emits top-leveltypefor boolean values. - Updated API dump and expanded serializer tests for boolean/string/unknown/backward-compat cases.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| build.gradle.kts | Version bump to publish the protocol-alignment change. |
| acp-model/src/commonMain/kotlin/com/agentclientprotocol/model/Requests.kt | Routes SetSessionConfigOptionRequest through the new custom serializer. |
| acp-model/src/commonMain/kotlin/com/agentclientprotocol/model/SessionConfig.kt | Implements SetSessionConfigOptionRequestSerializer flattening logic. |
| acp-model/src/commonTest/kotlin/com/agentclientprotocol/model/SessionConfigSelectOptionsSerializerTest.kt | Updates/extends tests around request serialization/deserialization formats. |
| acp-model/api/acp-model.api | Reflects regenerated public API surface (removes synthetic serializer class). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
acp-model/src/commonMain/kotlin/com/agentclientprotocol/model/SessionConfig.kt
Outdated
Show resolved
Hide resolved
acp-model/src/commonMain/kotlin/com/agentclientprotocol/model/SessionConfig.kt
Outdated
Show resolved
Hide resolved
acp-model/src/commonMain/kotlin/com/agentclientprotocol/model/SessionConfig.kt
Show resolved
Hide resolved
…d compat - Deserialize boolean primitives without type field as BoolValue (backward compat) - Preserve both type and value in UnknownValue for unknown types (forward compat) - Add regression tests for boolean-without-type and unknown-type round-trip Co-authored-by: Junie <junie@jetbrains.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| ?: throw SerializationException("Expected boolean primitive for type 'boolean'") | ||
| if (primitive.booleanOrNull != null) { | ||
| SessionConfigOptionValue.BoolValue(primitive.boolean) | ||
| } else { | ||
| SessionConfigOptionValue.UnknownValue(rawValue) |
There was a problem hiding this comment.
In the type == "boolean" branch, if value is not a JSON boolean (e.g., a string/number/object), the code either throws (non-JsonPrimitive) or falls back to UnknownValue(rawValue) which drops the original type field. That makes payloads like {...,"type":"boolean","value":"false"} impossible to round-trip and reduces forward/robust compatibility.
Consider avoiding the hard throw and, on any non-boolean value, preserving both fields by storing an UnknownValue wrapper object containing the original type and value (similar to the unknown-type branch).
| ?: throw SerializationException("Expected boolean primitive for type 'boolean'") | |
| if (primitive.booleanOrNull != null) { | |
| SessionConfigOptionValue.BoolValue(primitive.boolean) | |
| } else { | |
| SessionConfigOptionValue.UnknownValue(rawValue) | |
| if (primitive?.booleanOrNull != null) { | |
| SessionConfigOptionValue.BoolValue(primitive.boolean) | |
| } else { | |
| // Preserve both type and value for non-boolean or non-primitive values | |
| val unknownWrapper = buildJsonObject { | |
| put("type", JsonPrimitive("boolean")) | |
| put("value", rawValue) | |
| } | |
| SessionConfigOptionValue.UnknownValue(unknownWrapper) |
Aligns the Kotlin SDK with agent-client-protocol#576 (commit 2aca527).
What changed
Replaced the auto-generated serializer for
SetSessionConfigOptionRequestwith a customSetSessionConfigOptionRequestSerializerthat flattenstypeandvalueat the top levelof the JSON object.
Serialization format
{"sessionId":"…","configId":"…","type":"boolean","value":true}{"sessionId":"…","configId":"…","value":"model-1"}UnknownValueThe string/select format is intentionally unchanged to maintain backward compatibility.
Files changed
@Serializable(with = SetSessionConfigOptionRequestSerializer::class)SetSessionConfigOptionRequestSerializerwith custom serialize/deserialize logicapiDump(removed synthetic$$serializerclass)Testing
All 340 tests in
acp-modelpass. No breaking changes for existing consumers.