[Node] Add onElicitationRequest Callback for Elicitation Provider Support#908
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Cross-SDK Consistency ReviewThis PR adds Summary of Changes (Node.js Only)The PR adds:
Feature Parity Status
RecommendationThe Python, Go, and .NET SDKs should receive equivalent implementations to maintain feature parity. Based on the Node.js changes, each SDK would need: Python (
|
MRayermannMSFT
force-pushed
the
mrayermannmsft/elicitation-provider
branch
from
3a1ec67 to
75c9be1
Compare
Cross-SDK Consistency ReviewThis PR adds Current State✅ Node.js SDK (this PR):
❌ Python SDK: No elicitation provider support
❌ Go SDK: No elicitation provider support
❌ .NET SDK: No elicitation provider support
RecommendationTo maintain feature parity across SDKs, consider adding equivalent elicitation provider APIs to Python, Go, and .NET: Python: async def create_session(
*,
on_elicitation_request: ElicitationHandler | None = None,
# ... other params
) -> CopilotSession:Go: type SessionConfig struct {
// ...
OnElicitationRequest ElicitationHandler
}.NET: public class SessionConfig
{
public ElicitationRequestHandler? OnElicitationRequest { get; set; }
}All SDKs already support consuming elicitation as a client (via Note: If this feature is being rolled out incrementally, consider tracking the cross-language implementation in an issue to ensure consistency is achieved in future PRs.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #908
Cross-SDK Consistency ReviewI've reviewed PR #908 for consistency across the four SDK implementations (Node.js/TypeScript, Python, Go, and .NET). SummaryThis PR adds elicitation provider support to the Node.js SDK, enabling clients to handle
Feature Parity GapsThis PR widens an existing feature gap. The Node.js SDK has several features that are not available in Python, Go, or .NET: 1. UI Elicitation (Consumer Side) ❌ Python, Go, .NETThe Node.js SDK has had 2. Commands Support ❌ Python, Go, .NETThe Node.js SDK supports registering slash commands via 3. Elicitation Provider Support (This PR) ❌ Python, Go, .NETThis PR adds the ability for Node.js clients to handle elicitation requests, not just call them. None of the other SDKs have this new capability. RecommendationWhile this Node.js-specific feature is well-implemented and valuable, I recommend tracking these cross-SDK consistency issues:
The Node.js SDK is becoming the "reference implementation" with significantly more capabilities than the others. This is sustainable if intentional, but may surprise users expecting feature parity across languages. No blocking issues with this PR — it maintains consistency with Node.js's existing patterns and doesn't introduce inconsistencies within the Node.js SDK itself. The cross-SDK gap is pre-existing and growing.
|
There was a problem hiding this comment.
Pull request overview
Adds elicitation provider support to the Node.js SDK by introducing an onElicitationRequest callback in SessionConfig, allowing SDK clients to receive and answer fan-out elicitation requests and track dynamic capability changes during a session.
Changes:
- Add
onElicitationRequesttypes/exports and wire capability negotiation (requestElicitation) into session create/resume. - Handle
elicitation.requestedbroadcast events and applycapabilities.changedupdates tosession.capabilities. - Add unit + E2E coverage and update docs; bump
@github/copilotdependency versions used by Node + test harness.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/harness/package.json | Bumps @github/copilot used by the E2E harness to a version supporting new elicitation behavior. |
| test/harness/package-lock.json | Locks updated @github/copilot (+ platform packages) for the test harness. |
| nodejs/test/e2e/ui_elicitation.test.ts | Adds E2E coverage for callback-based elicitation capability + multi-client capabilities.changed propagation. |
| nodejs/test/client.test.ts | Adds unit tests asserting requestElicitation flag behavior in session.create. |
| nodejs/src/types.ts | Introduces ElicitationRequest/ElicitationHandler and adds onElicitationRequest to session config types. |
| nodejs/src/session.ts | Registers elicitation handler, listens for elicitation.requested, responds via session.ui.handlePendingElicitation, and applies capabilities.changed updates. |
| nodejs/src/index.ts | Exports new elicitation callback types from the public Node SDK entrypoint. |
| nodejs/src/client.ts | Wires onElicitationRequest registration and sends requestElicitation on create/resume. |
| nodejs/README.md | Documents onElicitationRequest, fan-out behavior, and dynamic capability changes. |
| nodejs/package.json | Bumps SDK dependency on @github/copilot to ^1.0.14-0. |
| nodejs/package-lock.json | Locks updated @github/copilot (+ platform packages) for the Node SDK package. |
Files not reviewed (2)
- nodejs/package-lock.json: Language not supported
- test/harness/package-lock.json: Language not supported
Cross-SDK Consistency ReviewThis PR adds elicitation provider support to the Node.js SDK only, enabling SDK clients to act as elicitation providers by handling What was added (Node.js SDK):
Cross-SDK Status Check:Python SDK (
Go SDK (
.NET SDK (
Recommendation:This PR creates a feature gap across the SDKs. The elicitation provider capability enables SDK clients to present form-based UI dialogs on behalf of agents, which is valuable functionality for all SDK users regardless of language. Suggested follow-up work:
Each would follow the same pattern as Node.js:
Note: The generated RPC types ( This is a valuable feature addition to Node.js! The consistency gap is acceptable as an incremental rollout, but tracking issues should be created to bring this capability to other SDKs soon.
|
MRayermannMSFT
force-pushed
the
mrayermannmsft/elicitation-provider
branch
from
521a25b to
f17ed33
Compare
Cross-SDK Consistency ReviewThis PR adds elicitation provider support to the Node.js SDK, enabling clients to respond to Current State✅ Node.js SDK (this PR):
ImpactUsers of Python, Go, and .NET SDKs cannot:
RecommendationConsider adding equivalent functionality to Python, Go, and .NET SDKs in follow-up PRs to maintain feature parity: Python: session = await client.create_session(
model="gpt-5",
on_elicitation_request=async lambda request, invocation: {
"action": "accept",
"content": {"field": "value"}
}
)
# Check capabilities
if session.capabilities.ui and session.capabilities.ui.elicitation:
result = await session.ui.confirm("Deploy to production?")Go: session, _ := client.CreateSession(ctx, &SessionConfig{
Model: "gpt-5",
OnElicitationRequest: func(req ElicitationRequest, inv ElicitationInvocation) (ElicitationResult, error) {
return ElicitationResult{Action: "accept", Content: map[string]any{"field": "value"}}, nil
},
})
// Check capabilities
if session.Capabilities.UI != nil && session.Capabilities.UI.Elicitation {
result, _ := session.UI.Confirm(ctx, "Deploy to production?")
}.NET: var session = await client.CreateSessionAsync(new SessionConfig {
Model = "gpt-5",
OnElicitationRequest = async (request, invocation, ct) => new ElicitationResult {
Action = "accept",
Content = new Dictionary(string, object) { ["field"] = "value" }
}
});
// Check capabilities
if (session.Capabilities?.UI?.Elicitation == true) {
var result = await session.UI.ConfirmAsync("Deploy to production?");
}The underlying protocol types are already generated in all SDKs (as evidenced by this PR updating
|
MackinnonBuck
left a comment
MackinnonBuck
left a comment
There was a problem hiding this comment.
Looks great! Just a couple comments, but I won't block the approval based on those.
Another question: What's the motivation for holding off on the Go/.NET/Python changes? Is there a consumer of the Node SDK that needs this change expedited? It would be great to get the other SDKs to match, but this can be done in a follow-up. I'd be happy to handle that if you want someone to take it off your plate.
Ya, trying to get capabilities land fast for NodeJS for extensions. This is the last thing I need to land. I'll be coming back in a day or so to get other languages landed. |
Cross-SDK Consistency Review: Elicitation Provider SupportThis PR adds a valuable new feature—elicitation provider support via the What This PR Adds (Node.js Only)
Current State of Other SDKs✅ Generated RPC types exist in all SDKs:
✅ Generated event types exist in all SDKs:
❌ Missing in Python, Go, and .NET:
Recommended Next StepsTo maintain cross-language feature parity, consider one of: Option 1: Expand this PR to add equivalent functionality to Python, Go, and .NET
Option 2: Track follow-up work in separate issues/PRs
Implementation Pattern ReferenceFor consistency, the implementation in other SDKs should mirror the existing user input handler pattern: // Node.js (this PR) - pattern to replicate
onElicitationRequest: async (request, invocation) => {
// request: { message, requestedSchema, mode, elicitationSource, url }
// invocation: { sessionId }
return { action: "accept", content: { field: "value" } };
}All three SDKs already have this pattern for user input:
The elicitation handler would follow the same structure with different parameter types. Why This MattersThe CLI runtime now supports fan-out elicitation—broadcasting structured input requests to all connected clients. Without SDK support, only the CLI's TUI can respond. By limiting this to Node.js, we prevent Python/Go/.NET SDK users from:
Recommendation: Either expand this PR to include all SDKs, or merge with follow-up issues tracked for the other languages. The generated types are already in place, so the remaining work is registering handlers and wiring up event listeners.
|
What
Adds
onElicitationRequestto the Node.js SDK'sSessionConfig, enabling SDK clients to act as elicitation providers. When a client registers this callback, the SDK negotiates therequestElicitationcapability during session create/resume, listens forelicitation.requestedbroadcast events, invokes the handler, and responds via thehandlePendingElicitationRPC. The SDK also automatically updatessession.capabilitieswhencapabilities.changedevents arrive as providers join or leave. Includes unit tests, E2E tests covering capability reporting, multi-client propagation, and provider disconnect, plus updated README documentation.Why
The CLI runtime now supports fan-out elicitation — broadcasting structured input requests to all connected clients that advertise elicitation support, with the first response winning. Without this SDK-side change, Node.js clients had no way to register as elicitation providers, limiting these requests to the CLI's own TUI.