Skip to content

[Node] Add Support For Commands and Simple UI#873

Draft
MRayermannMSFT wants to merge 1 commit intomainfrom
mrayermannmsft/uiandcommands
Draft

[Node] Add Support For Commands and Simple UI#873
MRayermannMSFT wants to merge 1 commit intomainfrom
mrayermannmsft/uiandcommands

Conversation

@MRayermannMSFT
Copy link
Contributor

No description provided.

@github-actions
Copy link
Contributor

Cross-SDK Consistency Review

Thank you for this PR! I've reviewed the changes for consistency across all four SDK implementations (Node.js, Python, Go, .NET). This PR adds two significant new features to the Node.js SDK:

  1. Command Handler Registration API (commands config, defineCommand, registerCommands)
  2. SessionUI API (session.ui with confirm(), select(), input() methods)

🔍 Findings

Event Infrastructure Exists (All SDKs)

All SDKs already support the underlying event infrastructure:

  • Command events: command.queued, command.completed (or command.requested in Node.js)
  • User input events: user_input.requested, user_input.completed

⚠️ Missing High-Level APIs (Python, Go, .NET)

However, the developer-facing APIs added in this PR are Node.js-only:

1. Command Handler Registration Pattern:

  • Node.js (this PR): commands: [defineCommand('deploy', { handler: async (args, ctx) => {...} })]
  • Python: No Command type in SessionConfig, no define_command() helper
  • Go: No Commands field in SessionConfig, no command registration API
  • .NET: No Commands property in SessionConfig, no command registration

2. SessionUI High-Level API:

  • Node.js (this PR): session.ui.confirm(), session.ui.select(), session.ui.input()
  • Python: Has UserInputHandler callback but no session.ui property
  • Go: Has OnUserInputRequest callback but no session.UI property
  • .NET: Has OnUserInputRequest callback but no session.UI property

🎯 Recommendation

To maintain cross-SDK feature parity, equivalent APIs should be added to Python, Go, and .NET before or shortly after merging this PR. The other SDKs have the event infrastructure but lack the ergonomic, handler-based APIs that this PR introduces.

Suggested approach:

  1. Track this as a known consistency gap in the PR description or follow-up issue
  2. Create tracking issues for adding these features to Python, Go, and .NET
  3. Ensure the API design translates well to each language's idioms:
    • Python: session.ui.confirm(), define_command("name", handler=...)
    • Go: session.UI.Confirm(), command config in SessionConfig.Commands
    • .NET: session.UI.Confirm(), command config in SessionConfig.Commands

📋 Next Steps

Would you like me to:

  1. File follow-up issues for each SDK to track feature parity?
  2. Identify the specific files in each SDK that would need modifications?
  3. Provide API design suggestions that respect each language's conventions?

Note: This review focuses on cross-language consistency. The Node.js implementation itself looks well-designed and thoroughly tested!

Generated by SDK Consistency Review Agent for issue #873 ·

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