-
-
Notifications
You must be signed in to change notification settings - Fork 396
Add Real-time Diagnostics System (VSCode-style) #162
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?
Add Real-time Diagnostics System (VSCode-style) #162
Conversation
- Implement extensible diagnostic provider system - Add TypeScript and ESLint providers with Python example - Integrate diagnostics into writeFile and edit_block handlers - Make diagnostics opt-in via configuration - Support provider selection and warning filtering - Add demo files showing the feature in action This allows users to get instant lint/type error feedback after file modifications, similar to VSCode's experience.
- Add extensible diagnostic provider architecture - Implement TypeScript, ESLint, and Python (flake8) providers - Add timeout and output size limits for robustness - Run providers in parallel for performance - Add configurable output limits (maxDiagnostics) - Add diagnostic configuration tools to MCP server - Update README and add comprehensive DIAGNOSTICS.md - Make diagnostics opt-in by default Features: - Real-time error/warning feedback after write_file and edit_block - Configurable providers, warnings, and output limits - Handles missing tools gracefully - Extensible system for community providers
- Change default enabled value from false to true - Update documentation to reflect the new default - Users can still disable with configure_diagnostics {"enabled": false}
- Add TypeScript, ESLint, and Flake8 diagnostic providers - Implement extensible provider architecture for community contributions - Add configure_diagnostics and list_diagnostic_providers MCP tools - Include robust error handling with timeouts and output limits - Filter diagnostics to show only errors from the edited file - Disable by default (opt-in) for conservative PR approach - Add comprehensive test suite covering all functionality - Update README with detailed documentation and examples - Maintain complete backwards compatibility with zero breaking changes This feature transforms Desktop Commander MCP into a comprehensive development environment that provides immediate code quality feedback similar to VS Code's error highlighting. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThis update introduces a real-time diagnostics system to Desktop Commander, modeled after VSCode's error and warning feedback. It includes a configurable, extensible diagnostics framework with built-in providers for TypeScript, ESLint, and Flake8. The system integrates with file write and edit operations, provides new configuration tools, and adds comprehensive documentation and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DesktopCommander
participant DiagnosticsFramework
participant Provider[DiagnosticProvider(s)]
participant ConfigManager
User->>DesktopCommander: Write/Edit file
DesktopCommander->>DiagnosticsFramework: collectDiagnostics(filePath)
DiagnosticsFramework->>ConfigManager: getDiagnosticsConfig()
ConfigManager-->>DiagnosticsFramework: DiagnosticsConfig
loop For each enabled provider
DiagnosticsFramework->>Provider: runDiagnostics(filePath)
Provider-->>DiagnosticsFramework: Diagnostic[]
end
DiagnosticsFramework-->>DesktopCommander: DiagnosticsResult
DesktopCommander->>User: Success message + formatted diagnostics
Suggested labels
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/tools/schemas.ts (1)
97-106
: LGTM: Well-structured schemas for diagnostics tools.The schema definitions are clean and follow good practices with optional fields and appropriate types.
Consider adding validation constraints for better robustness:
export const ConfigureDiagnosticsArgsSchema = z.object({ enabled: z.boolean().optional(), providers: z.array(z.string()).optional(), showWarnings: z.boolean().optional(), showInlineAnnotations: z.boolean().optional(), - maxDiagnostics: z.number().optional(), + maxDiagnostics: z.number().min(1).max(1000).optional(), });DIAGNOSTICS.md (1)
92-92
: Minor grammar improvement suggestion.Consider adding the article "the" for better readability.
-- Results are limited by `maxDiagnostics` setting +- Results are limited by the `maxDiagnostics` settingsrc/tools/diagnostics-config.ts (1)
43-43
: Remove redundant fallback for maxDiagnostics.The
|| 20
fallback is unnecessary sincemaxDiagnostics
should already have a default value from the configuration merge.- `- Max diagnostics shown: ${updatedDiagnostics.maxDiagnostics || 20}` + `- Max diagnostics shown: ${updatedDiagnostics.maxDiagnostics}`README.md (1)
237-244
: Add language specifier to code block.The code block showing diagnostic output should have a language specified for proper syntax highlighting.
-When diagnostics are enabled, you'll see immediate feedback after file operations: -``` +When diagnostics are enabled, you'll see immediate feedback after file operations: +```textPR_DESCRIPTION.md (1)
110-110
: Fix compound adjective hyphenation."Backwards compatible" should be hyphenated when used as a compound adjective.
-- **Backward compatible**: Works with all existing configurations +- **Backward-compatible**: Works with all existing configurationssrc/tools/diagnostics.ts (1)
349-349
: Use optional chaining for cleaner code.Replace the conditional checks with optional chaining for more concise and safer code.
-if (results && results[0] && results[0].messages) { +if (results?.[0]?.messages) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test-diagnostics/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (24)
DIAGNOSTICS.md
(1 hunks)PR_DESCRIPTION.md
(1 hunks)README.md
(4 hunks)demo-diagnostics.md
(1 hunks)src/config-manager.ts
(2 hunks)src/handlers/filesystem-handlers.ts
(2 hunks)src/server.ts
(3 hunks)src/tools/diagnostics-config.ts
(1 hunks)src/tools/diagnostics.ts
(1 hunks)src/tools/edit.ts
(2 hunks)src/tools/schemas.ts
(1 hunks)test-diagnostics/.eslintrc.json
(1 hunks)test-diagnostics/demo-disabled.ts
(1 hunks)test-diagnostics/demo-enabled.ts
(1 hunks)test-diagnostics/demo.ts
(1 hunks)test-diagnostics/package.json
(1 hunks)test-diagnostics/simple-test.js
(1 hunks)test-diagnostics/simple-test.ts
(1 hunks)test-diagnostics/test-eslint.js
(1 hunks)test-diagnostics/test-with-errors.ts
(1 hunks)test-diagnostics/test-with-tsconfig.ts
(1 hunks)test-diagnostics/tsconfig.json
(1 hunks)test-diagnostics/typescript-only-test.ts
(1 hunks)test/test-diagnostics.js
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
DIAGNOSTICS.md
[uncategorized] ~92-~92: You might be missing the article “the” here.
Context: ...un in parallel - Results are limited by maxDiagnostics
setting ## Extending Di...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
PR_DESCRIPTION.md
[uncategorized] ~110-~110: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...eaking changes**: Completely opt-in and backwards compatible - Production ready: Comprehensive t...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Biome (1.9.4)
src/tools/diagnostics.ts
[error] 349-349: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 markdownlint-cli2 (0.17.2)
README.md
237-237: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
🔇 Additional comments (29)
test-diagnostics/simple-test.js (1)
1-6
: Well-designed test case for ESLint diagnostics.This test file effectively triggers multiple ESLint violations (quote style, missing semicolon, unused variable) to verify the diagnostics system. The intentional errors are clearly documented with comments, making the test purpose explicit.
test-diagnostics/demo-disabled.ts (1)
1-4
: Effective test case for disabled diagnostics scenario.This file creates a clear TypeScript type error (returning number instead of string) to verify that diagnostics are properly suppressed when disabled. The intentional error is well-documented and serves its testing purpose effectively.
test-diagnostics/simple-test.ts (1)
1-4
: Clear and simple TypeScript diagnostics test case.This test file creates a straightforward type error to verify basic TypeScript diagnostics functionality. The error is well-documented with an explanatory comment and effectively serves its testing purpose.
test-diagnostics/tsconfig.json (1)
1-10
: Appropriate TypeScript configuration for diagnostic testing.The configuration enables strict type checking and uses modern ES2020 target, which is ideal for testing TypeScript diagnostics. The strict mode ensures intentional type errors in test files will be properly detected by the diagnostics system.
test-diagnostics/demo-enabled.ts (1)
1-6
: Comprehensive test case for multiple diagnostic providers.This test file effectively combines both TypeScript type errors and ESLint warnings in a single file, making it valuable for testing the integration of multiple diagnostic providers. Both intentional errors are clearly documented and serve distinct testing purposes.
test-diagnostics/.eslintrc.json (1)
1-18
: LGTM! Well-configured ESLint setup for testing.The ESLint configuration is appropriate for testing the diagnostics system. The chosen rules (indentation, quotes, semicolons) will generate predictable linting errors that can be verified by the test suite.
test-diagnostics/package.json (1)
1-13
: LGTM! Appropriate test package configuration.The package.json correctly sets up the test environment with TypeScript and ESLint dependencies. The placeholder test script is acceptable for this testing context.
test-diagnostics/test-eslint.js (1)
1-16
: LGTM! Intentional ESLint violations for testing.The file correctly contains intentional ESLint rule violations that align with the configuration in
.eslintrc.json
. These violations will effectively test the ESLint diagnostics provider:
- Double quotes instead of single quotes (line 4)
- Missing semicolon (line 4)
- Unused variable (line 9)
- Missing newline at end of file
This is exactly what's needed for comprehensive diagnostics testing.
src/tools/edit.ts (2)
10-10
: LGTM! Clean import of diagnostics functionality.The import statement properly brings in the diagnostics functions needed for integration.
184-194
: LGTM! Well-integrated diagnostics collection.The diagnostics integration is thoughtfully implemented:
- Only runs after successful file edits
- Uses existing configuration system
- Gracefully handles diagnostics results
- Provides immediate feedback without disrupting existing functionality
The implementation follows good practices by being non-intrusive and defensive.
test-diagnostics/typescript-only-test.ts (1)
1-12
: LGTM! Intentional TypeScript errors for testing.The file contains well-chosen TypeScript errors for testing the diagnostics system:
- Type mismatch: function declared to return
string
but returnsnumber
(line 3)- Missing type annotation: parameter lacks explicit type (line 7)
- Reference error: typo in
console
object name (line 12)These realistic errors will effectively test the TypeScript diagnostics provider.
src/handlers/filesystem-handlers.ts (3)
18-18
: LGTM: Clean import for diagnostics functionality.The import statement correctly brings in the necessary diagnostics functions for integration with file operations.
190-190
: LGTM: Clean integration of diagnostics output.The diagnostics message is properly appended to the success message, providing users with immediate feedback after file writes.
182-186
: To confirm whethercollectDiagnostics
already guards against failures, please inspect its full implementation:#!/bin/bash # Display the full collectDiagnostics function for review sed -n '450,550p' src/tools/diagnostics.tstest-diagnostics/test-with-tsconfig.ts (1)
1-13
: LGTM: Comprehensive test file for TypeScript diagnostics.This test file effectively covers key TypeScript diagnostic scenarios:
- Type mismatch errors (line 2-5)
- Implicit 'any' parameter warnings (line 8-10)
- Reference errors from typos (line 13)
The intentional errors are well-documented and realistic, making this an excellent test case for the diagnostics system.
test-diagnostics/demo.ts (1)
1-25
: LGTM: Excellent comprehensive test file for diagnostics system.This demo file provides outstanding coverage of both TypeScript and ESLint diagnostic scenarios:
TypeScript errors:
- Type mismatch (line 5: number returned where string expected)
- Implicit 'any' parameters (line 9)
- Undefined names (line 17: typo in console)
- Invalid type assignments (line 23: string to number)
ESLint violations:
- Quote style violations (line 14)
- Missing semicolons (line 14)
- Unused variables (line 20)
The intentional errors are realistic and well-documented, making this an ideal test case and demonstration of the diagnostics capabilities.
test-diagnostics/test-with-errors.ts (1)
1-24
: LGTM: Good complementary test file for diagnostics validation.This test file provides valuable additional test coverage for the diagnostics system:
TypeScript strict mode errors:
- Missing parameter type annotations (line 8-10)
- Type return mismatches (line 2-5)
Additional diagnostic scenarios:
- ESLint style violations (line 13)
- Type assignment errors (line 16)
- Unused variables (line 19)
- Reference errors (line 22)
Having multiple test files with different error combinations strengthens the test suite and ensures comprehensive validation of the diagnostics system.
src/config-manager.ts (2)
16-22
: Well-structured diagnostics configuration interface.The optional
diagnostics
configuration object provides comprehensive control over the diagnostics system with clear, intuitive field names and appropriate types.
137-142
: Conservative and sensible default configuration.The default diagnostics configuration is well-designed:
- Disabled by default (opt-in approach)
- Empty providers array for "all available" is intuitive
- Reasonable defaults for display options
test/test-diagnostics.js (3)
12-23
: Proper test setup and teardown.The setup and teardown functions correctly manage test configuration and cleanup, ensuring test isolation.
37-43
: Good test case with intentional TypeScript errors.The test content includes multiple types of TypeScript errors (type mismatch, undefined variable) which effectively validates the diagnostics system.
52-57
: Graceful handling of missing tools.The test properly handles the case where TypeScript compiler might not be available, providing clear feedback rather than failing.
demo-diagnostics.md (1)
1-93
: Excellent documentation structure and clarity.This POC documentation effectively explains the diagnostics system architecture, configuration options, and extensibility. The examples are practical and the extension guide provides clear implementation guidance.
src/server.ts (3)
35-36
: Proper imports for diagnostics functionality.The import statements correctly reference the new diagnostics schemas and configuration handlers.
Also applies to: 39-39
337-367
: Well-documented diagnostics tools.The tool descriptions are comprehensive, explaining functionality, options, and available providers clearly. The descriptions follow the established pattern and provide sufficient detail for users.
525-530
: Consistent handler integration.The new diagnostics tool handlers are properly integrated into the switch statement following the established error handling and invocation patterns.
DIAGNOSTICS.md (1)
1-188
: Comprehensive and well-structured documentation.This documentation thoroughly covers all aspects of the diagnostics system including configuration, usage, troubleshooting, and extensibility. The structure is logical and examples are practical and helpful.
README.md (1)
184-244
: Excellent documentation for the diagnostics feature!The documentation is comprehensive, with clear examples and configuration options. The quick start section and example usage patterns will help users adopt the feature easily.
src/tools/diagnostics.ts (1)
1-588
: Well-architected diagnostics system with extensible provider pattern!The implementation demonstrates excellent software engineering practices:
- Clean separation of concerns with the provider interface
- Robust error handling with timeouts and output limits
- Good telemetry integration for monitoring
- Extensible design allowing easy addition of new providers
The parallel execution of providers and smart filtering show good performance considerations.
const isEnabled = diagnosticsConfig.providers.length === 0 || | ||
diagnosticsConfig.providers.includes(p.name as never); | ||
return `- ${p.name}: ${p.fileExtensions.join(', ')} ${isEnabled ? '(enabled)' : '(disabled)'}`; |
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.
Fix type safety issue with provider name checking.
The as never
type cast indicates a type mismatch. This should be properly typed to avoid runtime issues.
The type cast suggests that diagnosticsConfig.providers
might not be properly typed as string[]
. Please verify the type definition and fix it properly:
- const isEnabled = diagnosticsConfig.providers.length === 0 ||
- diagnosticsConfig.providers.includes(p.name as never);
+ const isEnabled = diagnosticsConfig.providers.length === 0 ||
+ diagnosticsConfig.providers.includes(p.name);
If the types don't align, update the configuration type definition to ensure providers
is string[]
.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const isEnabled = diagnosticsConfig.providers.length === 0 || | |
diagnosticsConfig.providers.includes(p.name as never); | |
return `- ${p.name}: ${p.fileExtensions.join(', ')} ${isEnabled ? '(enabled)' : '(disabled)'}`; | |
const isEnabled = diagnosticsConfig.providers.length === 0 || | |
diagnosticsConfig.providers.includes(p.name); | |
return `- ${p.name}: ${p.fileExtensions.join(', ')} ${isEnabled ? '(enabled)' : '(disabled)'}`; |
🤖 Prompt for AI Agents
In src/tools/diagnostics-config.ts around lines 57 to 59, the use of 'as never'
to cast p.name indicates a type mismatch when checking if
diagnosticsConfig.providers includes p.name. To fix this, ensure that
diagnosticsConfig.providers is correctly typed as string[] in its type
definition. Then remove the 'as never' cast and use p.name directly, which
should be typed as string, to maintain type safety and avoid runtime issues.
const currentDiagnostics = config.diagnostics || { | ||
enabled: false, | ||
providers: [], | ||
showWarnings: true, | ||
showInlineAnnotations: false, | ||
maxDiagnostics: 20 | ||
}; |
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.
🛠️ Refactor suggestion
Avoid duplicating default diagnostics configuration.
The default diagnostics configuration is duplicated here and in diagnostics.ts
(lines 39-44). This could lead to inconsistencies if defaults are changed in one place but not the other.
Import and reuse the default configuration from diagnostics.ts:
-import { diagnosticRegistry } from './diagnostics.js';
+import { diagnosticRegistry, DEFAULT_DIAGNOSTICS_CONFIG } from './diagnostics.js';
Then use it directly:
- const currentDiagnostics = config.diagnostics || {
- enabled: false,
- providers: [],
- showWarnings: true,
- showInlineAnnotations: false,
- maxDiagnostics: 20
- };
+ const currentDiagnostics = config.diagnostics || DEFAULT_DIAGNOSTICS_CONFIG;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const currentDiagnostics = config.diagnostics || { | |
enabled: false, | |
providers: [], | |
showWarnings: true, | |
showInlineAnnotations: false, | |
maxDiagnostics: 20 | |
}; | |
// At the top of src/tools/diagnostics-config.ts | |
-import { diagnosticRegistry } from './diagnostics.js'; | |
+import { diagnosticRegistry, DEFAULT_DIAGNOSTICS_CONFIG } from './diagnostics.js'; | |
// … | |
// Replace the inline default object with the shared constant | |
- const currentDiagnostics = config.diagnostics || { | |
- enabled: false, | |
- providers: [], | |
- showWarnings: true, | |
- showInlineAnnotations: false, | |
- maxDiagnostics: 20 | |
- }; | |
+ const currentDiagnostics = config.diagnostics || DEFAULT_DIAGNOSTICS_CONFIG; | |
// … |
🤖 Prompt for AI Agents
In src/tools/diagnostics-config.ts around lines 15 to 21, the default
diagnostics configuration is duplicated from diagnostics.ts (lines 39-44),
risking inconsistencies. To fix this, import the default diagnostics
configuration object from diagnostics.ts and replace the inline default object
with this imported constant, ensuring a single source of truth for default
settings.
async runDiagnostics(filePath: string): Promise<Diagnostic[]> { | ||
return new Promise((resolve) => { | ||
const diagnostics: Diagnostic[] = []; | ||
const flake8 = spawn('flake8', ['--format', '%(path)s:%(row)d:%(col)d: %(code)s %(text)s', filePath], { | ||
shell: true | ||
}); | ||
|
||
let output = ''; | ||
|
||
flake8.stdout.on('data', (data) => { | ||
output += data.toString(); | ||
}); | ||
|
||
flake8.on('close', () => { | ||
const lines = output.split('\n'); | ||
for (const line of lines) { | ||
const match = line.match(/^(.+?):(\d+):(\d+): ([A-Z]\d+) (.+)$/); | ||
if (match) { | ||
diagnostics.push({ | ||
file: match[1], | ||
line: parseInt(match[2]), | ||
column: parseInt(match[3]), | ||
severity: match[4].startsWith('E') ? 'error' : 'warning', | ||
code: match[4], | ||
message: match[5], | ||
source: this.name | ||
}); | ||
} | ||
} | ||
resolve(diagnostics); | ||
}); | ||
|
||
flake8.on('error', () => resolve([])); | ||
}); | ||
} |
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.
🛠️ Refactor suggestion
Add timeout and output limits to Flake8 provider for consistency.
Unlike the TypeScript and ESLint providers, the Flake8 provider lacks timeout and output size limits, which could lead to hanging processes or memory issues.
The Flake8 provider should implement the same timeout and output size limits as the other providers for consistency and reliability. Would you like me to generate the implementation with these safeguards?
🤖 Prompt for AI Agents
In src/tools/diagnostics.ts between lines 403 and 437, the runDiagnostics method
for the Flake8 provider lacks timeout and output size limits, risking hanging
processes or excessive memory use. To fix this, add a timeout mechanism that
kills the Flake8 process if it runs too long, and implement a maximum output
size limit by truncating or stopping data collection when the output exceeds a
set threshold. This will align Flake8's behavior with the TypeScript and ESLint
providers and improve reliability.
// Default diagnostics configuration | ||
const DEFAULT_DIAGNOSTICS_CONFIG: DiagnosticsConfig = { | ||
enabled: false, // Disabled by default - opt-in feature | ||
providers: [], | ||
showWarnings: true, | ||
showInlineAnnotations: false | ||
}; |
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.
🛠️ Refactor suggestion
Export DEFAULT_DIAGNOSTICS_CONFIG to avoid duplication.
This constant should be exported so it can be reused in diagnostics-config.ts
instead of being duplicated.
-// Default diagnostics configuration
-const DEFAULT_DIAGNOSTICS_CONFIG: DiagnosticsConfig = {
+// Default diagnostics configuration
+export const DEFAULT_DIAGNOSTICS_CONFIG: DiagnosticsConfig = {
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
// Default diagnostics configuration | |
const DEFAULT_DIAGNOSTICS_CONFIG: DiagnosticsConfig = { | |
enabled: false, // Disabled by default - opt-in feature | |
providers: [], | |
showWarnings: true, | |
showInlineAnnotations: false | |
}; | |
// Default diagnostics configuration | |
export const DEFAULT_DIAGNOSTICS_CONFIG: DiagnosticsConfig = { | |
enabled: false, // Disabled by default - opt-in feature | |
providers: [], | |
showWarnings: true, | |
showInlineAnnotations: false | |
}; |
🤖 Prompt for AI Agents
In src/tools/diagnostics.ts around lines 38 to 44, the constant
DEFAULT_DIAGNOSTICS_CONFIG is defined but not exported, causing duplication in
diagnostics-config.ts. Fix this by adding an export statement to
DEFAULT_DIAGNOSTICS_CONFIG so it can be imported and reused in other files.
return new Promise((resolve) => { | ||
const check = spawn('which', ['flake8'], { shell: true }); | ||
check.on('close', (code) => resolve(code === 0)); | ||
check.on('error', () => resolve(false)); | ||
}); |
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.
🛠️ Refactor suggestion
❓ Verification inconclusive
Make platform detection cross-platform.
The which
command is Unix-specific and won't work on Windows. Consider using a cross-platform approach.
Consider using Node.js built-in methods or a cross-platform package to check for command availability:
-return new Promise((resolve) => {
- const check = spawn('which', ['flake8'], { shell: true });
- check.on('close', (code) => resolve(code === 0));
- check.on('error', () => resolve(false));
-});
+return new Promise((resolve) => {
+ const command = process.platform === 'win32' ? 'where' : 'which';
+ const check = spawn(command, ['flake8'], { shell: true });
+ check.on('close', (code) => resolve(code === 0));
+ check.on('error', () => resolve(false));
+});
🏁 Script executed:
#!/bin/bash
# Check if there's a cross-platform utility available for command detection
fd -t f "command.*\.ts|command.*\.js" --exec grep -l "command.*exists\|which\|where"
Length of output: 86
Make platform detection cross-platform.
The which
command is Unix-specific and won't work on Windows. Consider using a cross-platform approach.
#!/bin/bash
# Check if there's a cross-platform utility available for command detection
fd -t f "command.*\.ts|command.*\.js" --exec grep -l "command.*exists\|which\|where"
Consider using Node.js built-in methods or a cross-platform package to check for command availability:
-return new Promise((resolve) => {
- const check = spawn('which', ['flake8'], { shell: true });
- check.on('close', (code) => resolve(code === 0));
- check.on('error', () => resolve(false));
-});
+return new Promise((resolve) => {
+ const command = process.platform === 'win32' ? 'where' : 'which';
+ const check = spawn(command, ['flake8'], { shell: true });
+ check.on('close', (code) => resolve(code === 0));
+ check.on('error', () => resolve(false));
+});
🤖 Prompt for AI Agents
In src/tools/diagnostics.ts around lines 396 to 400, the current code uses the
Unix-specific 'which' command to check for 'flake8', which fails on Windows.
Replace this with a cross-platform method such as using the Node.js
'child_process' to run 'where' on Windows and 'which' on Unix, or better, use a
Node.js package like 'command-exists' to check command availability in a
platform-independent way. Update the Promise logic accordingly to resolve true
if the command is found and false otherwise.
Add Real-time Diagnostics System (VSCode-style)
🎯 Summary
This PR adds a comprehensive real-time diagnostics system to Desktop Commander MCP that provides immediate feedback on code quality, similar to VS Code's error highlighting. When enabled, users see TypeScript errors, ESLint warnings, and other diagnostics automatically after writing or editing files.
✨ Features
🔧 Supported Diagnostic Providers
⚙️ Configuration Options
enabled
: Enable/disable the diagnostics system (default: false - opt-in)providers
: Array of providers to use (empty = all available)showWarnings
: Include warnings in output (default: true)showInlineAnnotations
: Show inline code annotations (default: false)maxDiagnostics
: Maximum number of diagnostics to display (default: 20)🚀 New MCP Tools
configure_diagnostics
: Configure diagnostic providers and settingslist_diagnostic_providers
: List available providers and their status💡 Example Usage
Enable TypeScript diagnostics:
Immediate feedback after file operations:
🏗️ Architecture
Extensible Provider System
The diagnostics system uses a clean provider-based architecture:
Robust Error Handling
Performance Optimizations
🧪 Testing
Added comprehensive test suite covering:
🔒 Security & Performance
Conservative Defaults
Safe Implementation
📚 Documentation
🎁 Value Proposition
For Developers
For the Project
🔄 Migration & Compatibility
🚀 Future Possibilities
This PR establishes the foundation for:
This feature transforms Desktop Commander MCP from a powerful file manager into a comprehensive development environment that rivals dedicated IDEs, while maintaining its simplicity and performance.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests