feat: site shared paths - #93
Conversation
WalkthroughRefactors site creation to integrate PHP version selection into site info gathering; adds provisioning validation to shared commands; extends FilesystemService and GitService with new capabilities; registers new commands; and enhances deployment scripts with sudoers configuration. Changes
Sequence DiagramssequenceDiagram
participant User
participant SiteAddCmd as SiteAddCommand
participant SitesTrait as SitesTrait
participant Provisioner
User->>SiteAddCmd: execute()
SiteAddCmd->>SitesTrait: gatherSiteInfo(serverInfo)
Note over SitesTrait: Now accepts server info parameter
SiteAddCmd->>SitesTrait: selectPhpVersion(serverInfo)
SitesTrait-->>SiteAddCmd: phpVersion
Note over SiteAddCmd: Check if int (failure)
SiteAddCmd->>Provisioner: provision(site, phpVersion)
Note over SiteAddCmd: phpVersion now included throughout
Provisioner-->>SiteAddCmd: success
SiteAddCmd-->>User: Next steps
sequenceDiagram
participant User
participant SharedCmd as SiteShared(Push/Pull)Command
participant SitesTrait as SitesTrait
participant SSHService
participant Server
User->>SharedCmd: execute()
SharedCmd->>SitesTrait: getServerInfo()
SitesTrait-->>SharedCmd: serverInfo
Note over SharedCmd: NEW: Validate provisioning
SharedCmd->>SitesTrait: validateSiteProvisioned(server, site)
SitesTrait->>SSHService: executeRemoteCheck(test commands)
SSHService->>Server: Check directory & Caddy config
Server-->>SSHService: result
SSHService-->>SitesTrait: success/failure
SitesTrait-->>SharedCmd: null or Command::FAILURE
alt Validation Success
SharedCmd->>SharedCmd: Resolve paths & continue
else Validation Failure
SharedCmd-->>User: Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/Console/Site/SiteAddCommand.php (1)
231-286: Consider inlining single-use method selectPhpVersion.The
selectPhpVersionmethod is only called once fromgatherSiteInfo(Line 362). While the extraction improves readability, it adds method call overhead for a single-use case.As per coding guidelines, consider inlining this logic directly into
gatherSiteInfoto eliminate the single-use method, or keep it separate if you anticipate future reuse in other commands.app/Services/GitService.php (1)
93-98: Consider logging cleanup failures for observability.If
remove()fails in the finally block, the exception is silently swallowed. While this prevents masking the primary operation's result, it could leave temporary directories orphaned.Consider adding optional cleanup failure logging (if a logging mechanism is available) to track orphaned temp directories for operational awareness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
app/Console/Site/SiteAddCommand.php(4 hunks)app/Console/Site/SiteSharedPullCommand.php(1 hunks)app/Console/Site/SiteSharedPushCommand.php(1 hunks)app/Services/FilesystemService.php(1 hunks)app/Services/GitService.php(2 hunks)app/SymfonyApp.php(4 hunks)app/Traits/SitesTrait.php(2 hunks)playbooks/install-deployer.sh(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/00-main.mdc)
**/*.php: Eliminate single-use methods: inline if a method is called only once
Cache computed values: initialize expensive calculations in the constructor
Avoid method call overhead: prefer direct property access when appropriate
Files:
app/Console/Site/SiteSharedPullCommand.phpapp/Services/FilesystemService.phpapp/Services/GitService.phpapp/Traits/SitesTrait.phpapp/Console/Site/SiteSharedPushCommand.phpapp/SymfonyApp.phpapp/Console/Site/SiteAddCommand.php
🧠 Learnings (12)
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/@(Service|Services)/**/*.php : Services provide atomic, reusable functionality and must not perform console I/O
Applied to files:
app/Traits/SitesTrait.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/@(Service|Services)/**/*.php : Services may depend on other Services or utilities
Applied to files:
app/Traits/SitesTrait.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands must not duplicate orchestration logic—extract to shared Services
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Use SymfonyStyle consistently for all user-facing console output
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/@(Service|Services)/**/*.php : Extract complex orchestration shared by multiple Commands into dedicated Services
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands receive Services via constructor injection
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands handle user interaction (input/output) and orchestrate Services
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands are responsible for console styling, error formatting, and user prompts
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Only Commands perform console input/output operations
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands may only depend on Services (not other Commands)
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands must not contain business logic—delegate to Services
Applied to files:
app/SymfonyApp.php
📚 Learning: 2025-09-22T11:10:21.459Z
Learnt from: CR
Repo: deployer-plus/deployer-php PR: 0
File: .cursor/rules/01-architecture.mdc:0-0
Timestamp: 2025-09-22T11:10:21.459Z
Learning: Applies to app/**/Command/**/*.php : Commands should not invoke other commands (no proxy commands)
Applied to files:
app/SymfonyApp.php
🧬 Code graph analysis (5)
app/Console/Site/SiteSharedPullCommand.php (1)
app/Traits/SitesTrait.php (1)
validateSiteProvisioned(219-247)
app/Services/FilesystemService.php (2)
app/Services/DigitalOcean/DigitalOceanKeyService.php (1)
DigitalOceanKeyService(14-75)app/Services/ProcessService.php (1)
__construct(19-22)
app/Traits/SitesTrait.php (4)
app/DTOs/ServerDTO.php (1)
ServerDTO(7-19)app/DTOs/SiteDTO.php (1)
SiteDTO(7-24)app/Services/SSHService.php (2)
SSHService(37-333)executeCommand(69-102)app/Contracts/BaseCommand.php (1)
nay(186-190)
playbooks/install-deployer.sh (1)
playbooks/helpers.sh (1)
run_cmd(18-24)
app/Console/Site/SiteSharedPushCommand.php (1)
app/Traits/SitesTrait.php (1)
validateSiteProvisioned(219-247)
🔇 Additional comments (10)
playbooks/install-deployer.sh (1)
158-163: Ensure heredoc delimiters prevent variable expansion.The heredoc uses
'EOF'(quoted) which correctly prevents shell variable expansion. This is the right approach for sudoers content.app/Console/Site/SiteAddCommand.php (1)
98-109: LGTM! Clean integration of PHP version into site info gathering.The refactored flow consolidates PHP version selection within
gatherSiteInfo, making the site details gathering more cohesive and reducing the steps in the main execution flow.app/Console/Site/SiteSharedPullCommand.php (1)
88-96: LGTM! Proper early validation of site provisioning.The validation step correctly ensures the site is provisioned before attempting file operations, preventing cryptic errors from missing directories or configurations.
app/Console/Site/SiteSharedPushCommand.php (1)
88-96: LGTM! Consistent provisioning validation.The validation step matches the pattern in
SiteSharedPullCommand, ensuring consistent behavior across shared file operations.app/Services/FilesystemService.php (2)
67-76: LGTM! Clean wrapper following established patterns.The
removemethod maintains consistency with the existing Symfony Filesystem wrapper approach, delegating error handling appropriately.
156-178: LGTM! Useful utility method with proper error handling.The
getFirstExistingmethod provides a clean way to find the first valid path from candidates, with proper tilde expansion and exception handling.app/Services/GitService.php (1)
67-99: LGTM! Well-structured remote file existence check.The implementation uses shallow cloning efficiently and includes proper cleanup in the finally block. The 30-second timeout is appropriate for network operations.
app/SymfonyApp.php (1)
22-23: LGTM! Proper command registration.The new shared file commands are correctly imported and registered, following the established pattern in the application.
Also applies to: 166-167
app/Traits/SitesTrait.php (2)
210-247: LGTM! Robust provisioning validation with proper security.The validation method efficiently checks both directory and configuration file in a single SSH call, uses proper shell escaping with
escapeshellarg, and provides clear user guidance on failure.
19-25: LGTM! Trait dependencies properly documented.The SSHService dependency is correctly added to both the docblock description and @Property annotations, maintaining consistency with existing trait documentation patterns.
Based on learnings
Sudoers rules specified /bin/systemctl but run_cmd invokes unqualified systemctl commands. Sudoers requires exact path matches, so the rules would deny access at runtime. Remove /bin/ prefix from all four systemctl sudoers rules to match actual command invocations across all playbooks.
Summary by CodeRabbit
New Features
Improvements