diff --git a/.cursor/commands/deslop.md b/.cursor/commands/deslop.md index f1eafd10..cd86aaa9 100644 --- a/.cursor/commands/deslop.md +++ b/.cursor/commands/deslop.md @@ -1,4 +1,4 @@ -# Remove AI code slop +# Remove AI Code Slop Check the diff against main, and remove all AI-generated slop introduced in this branch. @@ -7,6 +7,6 @@ This includes: - Extra comments that a human wouldn't add or are inconsistent with the rest of the file - Extra defensive checks or try/catch blocks that are abnormal for that area of the codebase (especially if called by trusted / validated codepaths) - Casts to any to get around type issues -- Any other style that is inconsistent with the file +- Any other violation inconsistent with the rest of the file or our rules Report at the end with only a 1-3 sentence summary of what you changed diff --git a/.cursor/commands/refactor.md b/.cursor/commands/refactor.md deleted file mode 100644 index fec65c22..00000000 --- a/.cursor/commands/refactor.md +++ /dev/null @@ -1 +0,0 @@ -Refactor following our minimalist code philosophy then organize and catalog like a librarian and obsess over code consistency. Let's take this code from an A+ to an A++ 🚀 diff --git a/.cursor/rules/01-architecture.mdc b/.cursor/rules/01-architecture.mdc index 91c568b1..0ac9ddb6 100644 --- a/.cursor/rules/01-architecture.mdc +++ b/.cursor/rules/01-architecture.mdc @@ -12,7 +12,42 @@ All rules MANDATORY. - Explicit return types with generics: `Collection` - Dependency injection via Symfony patterns - Use Symfony classes over native PHP functions (Filesystem, Process) for testability -- **Yoda conditions:** Always place constants on the left side of comparisons to prevent accidental assignment +- **Yoda conditions:** Always place constants/literals on the LEFT side of comparisons to prevent accidental assignment +- **Always use braces:** ALL control structures (`if`, `else`, `elseif`, `for`, `foreach`, `while`, `do-while`) MUST use curly braces `{ }`, even for single-line bodies + +```php +// ✅ CORRECT - Yoda conditions (constant on left) +if (null === $value) { ... } +if ('' === trim($name)) { ... } +if (0 !== $exitCode) { ... } +if ('active' === $status) { ... } +if (null !== $this->repo->find($id)) { ... } + +// ❌ WRONG - Variable on left (risk of accidental assignment) +if ($value === null) { ... } +if (trim($name) === '') { ... } +if ($exitCode !== 0) { ... } +if ($status === 'active') { ... } + +// Note: Two variables - Yoda doesn't apply +if ($typedName !== $server->name) { ... } +if ($userInput === $expectedValue) { ... } +``` + +```php +// ✅ CORRECT - Always use braces +if (null === $value) { + return 'Value is required'; +} + +foreach ($items as $item) { + $this->process($item); +} + +// ❌ WRONG - Never omit braces +if (null === $value) return 'Value is required'; +foreach ($items as $item) $this->process($item); +``` ### PHPStan Type Hints diff --git a/.cursor/rules/03-commands.mdc b/.cursor/rules/03-commands.mdc index 36100df1..aadbb16a 100644 --- a/.cursor/rules/03-commands.mdc +++ b/.cursor/rules/03-commands.mdc @@ -108,66 +108,119 @@ protected function execute(InputInterface $input, OutputInterface $output): int Prompt wrappers (`$this->io->promptText()`, `$this->io->promptSelect()`, etc.) automatically suppress extra spacing for clean output. -### Input Validation Pattern +### Input Validation -Validation traits provide reusable validation for both Laravel Prompts and CLI options. +**Core Principles:** -**Validation Method Pattern:** +- Validate everything: CLI options, prompts, and auto-resolved values +- Fail fast: Validate before expensive operations (API calls, SSH connections) +- Never silent fallback: Explicit user input must fail validation, not fall back to defaults +- Complete errors: Include invalid value and guidance + +**Validator Signature:** ```php -// ✅ CORRECT - Accept mixed, return ?string (error message or null) -protected function validateNameInput(mixed $name): ?string +// Returns ?string: error message if invalid, null if valid +protected function validateNameInput(mixed $value): ?string { - if (!is_string($name)) { - return 'Server name must be a string'; + if (!is_string($value)) { + return 'Name must be a string'; } - if (trim($name) === '') { - return 'Server name cannot be empty'; + if ('' === trim($value)) { + return 'Name cannot be empty'; } - if ($this->servers->findByName($name) !== null) { - return "Server '{$name}' already exists"; + if (null !== $this->repo->findByName($value)) { + return "'{$value}' already exists"; } return null; } - -// ❌ WRONG - Throws exception (incompatible with Laravel Prompts) -protected function validateName(string $name): void -{ - if (trim($name) === '') { - throw new \InvalidArgumentException('Name cannot be empty'); - } -} ``` -**Usage:** +**Naming Convention:** + +- `validate*Input()` - Returns `?string` (for prompts/options) +- `validate*()` - Throws exceptions (for heavy I/O like git repo checks) + +**Usage with getValidatedOptionOrPrompt:** ```php $name = $this->io->getValidatedOptionOrPrompt( 'name', - fn ($validate) => $this->io->promptText( - label: 'Server name:', - placeholder: 'web1', - validate: $validate // Pass validator to prompt - ), - fn ($value) => $this->validateNameInput($value) // Returns ?string + fn ($validate) => $this->io->promptText(label: 'Name:', validate: $validate), + fn ($value) => $this->validateNameInput($value) ); -if ($name === null) { +if (null === $name) { return Command::FAILURE; // Validation failed, error already displayed } ``` -Validator injected into prompt callback, validates interactively and on CLI options. +**Optional Input with Fallback Resolution:** -**Naming Convention:** +```php +// Allow empty to trigger default resolution +protected function validateKeyPathInputAllowEmpty(mixed $path): ?string +{ + if (!is_string($path)) { + return 'Path must be a string'; + } -- `validate*Input()` - Returns `?string` (for prompts/options) -- `validate*()` - Throws exceptions (for heavy I/O like git repo checks) + if ('' === trim($path)) { + return null; // Allow empty - triggers default + } + + return $this->validateKeyPathInput($path); // Non-empty: validate strictly +} + +// After validation - only fallback for empty, expand explicit paths +$resolved = ('' === trim($pathRaw)) + ? $this->resolveDefaultPath() // Fallback resolution + : $this->fs->expandPath($pathRaw); // User's explicit path (validated) +``` + +**Selection from Dynamic Data:** + +```php +protected function validateRegion(mixed $region, array $validRegions): ?string +{ + if (!is_string($region)) { + return 'Region must be a string'; + } + + if (!isset($validRegions[$region])) { + return "Invalid region: '{$region}' not available"; + } + + return null; +} +``` + +**Error Message Guidelines:** + +- Include invalid value: `"Invalid region: 'xyz1' not available"` +- Provide guidance: `"Port must be 1-65535 (common: 22, 2222)"` +- Show format examples: `"UUID format: 12345678-1234-1234-1234-123456789abc"` + +**Common Mistakes:** + +```php +// ❌ Missing null check +$value = $this->io->getValidatedOptionOrPrompt(...); +$this->doSomething($value); // $value could be null! + +// ❌ Silent fallback on explicit input +$path = $this->getOptionOrPrompt('key-path', ...); +$resolved = $this->resolveKey($path); // Falls back even if user's path invalid! + +// ❌ CLI option bypasses validation +$env = $this->getOptionOrPrompt('env', fn() => promptSelect(..., options: $valid)); +// --env=invalid passes through unvalidated! +``` -**Examples:** ServerValidationTrait.php, SiteValidationTrait.php +See: ServerValidationTrait.php, KeyValidationTrait.php ### Boolean Flags @@ -217,13 +270,13 @@ $selected = $this->io->getOptionOrPrompt( if (is_string($selected)) { $selected = array_filter( array_map(trim(...), explode(',', $selected)), - static fn (string $item): bool => $item !== '' + static fn (string $item): bool => '' !== $item ); } // Validate CLI-provided values against allowed options $unknown = array_diff($selected, array_keys($allowedOptions)); -if ($unknown !== []) { +if ([] !== $unknown) { $this->nay('Unknown options: ' . implode(', ', $unknown)); return Command::FAILURE; } @@ -251,14 +304,14 @@ $generateKey = $input->getOption('generate-deploy-key'); $customKeyPath = $input->getOption('custom-deploy-key'); // Check for conflicting options -if ($generateKey && $customKeyPath !== null) { +if ($generateKey && null !== $customKeyPath) { $this->nay('Cannot use both --generate-deploy-key and --custom-deploy-key'); return Command::FAILURE; } if ($generateKey) { $deployKeyPath = null; // Use server-generated -} elseif ($customKeyPath !== null) { +} elseif (null !== $customKeyPath) { $deployKeyPath = $customKeyPath; // Use custom } else { // Interactive: prompt for choice, then conditionally prompt for path @@ -271,7 +324,7 @@ if ($generateKey) { default: 'generate' ); - if ($choice === 'generate') { + if ('generate' === $choice) { $deployKeyPath = null; } else { $deployKeyPath = $this->io->promptText( @@ -369,7 +422,7 @@ protected function selectServer(): ServerDTO|int // Validate CLI-provided name exists $server = $this->servers->findByName($name); - if ($server === null) { + if (null === $server) { $this->nay("Server '{$name}' not found in inventory"); return Command::FAILURE; } @@ -432,7 +485,7 @@ $replayOptions = [ 'php-version' => $phpVersion, ]; -if ($deployKeyPath !== null) { +if (null !== $deployKeyPath) { $replayOptions['custom-deploy-key'] = $deployKeyPath; } else { $replayOptions['generate-deploy-key'] = true; diff --git a/.cursor/rules/04-exceptions.mdc b/.cursor/rules/04-exceptions.mdc index 0ccce49c..3af74f26 100644 --- a/.cursor/rules/04-exceptions.mdc +++ b/.cursor/rules/04-exceptions.mdc @@ -59,11 +59,11 @@ protected function validateNameInput(mixed $name): ?string return 'Server name must be a string'; } - if (trim($name) === '') { + if ('' === trim($name)) { return 'Server name cannot be empty'; } - if ($this->servers->findByName($name) !== null) { + if (null !== $this->servers->findByName($name)) { return "Server '{$name}' already exists"; } @@ -115,7 +115,7 @@ protected function serverInfo(ServerDTO $server): array|int return Command::FAILURE; } - if ($result['exit_code'] !== 0) { + if (0 !== $result['exit_code']) { $this->nay('Failed to gather server information'); return Command::FAILURE; } diff --git a/app/Console/Key/KeyAddDigitalOceanCommand.php b/app/Console/Key/KeyAddDigitalOceanCommand.php index f03c9962..fc74ae87 100644 --- a/app/Console/Key/KeyAddDigitalOceanCommand.php +++ b/app/Console/Key/KeyAddDigitalOceanCommand.php @@ -121,6 +121,10 @@ protected function gatherKeyDeets(): ?array fn ($value) => $this->validateKeyPathInput($value) ); + if (null === $publicKeyPathRaw) { + return null; + } + /** @var ?string $publicKeyPath */ $publicKeyPath = $this->resolvePublicKeyPath($publicKeyPathRaw); diff --git a/app/Console/Server/ServerAddCommand.php b/app/Console/Server/ServerAddCommand.php index c4d2e011..7b0fe828 100644 --- a/app/Console/Server/ServerAddCommand.php +++ b/app/Console/Server/ServerAddCommand.php @@ -182,23 +182,8 @@ protected function gatherServerDeets(): ?array ) ); - /** @var string $privateKeyPathRaw */ - $privateKeyPathRaw = $this->io->getOptionOrPrompt( - 'private-key-path', - fn (): string => $this->io->promptText( - label: 'Path to SSH private key (leave empty for default ~/.ssh/id_ed25519 or ~/.ssh/id_rsa):', - default: '', - required: false, - hint: 'Used to connect to the server' - ) - ); - - /** @var ?string $privateKeyPath */ - $privateKeyPath = $this->resolvePrivateKeyPath($privateKeyPathRaw); - - if ($privateKeyPath === null) { - $this->nay('SSH private key not found.'); - + $privateKeyPath = $this->promptPrivateKeyPath(); + if (is_int($privateKeyPath)) { return null; } diff --git a/app/Console/Server/ServerProvisionDigitalOceanCommand.php b/app/Console/Server/ServerProvisionDigitalOceanCommand.php index e36825c8..bd443e0c 100644 --- a/app/Console/Server/ServerProvisionDigitalOceanCommand.php +++ b/app/Console/Server/ServerProvisionDigitalOceanCommand.php @@ -314,23 +314,8 @@ protected function gatherProvisioningDeets(array $accountData): ?array // // Prompt for local private key path - /** @var string $privateKeyPathRaw */ - $privateKeyPathRaw = $this->io->getOptionOrPrompt( - 'private-key-path', - fn (): string => $this->io->promptText( - label: 'Path to SSH private key (leave empty for default ~/.ssh/id_ed25519 or ~/.ssh/id_rsa):', - default: '', - required: false, - hint: 'Used to connect to the server' - ) - ); - - /** @var ?string $privateKeyPath */ - $privateKeyPath = $this->resolvePrivateKeyPath($privateKeyPathRaw); - - if ($privateKeyPath === null) { - $this->nay('SSH private key not found.'); - + $privateKeyPath = $this->promptPrivateKeyPath(); + if (is_int($privateKeyPath)) { return null; } diff --git a/app/Services/IOService.php b/app/Services/IOService.php index 92e8eccf..2a483589 100644 --- a/app/Services/IOService.php +++ b/app/Services/IOService.php @@ -224,7 +224,6 @@ public function getValidatedOptionOrPrompt( $error = $validator($value); if ($error !== null) { $this->error($error); - $this->out(''); return null; } diff --git a/app/Traits/KeysTrait.php b/app/Traits/KeysTrait.php index b5bb0147..ac287b3f 100644 --- a/app/Traits/KeysTrait.php +++ b/app/Traits/KeysTrait.php @@ -5,13 +5,16 @@ namespace Deployer\Traits; use Deployer\Services\FilesystemService; +use Deployer\Services\IOService; +use Symfony\Component\Console\Command\Command; /** * Reusable SSH key things. * - * Requires classes using this trait to have FilesystemService property. + * Requires classes using this trait to have FilesystemService and IOService properties. * * @property FilesystemService $fs + * @property IOService $io */ trait KeysTrait { @@ -79,6 +82,42 @@ protected function resolveKeyWithFallback(?string $path, array $fallback): ?stri return $this->fs->getFirstExisting($candidates); } + /** + * Prompt for private key path with validation and fallback resolution. + * + * @return string|int Resolved path or Command::FAILURE + */ + protected function promptPrivateKeyPath(): string|int + { + /** @var string|null $pathRaw */ + $pathRaw = $this->io->getValidatedOptionOrPrompt( + 'private-key-path', + fn ($validate) => $this->io->promptText( + label: 'Path to SSH private key (leave empty for default ~/.ssh/id_ed25519 or ~/.ssh/id_rsa):', + default: '', + required: false, + hint: 'Used to connect to the server', + validate: $validate + ), + fn ($value) => $this->validatePrivateKeyPathInputAllowEmpty($value) + ); + + if (null === $pathRaw) { + return Command::FAILURE; + } + + $resolved = ('' === trim($pathRaw)) + ? $this->resolvePrivateKeyPath('') + : $this->fs->expandPath($pathRaw); + + if (null === $resolved) { + $this->nay('No default SSH key found. Create ~/.ssh/id_ed25519 or ~/.ssh/id_rsa, or specify a path.'); + return Command::FAILURE; + } + + return $resolved; + } + // ---- // Validation // ---- @@ -246,6 +285,24 @@ protected function validatePrivateKeyPathInput(mixed $path): ?string return null; } + /** + * Validate SSH private key file, allowing empty paths. + * + * @return string|null Error message if invalid, null if valid + */ + protected function validatePrivateKeyPathInputAllowEmpty(mixed $path): ?string + { + if (!is_string($path)) { + return 'Key path must be a string'; + } + + if ('' === trim($path)) { + return null; // Allow empty - triggers default key resolution + } + + return $this->validatePrivateKeyPathInput($path); + } + /** * Validate deploy key pair (private key + corresponding public key). *