Conversation
There was a problem hiding this comment.
Pull request overview
Adds elicitation support to the ACP Kotlin client/model layers, including new RPC methods/types and client-side routing for session- and request-scoped elicitations.
Changes:
- Introduces elicitation protocol model types (schemas, request/response, capabilities) and method declarations.
- Adds client/agent operations for elicitation plus routing logic (session-scoped, request-scoped via in-flight request association, and global fallback).
- Updates protocol request tracking to associate outgoing requests with
SessionId, and adds broad test coverage for serialization + end-to-end flows.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| build.gradle.kts | Bumps library version to 0.19.0. |
| acp/src/jvmTest/kotlin/com/agentclientprotocol/client/ElicitationSessionMapTest.kt | Unit tests for bounded elicitation→session store behavior and eviction. |
| acp/src/commonMain/kotlin/com/agentclientprotocol/protocol/Protocol.kt | Tracks optional SessionId per outgoing request; exposes lookup API; updates cancellation logic. |
| acp/src/commonMain/kotlin/com/agentclientprotocol/protocol/Protocol.extensions.kt | Passes sessionId into raw requests by extracting it from AcpWithSessionId. |
| acp/src/commonMain/kotlin/com/agentclientprotocol/common/ElicitationOperations.kt | Adds client-side elicitation operation contract. |
| acp/src/commonMain/kotlin/com/agentclientprotocol/common/ClientSessionOperations.kt | Extends client session ops to include elicitation operations. |
| acp/src/commonMain/kotlin/com/agentclientprotocol/client/GlobalElicitationHandler.kt | Adds global fallback handler for non-session elicitations. |
| acp/src/commonMain/kotlin/com/agentclientprotocol/client/ElicitationSessionStore.kt | Implements bounded store mapping URL elicitation IDs to sessions for completion routing. |
| acp/src/commonMain/kotlin/com/agentclientprotocol/client/Client.kt | Wires elicitation request + completion handlers; routes by session/request scope; adds global fallback. |
| acp/src/commonMain/kotlin/com/agentclientprotocol/agent/RemoteClientSessionOperations.kt | Implements remote elicitation calls and adds convenience helpers. |
| acp/api/acp.api | Updates published API surface for new elicitation APIs and protocol signature changes. |
| acp-model/src/commonTest/kotlin/com/agentclientprotocol/model/ElicitationSerializationTest.kt | Serialization/deserialization coverage for elicitation requests/responses/capabilities/schemas. |
| acp-model/src/commonMain/kotlin/com/agentclientprotocol/model/Methods.kt | Adds elicitation/create and elicitation/complete method definitions. |
| acp-model/src/commonMain/kotlin/com/agentclientprotocol/model/Elicitation.kt | Adds elicitation model types, serializers, capabilities, and error code constant. |
| acp-model/src/commonMain/kotlin/com/agentclientprotocol/model/Capabilities.kt | Adds elicitation to ClientCapabilities. |
| acp-model/api/acp-model.api | Updates published model API surface for elicitation types/capabilities. |
| acp-ktor-test/src/commonTest/kotlin/com/agentclientprotocol/FeaturesTest.kt | Adds end-to-end feature tests for elicitation flows and routing behaviors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Schema for string properties in an elicitation form. | ||
| */ | ||
| @UnstableApi | ||
| @Serializable | ||
| public data class StringPropertySchema( | ||
| val title: String? = null, | ||
| val description: String? = null, | ||
| val minLength: Int? = null, | ||
| val maxLength: Int? = null, | ||
| val pattern: String? = null, | ||
| val format: StringFormat? = null, | ||
| val default: String? = null, | ||
| @SerialName("enum") val enumValues: List<String>? = null, | ||
| @SerialName("oneOf") val oneOf: List<EnumOption>? = null | ||
| ) | ||
|
|
||
| /** | ||
| * Schema for number (floating-point) properties in an elicitation form. | ||
| */ | ||
| @UnstableApi | ||
| @Serializable | ||
| public data class NumberPropertySchema( | ||
| val title: String? = null, | ||
| val description: String? = null, | ||
| val minimum: Double? = null, | ||
| val maximum: Double? = null, | ||
| val default: Double? = null | ||
| ) | ||
|
|
||
| /** | ||
| * Schema for integer properties in an elicitation form. | ||
| */ | ||
| @UnstableApi | ||
| @Serializable | ||
| public data class IntegerPropertySchema( | ||
| val title: String? = null, | ||
| val description: String? = null, | ||
| val minimum: Long? = null, | ||
| val maximum: Long? = null, | ||
| val default: Long? = null | ||
| ) | ||
|
|
||
| /** | ||
| * Schema for boolean properties in an elicitation form. | ||
| */ | ||
| @UnstableApi | ||
| @Serializable | ||
| public data class BooleanPropertySchema( | ||
| val title: String? = null, | ||
| val description: String? = null, | ||
| val default: Boolean? = null | ||
| ) |
There was a problem hiding this comment.
Elicitation.kt defines both top-level StringPropertySchema/NumberPropertySchema/etc and the sealed ElicitationPropertySchema variants with the same fields. These top-level schema types are currently unused in the codebase (all usages go through ElicitationPropertySchema.*), which increases public API surface and risks divergence/confusion. Consider removing them, making them internal, or reusing them from ElicitationPropertySchema to avoid duplicate representations.
…tocol.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…tocol.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.