Created the site:remove-atlantis-legacy-modules command#103
Created the site:remove-atlantis-legacy-modules command#103bernattorras wants to merge 6 commits intotrunkfrom
Conversation
This is a temporary command designed to bulk remove the legacy modules replaced by Atlantis (Colophon and the Plugin Autoupdate Filter) automatically.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Symfony CLI command class 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
commands/Site_Remove_Atlantis_Legacy_Modules.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ahegyes
Repo: a8cteam51/team51-cli PR: 97
File: commands/WPCOM_Site_WP_User_Delete.php:180-199
Timestamp: 2025-08-06T12:00:14.638Z
Learning: In the WPCOM_Site_WP_User_Delete command, when no administrators are found on a site, the process intentionally exits entirely rather than skipping that site, as this is considered an anomaly that should be investigated before attempting the deletion operation again.
🧬 Code graph analysis (1)
commands/Site_Remove_Atlantis_Legacy_Modules.php (4)
includes/functions.php (4)
get_enum_input(318-330)run_system_command(233-249)validate_user_choice(459-465)get_string_input(298-305)includes/functions-pressable.php (3)
get_pressable_site(104-108)get_pressable_site_deployhq_config(168-180)run_pressable_site_wp_cli_command(612-623)includes/functions-wpcom.php (3)
get_wpcom_site(69-71)get_wpcom_site_code_deployments(582-584)run_wpcom_site_wp_cli_command(631-642)includes/functions-github.php (3)
get_github_repository_from_deployhq_project(245-257)get_github_repository_branches(82-84)get_github_repository(32-34)
| if ( ! file_exists( $this->repo_dir ) ) { | ||
| mkdir( $this->repo_dir, 0755, true ); | ||
| $this->write_output( $output, "<info>Created folder: {$this->repo_dir}</info>" ); | ||
|
|
||
| // Clone the repository. | ||
| \run_system_command( array( 'git', 'clone', $this->gh_repository->clone_url, $this->repo_dir ) ); | ||
| $this->write_output( $output, "<info>Cloned repository: {$this->gh_repository->name} ({$this->gh_repository->clone_url}) into {$this->repo_dir}</info>" ); | ||
| } |
There was a problem hiding this comment.
Fix the clone path to avoid immediate failure.
We create $this->repo_dir before invoking git clone, but git clone refuses to run when the destination already exists (even if empty), so the first call throws a ProcessFailedException and the command aborts. Let Git create the directory instead.
// Create a folder named the repository name in the repos folder if it doesn't exist.
if ( ! file_exists( $this->repo_dir ) ) {
- mkdir( $this->repo_dir, 0755, true );
- $this->write_output( $output, "<info>Created folder: {$this->repo_dir}</info>" );
-
- // Clone the repository.
- \run_system_command( array( 'git', 'clone', $this->gh_repository->clone_url, $this->repo_dir ) );
+ // Clone the repository (git will create the target directory for us).
+ \run_system_command(
+ array( 'git', 'clone', $this->gh_repository->clone_url, $this->repo_dir ),
+ $this->repos_dir
+ );
$this->write_output( $output, "<info>Cloned repository: {$this->gh_repository->name} ({$this->gh_repository->clone_url}) into {$this->repo_dir}</info>" );
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
commands/Site_Remove_Atlantis_Legacy_Modules.php lines 731-738: the code
currently creates $this->repo_dir before running git clone which causes git to
fail because the destination must not pre-exist; instead, ensure the clone's
parent directory exists (mkdir dirname($this->repo_dir), 0755, true) but do not
create $this->repo_dir itself, remove the "Created folder" message for
$this->repo_dir, then run
run_system_command(['git','clone',$this->gh_repository->clone_url,$this->repo_dir])
and keep the "Cloned repository" output after the clone succeeds.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@commands/Site_Remove_Atlantis_Legacy_Modules.php`:
- Around line 328-360: The progress messages use the CSV row index ($index)
instead of a sequential site counter, so create a separate counter (e.g.
$site_count) initialized before the foreach over $csv_data and increment it at
the start of each iteration, then replace uses of $index in the output->writeln
calls (and in update_csv_row note messages if they should reflect sequential
position) with the new $site_count variable; update all occurrences inside the
loop (the Processing, Skipping, and Already processed messages around the
foreach handling) and keep $index for CSV-key lookups and update_csv_row calls
that rely on the original CSV row index.
- Around line 485-500: The condition is using the hardcoded
$this->legacy_modules (always >0); change the check to use the runtime-detected
list (e.g. $found_modules or $this->removed_modules) so the prompt only appears
when plugins were actually found, and pass that specific list to
uninstall_plugins_from_wordpress; update the $plugins_list construction and the
ConfirmationQuestion to use the chosen runtime array (and ensure
uninstall_plugins_from_wordpress accepts/receives that array instead of relying
on $this->legacy_modules).
🧹 Nitpick comments (3)
commands/Site_Remove_Atlantis_Legacy_Modules.php (3)
765-773: Redundant empty check and unsafe continuation path.The
get_wpcom_repository()method is only called frominitialize_repository()at line 624, after the empty repositories check at line 607. The check at line 765 is therefore redundant. Additionally, if somehow reached with empty repositories and the user continues,$this->gh_repositoryremains null.Consider removing this redundant check since the caller already handles the empty case:
♻️ Suggested refactor
private function get_wpcom_repository( InputInterface $input, OutputInterface $output ): void { $wpcom_gh_repositories = get_wpcom_site_code_deployments( $this->site->ID ); $gh_repository_name = null; - if ( empty( $wpcom_gh_repositories ) ) { - $output->writeln( '<error>Unable to find a WPCOM GitHub Deployments for the site.</error>' ); - - $question = new ConfirmationQuestion( '<question>Do you want to continue anyway? [y/N]</question> ', false ); - if ( true !== $this->getHelper( 'question' )->ask( $input, $output, $question ) ) { - $output->writeln( '<comment>Command aborted by user.</comment>' ); - exit( 1 ); - } - } - - if ( $wpcom_gh_repositories && 1 < count( $wpcom_gh_repositories ) ) { + if ( 1 < count( $wpcom_gh_repositories ) ) {
1062-1063: Consider making the Atlantis plugin version configurable.The plugin URL contains a hardcoded version (
v1.0.2). If this command lives beyond its temporary purpose, consider extracting this to a constant or configuration option for easier updates.
1107-1131: Uninstalls all legacy modules regardless of what was found.
uninstall_plugins_from_wordpress()uses$this->legacy_modulesto build the plugin list, which attempts to uninstall both legacy plugins even if only one (or neither) was found in the repository. This is related to the issue at line 486.♻️ Consider passing the found plugins explicitly
- private function uninstall_plugins_from_wordpress( OutputInterface $output ): void { + private function uninstall_plugins_from_wordpress( OutputInterface $output, array $plugins_to_uninstall = null ): void { $site_identifier = 'atomic' === $this->host ? $this->site->ID : $this->site->id; - $plugins_list = implode( ' ', $this->legacy_modules ); + $plugins_to_uninstall = $plugins_to_uninstall ?? $this->legacy_modules; + $plugins_list = implode( ' ', $plugins_to_uninstall );Then call it with the specific plugins found:
$this->uninstall_plugins_from_wordpress( $output, array_keys( $found_modules ) );
| // Uninstall plugins from WordPress if --uninstall option is provided. | ||
| if ( $this->uninstall_plugins && count( $this->legacy_modules ) > 0 ) { | ||
| // In quiet mode, automatically uninstall plugins. | ||
| if ( $this->quiet ) { | ||
| $this->uninstall_plugins_from_wordpress( $output ); | ||
| } else { | ||
| $output->writeln( '' ); | ||
| $plugins_list = implode( ', ', $this->legacy_modules ); | ||
| $question = new ConfirmationQuestion( "<question>Do you want to deactivate and uninstall these plugins from WordPress ({$plugins_list})? [y/N]</question> ", false ); | ||
| if ( true === $this->getHelper( 'question' )->ask( $input, $output, $question ) ) { | ||
| $this->uninstall_plugins_from_wordpress( $output ); | ||
| } else { | ||
| $output->writeln( '<comment>Skipping WordPress plugin uninstallation.</comment>' ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Condition count( $this->legacy_modules ) > 0 is always true.
$this->legacy_modules is a hardcoded array with 2 elements, so this condition is always satisfied. The intent seems to be uninstalling plugins that were actually found on the site. Consider using $found_modules or $this->removed_modules instead.
💡 Suggested fix
// Uninstall plugins from WordPress if --uninstall option is provided.
- if ( $this->uninstall_plugins && count( $this->legacy_modules ) > 0 ) {
+ if ( $this->uninstall_plugins && ! empty( $found_modules ) ) {
// In quiet mode, automatically uninstall plugins.
if ( $this->quiet ) {
$this->uninstall_plugins_from_wordpress( $output );You may also want to pass the specific plugins to uninstall rather than using the full $this->legacy_modules list:
- $plugins_list = implode( ', ', $this->legacy_modules );
+ $plugins_list = implode( ', ', array_keys( $found_modules ) );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Uninstall plugins from WordPress if --uninstall option is provided. | |
| if ( $this->uninstall_plugins && count( $this->legacy_modules ) > 0 ) { | |
| // In quiet mode, automatically uninstall plugins. | |
| if ( $this->quiet ) { | |
| $this->uninstall_plugins_from_wordpress( $output ); | |
| } else { | |
| $output->writeln( '' ); | |
| $plugins_list = implode( ', ', $this->legacy_modules ); | |
| $question = new ConfirmationQuestion( "<question>Do you want to deactivate and uninstall these plugins from WordPress ({$plugins_list})? [y/N]</question> ", false ); | |
| if ( true === $this->getHelper( 'question' )->ask( $input, $output, $question ) ) { | |
| $this->uninstall_plugins_from_wordpress( $output ); | |
| } else { | |
| $output->writeln( '<comment>Skipping WordPress plugin uninstallation.</comment>' ); | |
| } | |
| } | |
| } | |
| // Uninstall plugins from WordPress if --uninstall option is provided. | |
| if ( $this->uninstall_plugins && ! empty( $found_modules ) ) { | |
| // In quiet mode, automatically uninstall plugins. | |
| if ( $this->quiet ) { | |
| $this->uninstall_plugins_from_wordpress( $output ); | |
| } else { | |
| $output->writeln( '' ); | |
| $plugins_list = implode( ', ', array_keys( $found_modules ) ); | |
| $question = new ConfirmationQuestion( "<question>Do you want to deactivate and uninstall these plugins from WordPress ({$plugins_list})? [y/N]</question> ", false ); | |
| if ( true === $this->getHelper( 'question' )->ask( $input, $output, $question ) ) { | |
| $this->uninstall_plugins_from_wordpress( $output ); | |
| } else { | |
| $output->writeln( '<comment>Skipping WordPress plugin uninstallation.</comment>' ); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@commands/Site_Remove_Atlantis_Legacy_Modules.php` around lines 485 - 500, The
condition is using the hardcoded $this->legacy_modules (always >0); change the
check to use the runtime-detected list (e.g. $found_modules or
$this->removed_modules) so the prompt only appears when plugins were actually
found, and pass that specific list to uninstall_plugins_from_wordpress; update
the $plugins_list construction and the ConfirmationQuestion to use the chosen
runtime array (and ensure uninstall_plugins_from_wordpress accepts/receives that
array instead of relying on $this->legacy_modules).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@commands/Site_Remove_Atlantis_Legacy_Modules.php`:
- Around line 1124-1139: The code is passing the integer exit code returned by
run_pressable_site_wp_cli_command/run_wpcom_site_wp_cli_command into
json_decode; instead check the exit code stored in $result, return with
$this->write_output(...) if $result !== 0, then read the actual WP-CLI output
from $GLOBALS['wp_cli_output'] and pass that to json_decode; keep the existing
validation that json_decode($GLOBALS['wp_cli_output'], true) yields an array and
emit the same "Could not parse plugin list." message and return if it does not.
🧹 Nitpick comments (2)
commands/Site_Remove_Atlantis_Legacy_Modules.php (2)
765-777: Redundant API call for WPCOM code deployments.
get_wpcom_site_code_deployments()is already called at line 609 ininitialize_repository()before calling this method. Calling it again here at line 766 is redundant.♻️ Suggested optimization
Pass the already-fetched repositories as a parameter:
- private function get_wpcom_repository( InputInterface $input, OutputInterface $output ): void { - $wpcom_gh_repositories = get_wpcom_site_code_deployments( $this->site->ID ); + private function get_wpcom_repository( InputInterface $input, OutputInterface $output, array $wpcom_gh_repositories ): void {Then update the call site at line 628:
- $this->get_wpcom_repository( $input, $output ); + $this->get_wpcom_repository( $input, $output, $wpcom_gh_repositories );
1195-1219:uninstall_plugins_from_wordpressalways tries to uninstall all legacy modules.This method uses
$this->legacy_modules(line 1197) which includes all hardcoded legacy plugins, regardless of which were actually found on the site. This is related to the earlier issue at lines 489-504.Consider passing the list of found plugins as a parameter to avoid attempting to uninstall plugins that don't exist on the site (though WP-CLI handles this gracefully with warnings).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@commands/Site_Remove_Atlantis_Legacy_Modules.php`:
- Around line 430-436: The deployment error handling references an undefined
variable $site_host causing a warning; update the code in the error handling
block that computes $is_deployment_error and sets $deployment_type/$note to use
$this->host (or the appropriate property on the current class) instead of
$site_host so the check and message use the defined host value (replace all
occurrences of $site_host in that block with $this->host, e.g., where
$is_deployment_error is determined and when assigning $deployment_type).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@commands/Site_Remove_Atlantis_Legacy_Modules.php`:
- Around line 454-466: When running in quiet/automation mode ($this->quiet),
avoid creating a ChoiceQuestion and prompting; instead default to a safe
non-interactive choice (e.g., set $deployment_choice = 'skip') and skip calling
$this->getHelper('question')->ask. Locate the block that builds the $choices
array and creates the ChoiceQuestion (uses ChoiceQuestion and
$deployment_choice) and add a conditional: if ($this->quiet) assign the default
string ('skip') to $deployment_choice and only construct/ask the ChoiceQuestion
when not quiet.
- Around line 1440-1459: In uninstall_plugins_from_wordpress, capture the
integer return value from run_pressable_site_wp_cli_command or
run_wpcom_site_wp_cli_command (depending on host) instead of ignoring it, then
check if it equals 0 before printing the success message; if non-zero, write an
error via $this->write_output (including the exit code and the original $command
for context) and throw or return to avoid reporting success erroneously. Ensure
you reference the $command variable and use the same write_output pattern
currently used for success/failure messages.
---
Duplicate comments:
In `@commands/Site_Remove_Atlantis_Legacy_Modules.php`:
- Around line 956-969: In clone_repo, stop pre-creating the destination folder
($this->repo_dir) before calling \run_system_command('git clone ...') because
git fails if the target directory already exists; instead only ensure the parent
repos folder ($this->repos_dir) exists, remove the mkdir of $this->repo_dir,
call
\run_system_command(array('git','clone',$this->gh_repository->clone_url,$this->repo_dir))
to let git create the directory, and move the success write_output for the clone
to after the command succeeds; also add a guard to detect if $this->repo_dir
already exists and fail with a clear message (or skip) before attempting clone.
- Around line 774-788: The early-return when $wpcom_gh_repositories is empty
leaves $this->gh_repository null and later dereferences
$this->gh_repository->name; instead of returning after the interactive
confirmation, abort the command consistently (throw an exception or exit with
non-zero status) so execution cannot continue with a null $this->gh_repository;
update the block that currently does "return;" (after the
ConfirmationQuestion/exit(1) path) to throw new \Exception('No WPCOM GitHub
repository available') or call exit(1) so downstream references to
$this->gh_repository->name are never reached.
| $deployment_type = ( 'pressable' === $host ) ? 'DeployHQ' : 'WPCOM GitHub'; | ||
| $output->writeln( "<comment>⚠ {$site_name}: Unable to find a {$deployment_type} deployment for the site.</comment>" ); | ||
|
|
||
| // Present the user with choices for how to proceed. | ||
| $choices = array( | ||
| 'enter_repo' => 'Enter a repository URL or slug manually', | ||
| 'site_only' => 'Skip repository, process site only (install Atlantis, handle plugins)', | ||
| 'skip' => 'Skip this site entirely', | ||
| ); | ||
|
|
||
| $question = new ChoiceQuestion( '<question>How would you like to proceed?</question> ', $choices, 'skip' ); | ||
| $deployment_choice = $this->getHelper( 'question' )->ask( $input, $output, $question ); | ||
|
|
There was a problem hiding this comment.
Honor --no-output by avoiding interactive prompts in CSV runs.
ChoiceQuestion will block in quiet/automation mode. Default to a safe non-interactive choice (e.g., skip) when $this->quiet is true.
🔧 Suggested adjustment
- $question = new ChoiceQuestion( '<question>How would you like to proceed?</question> ', $choices, 'skip' );
- $deployment_choice = $this->getHelper( 'question' )->ask( $input, $output, $question );
+ if ( $this->quiet ) {
+ $deployment_choice = 'skip';
+ } else {
+ $question = new ChoiceQuestion( '<question>How would you like to proceed?</question> ', $choices, 'skip' );
+ $deployment_choice = $this->getHelper( 'question' )->ask( $input, $output, $question );
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@commands/Site_Remove_Atlantis_Legacy_Modules.php` around lines 454 - 466,
When running in quiet/automation mode ($this->quiet), avoid creating a
ChoiceQuestion and prompting; instead default to a safe non-interactive choice
(e.g., set $deployment_choice = 'skip') and skip calling
$this->getHelper('question')->ask. Locate the block that builds the $choices
array and creates the ChoiceQuestion (uses ChoiceQuestion and
$deployment_choice) and add a conditional: if ($this->quiet) assign the default
string ('skip') to $deployment_choice and only construct/ask the ChoiceQuestion
when not quiet.
| private function uninstall_plugins_from_wordpress( OutputInterface $output ): void { | ||
| $site_identifier = 'atomic' === $this->host ? $this->site->ID : $this->site->id; | ||
| $plugins_list = implode( ' ', $this->legacy_modules ); | ||
|
|
||
| $this->write_output( $output, '' ); | ||
| $this->write_output( $output, '<fg=cyan;options=bold>Deactivating and uninstalling plugins from WordPress...</>' ); | ||
|
|
||
| try { | ||
| // Run single command to deactivate and uninstall all plugins. | ||
| // The --uninstall flag will also deactivate if needed. | ||
| $command = "plugin uninstall {$plugins_list} --deactivate"; | ||
|
|
||
| // Skip output in silent mode. | ||
| if ( 'pressable' === $this->host ) { | ||
| run_pressable_site_wp_cli_command( $site_identifier, $command, $this->quiet ); | ||
| } else { | ||
| run_wpcom_site_wp_cli_command( $site_identifier, $command, $this->quiet ); | ||
| } | ||
|
|
||
| $this->write_output( $output, ' <info>✓ Plugins deactivated and uninstalled from WordPress</info>' ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the definitions of the WP-CLI command functions mentioned in the review
rg -n "function run_pressable_site_wp_cli_command|function run_wpcom_site_wp_cli_command" --type php -A 15Repository: a8cteam51/team51-cli
Length of output: 2165
🏁 Script executed:
#!/bin/bash
# Also search for run_app_command as mentioned in the verification request
rg -n "function run_app_command" --type php -A 40Repository: a8cteam51/team51-cli
Length of output: 2566
🏁 Script executed:
#!/bin/bash
# Look at the entire context of the reviewed method to understand the try-catch block
rg -n "uninstall_plugins_from_wordpress" --type php -B 5 -A 40Repository: a8cteam51/team51-cli
Length of output: 8573
Check WP-CLI exit code before reporting uninstall success.
The functions run_pressable_site_wp_cli_command() and run_wpcom_site_wp_cli_command() return an integer exit code but the current code ignores it, printing the success message regardless of whether the command succeeded. The try-catch block only protects against exceptions, which these functions don't throw.
🔧 Suggested fix
- if ( 'pressable' === $this->host ) {
- run_pressable_site_wp_cli_command( $site_identifier, $command, $this->quiet );
- } else {
- run_wpcom_site_wp_cli_command( $site_identifier, $command, $this->quiet );
- }
-
- $this->write_output( $output, ' <info>✓ Plugins deactivated and uninstalled from WordPress</info>' );
+ $exit_code = ( 'pressable' === $this->host )
+ ? run_pressable_site_wp_cli_command( $site_identifier, $command, $this->quiet )
+ : run_wpcom_site_wp_cli_command( $site_identifier, $command, $this->quiet );
+
+ if ( Command::SUCCESS !== $exit_code ) {
+ $this->write_output( $output, ' <comment>Plugin uninstall returned a non-zero exit code.</comment>' );
+ return;
+ }
+
+ $this->write_output( $output, ' <info>✓ Plugins deactivated and uninstalled from WordPress</info>' );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@commands/Site_Remove_Atlantis_Legacy_Modules.php` around lines 1440 - 1459,
In uninstall_plugins_from_wordpress, capture the integer return value from
run_pressable_site_wp_cli_command or run_wpcom_site_wp_cli_command (depending on
host) instead of ignoring it, then check if it equals 0 before printing the
success message; if non-zero, write an error via $this->write_output (including
the exit code and the original $command for context) and throw or return to
avoid reporting success erroneously. Ensure you reference the $command variable
and use the same write_output pattern currently used for success/failure
messages.
2 participants
This is a temporary command designed to bulk remove the legacy modules replaced by Atlantis (Colophon and the Plugin Autoupdate Filter) automatically.
I'm still testing it, so adjustments may be added.
Summary by CodeRabbit