Plugin Conflicts Guardian: migrate from jetpack-mu-wpcom to the Jetpack plugin#49048
Plugin Conflicts Guardian: migrate from jetpack-mu-wpcom to the Jetpack plugin#49048arthur791004 wants to merge 6 commits into
Conversation
…ck plugin Moves PCG out of the jetpack-mu-wpcom package (Atomic-only) into the Jetpack plugin so self-hosted Jetpack sites get conflict protection too. - Source files relocated to projects/plugins/jetpack/modules/plugin-conflicts-guardian/, loaded unconditionally from load-jetpack.php — not connection-gated, and not registered as a toggleable module (PCG is a safety net, not a user feature). - probe-endpoint.php now hooks plugins_loaded at PHP_INT_MIN instead of running inline at require time; the new load point also closes the early-priority plugins_loaded callback gap the mu-wpcom load point left open. - pcg-log.php rewritten for the Jetpack plugin: a Jetpack Tracks event when connected, Jetpack_Mu_Wpcom::log2logstash() when available (Atomic), and an error_log fallback otherwise. - Text domain, @Package headers and admin-notice copy updated for the Jetpack plugin context. - Removed from jetpack-mu-wpcom. PCG never shipped in a released version of the package, so its 11 unreleased PCG changelog entries are dropped; log2logstash() stays as a general-purpose helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
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:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 2 files.
9 files are newly checked for coverage. Only the first 5 are listed here.
Full summary · PHP report · JS report Coverage check overridden by
Coverage tests to be added later
|
…ibility
The Jetpack plugin supports PHP 7.2 (mu-wpcom required 7.4+), so the
`static fn () =>` arrow functions PCG carried over fail the parallel-lint
check. Convert all 7 to regular `static function () { return …; }`
closures — none captured outer scope, so no `use` clauses are needed.
A token scan confirms no trailing commas in function calls (PHP 7.3+)
remain either.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
A couple of questions:
|
The arrow-function replacements landed as single-line closures, which trip Squiz.Functions.MultiLineFunctionDeclaration.ContentAfterBrace (the opening brace must be the last content on its line). Pure `(string)` casts inside array_map() become the `'strval'` callback; the remaining filter/sanitize closures are expanded to multi-line form. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks @simison — good questions. Force-enabled, or a toggleable feature? Today it's behind two filters — My first instinct was a normal module: on by default, user can turn it off. But the module system doesn't fit PCG:
So if we want a user-facing toggle, it'd have to be a Jetpack Settings option, separate from the module system. But before adding one, I think the real question is: why would a user turn this off? It's a safety net — there's no plan or feature value in disabling it. The one scenario where an off switch genuinely matters is a false positive — PCG wrongly blocking a healthy plugin. PCG already works hard to avoid that (cache-intercepted, redirected, and otherwise inconclusive probes resolve as non-blocking, not as fatals), so a true false-block should be rare. And if one does happen, the better fix is probably to harden the probe, not to ship a switch that — for everyone who tripped it for a real reason — quietly turns the protection off. So right now I'm leaning toward no UI toggle, with the filters as the escape hatch for the rare edge case. Happy to be talked out of that if there's a concrete user-facing reason I'm missing. Part of Protect or Backup? Honestly, I hadn't considered that — I'm not that familiar with the Jetpack product lineup, so this is useful to hear. Conceptually it does overlap both — Protect (prevents a class of site-down events) and Backup (local snapshot + rollback). One thing to weigh: PCG is plan- and connection-independent infrastructure, whereas the Protect/Backup modules are connection- and plan-gated. Which product should own it is worth a conversation with those teams — and where it lives in the tree doesn't have to match which product owns it. WordPress core? Core already has adjacent pieces — fatal-error protection / recovery mode (5.2) and rollback for failed plugin auto-updates (6.3). What core doesn't have is the pre-flight check: probing an activation/update in an isolated request and refusing it before the live site is affected, instead of recovering after. I'd suggest letting it prove out on Jetpack first — once it's stable here and we've gathered real-world data, moving it (or proposing it) to core would be a reasonable next step to consider. |
The test was converted from WorDBless's BaseTestCase to WP_UnitTestCase during the migration, but missed `use \Automattic\Jetpack\PHPUnit\WP_UnitTestCase_Fix`. Without it, WP's test framework calls the PHPUnit\Util\Test::parseTestMethodAnnotations() method that PHPUnit 10+ removed, erroring every test method. The trait (loaded by tests/php/bootstrap.php, used by ~160 other Jetpack tests) supplies a PHPUnit 10/11/12-compatible getAnnotations() implementation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Just if the user doesn't want the feature; perhaps they wanted Jetpack for some other features. :-) There could be some technical "world of hosting sites is messy" reasons too, but I was mostly thinking marketing & perception angles of Jetpack. They might also have some alternative existing solutions for this problem that Jetpack might now compete with/take over. Is there any user-facing indication or marketing that the feature exists, or it is "hidden until triggered"? |
…f test output `test_update_guard_check_blocks_plugin_with_parse_error` exercises a real block, so `pcg_log_event()` reaches its `error_log()` fallback (no Tracks connection, no mu-wpcom logstash in the Jetpack plugin test env). PHPUnit's strict-output check then flags the test as risky. Point the `error_log` ini at a scratch file in `set_up()` and restore it in `tear_down()`, so the fallback writes there instead of into PHPUnit's captured output. Production behaviour is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ng in tests Pointing the `error_log` ini at a scratch file didn't keep PCG's logging fallback out of PHPUnit's captured output. Gate `pcg_log_event()` behind a new `pcg_log_enabled` filter instead — a legitimate hook for sites that don't want PCG telemetry — and have the test disable it in set_up(). With the dispatch short-circuited there's no error_log() call at all, so the strict-output check has nothing to flag, independent of how error_log is routed. Replaces the ineffective ini_set() approach. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Hidden until triggered — no Settings entry, no badge, no marketing. It only surfaces when it acts: an admin notice on a blocked activation or a rolled-back update. We can loop in the Jetpack team to do some marketing around it if that's worth pursuing. |
Proposed changes
Migrates Plugin Conflicts Guardian (PCG) out of the
jetpack-mu-wpcompackage (which only ships to WordPress.com Atomic sites) and into the Jetpack plugin, so self-hosted Jetpack installs get the same conflict protection.projects/plugins/jetpack/modules/plugin-conflicts-guardian/. It lives undermodules/but is not a registered toggleable module (noModule Name:header) — same pattern asmodules/seo-tools/andmodules/shortcodes/.load-jetpack.php. PCG is a site-safety net, so it deliberately does not go through the standard module loader: that loader runs atafter_setup_theme:-2and is connection-gated, which would leave disconnected sites (the most fragile moment) unprotected and would load the probe too late.probe-endpoint.phpnow hooksplugins_loadedatPHP_INT_MINinstead of running inline at require time. Forced by the new (pre-plugins_loaded) load point — and it closes the early-priorityplugins_loadedcallback gap the old mu-wpcom load point (load_features()priority 10) left open.pcg-log.phprewritten for the Jetpack plugin: a Jetpack Tracks event when the site has a working connection,Jetpack_Mu_Wpcom::log2logstash()when that helper is present (Atomic), and anerror_logfallback otherwise.@packageheaders,jetpacktext domain, and admin-notice copy ("WordPress.com blocked…" → "Jetpack blocked…") updated for the plugin context.jetpack-mu-wpcom. PCG never shipped in a released version of the package, so its 11 unreleased PCG changelog entries are dropped.Jetpack_Mu_Wpcom::log2logstash()stays as a general-purpose logging helper.Related product discussion/links
Does this pull request change what data or activity we track or use?
Yes — PCG's events (
Activation blocked,Update blocked,Update rolled back,Probe transport error) are now also emitted as Jetpack Tracks events (jetpack_pcg_*) on connected sites, in addition to the existing Atomic logstash bucket. No new personal data is collected; events carry plugin basenames and probe verdicts only (install-path prefixes are redacted).Testing instructions
phpcsandchangelogger validatepass for bothjetpackandjetpack-mu-wpcom.🤖 Generated with Claude Code