Port upstream PR #960: ElicitationContext single-arg handler#82
Conversation
…(upstream PR #960) Port upstream PR #960 cross-SDK consistency change: - Elicitation handler now takes single ElicitationContext arg instead of (request, ctx) — context includes :session-id alongside request fields - Rename ::elicitation-request spec to ::elicitation-context, add :session-id - Update all handler call sites, tests, example, and docs - BREAKING: handler signature change from (fn [request ctx]) to (fn [context]) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Ports upstream PR #960 to align elicitation handling with other cross-SDK “context map” patterns by switching :on-elicitation-request from a 2-arg handler to a single ElicitationContext argument that includes :session-id.
Changes:
- Renames/spec-updates inbound elicitation payload spec from
::elicitation-requestto::elicitation-contextand requires:session-id. - Updates v3 broadcast routing + session handler invocation to call elicitation handler with a single context map.
- Updates integration tests, example, API reference docs, and changelog for the breaking signature change.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/github/copilot_sdk/specs.clj |
Renames/adjusts spec to ::elicitation-context including required :session-id. |
src/github/copilot_sdk/client.clj |
Builds ElicitationContext in v3 handler and updates create-session docs to single-arg signature. |
src/github/copilot_sdk/session.clj |
Calls elicitation handler with a single context argument. |
test/github/copilot_sdk/integration_test.clj |
Updates elicitation provider tests to use the single-arg handler and asserts context contents. |
examples/elicitation_provider.clj |
Updates example handler to destructure the single context map. |
doc/reference/API.md |
Updates API docs to describe ElicitationContext single-arg handler. |
CHANGELOG.md |
Documents breaking change in [Unreleased]. |
src/github/copilot_sdk/client.clj
Outdated
| (let [context {:session-id session-id | ||
| :message (:message data) | ||
| :requested-schema (:requested-schema data) | ||
| :mode (:mode data) | ||
| :elicitation-source (:elicitation-source data) | ||
| :url (:url data)} |
There was a problem hiding this comment.
When building context, optional fields are always assoc’d even when missing in data (so :requested-schema, :elicitation-source, :url may be present with nil). That makes it impossible to distinguish “absent” vs “explicitly nil”, and it would fail ::elicitation-context if you ever validate it (those opt-un keys require non-nil values when present). Consider only assoc’ing these keys when non-nil (e.g., cond-> / merge with select-keys + keep).
| (let [context {:session-id session-id | |
| :message (:message data) | |
| :requested-schema (:requested-schema data) | |
| :mode (:mode data) | |
| :elicitation-source (:elicitation-source data) | |
| :url (:url data)} | |
| (let [requested-schema (:requested-schema data) | |
| elicitation-source (:elicitation-source data) | |
| url (:url data) | |
| context (cond-> {:session-id session-id | |
| :message (:message data) | |
| :mode (:mode data)} | |
| (some? requested-schema) (assoc :requested-schema requested-schema) | |
| (some? elicitation-source) (assoc :elicitation-source elicitation-source) | |
| (some? url) (assoc :url url)) |
| - :on-elicitation-request - Handler for elicitation requests from the agent (upstream PRs #908, #960). | ||
| When provided, sends requestElicitation=true and enables the | ||
| elicitation capability. Handler is (fn [request ctx]) returning | ||
| an ElicitationResult map ({:action \"accept\" :content {...}}). | ||
| elicitation capability. Single-arg handler receives an ElicitationContext | ||
| map with :session-id, :message, :requested-schema, :mode, | ||
| :elicitation-source, :url. Returns an ElicitationResult map. |
There was a problem hiding this comment.
The create-session docstring was updated to describe the new single-arg :on-elicitation-request handler, but the resume-session docstring later in this file still references elicitation as “upstream PR #908” without clarifying the new signature/context shape. Please update the resume-session docs too so both entry points describe the same breaking API.
- Use cond-> to only assoc optional context fields when non-nil - Update resume-session docstring to match create-session Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addressing Copilot Code Review (2 comments)Fixed in 2adfd4b: 1. Optional nil fields in context map (client.clj:372) 2. resume-session docstring outdated (client.clj:1445) |
Summary
Ports upstream PR #960 (Close Language Gaps for Commands + Dialogs/Elicitations) — the cross-SDK consistency change that merges the elicitation handler's two arguments into a single
ElicitationContext.Upstream Changes Ported
PR #960 — ElicitationRequest → ElicitationContext
BREAKING: The elicitation handler signature changes from 2-arg to single-arg:
The
ElicitationContextmap includes:session-idalongside the request fields, matching theCommandContextpattern used by command handlers.Changes
src/github/copilot_sdk/specs.clj::elicitation-request→::elicitation-context, added:session-idto required keyssrc/github/copilot_sdk/client.clj:session-idincluded, updated docstringssrc/github/copilot_sdk/session.clj(handler request {:session-id ...})test/.../integration_test.cljexamples/elicitation_provider.cljdoc/reference/API.mdCHANGELOG.mdUpstream Changes Skipped
Testing