@W-21741005: NEW - Implement Strategy Pattern for multi-engine custom rule creation#441
@W-21741005: NEW - Implement Strategy Pattern for multi-engine custom rule creation#441aruntyagiTutu wants to merge 8 commits into
Conversation
- Created strategy pattern infrastructure for extensible rule creation - Added IRuleCreationStrategy interface defining strategy contract - Implemented XPathRuleStrategy for PMD/XPath rules - Implemented RegexRuleStrategy for regex pattern rules - Created RuleCreationStrategyFactory for engine-based strategy selection - Refactored create_custom_rule tool to use strategy pattern - Tool now supports both engine: "pmd" and engine: "regex" - Added CreateRegexCustomRuleActionImpl for regex rule creation - Created temporary create_regex_rule tool for testing (to be deleted) - Updated provider to use RuleCreationStrategyFactory Benefits: - Single entry point for multiple engines - Easy to extend with new engines - Engine-specific logic isolated in strategies - Maintains backward compatibility with existing PMD functionality Regex rules support: - Inline YAML rules in code-analyzer.yml - Required: regex, violationMessage, tags, severity - Optional: fileExtensions, regexIgnore, includeMetadata - Validates regex format, severity range, file extensions
aruntyagiTutu
changed the title
- Refactored create_custom_rule.test.ts to use strategy mocks - Updated provider.test.ts to expect 7 tools (added temporary create_regex_rule) - All 228 tests passing - Coverage: 89.05% statements, 76.76% branches
…ests - Commented out create_regex_rule tool registration in provider - Tool code kept in codebase for reference only - Users can test regex rule creation via create_custom_rule with engine: 'regex' - Updated provider test to expect 6 tools instead of 7 - All 228 tests passing - E2E tests should now pass (9 tools expected)
- Added RegexRuleStrategy.test.ts with 26 tests - Added XPathRuleStrategy.test.ts with 20 tests - Added RuleCreationStrategyFactory.test.ts with 13 tests - Total: 59 new tests added - All 287 tests passing Coverage improvements: - RegexRuleStrategy: 8.69% → 100% - XPathRuleStrategy: 11.11% → 100% - RuleCreationStrategyFactory: 38.46% → 100% - Overall: 89.05% → 97.37% Test coverage includes: - Validation logic for all required/optional fields - Error handling for invalid inputs - Execute functionality for rule creation - File system operations (config creation/updates) - Factory pattern (strategy selection and registration) - Edge cases (case sensitivity, whitespace handling)
- Restored create_regex_rule.ts that was accidentally deleted - File kept for manual testing reference only (not registered in provider)
- Increased 'should enable 1 tool and a toolset' test timeout from 60s to 90s - Added connection timeout with Promise.race to fail fast on server issues - Connection timeout set to 90s for Windows compatibility This addresses E2E test failures on Windows runners where connection establishment takes longer than on Linux/macOS.
| errors.push("regex is required for regex engine"); | ||
| } else if (!regex.startsWith("/") || regex.lastIndexOf("/") <= 0) { | ||
| errors.push("regex must be in format '/pattern/flags' (e.g., '/todo/gi')"); | ||
| } |
There was a problem hiding this comment.
We can introduce more stringent validation on the regex pattern to ensure they are valid. Also we can check for performance of the regex pattern, so that the user does not ends up with regex pattern which can lead to backtracking.
| "For Apex and Visualforce, use tool 'get_ast_nodes_to_generate_xpath' to generate the XPath." | ||
| ); | ||
| } else { | ||
| errors.push("xpath is required for PMD engine. Provide a valid XPath expression."); |
There was a problem hiding this comment.
Is LLM going to read these error messages and act on the basis of that?
| - priority: PMD priority (1-5) | ||
| - workingDirectory: Workspace directory where files will be created | ||
|
|
||
| Output: Creates a ruleset XML file and updates code-analyzer.yml |
There was a problem hiding this comment.
We don't need the 2 step instruction we had created earlier and this is sufficient? We were previously specifying that the ruleset XML reference would be only in the config yml. Also user might be using custom config file as well.
ZLeventer
left a comment
ZLeventer
left a comment
There was a problem hiding this comment.
The Strategy Pattern refactor for multi-engine rule creation is a meaningful architectural improvement. Having a pluggable engine interface makes it straightforward to add new analysis engines without modifying existing code paths.
From a user perspective, the key benefit here is extensibility — custom rule engines can be dropped in without touching the core MCP tool registration. This matters for teams that have proprietary static analysis rules they want to integrate alongside the built-in engines.
The workingDirectory parameter addition in #439 (which this builds on) is also important — without it, the MCP server assumes CWD which breaks in many IDE integration scenarios where the MCP server's CWD doesn't match the project being analyzed.
3 participants
@W-21741005@
Benefits:
Regex rules support:
Note: unit tests and testing is pending after stretegy pattern and need to delete create-regex-rule tool, that was created only for testing puprpose.