Skip to content

Plugin Conflicts Guardian: Pre-flight activation gate#48261

Merged
taipeicoder merged 38 commits into
trunkfrom
try/plugin-conflicts-guardian
Apr 29, 2026
Merged

Plugin Conflicts Guardian: Pre-flight activation gate#48261
taipeicoder merged 38 commits into
trunkfrom
try/plugin-conflicts-guardian

Conversation

@arthur791004
Copy link
Copy Markdown
Contributor

@arthur791004 arthur791004 commented Apr 23, 2026

Summary

Adds a WP.com-side safety net that refuses plugin activations which would fatal at load or during the init / admin_init cycle. The admin click gets an error notice with the captured PHP error (class / errno, message, file, line) instead of the site falling into recovery mode. Install/update zips with PHP parse errors are also refused before they touch the live plugin files.

image

How it works

Full architecture + sequence diagram in README.md. Short version:

  • Activation guard (activation-guard.php) hooks load-plugins.php and load-update.php at priority 0, intercepting the admin-UI Activate paths:
    • plugins.php?action=activate (single)
    • plugins.php?action=activate-selected (bulk)
    • update.php?action=activate-plugin (post-upload "Activate Plugin" link from Add New → Upload Plugin)
  • For each plugin being activated, PCG_Load_Tester fires two loopback probes in parallel via \WpOrg\Requests\Requests::request_multiple():
    • Front-end probehome_url('/?pcg_probe=1&token=…'). Emits its verdict on wp_loaded (priority PHP_INT_MAX).
    • Admin probeadmin_url('index.php?pcg_probe=1&token=…&pcg_admin=1') with the admin's WordPress auth cookies forwarded so auth_redirect() clears. Defers its verdict to admin_init (priority PHP_INT_MAX) so admin-time hook fatals surface.
  • The first non-ok verdict wins. A 302 from the admin probe (cookies missing/expired) is treated as inconclusive ok so transport quirks don't block activation.
  • probe-endpoint.php handles each probe inline during jetpack-mu-wpcom's load_features. It sets WP_SANDBOX_SCRAPING so WP's own fatal handler steps aside, requires the plugin's main file, and lets the bootstrap continue. Any fatal/throwable along the way is captured by a shutdown handler and returned as JSON.
  • If either probe reports a fatal, the guard redirects back to the plugins list with a notice; activate_plugin() is never called, so active_plugins stays clean.
  • Update guard (update-guard.php) hooks upgrader_source_selection and tokenizes every .php in the extracted install/update package with token_get_all(..., TOKEN_PARSE). Parse errors abort the install/update before the live files are replaced. Syntax-only because during an update the active version is already loaded, so running the load probe would always fatal on "Cannot redeclare class". This also transparently covers plugin auto-updates.

Why not rely on core's plugin_sandbox_scrape()

  • Core's scrape runs inline in the activate request — a fatal still tears down the admin's current page and triggers recovery mode.
  • Core's scrape only catches errors at include time; our probes also catch fatals fired inside init / admin_init callbacks.
  • Side effects from the probe live in a separate PHP process that terminates when the probe finishes.

Gating

Ships dark behind a single filter:

  • pcg_guard_activation (default false) — flip to true to enable both the activation and the install/update guards.
add_filter( 'pcg_guard_activation', '__return_true' );

Testing instructions

  1. Ensure jetpack-mu-wpcom's dev plugin is active and pcg_guard_activation is filtered to true.
  2. Install a plugin that fatals during its init (or admin_init) hook. A sample plugin ("PCG Fatal Tester") was used during development — paste the 50-line snippet from the PR discussion.
  3. On Plugins, click Activate on the broken plugin. Expect: redirect back with a red notice carrying the PHP error class / name, message, and file:line; plugin remains inactive.
  4. Repeat via Plugins → Add New → Upload Plugin → Install Now → Activate Plugin (update.php path). Same expected outcome.
  5. Upload a plugin zip with a deliberate syntax error. Expect: the installer surfaces the parse error(s) and the previous plugin files stay untouched.
  6. Uninstall the fatal plugin; confirm normal (non-fatal) plugin activations still work unchanged.
  7. Filter pcg_guard_activation to false and confirm both gates are bypassed.

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

No new tracking or data collection. The feature is entirely local:

  • A short-lived transient (30s) stashes the path of a plugin the admin is trying to activate, keyed to a single-use random token.
  • Two internal wp_remote_get-style requests hit this same site's home_url() and admin_url() to trigger the probes; the admin probe forwards the admin's existing WP auth cookies.
  • The probe reads error_get_last() in its own request only.
  • No external HTTP calls, no analytics events.

🤖 Generated with Claude Code

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 23, 2026

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 try/plugin-conflicts-guardian branch.
  • To test on Simple, run the following command on your sandbox:
bin/jetpack-downloader test jetpack-mu-wpcom-plugin try/plugin-conflicts-guardian

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

@arthur791004 arthur791004 changed the title jetpack-mu-wpcom: Plugin Conflicts Guardian (pre-flight activation gate) Plugin Conflicts Guardian: Pre-flight activation gate Apr 23, 2026
@arthur791004 arthur791004 self-assigned this Apr 23, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 23, 2026

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:

  1. Ensure all required checks appearing at the bottom of this PR are passing.
  2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
  3. You can use GitHub's Reviewers functionality to request a review.
  4. 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!

@github-actions 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 23, 2026
@jp-launch-control
Copy link
Copy Markdown

jp-launch-control Bot commented Apr 23, 2026

Code Coverage Summary

Coverage changed in 1 file.

File Coverage Δ% Δ Uncovered
projects/packages/jetpack-mu-wpcom/src/class-jetpack-mu-wpcom.php 2/351 (0.57%) -0.00% 1 ❤️‍🩹

5 files are newly checked for coverage.

File Coverage
projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/plugin-conflicts-guardian.php 0/5 (0.00%) 💔
projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/probe-endpoint.php 0/61 (0.00%) 💔
projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/class-pcg-load-tester.php 1/100 (1.00%) 💔
projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/activation-guard.php 23/89 (25.84%) ❤️‍🩹
projects/packages/jetpack-mu-wpcom/src/features/plugin-conflicts-guardian/update-guard.php 50/61 (81.97%) 💚

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 .

@github-actions github-actions Bot added the Docs label Apr 24, 2026
@arthur791004 arthur791004 force-pushed the try/plugin-conflicts-guardian branch from 0d8d8da to 5c6e6af Compare April 24, 2026 04:57
@arthur791004 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 24, 2026
'pcg_probe' => '1',
'token' => $token,
),
home_url( '/' )
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to send the probe an admin page (like plugins.php) instead of the front-end, to ensure that the admin_init is run.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be better to check both!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in e415a0c1c3 + 8729578de4. The load tester now fires two parallel probes via \WpOrg\Requests\Requests::request_multiple(): home_url('/') (front-end) and admin_url('index.php') with pcg_admin=1 and the admin's WP auth cookies forwarded so auth_redirect() clears.

}

// Let init / admin_init fire with the plugin's hooks in place; emit "ok" at the end.
add_action( 'wp_loaded', 'pcg_probe_emit_ok', PHP_INT_MAX );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a follow up of https://github.com/Automattic/jetpack/pull/48261/changes#r3145321607, is wp_loaded too soon as admin_init is run later for admin flows?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The endpoint now selects the appropriate verdict hook based on the new pcg_admin=1 flag. Admin probes wait for admin_init at priority PHP_INT_MAX, while front-end probes continue to emit on wp_loaded. The load tester dispatches both loopback requests in parallel using \WpOrg\Requests\Requests::request_multiple(), forwarding the admin probe's authentication cookies so auth_redirect() completes successfully.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by the same change — when pcg_admin=1, the endpoint now defers its verdict to admin_init priority PHP_INT_MAX. Front-end probe stays on wp_loaded. See the add_action( $is_admin_probe ? 'admin_init' : 'wp_loaded', … ) line in probe-endpoint.php.

arthur791004 and others added 14 commits April 27, 2026 16:54
…eck)

Adds a prototype pre-flight plugin compatibility check that combines
WordPress.org plugin metadata (requires, requires_php, tested) with the
site's WP and PHP versions to produce a verdict:

  - block: site doesn't meet required WP or PHP
  - warn:  plugin untested on the current WP version
  - safe:  all checks pass

The comparison engine lives in four plain classes (PCG_Verdict,
PCG_Site_State, PCG_Wporg_Source, PCG_Compat_Checker) with no WP.com,
Atomic, or AI-tooling references so it can be lifted verbatim into the
AI ability that'll eventually wrap it.

Surfaces:
  - Tools → Plugin Compat Check (admin form + verdict card + raw JSON).
  - `wp plugin-compat check <slug>` WP-CLI command.

Both surfaces are gated behind `apply_filters( 'pcg_enable', false )`
so the prototype ships dark on WP.com until a site explicitly opts in.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends the Plugin Conflicts Guardian with a second input mode: admins
can upload a plugin zip and get a verdict before installing.

The upload flow adds three new layers to the existing slug check:

  PCG_Local_Source     parses plugin headers from an extracted dir.
  PCG_Syntax_Checker   tokenizes every .php file (catches parse errors
                       that would fatal at activation).
  PCG_Load_Tester      runs the plugin's main file in a short-lived
                       HTTP self-request; the probe endpoint (in
                       probe-endpoint.php) bootstraps WP, requires the
                       plugin, lets init/admin_init fire, and reports
                       any fatal via a shutdown handler.

The HTTP-based probe replaces an earlier CLI-subprocess approach so
it works inside web-PHP sandboxes (Atomic, managed hosts) where
proc_open can't reach a CLI binary. Token-authenticated, single-use,
30s expiry — no arbitrary file-include surface.

Admin page gains a second form for .zip uploads next to the slug
input; the verdict card renders the same way for both modes. Upload
handling: 25 MB cap, .zip-only, zip-slip guarded via WP's unzip_file,
extraction to pcg-tmp with recursive cleanup on every code path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hooks `load-plugins.php` at priority 0 to intercept the Activate (and
Activate-selected) action before wp-admin's inline handler calls
`activate_plugin()`. For each plugin being activated, runs the HTTP
load probe; any that return `fatal` or `throwable` are collected and
surfaced as an admin notice after redirecting back to the plugins
list. The plugin is never added to `active_plugins` — the site stays
up.

The gate is disable-able via the `pcg_guard_activation` filter for
environments that want to opt out. Nonce verification happens before
probing so a CSRF doesn't burn a probe slot.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ate-plugin

The post-install "Activate Plugin" link that WordPress renders after
Add New → Upload Plugin goes to update.php?action=activate-plugin,
not plugins.php. Hook `load-update.php` at priority 0 alongside
`load-plugins.php` and treat `activate-plugin` the same as `activate`
(same nonce, same single-plugin payload).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the admin Tools page, the wp-cli command, and the slug / upload
preview flows. The feature now consists solely of the pre-flight
activation guard and its HTTP probe infrastructure: any click on
Activate (plugins.php?action=activate, activate-selected, or
update.php?action=activate-plugin) runs the plugin in a short-lived
HTTP self-request and refuses the activation when a fatal is
captured during load or the init/admin_init cycle.

Gated behind `pcg_enable` (feature on/off) with a secondary filter
`pcg_guard_activation` for sites that want to skip the gate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hooks `upgrader_source_selection` at late priority and tokenizes every
`.php` file in the extracted package. Any parse error aborts the
install/update with a WP_Error — WordPress rolls back and leaves the
previous plugin files in place.

Syntax-only (not a full load probe) because during an update the
active version is already loaded, so `require`-ing the new version's
main file would always trip "Cannot redeclare" regardless of whether
the new code is otherwise correct. Parse errors are the high-
frequency failure mode for plugin releases, and catching them here
keeps that class of breakage out of production without the
redeclare false positives. Runtime regressions still trip on the
next Activate via the activation guard.

Shares the `pcg_guard_activation` filter with the activation guard
so the feature stays a single on/off surface.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Add @return never to pcg_probe_respond().
- Capture token_get_all()'s return so Phan sees the call used.
- Drop redundant (int)/(string) casts on ParseError getters.
Trim the rationale + flow comments from each PHP file and consolidate
them in a new README.md next to the feature. Net -134 lines of inline
comments; the prose stays discoverable in one place instead of being
scattered across five files.
… gate

pcg_enable and pcg_guard_activation overlapped. Collapse to the single
pcg_guard_activation filter, defaulting to false so the feature still
ships dark until a site opts in.
* @param string $dir Extracted package directory.
* @return array<int,array{file:string,line:int,message:string}>
*/
function pcg_update_guard_scan_for_parse_errors( $dir ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A concern here is that this function will run for a long time for big packages. Since cron auto-updates will run this as well, there's is an additional consideration for max-execution timeouts. We should allot a time budget to the scan, and bail if it exceeds it. Also consider a error_log/bump_stat to see how often are scans being bailed and which package is it.

Copy link
Copy Markdown
Contributor Author

@arthur791004 arthur791004 Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9cddc64. Added PCG_UPDATE_GUARD_BUDGET_SECONDS = 8.0; the scan checks the wall-clock budget before each file and bails out cleanly when exceeded. On bail with no errors found we don't fail-closed — we let the install/update through and error_log the slug + action so we can see how often this fires and on which packages.

}

// Verify the nonce up front so we don't run probes for a request core will reject anyway.
if ( ! isset( $_REQUEST['_wpnonce'] ) || false === check_admin_referer( $nonce_action ) ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider the following, as check_admin_referer() runs die() if the nonce is invalid.

Suggested change
if ( ! isset( $_REQUEST['_wpnonce'] ) || false === check_admin_referer( $nonce_action ) ) {
if ( ! isset( $_REQUEST['_wpnonce'] ) ) {
return;
}
check_admin_referer( $nonce_action );

Copy link
Copy Markdown
Contributor Author

@arthur791004 arthur791004 Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9cddc64. Dropped the redundant false === check_admin_referer(…) comparison; we now just isset( $_REQUEST['_wpnonce'] ) early-return then call check_admin_referer( $nonce_action ) and let it die() on a bad nonce.

$blocked = array();
$pcg_load_tester = new PCG_Load_Tester();

foreach ( $plugins as $plugin ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also performance consideration here that doesn't scale well with multiple plugins (2 probes per plugin), but we can follow up later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged — memo'd as a follow-up. Batching all probes for a bulk activation into a single request_multiple call would need probe-endpoint to handle multiple plugin_mains per token, which is a real refactor.

$this->rrmdir( $this->tmp_dir );
$this->tmp_dir = null;
}
remove_all_filters( 'pcg_guard_activation' );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also add:

remove_action( 'load-plugins.php', 'pcg_guard_maybe_block_activation', 0 );
remove_action( 'load-update.php', 'pcg_guard_maybe_block_activation', 0 );
remove_action( 'admin_notices', 'pcg_guard_render_block_notice' );

Since those are added by:

require_once Jetpack_Mu_Wpcom::PKG_DIR . 'src/features/plugin-conflicts-guardian/activation-guard.php';

Copy link
Copy Markdown
Contributor Author

@arthur791004 arthur791004 Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9cddc64. Added remove_action for load-plugins.php, load-update.php, and admin_notices in tear_down so the activation-guard hooks don't leak across cases.

return new WP_Error(
'pcg_update_parse_error',
sprintf(
"WordPress.com blocked the %s: the package contains PHP parse error(s).\n- %s",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WYDT if we just formatted the first error message, and output the rest so that consumers can format them accordingly? Basically return:

  return new WP_Error(
      'pcg_update_parse_error',
      $message,
      array( 'errors' => $errors )
  );

Copy link
Copy Markdown
Contributor Author

@arthur791004 arthur791004 Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9cddc64. The error message now formats just the first parse error; the full list is in $data['errors'] so consumers can render / surface them however they like.

@taipeicoder
Copy link
Copy Markdown
Contributor

For future maintainers, it would be useful to add a README section comparing this with WordPress core’s sandboxed activation via plugin_sandbox_scrape().

arthur791004 and others added 4 commits April 28, 2026 22:22
Use a separate `ok-inconclusive` status for the admin-probe 302 → login
case so it can be measured separately from a clean `ok`. Existing
non-blocking paths in activation-guard and update-healthcheck already
treat anything that isn't fatal/throwable as a pass-through, so behavior
is unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- update-guard: WP_Error now carries the full errors list in $data so
  consumers can render them however they like (only the first error is
  formatted into the message). Add an 8s wall-clock budget to the
  parse-error scan and bail (with an error_log entry) when packages are
  too big to scan in budget — keeps cron auto-updates from blowing the
  request timeout.
- activation-guard: drop the redundant `false === check_admin_referer`
  guard. Core's check_admin_referer() die()s on a bad nonce, so the
  comparison is dead code.
- Tests: tear down the activation-guard hooks (load-plugins.php,
  load-update.php, admin_notices) so they don't leak across cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Net -95 lines, no behavior change, all 23 tests pass.

- probe-endpoint: drop the dev-only `diagnostic` block from `ok`
  payloads (never read in production); route `pcg_probe_shutdown`
  through `pcg_probe_respond` so the buffer-clearing loop and the
  exit-after-emit live in one place.
- load-tester: use try/finally for transient cleanup instead of two
  separate cleanup paths; trim verbose docblocks.
- activation-guard: switch errno → name lookup to a `match` expression;
  collapse the action/action2 fallback into one statement.
- update-guard: drop the unused `files_scanned` field from the scan
  result; flatten the success / budget-exceeded / errors branches.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cumulative result of several self-review passes; behavior unchanged,
all 23 tests pass.

- probe-endpoint: pcg_probe_bail_error() helper replaces the three
  identical {status:error, reason:…} bail blocks.
- Probe transient is now the raw plugin_main string (no more one-key
  array wrapper; the user_id field was already dead).
- load-tester::parse_response: ?? null-coalesce on the response props
  instead of defensive isset() (WpOrg\Requests\Response always
  initializes status_code/body).
- Cookie collection uses str_starts_with() instead of `0 !== strpos`.
- Drop the dead is_ok() helper; merge the two if($is_admin) blocks in
  prepare_probe; trim stale "richer admin signal" docblock paragraph.
- match expression in pcg_guard_format_block_reason and
  pcg_guard_errno_name.
- Collapse bulk-vs-single plugin branch in activation-guard into one
  empty check after the branch.
- update-guard: drop unused files_scanned scan field; flatten the
  success / budget-exceeded / errors branches.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@arthur791004
Copy link
Copy Markdown
Contributor Author

arthur791004 commented Apr 28, 2026

Pushed a batch of changes addressing the review:

Probe coverage

  • Two parallel probes now run per plugin via \WpOrg\Requests\Requests::request_multiple() — one against home_url('/') (front-end), one against admin_url('index.php') with the admin's WP auth cookies forwarded. The admin probe defers its verdict to admin_init priority PHP_INT_MAX so admin-time hook fatals surface; the front-end probe stays on wp_loaded.
  • A captured fatal / throwable from either probe wins. An inconclusive error from one no longer shadows a real fatal from the other.
  • New ok-inconclusive status for admin-probe-302-to-login (no/expired cookie) so we can measure it without coupling to clean ok.

Guards

  • update-guard.php is now loaded unconditionally — upgrader_source_selection fires for cron auto-updates too, not just admin. The is_admin() gate was suppressing the parse-error gate during background flows.
  • Added an 8s wall-clock budget to the parse-error scan; on big packages we bail out cleanly (with an error_log line carrying the slug + action) instead of risking the cron / request timeout. We don't fail-closed on a slow scan.
  • The scan's WP_Error now carries the full list in $data['errors']; the message formats just the first error so consumers can render the rest however they want.

Activation guard

  • Bottom-of-page bulk dropdown handled (action=-1 + action2=…).
  • Dropped the redundant false === check_admin_referer() guard — core die()s on a bad nonce.
  • match expression for the errno → name lookup; tighter format helper.

Tests

  • tear_down now removes the activation-guard hooks (load-plugins.php, load-update.php, admin_notices) so they don't leak across cases.

Cleanup

  • Several self-review passes shaved the feature down by ~110 lines without changing behaviour: pcg_probe_bail_error() helper, transient stores plugin_main as a string instead of a one-key array, dropped a dead is_ok() helper and the diagnostic block from ok payloads, str_starts_with for cookie-prefix checks.

Documented as Limitations / follow-ups

  • Probe is registered during load_features() at plugins_loaded priority 10, so plugin callbacks at priorities < 10 fire before our require runs and aren't observed. Documented; fixing it requires splitting probe-handler registration out of load_features() (separate PR).
  • Two probes × N plugins doesn't batch; scaling that needs probe-endpoint to handle a list of plugin_mains per token — also a follow-up.

arthur791004 and others added 3 commits April 28, 2026 23:00
- Probe step describes the new ok-inconclusive status by name.
- Update-guard section covers the 8s scan budget + the unconditional
  loading (cron auto-updates).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collapse `isset( $x ) ? sanitize( wp_unslash( $x ) ) : ''` patterns
into `sanitize( wp_unslash( $x ?? '' ) )`. For the bulk-action
checked-array, replace the two-condition guard with
`is_array( $_REQUEST['checked'] ?? null )`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The notice was rendering log-style strings like "E_USER_ERROR: PCG fatal
(plugin.php, line 6)" or, worse, "error 99: unknown error" — neither
reads as a sentence.

Drop the technical label (E_USER_ERROR / error %d) from the per-plugin
line; the notice header already says "the pre-flight check detected a
fatal". Build a complete sentence around <message> + (in <file>, line
<line>):

  - With message + file + line: "PCG fatal (in plugin.php, line 6)."
  - With message only:          "PCG fatal."
  - No message but file + line: "A fatal PHP error was detected in plugin.php, line 6."
  - Neither:                    "A fatal PHP error was detected."

Drop the now-orphan pcg_guard_errno_name() and its data provider tests;
update the format-reason data provider to the new sentences. Translator
notes spell out where each fragment composes so translators can keep
the leading "in" preposition + parenthetical punctuation right.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…mid-bootstrap

A plugin that calls exit/die during load, init, or admin_init returns
HTTP 200 with a non-JSON body. parse_response previously classified
that as `error` (non-blocking), so activation proceeded and the same
termination would affect matching future requests.

The probe endpoint always emits JSON via wp_send_json, so a 2xx without
a JSON verdict means the bootstrap was truncated. Treat as a captured
fatal so the activation is blocked.
Two defense-in-depth tightenings:

- pcg_guard_evaluate_plugins() now rejects plugin basenames that fail
  validate_file() (.. segments, drive letters, absolute paths) before
  building the require path. The nonce check already gates the activation
  flow to admins, but a future entry point that forgets the nonce
  shouldn't turn this into an arbitrary-file-include.

- pcg_maybe_handle_probe() bails when pcg_guard_activation is filtered
  off, after the cheap probe-flag check. Without the gate, hitting
  ?pcg_probe=1 with a junk token short-circuits the rest of the
  plugins_loaded chain even when the feature is disabled. Filter-off
  should make the URL fully inert.
The previous filter check returned silently, so any request that
reached the endpoint with the feature disabled got whatever HTML the
WP bootstrap had buffered. Combined with the new 2xx-without-JSON
verdict, a direct caller of PCG_Load_Tester::test() with the filter
off would interpret that buffered body as a captured fatal.

Bail with pcg_probe_bail_error() so the endpoint preserves its "always
emits JSON" invariant. The error verdict is non-blocking at the
activation guard, which is the correct fail-open when the feature is
intentionally off.
@taipeicoder
Copy link
Copy Markdown
Contributor

This looks good to merge! We still have to address #48261 (comment) in a follow-up.

@taipeicoder taipeicoder merged commit eacc259 into trunk Apr 29, 2026
70 checks passed
@taipeicoder taipeicoder deleted the try/plugin-conflicts-guardian branch April 29, 2026 01:41
@github-actions 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] Needs Review This PR is ready for review. labels Apr 29, 2026
taipeicoder added a commit that referenced this pull request Apr 29, 2026
Collapse the action + default listener pair into a direct call from
the screen filter. The action existed for hypothetical third-party
listeners; none exist. Other mu-plugins that want the same signature
shape can call `wpcom_build_fatal_error_signature()` directly from
their own fatal-detection paths (e.g. #48261's activation probe) — the
shared helper in jetpack-mu-wpcom is the correct extension point, not
a parallel wpcomsh-specific hook.

Also log the decoded parts (kind, slug, extension_version, wp_version,
php_version) alongside the encoded signature so Kibana queries can
term-aggregate without an ingest-time base64+JSON decode step. Use
`class_exists( 'WPCOMSH_Log', false )` to skip autoloader I/O during
fatal handling.

Net: -1 file, ~150 fewer lines, no public API surface for nonexistent
consumers. Helper in jetpack-mu-wpcom is unchanged and remains
available for cross-mu-plugin reuse.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
arthur791004 added a commit that referenced this pull request May 5, 2026
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>
arthur791004 added a commit that referenced this pull request May 6, 2026
#48402)

* Plugin Conflicts Guardian: batch the probe across all selected plugins

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>

* Rename pcg_guard_attribute_block → pcg_guard_get_blocked_plugin

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>

* Add tests for pcg_guard_get_blocked_plugin and load tester guard rail

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>

* Satisfy Phan: assertArrayHasKey('reason') before assertNotEmpty

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>

* PCG: trim verbose comments + fix \$status alignment

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>

* PCG load tester: filter unreadable plugin paths up front

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>

* PCG activation guard: surface batch-level message when attribution fails

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>

* PCG probe endpoint: skip unreadable entries instead of bailing the batch

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>

* PCG: update flat-file prefix test to expect undetermined ''

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>

* PCG activation guard: number-agnostic blocked notice

"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>

* PCG activation guard: locale-aware list separator in batch-level notice

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>

* PCG load tester: log probe transport errors to logstash

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>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
matticbot pushed a commit to Automattic/jetpack-mu-wpcom-plugin that referenced this pull request May 6, 2026
…s (#48402)

* Plugin Conflicts Guardian: batch the probe across all selected plugins

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 Automattic/jetpack#48261 (comment)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Rename pcg_guard_attribute_block → pcg_guard_get_blocked_plugin

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>

* Add tests for pcg_guard_get_blocked_plugin and load tester guard rail

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>

* Satisfy Phan: assertArrayHasKey('reason') before assertNotEmpty

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>

* PCG: trim verbose comments + fix \$status alignment

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>

* PCG load tester: filter unreadable plugin paths up front

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>

* PCG activation guard: surface batch-level message when attribution fails

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>

* PCG probe endpoint: skip unreadable entries instead of bailing the batch

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>

* PCG: update flat-file prefix test to expect undetermined ''

The undetermined-attribution change in 59f6cc90 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>

* PCG activation guard: number-agnostic blocked notice

"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>

* PCG activation guard: locale-aware list separator in batch-level notice

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>

* PCG load tester: log probe transport errors to logstash

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>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/25415426493

Upstream-Ref: Automattic/jetpack@337468a
matticbot pushed a commit to Automattic/jetpack-storybook that referenced this pull request May 6, 2026
…s (#48402)

* Plugin Conflicts Guardian: batch the probe across all selected plugins

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 Automattic/jetpack#48261 (comment)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Rename pcg_guard_attribute_block → pcg_guard_get_blocked_plugin

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>

* Add tests for pcg_guard_get_blocked_plugin and load tester guard rail

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>

* Satisfy Phan: assertArrayHasKey('reason') before assertNotEmpty

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>

* PCG: trim verbose comments + fix \$status alignment

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>

* PCG load tester: filter unreadable plugin paths up front

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>

* PCG activation guard: surface batch-level message when attribution fails

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>

* PCG probe endpoint: skip unreadable entries instead of bailing the batch

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>

* PCG: update flat-file prefix test to expect undetermined ''

The undetermined-attribution change in 59f6cc90 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>

* PCG activation guard: number-agnostic blocked notice

"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>

* PCG activation guard: locale-aware list separator in batch-level notice

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>

* PCG load tester: log probe transport errors to logstash

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>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/25415426493

Upstream-Ref: Automattic/jetpack@337468a
matticbot pushed a commit to Automattic/jetpack-mu-wpcom that referenced this pull request May 6, 2026
…s (#48402)

* Plugin Conflicts Guardian: batch the probe across all selected plugins

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 Automattic/jetpack#48261 (comment)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Rename pcg_guard_attribute_block → pcg_guard_get_blocked_plugin

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>

* Add tests for pcg_guard_get_blocked_plugin and load tester guard rail

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>

* Satisfy Phan: assertArrayHasKey('reason') before assertNotEmpty

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>

* PCG: trim verbose comments + fix \$status alignment

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>

* PCG load tester: filter unreadable plugin paths up front

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>

* PCG activation guard: surface batch-level message when attribution fails

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>

* PCG probe endpoint: skip unreadable entries instead of bailing the batch

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>

* PCG: update flat-file prefix test to expect undetermined ''

The undetermined-attribution change in 59f6cc90 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>

* PCG activation guard: number-agnostic blocked notice

"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>

* PCG activation guard: locale-aware list separator in batch-level notice

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>

* PCG load tester: log probe transport errors to logstash

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>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/25415426493

Upstream-Ref: Automattic/jetpack@337468a
matticbot pushed a commit to Automattic/wpcom-site-helper that referenced this pull request May 6, 2026
…s (#48402)

* Plugin Conflicts Guardian: batch the probe across all selected plugins

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 Automattic/jetpack#48261 (comment)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Rename pcg_guard_attribute_block → pcg_guard_get_blocked_plugin

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>

* Add tests for pcg_guard_get_blocked_plugin and load tester guard rail

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>

* Satisfy Phan: assertArrayHasKey('reason') before assertNotEmpty

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>

* PCG: trim verbose comments + fix \$status alignment

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>

* PCG load tester: filter unreadable plugin paths up front

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>

* PCG activation guard: surface batch-level message when attribution fails

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>

* PCG probe endpoint: skip unreadable entries instead of bailing the batch

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>

* PCG: update flat-file prefix test to expect undetermined ''

The undetermined-attribution change in 59f6cc90 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>

* PCG activation guard: number-agnostic blocked notice

"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>

* PCG activation guard: locale-aware list separator in batch-level notice

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>

* PCG load tester: log probe transport errors to logstash

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>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/25415426493

Upstream-Ref: Automattic/jetpack@337468a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Coverage tests to be added later Use to ignore the Code coverage requirement check when tests will be added in a follow-up PR Docs [mu wpcom Feature] Plugin Conflicts Guardian [Package] Jetpack mu wpcom WordPress.com Features [Status] UI Changes Add this to PRs that change the UI so documentation can be updated. [Tests] Includes Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants