Plugin Conflicts Guardian: batch the probe across all selected plugins by arthur791004 · Pull Request #48402 · Automattic/jetpack

arthur791004 · 2026-04-30T08:47:23Z

Summary

Follow-up to #48261 addressing #48261 (comment): bulk plugin activation no longer scales N round-trips with the number of plugins.

PCG_Load_Tester::test() now takes the full list of plugin main files, stashes them in a single transient, and the probe endpoint require_onces each plugin in order under one WP_SANDBOX_SCRAPING request. Probe cost is constant in N (still one front-end + one admin request, fired in parallel via Requests::request_multiple()).

As a side effect this also catches conflicts that only fire when two plugins load together (duplicate class, shared global, etc.) — which the per-plugin probe model could never see, and which the feature's name implies it should.

Attribution on a fatal

When the batched probe captures a fatal/throwable, the guard maps it back to one plugin in the batch using (in order):

The explicit plugin field set on the verdict when a Throwable is caught around the require_once.
An exact path match of the captured file against a plugin's main file (covers flat-file plugins like hello.php).
A directory-prefix match of file against each plugin's own subdirectory under WP_PLUGIN_DIR (excluding flat-file plugins so they can't false-match siblings).

The whole batch is blocked as a unit; the admin notice names the offending plugin so the admin can retry without it. If attribution can't resolve cleanly (fatal in shared/vendor code), the first plugin in the batch is named — the batch is blocked anyway.

Testing instructions

Filter pcg_guard_activation to true.
Single Activate on a plugin that fatals during init: same as before — redirected back with the named plugin and captured fatal.
Bulk-activate (Activate-selected) two or more plugins, one of which fatals: expect one round-trip's worth of latency (no longer N×); notice names the offending plugin.
Two plugins that each load fine alone but conflict together (e.g. both declare the same top-level class): expect the batch to be blocked, attributing to whichever plugin loaded second (its require_once is when the duplicate-class fatal fires).
Filter to false — both gates bypassed, behavior unchanged.

Does this pull request change what data or activity we track or use?

No. Same loopback-only probe shape as #48261; only the request count changes (one pair regardless of batch size).

🤖 Generated with Claude Code

github-actions · 2026-04-30T08:48:13Z

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (WordPress.com Site Helper), and enable the add/pcg-batch-probes branch.
To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack-mu-wpcom-plugin add/pcg-batch-probes

Interested in more tips and information?

In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
Read more about our development workflow here: PCYsg-eg0-p2
Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

github-actions · 2026-04-30T08:48:26Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ Add a "[Status]" label (In Progress, Needs Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

Follow this PR Review Process:

Ensure all required checks appearing at the bottom of this PR are passing.
Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
You can use GitHub's Reviewers functionality to request a review.
When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

jp-launch-control · 2026-04-30T08:52:33Z

Code Coverage Summary

Coverage changed in 4 files.

File	Coverage	Δ%	Δ Uncovered
projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/class-pcg-load-tester.php	16/138 (11.59%)	6.79%	23 💔
projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/activation-guard.php	40/126 (31.75%)	5.90%	20 💔
projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/probe-endpoint.php	0/80 (0.00%)	0.00%	13 💔
projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/update-healthcheck.php	19/128 (14.84%)	-0.12%	1 ❤️‍🩹

Full summary · PHP report

_{Coverage check overridden by

Coverage tests to be added later
Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR
.}

taipeicoder · 2026-05-05T03:48:25Z

@arthur791004 can we rebase this PR after merging #48292?

Previously the activation guard called PCG_Load_Tester::test() once per plugin, so each plugin in a bulk-activate request fired its own pair of loopback probes — N plugins meant N sequential round-trips. Reviewer flagged the cost on PR #48261. Now the load tester takes the full list, stashes it in one transient, and the probe endpoint require_once's each plugin under a single WP_SANDBOX_SCRAPING request. Probe cost is constant in N. As a side effect this also surfaces conflicts that only fire when two plugins are loaded together (duplicate class, shared global, etc.) — which the per-plugin model couldn't see. On a fatal/throwable the guard attributes the failure to one plugin in the batch using (in order) the explicit plugin field set when a Throwable is caught around the require, an exact-path match against the captured fatal file, and a directory-prefix match scoped to subdirectory plugins only (so flat-file plugins don't false-match siblings). The whole batch is blocked as a unit; the notice tells the admin which plugin caused the fatal. Addresses #48261 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The function identifies which plugin is being blocked; "attribute_block" read awkwardly. Local var renamed to $blocked_plugin to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Covers the new attribution logic introduced with batched probes: explicit plugin field wins, falls through to file-exact then file-prefix matching, flat-file plugins are matched only by exact path (not by their dirname, which is WP_PLUGIN_DIR and would false-match siblings), and the first plugin in the batch is returned when nothing attributes cleanly. Also covers PCG_Load_Tester::test()'s empty / missing-file rejection branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The verdict's 'reason' key is optional in the typed shape, so accessing it directly without a presence check makes Phan flag a possibly-invalid offset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Net -34 lines across class-pcg-load-tester.php, probe-endpoint.php, update-healthcheck.php — collapsing the multi-paragraph docblocks introduced during conflict resolution. Also align \$status assignment to satisfy phpcs Generic.Formatting.MultipleStatementAlignment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

taipeicoder · 2026-05-05T07:36:59Z

+		$plugin_mains = array_values(
+			array_filter(
+				array_map( static fn( $p ) => (string) $p, $plugin_mains ),
+				static fn( $p ) => '' !== $p && is_file( $p )


Let's also check for is_readable() following pcg_maybe_handle_probe()

Suggested change

static fn( $p ) => '' !== $p && is_file( $p )

static fn( $p ) => '' !== $p && is_file( $p ) && is_readable( $p )

Done in 39711d8 — added is_readable($p) to the up-front filter.

taipeicoder · 2026-05-05T08:08:05Z

 	}

-	return $blocked;
+	return (string) array_key_first( $paths );


In cases where the $result payload doesn't contain plugin or file (for instance, https://github.com/Automattic/jetpack/pull/48402/changes#diff-babcd1ba3a7b2e3b238816b14f26e591dfd6bfed2dcb3578bb05b6b0ceb4eacaR190-R195), then returning the first plugin might lead to wrong attribution. The batch would still be blocked, but the notice might point to an incorrect plugin.

Maybe we can have a batch-level attribution, something like "One of these plugins caused a fatal error during the pre-flight check: A, B, C. Investigate before trying again.".

Done in 59f6cc9 — pcg_guard_get_blocked_plugin returns '' when attribution can't be pinned; caller emits a batch-level notice line: "One of these plugins caused a fatal during the pre-flight check: A, B, C. Reason: …". Renderer drops the <code>plugin</code> prefix when the key is empty.

taipeicoder · 2026-05-05T08:08:47Z


-	if ( ! is_file( $plugin_main ) || ! is_readable( $plugin_main ) ) {
-		pcg_probe_bail_error( 'Probe target is no longer readable.', 404 );
+	foreach ( $plugin_mains as $plugin_main ) {


Bailing on the first bad plugin aborts the entire batch. This can cause a false negative where PCG_Load_Tester::test() returns error, which is ignored by pcg_guard_evaluate_plugins() since it's not fatal/throwable. That means that if a later plugin in the batch causes a fatal error, it wouldn't be blocked. I think we can bail when none of the plugins are readable.

Fixed in 16232b1. Endpoint now filters unreadable entries out and continues with the readable subset; only bails 404 when nothing remains. A transient unreadable file no longer masks a fatal in a sibling.

Saves a wasted loopback round-trip for files that the probe endpoint would 404 on anyway via its own is_readable check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

When the probe verdict has no \`plugin\` and no usable \`file\` (e.g. probe terminated mid-bootstrap with no JSON body, or fatal originated outside any candidate's tree), pcg_guard_get_blocked_plugin used to fall back to the first plugin in the batch — which could blame an innocent plugin in the notice. Now it returns '' to signal "undetermined", and pcg_guard_evaluate_plugins emits a single batch-level entry naming every plugin in the batch: "One of these plugins caused a fatal during the pre-flight check: A, B, C. Reason: …". The renderer drops the \`<code>plugin</code>\` prefix when the key is empty so the message reads cleanly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Bailing on the first unreadable plugin emitted \`error\`, which the activation guard treats as a non-block — so a genuine fatal in a later readable plugin in the same batch would slip through. Filter unreadable entries out and continue with the readable subset. Only bail with 404 when nothing remains. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The undetermined-attribution change in 59f6cc9 means this test (which covers a fatal whose path is in WP_PLUGIN_DIR but not in any candidate's tree) no longer falls back to the first plugin; it now returns '' so the caller can emit a batch-level notice. Update the assertion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

taipeicoder

Left some minor feedback. Good to go 👍

taipeicoder · 2026-05-05T13:51:06Z

+				</li>
 			<?php endforeach; ?>
 		</ul>
 		<p><?php esc_html_e( 'The plugin was not activated. Investigate the error before trying again.', 'jetpack-mu-wpcom' ); ?></p>


This message needs to be updated. Perhaps number-agnostic would be better? Something like "No plugins were activated to prevent a site crash. Investigate the error before trying again."

Done in 1ecfcc5

taipeicoder · 2026-05-05T13:58:38Z

+	$reason = sprintf(
+		/* translators: 1: comma-separated plugin basenames; 2: probe verdict reason. */
+		__( 'One of these plugins caused a fatal during the pre-flight check: %1$s. Reason: %2$s', 'jetpack-mu-wpcom' ),
+		implode( ', ', array_keys( $paths ) ),


It'd be better to use wp_sprintf_l( '%l', array_keys( $paths ) ) for locale-aware separators.

"The plugin was not activated" reads wrong for bulk activation. Switch to "No plugins were activated to prevent a site crash" so the same copy works for single and batch flows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Use wp_sprintf_l('%l', ...) instead of implode(', ', ...) so the list of plugin basenames in the unattributed-batch notice picks up locale-appropriate separators (e.g. "A, B, and C" in en_US, "A, B et C" in fr_FR). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

taipeicoder · 2026-05-06T00:08:43Z

 					'admin' => $admin['request'],
 				),
 				array(
 					'timeout'   => self::PROBE_TIMEOUT,


With batch activation, a PROBE_TIMEOUT of 15 seconds might not be enough. I'm not sure if scaling it with the number of plugins is a good idea. For now, let's add logging to see how often this happens? Properties that we should include in the log: mode, and plugin base names.

Done by d4c7cc9.

Logs whenever either probe (front-end or admin) returns a transport error — covers PROBE_TIMEOUT timeouts, connection failures, and non-JSON bodies. Lands in logstash via the WordPress.com log2logstash lib; no-op outside .com (lib not present), which keeps tests and self-hosted installs quiet. Logged fields: mode, plugin basenames in the batch, and per-probe reason. Lets us measure how often the 15s timeout fires vs. batch size before deciding whether to scale PROBE_TIMEOUT with N. Adds is_error() alongside is_block() to keep dispatch in test() symmetric, and splits log_probe_error into relative_basenames / probe_error_reason helpers so the log call reads as data assembly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

StefMattana · 2026-05-08T16:10:10Z

Hey @arthur791004! Just checking this PR and related #48553
Are they going to be shippped via Jetpack standalone or is WP.com-only? I'd like to confirming the scope before creating a doc-related Linear issue for Jetpack. Thanks!

arthur791004 self-assigned this Apr 30, 2026

github-actions Bot added [mu wpcom Feature] Plugin Conflicts Guardian [Package] Jetpack mu wpcom WordPress.com Features [Status] In Progress Docs labels Apr 30, 2026

github-actions Bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Apr 30, 2026

arthur791004 marked this pull request as ready for review April 30, 2026 09:07

github-actions Bot added the [Tests] Includes Tests label Apr 30, 2026

arthur791004 added the Coverage tests to be added later Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR label Apr 30, 2026

arthur791004 force-pushed the add/pcg-batch-probes branch from f4a10d3 to cbbcee7 Compare April 30, 2026 10:36

arthur791004 requested a review from taipeicoder May 4, 2026 10:51

arthur791004 and others added 4 commits May 5, 2026 13:46

Rename pcg_guard_attribute_block → pcg_guard_get_blocked_plugin

957b3dd

The function identifies which plugin is being blocked; "attribute_block" read awkwardly. Local var renamed to $blocked_plugin to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Satisfy Phan: assertArrayHasKey('reason') before assertNotEmpty

0300441

The verdict's 'reason' key is optional in the typed shape, so accessing it directly without a presence check makes Phan flag a possibly-invalid offset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

arthur791004 force-pushed the add/pcg-batch-probes branch from cbbcee7 to 0300441 Compare May 5, 2026 05:54

taipeicoder reviewed May 5, 2026

View reviewed changes

arthur791004 and others added 4 commits May 5, 2026 17:20

PCG load tester: filter unreadable plugin paths up front

39711d8

Saves a wasted loopback round-trip for files that the probe endpoint would 404 on anyway via its own is_readable check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

taipeicoder previously approved these changes May 5, 2026

View reviewed changes

arthur791004 and others added 2 commits May 5, 2026 22:11

arthur791004 dismissed taipeicoder’s stale review via 7147b8f May 5, 2026 14:14

taipeicoder reviewed May 6, 2026

View reviewed changes

arthur791004 force-pushed the add/pcg-batch-probes branch from 97ef45d to d4c7cc9 Compare May 6, 2026 02:30

taipeicoder approved these changes May 6, 2026

View reviewed changes

arthur791004 merged commit 337468a into trunk May 6, 2026
73 checks passed

arthur791004 deleted the add/pcg-batch-probes branch May 6, 2026 03:49

github-actions Bot added [Status] UI Changes Add this to PRs that change the UI so documentation can be updated. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Status] In Progress labels May 6, 2026

	static fn( $p ) => '' !== $p && is_file( $p )
	static fn( $p ) => '' !== $p && is_file( $p ) && is_readable( $p )

Conversation

arthur791004 commented Apr 30, 2026

Summary

Attribution on a fatal

Testing instructions

Does this pull request change what data or activity we track or use?

Uh oh!

github-actions Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jp-launch-control Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Uh oh!

taipeicoder commented May 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taipeicoder left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

StefMattana commented May 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Apr 30, 2026 •

edited

Loading

github-actions Bot commented Apr 30, 2026 •

edited

Loading

jp-launch-control Bot commented Apr 30, 2026 •

edited

Loading