|
| 1 | +# Implementation Plan: PromptGetException in Prompts |
| 2 | + |
| 3 | +## Objective |
| 4 | +Add proper error handling to all prompt classes by implementing validation and throwing `PromptGetException` from `Mcp\Exception\PromptGetException` when invalid parameters are provided. |
| 5 | + |
| 6 | +## Overview |
| 7 | + |
| 8 | +Each prompt accepts parameters that define behavior. These should be validated before processing, and `PromptGetException` should be thrown for invalid inputs. |
| 9 | + |
| 10 | +- **PromptNotFoundException**: For prompts that don't exist (handled by framework) |
| 11 | +- **PromptGetException**: For runtime errors during prompt execution (our responsibility) |
| 12 | + |
| 13 | +## Current State Analysis |
| 14 | + |
| 15 | +| Prompt | Parameters | Current Validation | Status | |
| 16 | +|--------|-----------|-------------------|--------| |
| 17 | +| DocumentationExpertPrompt | `$depth` | Falls back to default | ❌ Needs validation | |
| 18 | +| DebugHelperPrompt | `$context` | Checks empty only | ❌ Needs validation | |
| 19 | +| FeatureBuilderPrompt | `$component` | Likely no validation | ❌ Needs validation | |
| 20 | +| DatabaseExplorerPrompt | `$table`, `$show` | No validation | ❌ Needs validation | |
| 21 | +| CodeReviewerPrompt | `$focus` | Unknown | ❌ Needs validation | |
| 22 | +| MigrationGuidePrompt | `$fromVersion`, `$toVersion`, `$area` | No validation | ❌ Needs validation | |
| 23 | +| TestingAssistantPrompt | `$testType` | Unknown | ❌ Needs validation | |
| 24 | +| PerformanceAnalyzerPrompt | `$context` | Unknown | ❌ Needs validation | |
| 25 | +| OrmQueryHelperPrompt | `$queryGoal`, `$tables` | Unknown | ❌ Needs validation | |
| 26 | +| TinkerWorkshopPrompt | `$goal` | Unknown | ❌ Needs validation | |
| 27 | +| QualityAssurancePrompt | `$context`, `$tools` | Unknown | ❌ Needs validation | |
| 28 | + |
| 29 | +## Implementation Strategy |
| 30 | + |
| 31 | +### Phase 1: Add Validation Infrastructure to AbstractPrompt |
| 32 | + |
| 33 | +**File**: `src/Prompts/AbstractPrompt.php` |
| 34 | + |
| 35 | +Add utility methods for parameter validation: |
| 36 | + |
| 37 | +```php |
| 38 | +/** |
| 39 | + * Validate that a parameter value is one of allowed values |
| 40 | + * |
| 41 | + * @param string $value The parameter value |
| 42 | + * @param array<string> $allowed Allowed values |
| 43 | + * @param string $paramName The parameter name (for error message) |
| 44 | + * @return void |
| 45 | + * @throws \Mcp\Exception\PromptGetException |
| 46 | + */ |
| 47 | +protected function validateEnumParameter( |
| 48 | + string $value, |
| 49 | + array $allowed, |
| 50 | + string $paramName |
| 51 | +): void; |
| 52 | + |
| 53 | +/** |
| 54 | + * Validate that a parameter is not empty |
| 55 | + * |
| 56 | + * @param string $value The parameter value |
| 57 | + * @param string $paramName The parameter name (for error message) |
| 58 | + * @return void |
| 59 | + * @throws \Mcp\Exception\PromptGetException |
| 60 | + */ |
| 61 | +protected function validateNonEmptyParameter(string $value, string $paramName): void; |
| 62 | + |
| 63 | +/** |
| 64 | + * Validate comma-separated values against allowed list |
| 65 | + * |
| 66 | + * @param string $value Comma-separated values |
| 67 | + * @param array<string> $allowed Allowed values |
| 68 | + * @param string $paramName The parameter name (for error message) |
| 69 | + * @return void |
| 70 | + * @throws \Mcp\Exception\PromptGetException |
| 71 | + */ |
| 72 | +protected function validateCommaSeparatedParameter( |
| 73 | + string $value, |
| 74 | + array $allowed, |
| 75 | + string $paramName |
| 76 | +): void; |
| 77 | +``` |
| 78 | + |
| 79 | +### Phase 2: Update Prompts (Priority Order) |
| 80 | + |
| 81 | +#### High Priority |
| 82 | + |
| 83 | +**1. DocumentationExpertPrompt** |
| 84 | +- Validate: `$depth` ∈ ['basic', 'intermediate', 'advanced'] |
| 85 | +- Action: Replace `match()` default with exception |
| 86 | + |
| 87 | +**2. DebugHelperPrompt** |
| 88 | +- Validate: `$context` ∈ ['', 'controller', 'model', 'database', 'view'] |
| 89 | +- Action: Add enum validation |
| 90 | + |
| 91 | +**3. FeatureBuilderPrompt** |
| 92 | +- Validate: `$component` ∈ ['controller', 'model', 'behavior', 'helper', 'middleware', 'command', 'full-stack'] |
| 93 | +- Action: Add enum validation |
| 94 | + |
| 95 | +#### Medium Priority |
| 96 | + |
| 97 | +**4. DatabaseExplorerPrompt** |
| 98 | +- Validate: `$show` ∈ ['schema', 'data', 'relationships', 'all'] |
| 99 | +- Validate: `$table` is not empty |
| 100 | +- Action: Add enum and non-empty validation |
| 101 | + |
| 102 | +**5. CodeReviewerPrompt** |
| 103 | +- Validate: `$focus` ∈ ['conventions', 'security', 'performance', 'testing', 'all'] |
| 104 | +- Action: Add enum validation |
| 105 | + |
| 106 | +**6. QualityAssurancePrompt** |
| 107 | +- Validate: `$context` ∈ ['guidelines', 'integration', 'troubleshooting', 'all'] |
| 108 | +- Validate: `$tools` contains valid tool names |
| 109 | +- Action: Add enum and comma-separated validation |
| 110 | + |
| 111 | +#### Low Priority |
| 112 | + |
| 113 | +**7. TestingAssistantPrompt** |
| 114 | +- Validate: `$testType` ∈ ['unit', 'integration', 'fixture', 'all'] |
| 115 | +- Action: Add enum validation |
| 116 | + |
| 117 | +**8. OrmQueryHelperPrompt** |
| 118 | +- Validate: `$queryGoal` is not empty |
| 119 | +- Validate: `$tables` (optional) contains valid table names or is empty |
| 120 | +- Action: Add non-empty validation for required params |
| 121 | + |
| 122 | +**9. TinkerWorkshopPrompt** |
| 123 | +- Validate: `$goal` ∈ ['explore', 'test', 'debug'] |
| 124 | +- Action: Add enum validation |
| 125 | + |
| 126 | +**10. PerformanceAnalyzerPrompt** |
| 127 | +- Validate: `$concern` is not empty |
| 128 | +- Action: Add non-empty validation |
| 129 | + |
| 130 | +**11. MigrationGuidePrompt** |
| 131 | +- Validate: `$fromVersion` and `$toVersion` match semantic version format |
| 132 | +- Validate: `$area` (optional) is reasonable |
| 133 | +- Action: Add version format validation |
| 134 | + |
| 135 | +### Phase 3: Testing |
| 136 | + |
| 137 | +**New Test File**: `tests/TestCase/Prompts/PromptValidationTest.php` |
| 138 | + |
| 139 | +For each prompt, add tests: |
| 140 | +```php |
| 141 | +public function testValidParametersDoNotThrow(): void |
| 142 | +public function testInvalidParameterThrowsPromptGetException(): void |
| 143 | +public function testErrorMessageContainsParameterInfo(): void |
| 144 | +``` |
| 145 | + |
| 146 | +## Error Message Format |
| 147 | + |
| 148 | +Consistent error messages: |
| 149 | + |
| 150 | +``` |
| 151 | +Invalid value for parameter '{paramName}': '{value}'. |
| 152 | +Expected one of: {allowed values}. |
| 153 | +Prompt: {prompt name} |
| 154 | +``` |
| 155 | + |
| 156 | +Example: |
| 157 | +``` |
| 158 | +Invalid value for parameter 'depth': 'expert'. |
| 159 | +Expected one of: basic, intermediate, advanced. |
| 160 | +Prompt: documentation-expert |
| 161 | +``` |
| 162 | + |
| 163 | +## Files to Modify |
| 164 | + |
| 165 | +1. **src/Prompts/AbstractPrompt.php** - Add validation methods |
| 166 | +2. **src/Prompts/DocumentationExpertPrompt.php** - Add depth validation |
| 167 | +3. **src/Prompts/DebugHelperPrompt.php** - Add context validation |
| 168 | +4. **src/Prompts/FeatureBuilderPrompt.php** - Add component validation |
| 169 | +5. **src/Prompts/DatabaseExplorerPrompt.php** - Add show/table validation |
| 170 | +6. **src/Prompts/CodeReviewerPrompt.php** - Add focus validation |
| 171 | +7. **src/Prompts/QualityAssurancePrompt.php** - Add context/tools validation |
| 172 | +8. **src/Prompts/TestingAssistantPrompt.php** - Add testType validation |
| 173 | +9. **src/Prompts/OrmQueryHelperPrompt.php** - Add queryGoal validation |
| 174 | +10. **src/Prompts/TinkerWorkshopPrompt.php** - Add goal validation |
| 175 | +11. **src/Prompts/PerformanceAnalyzerPrompt.php** - Add concern validation |
| 176 | +12. **src/Prompts/MigrationGuidePrompt.php** - Add version validation |
| 177 | + |
| 178 | +## Testing |
| 179 | + |
| 180 | +Create/update: `tests/TestCase/Prompts/PromptValidationTest.php` |
| 181 | +- Test each prompt's parameter validation |
| 182 | +- Verify correct exception is thrown |
| 183 | +- Verify error message format |
| 184 | + |
| 185 | +## Acceptance Criteria |
| 186 | + |
| 187 | +- [ ] All prompts validate their parameters |
| 188 | +- [ ] Invalid parameters throw `PromptGetException` |
| 189 | +- [ ] Error messages are clear and helpful |
| 190 | +- [ ] All tests pass |
| 191 | +- [ ] PHPStan level 8 compliance maintained |
| 192 | +- [ ] Code follows CakePHP 5.2+ standards |
| 193 | +- [ ] PHPUnit tests added for all validations |
| 194 | + |
| 195 | +## Timeline Estimate |
| 196 | + |
| 197 | +- Phase 1 (Infrastructure): 30 minutes |
| 198 | +- Phase 2 (Updates): 1-2 hours |
| 199 | +- Phase 3 (Testing): 1 hour |
| 200 | +- Total: 2.5-3.5 hours |
0 commit comments