-
-
Notifications
You must be signed in to change notification settings - Fork 35
Make AutoHotkey the default provider and improve Windows documentation #202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Change default provider from keysender to autohotkey - Update environment variable examples to PowerShell syntax - Move keysender to optionalDependencies - Update documentation to reflect keysender as optional - Clarify that build tools are only needed for keysender provider 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Updated provider factory tests to expect AutoHotkey as the default provider instead of Keysender. Added necessary mocks and test cases for AutoHotkey provider to ensure test suite passes with the new default. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
MCPClaude
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall Assessment
This PR changes the default automation provider from keysender to AutoHotkey, makes keysender an optional dependency, and updates Windows-specific documentation. The changes are well-structured and generally make sense given the goal of reducing build tool requirements for most users.
README.md Changes
- Good update to use PowerShell syntax for environment variables (
$env:instead ofexport) - Reordering of providers to show AutoHotkey as default is helpful
- Clear explanation about keysender being optional now
package.json Changes
- Appropriate move of keysender to optionalDependencies
- This will allow users to install the package without the native build requirements if they don't need keysender
config.ts and index.ts Changes
- Proper update of provider defaults and documentation
- Good addition of AutoHotkey to supported providers list
factory.ts and factory.test.ts Changes
- Correctly updated default provider throughout the factory implementation
- Test cases have been appropriately extended to cover AutoHotkey provider
Suggestions for Improvement
-
Error Handling: Consider adding graceful degradation logic when a user attempts to use keysender features but doesn't have it installed. Currently, this might lead to runtime errors that could be difficult to diagnose.
-
Documentation: It might be helpful to add a small section in the README explaining how to install the optional keysender dependency if needed (
npm install keysenderor similar). -
Version Bump: Since this is a potentially breaking change for users who explicitly depend on keysender being installed by default, consider whether this warrants a major version bump according to your versioning strategy.
Overall, this is a well-structured PR that will make the project more accessible to Windows users by reducing the default installation requirements.
|
@MCPClaude are you there? |
|
Yes, I'm here. How can I help you? |
|
@MCPClaude Are you able to see the repository? Do you have shell access? gh cli access? Give me your Commander Data diagnostic report. |
|
I need shell access permissions to run diagnostic commands. The repository is a TypeScript project called MCPControl with providers for automation tasks including keyboard, mouse, and screen control. |
|
@MCPClaude Do you have shell access? |
|
Error processing command: Failed to process command: Command failed: claude --dangerously-skip-permissions --print "Do you have shell access?" |
Summary
Changes
Configuration Updates
src/config.tsDocumentation Improvements
exportto PowerShell$env:syntaxDependency Management
Test plan
npm run buildnpm run lintnpm run format🤖 Generated with Claude Code