refactor: update branding, visual identity and output style - #104
Conversation
Add detailed patterns for: - Core principle: non-interactive CLI option support - Multi-path prompts with separate options - Confirmation patterns (simple and type-to-confirm) - Post-CLI validation - Resource selection pattern - Summary checklist for command builders
Add --generate-deploy-key and --custom-deploy-key options to server:install. Extract setupDeployerUser() method for better organization. Add validatePrivateKeyPathInput() and validateDeployKeyPairInput() to KeysTrait. Update user-install playbook to support custom key installation with proper idempotency: custom keys overwrite existing, auto-generated keys preserve existing.
Update displayDeets parameter type from array<string, mixed> to array<int|string, mixed> to support arrays with integer keys (e.g., DigitalOcean key lists). Also fix array_map call to handle integer keys by casting to string before calculating length.
Add error() method that was referenced in getValidatedOptionOrPrompt but was not defined. Method displays error messages with red styling consistent with other error output methods.
Replace incorrect executePlaybook calls with true argument (which was being passed to a by-reference parameter) with executePlaybookSilently method. This correctly handles silent execution with spinner display and fixes both type mismatch and pass-by-reference errors.
WalkthroughA repository-wide namespace migration from Bigpixelrocket\DeployerPHP to PHPDeployer, plus coordinated refactors: new BaseCommand I/O helpers, IOService changes, updated traits and DTOs, playbook edits (adds/removals), documentation/CLI instruction trims, composer/PSR-4 updates, and multiple command behavior adjustments. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.cursor/rules/rules.mdc (1)
127-132: Token budget target is unrealistic and contradicts actual corpus size.The stated target of <3000 tokens (~600-800 lines) is unachievable: the existing rule corpus totals ~6804 tokens across 1701 lines in 8 files. The largest single file (03-commands.mdc) already exceeds the entire stated budget at ~1888 tokens.
Either clarify that these are aspirational targets requiring substantial refactoring, or recalibrate targets to reflect realistic thresholds for the existing codebase. Prescribing unattainable constraints in guidance documentation will confuse future maintainers.
playbooks/php-install.sh (1)
77-106: Guard Composer installation on PHP CLI availability
install_composer()is called unconditionally inmain()(line 262) and assumesphpCLI is available on PATH. If a user installs only FPM and extensions withoutcliinDEPLOYER_PHP_EXTENSIONS, thephp /tmp/composer-setup.phpcall at line 93 will fail and abort the playbook, which is a behavior change from earlier versions that didn't require Composer.Add a guard to check for PHP CLI presence before attempting Composer installation:
install_composer() { # Ensure composer is installed if not already present if ! command -v composer > /dev/null 2>&1; then if ! command -v php > /dev/null 2>&1; then echo "Warning: php CLI not found; skipping Composer installation" >&2 return 0 fi # … existing install logic … fi }Optionally, consider verifying the installer's signature before execution to harden the supply-chain story.
tests/Integration/ArchitectureTest.php (2)
14-14: Update the namespace reference to match the migration.The arch test is checking for classes in the old
Bigpixelrocket\\DeployerPHP\\Console\\namespace, but the import on Line 5 and the broader PR refactor have migrated toPHPDeployer. This will cause test failures since commands now reside in the new namespace.Apply this diff to fix the namespace reference:
- expect('Bigpixelrocket\\DeployerPHP\\Console\\') + expect('PHPDeployer\\Console\\')
28-28: Update the namespace reference to match the migration.Same issue as Line 14—this arch test still references the old namespace.
Apply this diff:
- expect('Bigpixelrocket\\DeployerPHP\\Console\\') + expect('PHPDeployer\\Console\\')app/SymfonyApp.php (1)
46-49: UpdateVersionServicedefault package name from'bigpixelrocket/deployer-php'to'php-deployer/php-deployer'.The
VersionServiceconstructor (line 20 inapp/Services/VersionService.php) still uses the outdated package name'bigpixelrocket/deployer-php', butcomposer.jsondefines the package as'php-deployer/php-deployer'. This mismatch causesgetVersion()to throw anOutOfBoundsExceptionwhen callingInstalledVersions::getPrettyVersion(), which is caught and returns'dev'as a fallback, resulting in incorrect version display.
🧹 Nitpick comments (13)
playbooks/php-install.sh (1)
70-75:apt_get_with_retryintegration looks good; consider dropping extra2>&1Using
apt_get_with_retryfor package installation is a solid improvement for dealing with dpkg locks. The trailing2>&1is likely unnecessary sinceapt_get_with_retryalready captures and routes stdout/stderr; you could omit it for slightly clearer intent.playbooks/user-install.sh (1)
124-144: Verify key handling security and consider partial input validation.The custom key installation logic correctly uses base64 decoding and
teewithrun_cmdfor privilege handling. However, if only one ofDEPLOYER_KEY_PRIVATEorDEPLOYER_KEY_PUBLICis provided (but not both), the script silently falls through to auto-generation, which could mask configuration errors.Consider adding a warning or validation when only one key is provided:
+ if [[ -n ${DEPLOYER_KEY_PRIVATE:-} && -z ${DEPLOYER_KEY_PUBLIC:-} ]] || \ + [[ -z ${DEPLOYER_KEY_PRIVATE:-} && -n ${DEPLOYER_KEY_PUBLIC:-} ]]; then + echo "Warning: Both DEPLOYER_KEY_PRIVATE and DEPLOYER_KEY_PUBLIC must be provided together" >&2 + echo "→ Falling back to auto-generated key pair..." + fi + if [[ -n ${DEPLOYER_KEY_PRIVATE:-} && -n ${DEPLOYER_KEY_PUBLIC:-} ]]; thenapp/Traits/KeysTrait.php (1)
218-241: Static analysis false positive; validation logic is sound.The Gitleaks warning on lines 219-221 is a false positive — these are PEM header strings used for format validation, not actual private key material.
Note: The code validates DSA keys and returns an error after initially accepting the format at line 222. Consider restructuring to exclude DSA from
$validPrefixesentirely for cleaner logic:$validPrefixes = [ '-----BEGIN OPENSSH PRIVATE KEY-----', '-----BEGIN RSA PRIVATE KEY-----', '-----BEGIN EC PRIVATE KEY-----', - '-----BEGIN DSA PRIVATE KEY-----', '-----BEGIN PRIVATE KEY-----', ]; $isValid = false; foreach ($validPrefixes as $prefix) { if (str_starts_with($key, $prefix)) { $isValid = true; break; } } if (!$isValid) { + // Explicit error for obsolete DSA keys + if (str_starts_with($key, '-----BEGIN DSA PRIVATE KEY-----')) { + return 'DSA keys are obsolete and insecure'; + } return 'Invalid SSH private key format'; } - - // Check for DSA key (obsolete) - if (str_starts_with($key, '-----BEGIN DSA PRIVATE KEY-----')) { - return 'DSA keys are obsolete and insecure'; - }app/Repositories/ServerRepository.php (1)
5-9: ServerRepository doesn’t persist the newServerDTO::infofieldWith
ServerDTOnow exposing aninfoarray and awithInfo()helper,dehydrateServerDTO()/hydrateServerDTO()still ignore that property, so any info attached to a server will be dropped when saving/loading via inventory. Ifinfois meant to be persisted (rather than purely transient metadata), consider extending the mapping:private function dehydrateServerDTO(ServerDTO $server): array { return [ 'name' => $server->name, 'host' => $server->host, 'port' => $server->port, 'username' => $server->username, 'privateKeyPath' => $server->privateKeyPath, 'provider' => $server->provider, 'dropletId' => $server->dropletId, + 'info' => $server->info, ]; } private function hydrateServerDTO(array $data): ServerDTO { @@ - $dropletId = $data['dropletId'] ?? null; + $dropletId = $data['dropletId'] ?? null; + $info = $data['info'] ?? null; return new ServerDTO( name: is_string($name) ? $name : '', host: is_string($host) ? $host : '', port: is_int($port) ? $port : 22, username: is_string($username) ? $username : 'root', privateKeyPath: is_string($privateKeyPath) ? $privateKeyPath : null, provider: is_string($provider) ? $provider : null, - dropletId: is_int($dropletId) ? $dropletId : null, + dropletId: is_int($dropletId) ? $dropletId : null, + info: is_array($info) ? $info : null, ); }If
infois intentionally non‑persistent, a short comment on the DTO or repository to clarify that would also help.Also applies to: 99-120, 157-193
app/Console/Server/ServerRunCommand.php (1)
89-91: Consider simplifying the control flow.Throwing a
RuntimeExceptiononly to catch it in the same try block is functionally correct but slightly unconventional. The previous approach of directly callingnay()and returningCommand::FAILUREhere would be more direct.That said, this pattern does consolidate all error handling through the single catch block, which could be intentional for consistency.
If you prefer a more direct approach:
if ($result['exit_code'] !== 0) { - throw new \RuntimeException("Command failed with exit code {$result['exit_code']}"); + $this->nay("Command failed with exit code {$result['exit_code']}"); + + return Command::FAILURE; } - } catch (\RuntimeException $e) { - $this->nay($e->getMessage()); - - return Command::FAILURE; + } catch (\RuntimeException $e) { + $this->nay($e->getMessage()); + + return Command::FAILURE; }app/Console/Server/ServerInfoCommand.php (2)
79-79: Remove unused$hostparameter.The
$hostparameter is never used withindisplayServerInfo(). As flagged by static analysis, this should be removed to avoid confusion.- private function displayServerInfo(array $info, string $host): void + private function displayServerInfo(array $info): voidAnd update the call site on line 57:
- $this->displayServerInfo($server->info, $server->host); + $this->displayServerInfo($server->info);
79-350: Consider extracting sub-methods fromdisplayServerInfo().This method spans ~270 lines with distinct rendering sections for hardware, ports, Caddy, PHP versions, PHP-FPM, and sites. Extracting these into private helper methods would improve readability and testability:
displayHardwareInfo(array $hardware)displayServicesInfo(array $ports)displayCaddyInfo(array $caddy)displayPhpInfo(array $php)displayPhpFpmInfo(array $phpFpm)displaySitesInfo(array $sitesConfig)app/Console/Key/KeyDeleteDigitalOceanCommand.php (1)
76-82: Consider using consistent output methods for the separator.Lines 81-82 mix
out()andio->write(). PerIOService.php(lines 56-73),out()prepends a▒prefix to each line, whileio->write()does not. This creates visual inconsistency.If the intent is a plain separator without prefix, use:
- $this->out('───'); - $this->io->write('', true); + $this->io->write('───', true); + $this->io->write('', true);Or if the prefix is desired, the current approach is fine but the blank line could use
out('')for consistency.app/Console/Server/ServerProvisionDigitalOceanCommand.php (1)
241-243: Consider removing empty default for region selection.Setting
default: ''for a required selection means no pre-selected option. If the user accidentally presses Enter without selecting, the validation should catch it, but consider using the first available region as the default for better UX.fn ($validate) => $this->io->promptSelect( label: 'Select region:', options: $accountData['regions'], hint: 'Choose the datacenter location', - default: '', + default: array_key_first($accountData['regions']) ?? '', scroll: 15 ),app/Console/Server/ServerInstallCommand.php (1)
113-113: Minor redundancy in parameter passing.The
installPhpmethod receives both$serverand$server->infoas separate parameters. Consider using only$serverand accessing$server->infowithin the method to reduce redundancy.- $phpResult = $this->installPhp($server, $server->info, $packageList); + $phpResult = $this->installPhp($server, $packageList);Then update the method signature and internal references accordingly.
app/Contracts/BaseCommand.php (2)
232-257: Potential issue with non-array iterables inul()andol().If
$linesis a non-array iterable (e.g., aGenerator), the foreach by-reference modification won't persist because iterables are consumed on iteration and the reference assignment won't affect the original. Consider converting to an array explicitly:protected function ul(string|iterable $lines): void { - $writeLines = is_string($lines) ? [$lines] : $lines; + $writeLines = is_string($lines) ? [$lines] : [...$lines]; foreach ($writeLines as &$line) { $line = "• {$line}"; } unset($line); $this->out($writeLines); }Apply the same pattern to
ol().
184-193: Multibyte character support inh2()underline.
strlen()counts bytes, not characters. If$textcontains multibyte characters (e.g., UTF-8 emojis or non-ASCII text), the underline will be shorter than the visible heading width. Consider usingmb_strlen()for accurate character counting:- $underline = str_repeat('─', strlen($h2)); + $underline = str_repeat('─', mb_strlen($h2));app/Traits/ServersTrait.php (1)
214-242:getSiteConfig()is defensive and well-typed.The method properly validates the
sites_configstructure and provides sensible defaults. The return type annotation accurately describes the expected shape.Minor: Lines 238-239 have extra blank lines that are inconsistent with the rest of the file's formatting.
]; } - - // ----
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (80)
.cursor/cli.json(0 hunks).cursor/commands/_analyze.md(0 hunks).cursor/commands/_create-branch-and-commits.md(0 hunks).cursor/commands/_create-branch.md(0 hunks).cursor/commands/_create-commits.md(0 hunks).cursor/commands/_in-branch.md(0 hunks).cursor/commands/_in-diff.md(0 hunks).cursor/commands/_report.md(0 hunks).cursor/commands/_review.md(0 hunks).cursor/commands/create-branch-and-commits.md(0 hunks).cursor/commands/create-branch.md(1 hunks).cursor/commands/create-commits.md(1 hunks).cursor/commands/deslop.md(1 hunks).cursor/rules/00-main.mdc(3 hunks).cursor/rules/01-architecture.mdc(1 hunks).cursor/rules/02-tests.mdc(1 hunks).cursor/rules/03-commands.mdc(8 hunks).cursor/rules/06-playbooks.mdc(1 hunks).cursor/rules/rules.mdc(1 hunks).gitignore(1 hunks)README.md(1 hunks)app/Console/HelloCommand.php(1 hunks)app/Console/Key/KeyAddDigitalOceanCommand.php(3 hunks)app/Console/Key/KeyDeleteDigitalOceanCommand.php(5 hunks)app/Console/Key/KeyListDigitalOceanCommand.php(3 hunks)app/Console/ScaffoldHooksCommand.php(5 hunks)app/Console/Server/ServerAddCommand.php(4 hunks)app/Console/Server/ServerDeleteCommand.php(8 hunks)app/Console/Server/ServerInfoCommand.php(2 hunks)app/Console/Server/ServerInstallCommand.php(11 hunks)app/Console/Server/ServerListCommand.php(3 hunks)app/Console/Server/ServerLogsCommand.php(10 hunks)app/Console/Server/ServerProvisionDigitalOceanCommand.php(8 hunks)app/Console/Server/ServerRunCommand.php(7 hunks)app/Console/Site/SiteAddCommand.php(9 hunks)app/Console/Site/SiteDeleteCommand.php(7 hunks)app/Console/Site/SiteDeployCommand.php(12 hunks)app/Console/Site/SiteHttpsCommand.php(7 hunks)app/Console/Site/SiteListCommand.php(3 hunks)app/Console/Site/SiteSharedPullCommand.php(6 hunks)app/Console/Site/SiteSharedPushCommand.php(5 hunks)app/Container.php(1 hunks)app/Contracts/BaseCommand.php(3 hunks)app/DTOs/ServerDTO.php(2 hunks)app/DTOs/SiteDTO.php(1 hunks)app/Enums/Distribution.php(1 hunks)app/Enums/DistributionFamily.php(1 hunks)app/Exceptions/SSHTimeoutException.php(1 hunks)app/Repositories/ServerRepository.php(3 hunks)app/Repositories/SiteRepository.php(1 hunks)app/Services/DigitalOcean/BaseDigitalOceanService.php(1 hunks)app/Services/DigitalOcean/DigitalOceanAccountService.php(4 hunks)app/Services/DigitalOcean/DigitalOceanDropletService.php(1 hunks)app/Services/DigitalOcean/DigitalOceanKeyService.php(1 hunks)app/Services/DigitalOceanService.php(1 hunks)app/Services/EnvService.php(2 hunks)app/Services/FilesystemService.php(1 hunks)app/Services/GitService.php(1 hunks)app/Services/HttpService.php(1 hunks)app/Services/IOService.php(3 hunks)app/Services/InventoryService.php(2 hunks)app/Services/ProcessService.php(1 hunks)app/Services/SSHService.php(1 hunks)app/Services/VersionService.php(1 hunks)app/SymfonyApp.php(3 hunks)app/Traits/DigitalOceanTrait.php(5 hunks)app/Traits/KeysTrait.php(2 hunks)app/Traits/PlaybooksTrait.php(7 hunks)app/Traits/ServersTrait.php(5 hunks)app/Traits/SiteSharedPathsTrait.php(1 hunks)app/Traits/SitesTrait.php(4 hunks)bin/deployer(1 hunks)composer.json(3 hunks)playbooks/demo-site.sh(0 hunks)playbooks/php-install.sh(3 hunks)playbooks/server-info.sh(1 hunks)playbooks/site-add.sh(2 hunks)playbooks/user-install.sh(3 hunks)tests/Integration/ArchitectureTest.php(1 hunks)tests/TestCase.php(1 hunks)
💤 Files with no reviewable changes (11)
- .cursor/commands/_review.md
- .cursor/commands/_create-branch.md
- .cursor/commands/_analyze.md
- .cursor/commands/_in-diff.md
- .cursor/commands/_in-branch.md
- .cursor/cli.json
- .cursor/commands/create-branch-and-commits.md
- playbooks/demo-site.sh
- .cursor/commands/_create-branch-and-commits.md
- .cursor/commands/_create-commits.md
- .cursor/commands/_report.md
🧰 Additional context used
🧬 Code graph analysis (31)
app/Console/HelloCommand.php (1)
app/Contracts/BaseCommand.php (1)
BaseCommand(30-355)
app/Services/DigitalOcean/DigitalOceanKeyService.php (1)
app/Services/FilesystemService.php (1)
FilesystemService(28-179)
app/Repositories/ServerRepository.php (2)
app/DTOs/ServerDTO.php (1)
ServerDTO(7-40)app/Services/InventoryService.php (1)
InventoryService(33-264)
app/Services/DigitalOceanService.php (3)
app/Services/DigitalOcean/DigitalOceanAccountService.php (1)
DigitalOceanAccountService(17-186)app/Services/DigitalOcean/DigitalOceanDropletService.php (1)
DigitalOceanDropletService(14-186)app/Services/DigitalOcean/DigitalOceanKeyService.php (1)
DigitalOceanKeyService(14-75)
app/Console/ScaffoldHooksCommand.php (2)
app/Contracts/BaseCommand.php (5)
BaseCommand(30-355)h1(171-179)commandReplay(305-354)displayDeets(277-298)out(264-267)app/Services/IOService.php (1)
out(57-74)
app/Traits/SiteSharedPathsTrait.php (1)
app/DTOs/SiteDTO.php (1)
SiteDTO(7-24)
app/Console/Server/ServerListCommand.php (1)
app/Contracts/BaseCommand.php (2)
BaseCommand(30-355)h1(171-179)
app/Services/SSHService.php (2)
app/DTOs/ServerDTO.php (1)
ServerDTO(7-40)app/Exceptions/SSHTimeoutException.php (1)
SSHTimeoutException(10-19)
playbooks/user-install.sh (1)
playbooks/helpers.sh (1)
run_cmd(19-25)
app/Traits/KeysTrait.php (1)
app/Services/FilesystemService.php (4)
FilesystemService(28-179)expandPath(124-154)exists(42-45)readFile(52-55)
app/Console/Server/ServerAddCommand.php (3)
app/Contracts/BaseCommand.php (3)
BaseCommand(30-355)h1(171-179)commandReplay(305-354)app/DTOs/ServerDTO.php (1)
ServerDTO(7-40)app/Traits/ServersTrait.php (1)
serverInfo(73-121)
app/Console/Site/SiteListCommand.php (1)
app/Contracts/BaseCommand.php (2)
BaseCommand(30-355)h1(171-179)
app/Console/Key/KeyDeleteDigitalOceanCommand.php (2)
app/Contracts/BaseCommand.php (6)
BaseCommand(30-355)h1(171-179)displayDeets(277-298)out(264-267)warn(214-217)commandReplay(305-354)app/Services/IOService.php (2)
out(57-74)write(90-93)
app/Console/Site/SiteHttpsCommand.php (5)
app/Contracts/BaseCommand.php (6)
BaseCommand(30-355)h1(171-179)info(198-201)warn(214-217)out(264-267)commandReplay(305-354)app/Traits/SitesTrait.php (1)
selectSite(69-105)app/Traits/ServersTrait.php (2)
serverInfo(73-121)getSiteConfig(214-236)app/Services/IOService.php (1)
out(57-74)app/Traits/PlaybooksTrait.php (1)
executePlaybookSilently(37-52)
app/Traits/PlaybooksTrait.php (4)
app/DTOs/ServerDTO.php (1)
ServerDTO(7-40)app/Services/IOService.php (4)
IOService(30-489)out(57-74)write(90-93)promptSpin(475-488)app/Services/SSHService.php (2)
SSHService(37-333)executeCommand(69-102)app/Contracts/BaseCommand.php (2)
out(264-267)nay(222-225)
app/Console/Key/KeyAddDigitalOceanCommand.php (1)
app/Contracts/BaseCommand.php (3)
BaseCommand(30-355)h1(171-179)commandReplay(305-354)
app/Console/Site/SiteDeployCommand.php (4)
app/Contracts/BaseCommand.php (7)
BaseCommand(30-355)h1(171-179)warn(214-217)out(264-267)info(198-201)commandReplay(305-354)displayDeets(277-298)app/DTOs/SiteDTO.php (1)
SiteDTO(7-24)app/Services/IOService.php (1)
out(57-74)app/Traits/ServersTrait.php (1)
serverInfo(73-121)
app/Console/Key/KeyListDigitalOceanCommand.php (1)
app/Contracts/BaseCommand.php (4)
BaseCommand(30-355)h1(171-179)displayDeets(277-298)commandReplay(305-354)
playbooks/php-install.sh (1)
playbooks/helpers.sh (1)
apt_get_with_retry(130-166)
app/Repositories/SiteRepository.php (2)
app/DTOs/SiteDTO.php (1)
SiteDTO(7-24)app/Services/InventoryService.php (1)
InventoryService(33-264)
app/Services/IOService.php (1)
app/Contracts/BaseCommand.php (1)
out(264-267)
app/Services/VersionService.php (2)
app/Services/GitService.php (1)
__construct(14-18)app/SymfonyApp.php (1)
__construct(44-55)
app/Console/Server/ServerLogsCommand.php (3)
app/Contracts/BaseCommand.php (7)
BaseCommand(30-355)h1(171-179)info(198-201)commandReplay(305-354)h2(184-193)out(264-267)nay(222-225)app/DTOs/ServerDTO.php (1)
ServerDTO(7-40)app/Services/IOService.php (2)
write(90-93)out(57-74)
tests/Integration/ArchitectureTest.php (1)
app/Contracts/BaseCommand.php (1)
BaseCommand(30-355)
app/DTOs/ServerDTO.php (1)
app/Contracts/BaseCommand.php (1)
info(198-201)
app/Console/Site/SiteSharedPushCommand.php (4)
app/Contracts/BaseCommand.php (5)
BaseCommand(30-355)h1(171-179)info(198-201)out(264-267)commandReplay(305-354)app/DTOs/ServerDTO.php (1)
ServerDTO(7-40)app/Traits/ServersTrait.php (1)
serverInfo(73-121)app/Services/IOService.php (1)
out(57-74)
app/SymfonyApp.php (1)
app/Services/VersionService.php (1)
VersionService(12-39)
app/Console/Site/SiteDeleteCommand.php (4)
app/Contracts/BaseCommand.php (4)
BaseCommand(30-355)out(264-267)info(198-201)commandReplay(305-354)app/Services/IOService.php (1)
out(57-74)app/Traits/ServersTrait.php (1)
serverInfo(73-121)app/Traits/PlaybooksTrait.php (1)
executePlaybookSilently(37-52)
app/Console/Site/SiteAddCommand.php (4)
app/Contracts/BaseCommand.php (4)
BaseCommand(30-355)h1(171-179)out(264-267)commandReplay(305-354)app/Traits/ServersTrait.php (1)
selectServer(128-164)app/Traits/PlaybooksTrait.php (1)
executePlaybookSilently(37-52)app/Services/IOService.php (1)
out(57-74)
app/Console/Server/ServerProvisionDigitalOceanCommand.php (4)
app/Contracts/BaseCommand.php (4)
BaseCommand(30-355)h1(171-179)commandReplay(305-354)warn(214-217)app/DTOs/ServerDTO.php (1)
ServerDTO(7-40)app/Traits/ServersTrait.php (1)
serverInfo(73-121)app/Repositories/ServerRepository.php (1)
create(48-65)
app/Traits/DigitalOceanTrait.php (1)
app/Contracts/BaseCommand.php (3)
nay(222-225)info(198-201)ul(232-240)
🪛 Gitleaks (8.30.0)
app/Traits/KeysTrait.php
[high] 219-221: Identified a Private Key, which may compromise cryptographic security and sensitive data encryption.
(private-key)
🪛 LanguageTool
.cursor/commands/deslop.md
[uncategorized] ~3-~3: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...k the diff against main, and remove all AI generated slop introduced in this branch. This i...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 PHPMD (2.15.0)
app/Console/Server/ServerInfoCommand.php
79-79: Avoid unused parameters such as '$host'. (undefined)
(UnusedFormalParameter)
Update package name from bigpixelrocket/deployer-php to php-deployer/php-deployer Update namespace references from Bigpixelrocket\DeployerPHP to PHPDeployer
Fix typo in PlaybooksTrait comment (ouput -> output) Fix typos and formatting in deslop.md command documentation
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/Traits/PlaybooksTrait.php (1)
117-118: Typo fix confirmed.The previous review comment about "ouput" → "output" has been addressed.
🧹 Nitpick comments (1)
app/Traits/PlaybooksTrait.php (1)
180-190: Consider a distinct spinner message for YAML retrieval.The YAML read operation reuses
$statusMessagefor the spinner. In captured mode, users might briefly see the same spinner message twice (once for execution, once for reading output). This is minor since the read is quick, but a distinct message like"Retrieving results..."could improve clarity.$yamlResult = $this->io->promptSpin( callback: fn () => $this->ssh->executeCommand( $server, sprintf('cat %s 2>/dev/null && rm -f %s', escapeshellarg($outputFile), escapeshellarg($outputFile)), null, 30 ), - message: $statusMessage + message: 'Retrieving results...' );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.cursor/commands/deslop.md(1 hunks)app/Services/VersionService.php(1 hunks)app/Traits/PlaybooksTrait.php(7 hunks)tests/Integration/ArchitectureTest.php(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/Integration/ArchitectureTest.php
- .cursor/commands/deslop.md
🧰 Additional context used
🧬 Code graph analysis (2)
app/Traits/PlaybooksTrait.php (4)
app/DTOs/ServerDTO.php (1)
ServerDTO(7-40)app/Services/IOService.php (4)
IOService(30-489)out(57-74)write(90-93)promptSpin(475-488)app/Services/SSHService.php (2)
SSHService(37-333)executeCommand(69-102)app/Contracts/BaseCommand.php (2)
out(264-267)nay(222-225)
app/Services/VersionService.php (1)
app/SymfonyApp.php (1)
__construct(44-55)
🔇 Additional comments (9)
app/Services/VersionService.php (3)
5-5: LGTM: Namespace updated correctly.The namespace migration to
PHPDeployer\Servicesaligns with the repository-wide refactoring objectives.
19-21: Previous issue resolved: Package name updated correctly.The default package name has been updated to
'php-deployer/php-deployer', which addresses the concern raised in the previous review. This now matches the package name incomposer.jsonand will allowInstalledVersions::getPrettyVersion()to correctly retrieve the package version.
27-38: LGTM: Clean and defensive implementation.The simplified version detection logic is well-structured with appropriate error handling:
- Gracefully handles missing
InstalledVersionsclass- Catches
OutOfBoundsExceptionwhen package isn't found- Provides sensible 'dev' fallback for all error cases
app/Traits/PlaybooksTrait.php (6)
5-14: Namespace migration looks correct.The namespace and import updates are consistent with the repository-wide refactor from
Bigpixelrocket\DeployerPHPtoPHPDeployer.
28-52: LGTM!The
executePlaybookSilentlymethod cleanly wrapsexecutePlaybookby providing a non-null$captureto trigger the spinner-based silent mode. The implementation correctly delegates all parameters.
66-79: Signature changes are well-documented.The refactored signature with
$statusMessageand the by-reference$captureparameter provides a clean API for both streaming and captured execution modes. The docblock clearly explains the semantics.
84-115: Playbook preparation logic is solid.The unique output filename generation using
time()andrandom_bytes(8)provides adequate collision avoidance. The heredoc wrapping with'DEPLOYER_SCRIPT_EOF'(quoted) correctly prevents variable expansion in the script content.
121-150: Execution branching logic is well-structured.The dual-mode execution (streaming vs. captured with spinner) is cleanly implemented. The error output display on line 143-148 correctly shows captured output only when an error occurs, preventing unnecessary noise on success.
203-222: YAML parsing with good error visibility.The error handling correctly displays the raw YAML content when parsing fails, which aids debugging malformed output from playbooks.
1 participant
Summary by CodeRabbit
Chores
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.