Skip to content

Close Language Gaps for Commands + Dialogs/Elicitations#960

Draft
MRayermannMSFT wants to merge 9 commits intomainfrom
mrayermannmsft/uiandcommandsgaps
Draft

Close Language Gaps for Commands + Dialogs/Elicitations#960
MRayermannMSFT wants to merge 9 commits intomainfrom
mrayermannmsft/uiandcommandsgaps

Conversation

@MRayermannMSFT
Copy link
Copy Markdown
Contributor

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

✅ Cross-SDK Consistency Review Complete

I've reviewed this PR for consistency across all four SDK implementations (Node.js, Python, Go, .NET).

🎯 Overall Assessment: Exemplary Consistency

This PR successfully adds Commands and UI Elicitation features to all four SDKs with excellent cross-language consistency while appropriately adapting to each language's idioms.


Commands Feature ✅

All four SDKs provide:

  • Consistent command definition: CommandDefinition with name, description, and handler
  • Consistent context object: CommandContext with session ID, command name, command object, and args
  • Appropriate naming conventions:
    • Node.js: registerCommands() with commands?: CommandDefinition[]
    • Python: _register_commands() with commands: list[CommandDefinition]
    • Go: registerCommands() with Commands []CommandDefinition
    • .NET: RegisterCommands() with Commands: List(CommandDefinition)?

UI Elicitation Feature ✅

All four SDKs provide the same four UI methods:

  1. elicitation() - Generic elicitation with custom schemas
  2. confirm() - Boolean confirmation dialogs
  3. select() - Single-choice selection from options
  4. input() - Text input with validation

Handler registration is consistent:

  • Config parameter: onElicitationRequest / on_elicitation_request / OnElicitationRequest
  • Registration method: registerElicitationHandler() / _register_elicitation_handler() / RegisterElicitationHandler()
  • Capabilities check: All SDKs check capabilities.ui.elicitation before allowing calls

Language-appropriate async patterns:

  • Node.js/Python: async/await
  • Go: context.Context parameter
  • .NET: CancellationToken + Task(T)

Documentation & Tests ✅

Documentation: All four README files include complete sections on Commands and UI Elicitation with code examples
Test Coverage: Shared test snapshots ensure behavioral equivalence across SDKs
E2E Tests: Each language has comprehensive E2E tests for both features


🏆 Conclusion

No consistency issues found. This PR maintains excellent feature parity across all four SDKs and demonstrates the gold standard for cross-language SDK development. All language-specific variations are appropriate adaptations to each ecosystem's conventions.

Great work! 🚀

Generated by SDK Consistency Review Agent for issue #960 ·

- Python: fix ruff formatting, add comments to empty except blocks, remove unused imports
- .NET: simplify boolean expressions, combine nested ifs, narrow generic catch clause
- Go: fix struct field alignment for go fmt compliance
@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review: Commands & UI Elicitation ✅

This PR successfully closes the language gaps for Commands and UI Elicitation features across Python, Go, and .NET SDKs. The implementations maintain excellent consistency with the Node.js SDK reference implementation.

✅ Consistent API Surface

All four SDKs now expose parallel APIs with appropriate language conventions:

Feature Node.js Python Go .NET
Commands config commands: CommandDefinition[] commands: list[CommandDefinition] Commands []CommandDefinition Commands IEnumerable(CommandDefinition)
UI confirm session.ui.confirm() session.ui.confirm() session.UI().Confirm() session.Ui.ConfirmAsync()
UI select session.ui.select() session.ui.select() session.UI().Select() session.Ui.SelectAsync()
UI input session.ui.input() session.ui.input() session.UI().Input() session.Ui.InputAsync()
Capabilities check capabilities.ui?.elicitation capabilities.get("ui", {}).get("elicitation") Capabilities().UI.Elicitation Capabilities.Ui?.Elicitation

The naming differences (camelCase vs PascalCase, Async suffix) correctly follow each language's conventions. ✨

✅ Consistent Behavior

All SDKs implement:

  • ✅ Command registration at create_session/resume_session with handler storage
  • ✅ Event routing for command.execute → handler → RPC response via commands.handlePendingCommand
  • ✅ Capability gating for UI methods (throws if elicitation not supported)
  • ✅ Auto-cancel on handler errors
  • ✅ Comprehensive test coverage (E2E + unit tests)
  • ✅ README documentation with code examples

⚠️ Minor Inconsistency Found

One small gap: Go's CommandDefinition.Description field is required while it's optional in Node.js, Python, and .NET.

I've added an inline comment on go/types.go suggesting making it *string to allow nil values. This ensures developers can define commands without descriptions consistently across all SDKs.


Overall: Excellent work bringing feature parity to all SDKs! 🎉 The implementations are well-structured, thoroughly tested, and maintain the parallel API design that makes the SDK easy to use across languages.

Generated by SDK Consistency Review Agent for issue #960 ·

@github-actions
Copy link
Copy Markdown
Contributor

✅ Cross-SDK Consistency Review

I've reviewed this PR for consistency across all four SDK implementations (Node.js, Python, Go, and .NET).

Summary

This PR successfully adds Commands and UI Elicitation features to Go, .NET, and Python SDKs to achieve feature parity with the Node.js SDK. The implementation is excellent — all APIs are semantically consistent while following their respective language conventions.

What This PR Adds

Feature Node.js Python Go .NET
Command Definitions ✅ (baseline) ✅ Added ✅ Added ✅ Added
UI Elicitation Methods ✅ (baseline) ✅ Added ✅ Added ✅ Added
Session Capabilities ✅ (baseline) ✅ Added ✅ Added ✅ Added
Elicitation Handler Callback ✅ (baseline) ✅ Added ✅ Added ✅ Added

API Consistency Verification

Command Definitions

All SDKs now expose CommandDefinition with equivalent fields:

  • Node.js: { name, description?, handler }
  • Python: CommandDefinition(name, handler, description?)
  • Go: CommandDefinition{ Name, Description, Handler }
  • .NET: CommandDefinition { Name, Description?, Handler }

UI Elicitation Methods

All SDKs provide the same four methods with appropriate language naming:

  • Generic: elicitation() / Elicitation() / ElicitationAsync()
  • Confirm: confirm() / Confirm() / ConfirmAsync()
  • Select: select() / Select() / SelectAsync()
  • Input: input() / Input() / InputAsync()

Return types follow language idioms:

  • Node.js/Python/.NET: string | null (nullable)
  • Go: (string, bool, error) (idiomatic tuple return) ✅

Capabilities Access

  • Node.js: session.capabilities.ui?.elicitation
  • Python: session.capabilities["ui"]["elicitation"]
  • Go: session.Capabilities().UI.Elicitation
  • .NET: session.Capabilities.Ui?.Elicitation

Test Coverage

The PR includes comprehensive test coverage:

  • ✅ Go: E2E tests in go/internal/e2e/commands_and_elicitation_test.go
  • ✅ .NET: Unit tests in dotnet/test/{CommandsTests,ElicitationTests}.cs
  • ✅ Python: E2E tests in python/e2e/test_{commands,ui_elicitation}.py
  • ✅ Shared test snapshots in test/snapshots/{commands,elicitation}/

Conclusion

No consistency issues found. This PR maintains excellent cross-SDK consistency while respecting language-specific conventions. All four SDKs now have feature parity for Commands and UI Elicitation. 🎉

Generated by SDK Consistency Review Agent for issue #960 ·

…apabilities.changed reliably in replay proxy
@github-actions
Copy link
Copy Markdown
Contributor

✅ Cross-SDK Consistency Review: PR #960

I've completed a comprehensive review of this PR to ensure consistency across all four SDK implementations (Node.js, Python, Go, and .NET).

Summary

Overall Assessment: Excellent consistency 🎉

This PR successfully brings Python, Go, and .NET up to feature parity with the existing Node.js SDK for Commands and UI Elicitation APIs. The implementations are highly consistent across all languages, with appropriate adaptations for language-specific idioms.


Features Added

1. Commands API

All three SDKs now support slash commands with consistent structure:

  • Type Definition: CommandDefinition with Name, Description (optional), and Handler
  • Handler Context: CommandContext with session ID, command text, command name, and arguments
  • Registration: Commands registered via SessionConfig during createSession/resumeSession
  • Event Routing: Automatic dispatch to handlers on command.execute events

2. UI Elicitation API

All three SDKs provide four UI methods:

  • elicitation() — Generic method with full schema control
  • confirm() — Boolean yes/no dialog
  • select() — Selection from a list of options
  • input() — Text input with validation options

All methods:

  • Are gated by capability checks (session.capabilities.ui?.elicitation)
  • Throw/error when capabilities are missing
  • Support accept, decline, and cancel actions

Language-Specific Adaptations (All Appropriate ✅)

The implementations correctly adapt to each language's conventions:

Aspect Node.js Python Go .NET
Naming camelCase snake_case PascalCase (exported) PascalCase
Null/None string | null str | None (string, bool, error) string?
Async async/await async/await context.Context Task(T) + CancellationToken
Error Handling Throws Error Raises RuntimeError Returns error Throws InvalidOperationException

Note on Go's (value, ok, error) pattern: Go's Select and Input methods return (string, bool, error) instead of nullable strings. This is an appropriate Go idiom (similar to map lookups) and is well-documented in the README.


Testing Coverage ✅

Comprehensive test coverage added for all three SDKs:

Python:

  • python/test_commands_and_elicitation.py (unit tests)
  • python/e2e/test_commands.py
  • python/e2e/test_ui_elicitation.py
  • python/e2e/test_ui_elicitation_multi_client.py

Go:

  • go/internal/e2e/commands_and_elicitation_test.go
  • Updated existing unit tests in go/client_test.go and go/session_test.go

.NET:

  • dotnet/test/CommandsTests.cs
  • dotnet/test/ElicitationTests.cs
  • dotnet/test/MultiClientCommandsElicitationTests.cs

Shared test snapshots added for replay proxy:

  • test/snapshots/commands/*.yaml
  • test/snapshots/elicitation/*.yaml

Documentation ✅

All README files updated with:

  • Clear examples of command registration
  • UI elicitation capability checking
  • All four UI methods with code samples
  • Elicitation request handlers (server→client)
  • Proper error handling guidance

Verified Consistency Points ✅

  1. Field Names: Consistent across SDKs (accounting for casing conventions)
  2. Method Signatures: Semantically equivalent with language-appropriate types
  3. Capability Checks: All SDKs check capabilities.ui.elicitation before calling UI methods
  4. Error Messages: Consistent wording across SDKs for capability errors
  5. Command Registration: All SDKs support commands in both create and resume flows
  6. RPC Wire Format: Commands serialized as {name, description} arrays (handlers stay client-side)
  7. Action Values: All use "accept", "decline", "cancel" (enum in .NET/Python, string in Go)

No Issues Found

I found no cross-SDK inconsistencies that need to be addressed. The PR maintains excellent feature parity while respecting language-specific conventions.


Recommendation

✅ Approve from a cross-SDK consistency perspective

This PR exemplifies how to properly implement cross-language SDKs: consistent APIs that feel natural in each language.

Generated by SDK Consistency Review Agent for issue #960 ·

…connect

Python's socket.makefile() holds its own reference to the socket.
Calling socket.close() alone won't release the OS-level resource
until the makefile wrapper is also closed. This meant force_stop()
wasn't actually closing the TCP connection, so the server never
detected the disconnect and never sent capabilities.changed events
to other clients.

Fix: close the file wrapper before the socket in SocketWrapper.terminate().
Unskip test_capabilities_changed_when_elicitation_provider_disconnects.
- Narrow generic catch clauses in .NET command/elicitation handlers
  with 'when (ex is not OperationCanceledException)' filter
- Remove redundant null-conditional (val?.ToString -> val.ToString)
  in SelectAsync and InputAsync switch expressions
- Add explanatory comments to Python empty except blocks
@github-actions
Copy link
Copy Markdown
Contributor

✅ Cross-SDK Consistency Review

I've reviewed PR #960 for consistency across all four SDK implementations (Node.js, Python, Go, and .NET). This PR successfully adds Commands and UI Elicitation features to Python, Go, and .NET, bringing them to feature parity with the Node.js SDK.

Summary of Changes

Commands Feature:

  • ✅ All three SDKs (Python, Go, .NET) now support registering slash commands via CommandDefinition with name, description, and handler
  • ✅ Consistent command execution flow: command.execute event → handler dispatch → auto-response to CLI
  • ✅ Commands forwarded on both create_session/CreateSession/CreateSessionAsync and resume_session/ResumeSession/ResumeSessionAsync
  • ✅ Error handling: exceptions/errors in handlers are properly reported back to the CLI

UI Elicitation Feature:

  • ✅ All SDKs support the generic elicitation() method with custom schemas
  • ✅ Convenience methods added: confirm(), select(), input() with appropriate language-specific naming (snake_case in Python, PascalCase in Go/.NET)
  • ✅ Capabilities checking: All SDKs properly check session.capabilities.ui?.elicitation and throw/error if not supported
  • ✅ Multi-client support: All SDKs support OnElicitationRequest/on_elicitation_request handler to act as elicitation providers
  • ✅ Capabilities auto-update via capabilities.changed event handling

API Consistency Across Languages

The implementations properly account for language-specific conventions:

Feature Node.js Python Go .NET
Naming camelCase snake_case PascalCase (public) PascalCase
Commands commands?: CommandDefinition[] commands: list[CommandDefinition] Commands []CommandDefinition Commands CommandDefinition[]
Confirm Promise(boolean) bool (bool, error) Task(bool)
Select Promise(string | null) str | None (string, bool, error) ⚠️ Task(string?)
Input Promise(string | null) str | None (string, bool, error) ⚠️ Task(string?)

⚠️ Minor Inconsistency: Go Return Types

Go's Select() and Input() methods return (string, bool, error) where the second boolean indicates whether a selection was made, while other SDKs use nullable/optional types (string | null, string?, str | None) to represent "no selection".

Assessment: This is an acceptable language idiom difference. Go doesn't have nullable reference types, so using multiple return values (value, wasSelected, error) is idiomatic Go. The semantic meaning is equivalent - callers can check the bool flag the same way they'd check for null in other languages.

Example comparison:

// Node.js / TypeScript
const result = await session.ui.select("Pick one", ["a", "b"]);
if (result !== null) { /* user selected */ }
# Python
result = await session.ui.select("Pick one", ["a", "b"])
if result is not None: # user selected
// Go
result, selected, err := session.UI().Select(ctx, "Pick one", []string{"a", "b"})
if err != nil { return err }
if selected { /* user selected */ }
// .NET
var result = await session.Ui.SelectAsync("Pick one", ["a", "b"], cancellationToken);
if (result != null) { /* user selected */ }

Conclusion

✅ This PR maintains excellent cross-SDK consistency. The Commands and UI Elicitation features are implemented uniformly across all languages, with only idiomatic differences appropriate to each language's conventions. No changes needed - the Go tuple return is the correct design for Go.

Generated by SDK Consistency Review Agent for issue #960 ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant