Skip to content

Review followup: PR #1470 — feat: add main site promotion workflow #1502

Description

@superdav42

Unaddressed review bot suggestions

PR #1470 was merged with unaddressed review bot feedback. Each comment
below includes its file path, line number, a direct link to the inline
review comment, and a diff fence with the code context the bot was
flagging. Resolved and outdated threads are filtered out via GitHub's
GraphQL review-thread state. Read the relevant lines, decide whether
the suggestion is correct, and either apply the fix or close this issue
with a wontfix rationale.

Source PR: #1470


You are the triager (worker-is-triager rule)

This issue is auto-created from review bot output and dispatched
directly to you. Review bots can be wrong: hallucinated line refs, false
premises about codebase structure, template-driven sweeps without
measurements (see GH#17832-17835 for prior art and AGENTS.md
"AI-Generated Issue Quality"). Do not assume the bot is correct. Verify before acting.

You must end in exactly one of three outcomes — no fourth "hand it back
to the human" path exists. Humans approve decisions; they do not re-do
analysis.

Outcome A — Premise falsified → close the issue

  1. Read the cited file:line (listed under Files to modify below).

  2. If the bot's claim is factually wrong (file doesn't exist at that
    line, function doesn't behave as described, "auto-generated" section
    isn't actually auto-generated, etc.), close the issue with a
    comment in this shape:

    Premise falsified. <what the bot claimed>. <what the code
    actually shows, with a file:line citation or one-line quote>.
    Not acting.

    No PR. No further dispatch. The closing comment trains the next
    session reading this thread and the noise filter.

Outcome B — Premise correct + fix is obvious → implement and PR

  1. Verify the bot's premise as above.
  2. Read the Worker Guidance section below, open a worktree, implement.
  3. Open a PR with Resolves #<this-issue-number> in the body
    (use THIS issue's number, not the source PR's) so merge auto-closes it.
  4. Follow the normal Lifecycle Gate (brief, tests, review-bot-gate,
    merge, postflight).

Outcome C — Premise correct but approach is a genuine judgment call

Only use this path if you reach it after Outcomes A and B don't apply:
the bot's finding is real, but the fix requires a decision that is
architectural, policy, breaking-change, or otherwise genuinely outside
what you can resolve autonomously. In that case, post a decision
comment
with exactly these fields:

  • Premise check: one line, confirming the finding is real.
  • Analysis: 2-4 bullets on the trade-offs.
  • Recommended path: the option you would take if the decision were
    yours, with rationale.
  • Specific question: the single decision the human needs to make
    (yes/no or pick-one, not open-ended).

Then apply needs-maintainer-review and stop. The human wakes up to a
ready-to-approve recommendation, not a blank task.

Ambiguity about scope or style is not Outcome C. Per
AGENTS.md "Reasoning responsibility", the model does the
thinking and delivers a recommendation. Only escalate what is genuinely
a maintainer-only decision.

Worker Guidance

Files to modify:

  • inc/site-exporter/class-main-site-promoter.php:285
  • inc/site-exporter/class-main-site-promoter.php:77
  • tests/WP_Ultimo/Site_Exporter/Main_Site_Promoter_Test.php:8

Implementation steps (Outcome B path):

  1. Read the diff block under each inline comment below — it shows the
    exact code the bot was flagging. Open the file only if you need
    surrounding context beyond what the diff tail shows.
  2. Read the bot's full comment below the diff — it contains the rationale
    and any suggested change.
  3. Verify the premise before implementing (see Outcome A). If the premise
    is wrong, switch to Outcome A instead of burning iterations trying to
    satisfy a wrong suggestion.
  4. If multiple comments target the same file, group your edits into one
    logical commit.
  5. Run shellcheck / markdownlint-cli2 / project tests as appropriate.

Verification:

  • Open the new PR with Resolves #<this-issue> so this followup is auto-closed on merge.
  • If the bot's suggestion was incorrect, close this issue with a Outcome A comment — do not open a no-op PR.

Inline comments

coderabbitai on inc/site-exporter/class-main-site-promoter.php:77

View inline comment

+
+		$validation = $this->validate($source_blog_id, $args);
+
+		if (is_wp_error($validation)) {
+			return $validation;
+		}
+
+		$main_site_id = wu_get_main_site_id();
+		$main_url     = get_site_url($main_site_id);
+		$source_url   = get_site_url($source_blog_id);
+		$source_title = get_blog_option($source_blog_id, 'blogname');
+		$backups      = [];

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Bug: Original main site title not captured for preserve_main_title feature.

The code captures $source_title but not the main site's original blogname. After Site_Duplicator::override_site() copies the source to main, the main site's blogname is overwritten with the source's title. When preserve_main_title is true, restore_main_identity() does nothing to blogname, leaving the source title in place — the opposite of the intended behavior.

Proposed fix
 $main_site_id = wu_get_main_site_id();
 $main_url     = get_site_url($main_site_id);
 $source_url   = get_site_url($source_blog_id);
-$source_title = get_blog_option($source_blog_id, 'blogname');
+$main_title   = get_blog_option($main_site_id, 'blogname');
+$source_title = get_blog_option($source_blog_id, 'blogname');
 $backups      = [];

Then update the call at line 121:

-$this->restore_main_identity($main_site_id, $main_url, $source_title, (bool) $args['preserve_main_title']);
+$this->restore_main_identity($main_site_id, $main_url, $main_title, $source_title, (bool) $args['preserve_main_title']);

And update restore_main_identity() — see the corresponding comment on that method.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@inc/site-exporter/class-main-site-promoter.php` around lines 73 - 77, Capture
the main site's original blogname option before it gets overwritten by the
duplication process. Add a variable assignment using get_blog_option() to
retrieve the main site's current blogname (similar to how $source_title is
captured for the source site) and store it alongside the other main site
initialization code near where $main_url is set. Then update the call to
restore_main_identity() at line 121 to pass this cap

coderabbitai on inc/site-exporter/class-main-site-promoter.php:285

View inline comment

+	 * @param bool   $preserve_main_title Whether to preserve the current main title.
+	 * @return void
+	 */
+	private function restore_main_identity($main_site_id, $main_url, $source_title, $preserve_main_title) {
+
+		update_blog_option($main_site_id, 'home', $main_url);
+		update_blog_option($main_site_id, 'siteurl', $main_url);
+
+		if (! $preserve_main_title) {
+			update_blog_option($main_site_id, 'blogname', $source_title);
+		}
+	}

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Fix required: Restore original main title when preserve_main_title is true.

The method signature and logic need to accept both the original main title and source title, then choose based on the flag.

Proposed fix
-private function restore_main_identity($main_site_id, $main_url, $source_title, $preserve_main_title) {
+private function restore_main_identity($main_site_id, $main_url, $main_title, $source_title, $preserve_main_title) {

     update_blog_option($main_site_id, 'home', $main_url);
     update_blog_option($main_site_id, 'siteurl', $main_url);

-    if (! $preserve_main_title) {
-        update_blog_option($main_site_id, 'blogname', $source_title);
-    }
+    $title_to_use = $preserve_main_title ? $main_title : $source_title;
+    update_blog_option($main_site_id, 'blogname', $title_to_use);
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@inc/site-exporter/class-main-site-promoter.php` around lines 266 - 285, The
restore_main_identity method currently only updates the blogname when
preserve_main_title is false using source_title, but does not restore the
original main title when preserve_main_title is true. Add a new parameter to the
method signature to accept the original main title, then update the conditional
logic so that when preserve_main_title is true, the blogname is restored to the
original main title using update_blog_option, and when preserve_main_title is
false, it is set to the source_title. This ensures the main site title is
properly restored in both cases.

coderabbitai on tests/WP_Ultimo/Site_Exporter/Main_Site_Promoter_Test.php:8

View inline comment

@@ -0,0 +1,219 @@
+<?php
+/**
+ * Tests for the main-site promotion service.
+ *
+ * @package WP_Ultimo\Tests\Site_Exporter
+ */
+
+namespace WP_Ultimo\Tests\Site_Exporter;

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix test namespace to mirror source namespace.

The test namespace should mirror the source class namespace without the extra Tests segment. As per coding guidelines, the test namespace must mirror source (e.g., WP_Ultimo\Checkout\Cart_Test tests WP_Ultimo\Checkout\Cart).

Since the source class is in namespace WP_Ultimo\Site_Exporter, this test should use the same namespace.

📦 Proposed fix for namespace
-namespace WP_Ultimo\Tests\Site_Exporter;
+namespace WP_Ultimo\Site_Exporter;
📝 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.

namespace WP_Ultimo\Site_Exporter;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/WP_Ultimo/Site_Exporter/Main_Site_Promoter_Test.php` at line 8, The
namespace declaration at the top of the test file includes an extra Tests
segment that should not be there. Change the namespace from
WP_Ultimo\Tests\Site_Exporter to WP_Ultimo\Site_Exporter to mirror the source
class namespace according to coding guidelines. Remove the Tests path segment
from the namespace declaration so the test namespace exactly matches the source
class namespace.

Source: Coding guidelines

PR review summaries

(none)


aidevops.sh v3.21.11 automated scan.

Metadata

Metadata

Assignees

No one assigned

    Labels

    origin:workerAuto-created by pulse labelless backfill (t2112)review-followupUnaddressed review bot feedbacksource:review-scannerAuto-created by post-merge-review-scanner.shtier:standardAuto-created by pulse labelless backfill (t2112)

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions