refactor: site remove local source support

[loadinglucian]

Summary by CodeRabbit

• New Features

  • Sites now exclusively use git-based deployment workflow.

• Bug Fixes

  • Removed support for local deployment paths to simplify site configuration.

• Refactor

  • Repository and branch are now always required fields for all sites.
  • Consolidated site setup to a single git-focused flow, eliminating multi-path configuration logic.
  • Streamlined site details display to consistently show git source information.

[coderabbitai]

Walkthrough

The pull request eliminates local deployment support, consolidating to a git-only model. It removes the siteSource option from site configuration, makes repo and branch mandatory string properties in SiteDTO, removes the isLocal() method, and updates data persistence and display logic accordingly.

Changes

Cohort / File(s)	Summary
Command Input Refactoring app/Console/Site/SiteAddCommand.php	Removed siteSource option and related conditional branching; consolidated input gathering to always collect git-based repo and branch details; updated command replay hint construction and return type documentation to exclude siteSource and make repo and branch mandatory.
Domain Model Update app/DTOs/SiteDTO.php	Changed repo and branch from nullable strings (?string) to mandatory strings (string); removed isLocal() method; updated constructor and docblock annotations to reflect non-nullable types.
Data Persistence Layer app/Repositories/SiteRepository.php	Updated dehydrateSiteDTO to always include repo and branch in serialized data; changed hydrateSiteDTO to use empty string defaults instead of null for repo and branch, aligning with non-nullable type expectations.
Display Logic app/Traits/SitesTrait.php	Removed conditional branching in displaySiteDeets that previously differentiated between local and git-based sites; now uniformly initializes details with Domain, Source (Git), Repo, and Branch.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• SiteAddCommand.php: Verify all conditional flows for repo/branch detection and validation are correct; ensure command replay hints capture all necessary parameters.
• SiteDTO.php: Confirm all call sites that previously checked isLocal() are updated to handle mandatory repo/branch; review for any backward compatibility concerns with existing persisted sites.
• SiteRepository.php: Inspect hydration of existing data to ensure empty string defaults don't introduce unexpected behavior; verify serialization consistency across all site types.
• SitesTrait.php: Ensure display output is correct for all site configurations without the Local/Git branching logic.

Possibly related PRs

• refactor: process service replacement #38: Directly modifies SiteDTO and SiteRepository to change repo/branch types, constructor shapes, and hydration/dehydration behavior—core changes aligned with this PR's domain model consolidation.
• refactor: site and server CRUD improvements #60: Modifies SiteAddCommand.php input flow and introduces source option handling—related through shared command logic and potential interactions with this PR's removal of siteSource.

Poem

🐰 Gone are the paths both local and remote,
One git-based way now carries the boat,
No branching logic, no isLocal fret,
Just repo and branch—the simplest duet!
Deploying with focus, clean and concise—
Your code hops forward with strategy precise! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main refactoring objective: removing local source support from the site management system.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/site-remove-local-source-support

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

app/Console/Site/SiteAddCommand.php (1)

152-152: Consider handling when git detection returns null.

Line 152 sets $defaultRepo = $this->git->detectRemoteUrl() ?? '', which means an empty string will be used as the default if detection fails. This could result in a confusing UX where the prompt shows an empty default.

Consider providing a more helpful placeholder or informing the user when detection fails:

-$defaultRepo = $this->git->detectRemoteUrl() ?? '';
+$detectedRepo = $this->git->detectRemoteUrl();
+$defaultRepo = $detectedRepo ?? '';
+
+if ($detectedRepo === null) {
+    $this->io->note('Could not detect git repository. Please enter manually.');
+}

Alternatively, you could omit the default parameter when detection fails to avoid showing an empty default.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5406bd1 and 46f421e.

📒 Files selected for processing (4)

• app/Console/Site/SiteAddCommand.php (4 hunks)
• app/DTOs/SiteDTO.php (1 hunks)
• app/Repositories/SiteRepository.php (1 hunks)
• app/Traits/SitesTrait.php (1 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/Traits/SitesTrait.php
• app/DTOs/SiteDTO.php
• app/Console/Site/SiteAddCommand.php
• app/Repositories/SiteRepository.php

🔇 Additional comments (13)

app/Traits/SitesTrait.php (1)

113-118: LGTM! Clean removal of local/git branching.

The uniform initialization of the details array correctly reflects the git-only model. The hardcoded 'Source' => 'Git' aligns with the PR objective to remove local source support.

app/DTOs/SiteDTO.php (2)

13-14: LGTM! Documentation correctly reflects non-nullable types.

The docblock updates accurately reflect the constructor signature changes for repo and branch.

---

19-20: Type change from ?string to string is valid; empty strings are prevented at creation.

The validators in SitesTrait.php (lines 165-173, 183-191) enforce that both repo and branch cannot be empty strings—they check trim($value) === '' and reject accordingly. New sites created via SiteAddCommand go through these validators.

The SiteRepository::hydrateSiteDTO method defaults to empty strings only when loading from existing inventory data that may lack these fields. This is a safe defensive pattern: empty strings represent missing legacy data, not invalid new sites. The type change to non-nullable string is correct because:

• Empty strings are still non-null values
• Creation paths enforce non-empty via validation
• Load paths handle missing data gracefully with empty string fallbacks

app/Repositories/SiteRepository.php (5)

170-170: LGTM! Documentation accurately reflects the uniform data structure.

The docblock correctly indicates that repo and branch are always included in the serialized output.

---

174-179: LGTM! Simplified serialization logic.

The uniform serialization correctly reflects the git-only model by always including repo and branch in the stored data.

---

186-186: LGTM! Clear documentation of fallback behavior.

The docblock explicitly documents that repo and branch default to empty strings when missing, which aligns with the implementation.

---

191-192: Consider validation for empty string defaults.

The defaults to empty strings for missing repo and branch values are consistent with the non-nullable contract, but may allow invalid sites to be hydrated from corrupted or legacy inventory data.

This relates to the validation concern raised in SiteDTO.php (lines 19-20). Consider whether hydration should:

1. Accept empty strings as valid fallbacks, or
2. Throw an exception or log a warning when required fields are missing

---

197-198: Good defensive type checking.

The is_string() checks provide additional safety against corrupted inventory data, though empty strings remain possible as discussed in lines 191-192.

app/Console/Site/SiteAddCommand.php (5)

34-36: LGTM! Command options correctly reflect git-only model.

The removal of siteSource and the retention of repo and branch options align with the PR objective to consolidate to git-based deployments.

---

97-102: LGTM! Command replay correctly reflects updated options.

The replay hint now includes repo and branch without siteSource, matching the new command signature.

---

114-114: LGTM! Return type accurately reflects git-only structure.

The updated return type correctly shows non-nullable repo and branch strings without siteSource.

---

154-188: LGTM! Comprehensive validation ensures non-empty values.

The git details gathering flow correctly:

• Validates repo and branch using the trait's validation methods
• Ensures non-empty strings through validateSiteRepo and validateSiteBranch
• Handles null returns from prompts appropriately
• Provides sensible defaults ('main' for branch when detection fails)

This validation ensures that empty strings flagged in SiteDTO.php and SiteRepository.php reviews can only occur from legacy/corrupted inventory data, not from the creation flow.

---

190-195: LGTM! Return structure matches type annotation.

The return statement correctly provides all required fields matching the updated return type on line 114.