Boost: let the host decide whether static cache URLs can be used - #50962
Open
LiamSarsfield wants to merge 17 commits into
Open
Boost: let the host decide whether static cache URLs can be used#50962LiamSarsfield wants to merge 17 commits into
LiamSarsfield wants to merge 17 commits into
Conversation
The 404 tester decides whether concatenated bundles can be linked straight out of wp-content/boost-cache/static, and records its verdict in the jetpack_boost_static_minification site option. That verdict describes the host, but the option travels with the database: a site migrated onto Atomic or WP Cloud arrives carrying its previous host's 1, and the concat classes read the option with no host check. Every bundle URL then points somewhere the new host answers with a web server 404 that never reaches WordPress, so the file is never built and the frontend loses all of its CSS and JS. The host check already existed, but it guarded jetpack_boost_404_setup() -- reachable only from module activation and plugin upgrade -- so nothing re-evaluated it when the host underneath the database changed. Move the check to the point of use: jetpack_boost_minify_use_static_cache_urls() asks the host first and only then consults the option, and the setup path now deletes a stale verdict rather than stepping over it. Deleting rather than zeroing matters: show_legacy_notice() treats 0 as "tested, unsupported" and surfaces a notice, while a missing option is correctly read as "never tested".
LiamSarsfield
added
[Status] Needs Review
Contributor
|
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! Boost plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryCoverage changed in 19 files. Only the first 5 are listed here.
|
Follow-up to review feedback on the BOOST-608 fix. The read guard stops a migrated verdict from being acted on, but three gaps around it were worth closing: * jetpack_boost_404_tester() had no host check, and the cron event's schedule travels with the database like the option does. On a migrated site the daily run would put back the verdict jetpack_boost_404_setup() deletes, and spend a loopback request on a question whose answer is known a priori. Guard the tester instead of unscheduling the event, so a site that later migrates off Atomic/WP Cloud still re-tests within a day. The comment claiming nothing else would clear the option was wrong, and is corrected. * Nothing exercised the two do_items() call sites that are the actual fix - reverting either left the whole suite green. Add a with-wordpress test that registers real assets, runs both concatenators, and asserts on the emitted URL. Reverting either call site now fails it. * Rename jetpack_boost_minify_host_supports_static_cache() to jetpack_boost_minify_host_handles_wp_content_404s(). It sat in the same subsystem as Minify\Config::can_use_static_cache(), which asks whether the cache directory is writable - a different question. Name it for the condition it actually tests. Also covers the show_legacy_notice() host guard, the supported-host scheduling branch, and stubs get_current_blog_id() in Functions_Service_Test, which now reaches Host through the new guard.
… comments
handle_version_change() unschedules every minify event before re-running
activation, and jetpack_boost_404_setup()'s Atomic/WP Cloud early return sat
above the only re-schedule call. On those hosts the daily tester was torn down
at each plugin upgrade and never recreated, so a site migrating back off would
never re-test and would stay on the fallback delivery method indefinitely. The
comment added in the previous commit claimed the opposite.
Hoist jetpack_boost_page_optimize_schedule_404_tester() above the guard. The
guard inside jetpack_boost_404_tester() makes a scheduled-but-unsupported event
free -- it short-circuits without a loopback request -- so this costs nothing on
the hosts that can't use the static cache and restores the ~24h recovery path.
Ordering is unchanged for supported hosts: the inline probe writes a real verdict
where the placeholder 0 would have gone.
Also:
* Correct "nothing on these hosts would otherwise clear it" -- handle_version_change()
deletes that option unconditionally on every host. State what the delete is
actually load-bearing for: the module-activation window between upgrades.
* Write the host predicate as an allowlist ('other' === get_hosting_provider())
rather than a deny-list. get_hosting_provider() documents itself as tracking
Atomic and WoA "for now", and a future fourth value should fall back to the
slower-but-correct delivery method, not re-create the outage. No behaviour
change for the three values it returns today.
* Document the short-circuit in jetpack_boost_404_tester()'s DocBlock, and note
in the predicate's that true means "not a host we know answers these itself",
not "verified".
* Test hygiene: 0755 rather than 0777 for the fixture directory, teardown that
only removes what set_up() created, and ob_start() paired with finally so a
throw inside do_items() surfaces as that failure rather than a leaked buffer.
* Cover the two gaps this leaves: the daily event still being scheduled on WP
Cloud, and a genuinely unset option on a supported host.
…hes when the verdict is dropped Three fixes and a correction, from a third review round. The early fallback router in jetpack-boost.php compared request paths against the raw JETPACK_BOOST_STATIC_PREFIX constant, while the concatenators canonicalize it before building URLs. A site defining it as `assets` or `/assets` emitted /assets/??… and then had those exact requests fall through to a 404. Pre-existing, but this change forces Atomic and WP Cloud sites onto that router unconditionally, so leaving it would have moved their breakage rather than fixed it. The minify loader is already required eight lines above, so the router can just use jetpack_boost_get_static_prefix(). Dropping a stale verdict rewrites every concatenated URL the next render emits, which makes any cached HTML holding the old ones stale. Both delete sites now go through one jetpack_boost_minify_forget_static_cache_verdict() helper that fires jetpack_boost_page_output_changed only when a value was actually removed. The previous round's stated reason for not doing this was wrong twice over: Page_Cache::is_available() enables Boost's own page cache on Atomic and WoA whenever the cache can run, and its listener schedules garbage collection with older_than = time(), which evaluates the TTL to zero and purges everything. The reverse-migration test was vacuous - it asserted that an unset option renders the fallback, which was true before this branch existed. It now drives the transition that produces the unset option, with the cron event pre-scheduled so the guard under test is deterministic rather than dependent on process state. Also: the predicate's DocBlock claimed both its callers combine it with the probed verdict; one of four does, and the other three treat it as authoritative, which makes the fallback permanent rather than merely slower for any future hosting provider value. The fixture teardown tracked directory ownership, so one crashed run left the assets behind for good; it now tracks the files it wrote. Adds an @return to jetpack_boost_404_tester(), a note at get_hosting_provider() where a future editor will be standing, and clears host state in set_up() as well as tear_down().
github-actions
Bot
added
the
Admin Page
… a root static prefix Three follow-ups to the BOOST-608 fix, all found by review. Jetpack_Boost::handle_version_change() raw-deleted jetpack_boost_static_minification before running minify activation, so the activation path behind it found the row already gone and never announced the change. That is the primary upgrade path for a migrated site, so the invalidation added in the previous commit was skipped on exactly the run that detects the migration. Route it through the same helper. While doing that, tighten when the helper announces: only a truthy verdict was pointing renders at the static cache directory, so only dropping one of those changes what the next render emits. Dropping an absent or 0 verdict changes nothing and now stays quiet, which also stops an upgrade on a healthy site from asking for a page cache purge it does not need. Canonicalizing the router's prefix in the previous commit had a degenerate case: JETPACK_BOOST_STATIC_PREFIX defined as '', '/' or '//' all canonicalize to '/', and the router matches that value against the end of the request path - so a prefix of '/' claims every URL ending in a slash, including the home page and /wp-admin/, and serves them from the concatenator before WordPress loads. '/' already did this on trunk; the canonicalization would have extended it to the other two spellings. Treat all three as if the constant were unset. The router's decision moves into jetpack_boost_minify_request_is_for_static_prefix() so it can be tested at all - the router block itself ends in exit(). The matrix covers the URLs Boost emits, the near misses, and the three root spellings. Also correct the predicate's DocBlock, which claimed all three non-emit callers skip the probe; Minify_Common::show_legacy_notice() does not probe at all. The callers are now listed by name so a grep shows when the list drifts.
…ing a drop that reverses itself Two costs this branch introduced, both found by review after the last round. Canonicalizing the static prefix widened the bootstrap router's suffix match: a site defining JETPACK_BOOST_STATIC_PREFIX as `assets` now had every page whose path ends in /assets/ claimed pre-auth and answered with a bare 400, with no way back into WordPress. The router now also requires the ??<files> payload that the two concatenators always emit, which is what separates a bundle URL Boost built from an ordinary permalink. The service already rejected a request without it, so this only moves that rejection above the point of no return. The predicate takes the whole request URI rather than a pre-split path, so the query handling is covered by its tests instead of living in the untestable bootstrap block. Routing handle_version_change() through the forget helper made every release purge the page cache on hosts that can serve the static cache. That call runs unconditionally, and the minify activation below it re-probes inline and restores the same verdict before anything renders - so the emitted URLs were identical either side of a full cache flush, and pages rendered during the blocking loopback were cached with fallback URLs and deliberately spared by the garbage collector's cutoff. The announcement is now gated on the host as well as on the verdict: it fires only where the drop survives the request, which is exactly the Atomic and WP Cloud migration this branch is for. The two other callers already sit inside that condition, so nothing changes for them. Also delete jetpack_boost_minify_serve_concatenated(), a third copy of the router match that is defined and never called, and correct two changelog entries: the root-prefix one claimed all three root spellings over-matched on trunk when only '/' did, and the headline one read as a complete fix without the already-cached pages caveat the PR body carries. 218 unit tests and 111 with-wordpress tests, both new behaviours mutation-anchored.
…nts the last round got wrong The unit suite runs without WordPress, and its bootstrap only polyfills str_contains. The prefix and router tests added on this branch are the first here to reach a production line that calls str_starts_with, and CI runs this suite on PHP 7.2, 7.3 and 7.4, so those three jobs fatal. Production is unaffected: wp-settings.php loads wp-includes/compat.php long before plugins. The router also refuses a bare <prefix>?? now. The fallback parser strips the delimiter and reads offset 0 of what is left without checking, so claiming that request traded a WordPress 404 for a warning and the same 400. The rest is comment accuracy, in the comments this branch itself added: - The host predicate's DocBlock lists its callers so a grep shows when the list has drifted, and it shipped one caller short - the forget helper added in the same commit, which is also a second caller that pairs the host with the verdict rather than using it alone. - The router comment claimed the ?? requirement only moves a rejection the service already performs. The service accepts a superset: it asks for a '?' anywhere in the query, not for the payload at the front. Nothing emits the divergent shape, so no URL changes hands, but the router is stricter on purpose and should say so. - "The re-probe restores the same verdict" was the stated reason for the host gate on the announcement, in three places, and it holds only when minify is enabled - handle_version_change() calls the helper above that check. The gate is right for a better reason: on hosts that route wp-content 404s through WordPress, the handler in loader.php keeps answering the static cache URLs cached HTML holds, whatever the verdict now says, so those pages were never broken. That argument does not depend on the module state or on which way the re-probe goes. The test asserting it is renamed to match.
… the announcement gate rests on jetpack_boost_404_setup() wrote a placeholder 0 to jetpack_boost_static_minification to save an option lookup. That contradicts the absent-vs-0 distinction the rest of this branch relies on, and hoisting the scheduler above the host guard made it reachable: on a site that migrates off WP Cloud the daily event is already on the books, so the inline probe is skipped and the placeholder stands as if it had been measured. show_legacy_notice() reads it as "probed, unsupported" and offers a super-admin a delivery method they already have, until the next cron run. Take the emitted static cache URL back through jetpack_boost_build_minify_output() and assert the bundle rebuilds. Nothing exercised that, and it is what the announcement gate's argument for staying quiet on ordinary hosts rests on. Also pin what bounds it - the File_Paths entry behind the hash - and correct the comment, which claimed a 200 unconditionally. Guard the fallback parser's own read of offset 0. The router's length term only covers one spelling of an empty payload; a request for <static cache>/.js reaches the same read through the loader's 404 handler. Also: drop the caller count from the anti-drift DocBlock, which has now been wrong in three consecutive rounds, and state the announcement-gate rationale once rather than in three files.
…elper rather than deleting it The PHPCompatibility job is failing on a one-argument html_entity_decode() added by the previous commit: PHP 8.1 changed the default $flags, and Boost supports 7.2 up. Spell the flags and encoding out. Two fixes to last round's tests. The "nothing has written the file yet" precondition was pasting a request URI that already starts at /wp-content onto WP_CONTENT_DIR, so it asserted against a doubled path that can never exist and passed whatever the real cache directory held; it now goes through jetpack_boost_get_minify_file_path(). And the helper that pulls a static cache URL out of rendered markup now asserts the /wp-content/boost-cache/static/ literal, which is the one string the emit side and loader.php's 404 handler have to agree on - the emitter builds it from content_url() and the handler matches a hardcoded copy. jetpack_boost_minify_serve_concatenated() comes back as a deprecated wrapper. It is dead in-repo and was on trunk too, but it is a jetpack_-prefixed global that has shipped since 2023, and docs/coding-guidelines.md asks for a six-month window before removal rather than a straight delete. It routes through the corrected predicate, so it no longer claims an ordinary permalink whose last segment matches the prefix. The rest is comments that claimed more than the code does. forget_static_cache_verdict() said a one-shot purge could not repair either of the two cases it names, which is true for a relocated content directory and false for an ordinary host whose web server answers wp-content 404s: there the probe measures the host correctly, fresh renders move to /_jb_static/ and work, and only already-cached HTML is left holding dead URLs. Say so, and say why that transition is still left unannounced - a loopback that times out once is indistinguishable from a host that started answering its own 404s, and paying a full page cache purge for the former is worse than waiting out the latter. Also record that the announcement precondition omits the minify-enabled term the upgrade path needs, and that on multisite the delete consumes the only evidence of a drop, so the first blog to run announces and the rest return above the action.
…claims in the verdict docblock The new WorDBless test only passed because another test file booted the whole plugin first: run on its own it produced ten "Call to undefined function jetpack_boost_ds_get()" errors, because the exclude lists both concatenators consult live outside the minify library. Require that file, guarded, so the suite's own load order stays a convenience rather than a precondition. Three claims in jetpack_boost_minify_forget_static_cache_verdict()'s docblock were wrong or incomplete: - "announces once for HTML that never held a static cache URL" - the current module state says nothing about what was rendered into the cached pages. A site that switched minify off last week still has HTML full of static cache URLs, so the announcement is the conservative reading rather than waste. - The relocated-content-directory case is worse than described: the probe asks for a hardcoded /wp-content/ path under home_url(), which such a site 404s, so loader.php answers it, the marker lands under WP_CONTENT_DIR, and the tester stores a truthy verdict for a delivery method that cannot work anywhere on that site. Both lines are unchanged from trunk, so it is documented here and left for its own issue. - "It does not reach a platform edge cache" implied no mechanism exists. One does - packages/forms fires edge_cache_purge_domain - and the reasons not to fire it from a version change now say so. Also: the test docblock claimed the new literal assertion anchors the emit side against loader.php's copy. It anchors it against a third copy written in the test, since nothing executes the template_redirect closure. Two docblocks in Static_Cache_Urls_Test still carried the retired "the tester restores the verdict" rationale, and three admin-context stubs were left over from when jetpack_boost_404_setup() read that state.
… the regression anchor needs Three review rounds converged on the test-setup line removed last round. Restoring `set_current_screen( 'dashboard' )` was the right call and the removal was not: trunk's placeholder write is gated on `is_admin()`, which is false under WorDBless until a screen is set, so without it the anchor stays green when that exact write comes back. Reproduced both directions by mutation. The file still passes standalone, so the self-containment goal the removal was serving costs nothing. `jetpack_boost_404_tester()` now deletes the 404 marker on the Atomic/WP Cloud path too. The host guard returns above the only other code that deletes it, so a marker migrated in - or written by any request for the tester path - stayed on disk there for good; a later downgrade to a release whose tester has no host check finds it and stores a verdict of 1 for a delivery method the host cannot serve. Deliberately not hoisted above the guard: past that point a marker on disk is the probe's own measurement, and the existing tester tests are what pin that distinction. The forget-verdict docblock loses 24 lines. Four separate over-claims in it were flagged this round - an exhaustive "two ways" enumeration that has a third case, a verdict-write route list missing a route, a multisite purge scope that inverts under WP-CLI cron, and a "the probe measures it correctly" premise an unauthenticated request can falsify - and correcting each in place is what has been growing this block by a paragraph per round. Trimmed to the durable invariants instead, with the out-of-scope mismatch pointed at the pull request rather than restated. Also: the unit suite pinned WP_CONTENT_DIR to a fixed /tmp path, so whatever happened to sit there decided what the probe measured - two reviewers got opposite results from the same experiment. It is per-process and never created now. Changelog names the edge cache as the layer a manual purge is for.
…onstant from suppressing cleanup The PHP 7.2 lane was failing on assertFileDoesNotExist(), which PHPUnit 8.5 does not have and which this class cannot polyfill because it extends PHPUnit\Framework\TestCase directly rather than a WorDBless base. Replaced with a version-neutral file_exists() check. JETPACK_BOOST_DISABLE_404_TESTER returned above both the verdict drop and the marker reclaim, so a site that set the constant and then migrated onto Atomic or WP Cloud kept emitting static cache URLs the host can only 404, with nothing scheduled that would correct it. The constant turns off a loopback request the unsupported-host branch never sends, so that branch now runs above the guard. Supported hosts are unaffected: the host check returns true there and falls through to the same guard as before. The reclaim itself moved into jetpack_boost_minify_reclaim_404_marker(), is called before jetpack_boost_minify_forget_static_cache_verdict() rather than after - the forget helper fires a public action, so anything ordered behind it runs behind third-party listener code - and is now also called from jetpack_boost_404_setup(), which dropped the verdict but left the marker. Also: reset the current screen in the concatenation suite's tear_down, since WorDBless does not and jetpack_boost_page_optimize_bail() memoises is_admin() in a static; restore the rationale for why a 1 -> 0 probe flip announces nothing, next to the write it describes; give the marker fixture owned directory cleanup in a finally block, which takes the suite's temp residue from one tree per run to none; and align the changelog entries with the Concatenate JS/CSS prefix the plugin has used 24 times and Minify zero.
…t four false comments Both review vendors converged on the same restructure, from opposite directions. The reclaim introduced two rounds ago had two call sites and an ordering constraint expressed only in a comment, and each round since has produced a fresh finding about where it sits. Moving the marker deletion inside jetpack_boost_minify_forget_static_cache_verdict() removes the ordering question entirely - there is one call site, and no caller can reach the verdict drop without it. That also closes a real gap: Jetpack_Boost::handle_version_change() forgets the verdict unconditionally, but is the only cleanup a migrated site reaches with both minify modules switched off. It previously dropped the option and left the marker, which is the input the next probe reads. Two new anchors in Jetpack_Boost_Test cover it, and both mutants - deleting the reclaim, and hoisting it above the host check - fail the suite. The remaining changes correct comments that no longer describe the code: - The tester's "past this point a marker is the probe's own measurement" was false; nothing clears the marker before the loopback, as the pre-existing Functions_Service_Test::test_404_tester_when_404_file_exists demonstrates. - The reclaim helper's claim that loader.php answers the tester path "on every host" was false precisely on Atomic and WP Cloud, where the web server answers wp-content 404s and the template_redirect handler never runs. On those hosts a marker can only be inherited. - The router's length-check rationale cited an "Uninitialized string offset" warning whose cause this same PR removed in functions-service-fallback.php. The check is still right; the reason is now simply that a bare `??` names no bundle. - jetpack_boost_404_setup()'s "those two paths" was orphaned when that block became one path. Changelog: the legacy-notice entry described behaviour the code does not have - the notice is a title and a link, and asks nothing. The migration entry now also covers reverse migration, which the newly-scheduled daily cron is what delivers. The deprecation entry moves to minor. Declined again: the literal-root `/??<bundle>` compatibility route, on its sixth outing. Serving those URLs means claiming a site-root path before authentication, which is the hazard this PR exists to remove, and the affected population is sites that set JETPACK_BOOST_STATIC_PREFIX to a non-default value of exactly `/`. It stays documented as a known limitation.
…rdering it depends on Three of these are comments that did not survive being checked against the code they describe. The forget helper's docblock said a 404 probe marker "is only ever a leftover" on Atomic and WP Cloud, because "no request can reach the handler that writes one". That is not true. loader.php's listener tests the request URI with str_contains(), not a prefix match, so /anything/wp-content/boost-cache/static/ testing_404.js is an ordinary 404 that no wp-content web server rule intercepts, reaches the handler, and plants a marker. The tester's own docblock a few functions down already says a marker "can equally be one left by an earlier request for the tester path", so the two were contradicting each other. The sentence is deleted rather than rewritten: the marker drop is justified by what the next probe would read, which is the sentence after it, and does not need this one. jetpack_boost_404_setup() said a placeholder verdict would leave Minify_Common::show_legacy_notice() "offering a super-admin a switch to the delivery method they are already on". The notice offers no switch - it is a title, a sentence and a Learn more link - and the placeholder is what puts the site on the slower method in the first place, so "already on" had it backwards. Same claim, same correction, in Concatenate_Static_Cache_Urls_Test. The changelog promised the faster delivery method back "within a day" of a site migrating off WP Cloud. wp_schedule_event() starts the daily event a full day out and jetpack_boost_404_tester_last_run travels with the database, so the first post-migration run can be throttled by a timestamp the previous host wrote. The PR's own known limitations already say ~48h. It now promises automatic re-testing and no interval. Two anchors, both mutation-verified: - The marker drop is not gated on the verdict, only on the host. Nothing pinned that: both existing marker tests carried a truthy verdict, so re-gating the drop on $verdict left the suites green. - handle_version_change() has to call the forget helper above jetpack_boost_minify_activation(). The comment argues one direction of that; the other is that activation re-runs the inline probe, so a forget call moved below it deletes a fresh measurement of the current host. That move also left both suites green. The two marker tests shared their fixture boilerplate, so it moves into a with_probe_marker() helper and the third case costs six lines. Also drops the paragraph about loader.php's hardcoded /wp-content/ prefix. It documents a pre-existing mismatch that is out of scope here and set out in full in the PR's known limitations.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes BOOST-608 by ensuring Boost’s “static cache URL” decision is host-aware (so a migrated database can’t force an incompatible delivery method on WP Cloud / Atomic), and by tightening the early minify router/prefix handling to avoid false positives and broken asset URLs.
Changes:
- Introduces host-gated helpers for static cache URL usage and centralizes verdict/marker cleanup (with guarded cache invalidation signaling).
- Updates concatenation URL selection (CSS/JS), the 404 tester/setup flows, and the early bootstrap router to use canonicalized prefixes and stricter request matching.
- Adds/adjusts unit + WorDBless coverage and test suite configuration to lock in migration, router, and regression behaviors.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| projects/plugins/boost/app/lib/minify/functions-helpers.php | Adds host predicate + URL decision helper; canonicalizes static prefix; introduces testable router predicate; deprecates unused legacy router. |
| projects/plugins/boost/app/lib/minify/functions-service.php | Centralizes “forget verdict” cleanup; guards 404 tester/setup by host and schedules tester on all hosts. |
| projects/plugins/boost/app/lib/minify/functions-service-fallback.php | Hardens fallback parser against empty payload edge case. |
| projects/plugins/boost/app/lib/minify/class-concatenate-css.php | Uses host-aware helper instead of raw site option for URL selection. |
| projects/plugins/boost/app/lib/minify/class-concatenate-js.php | Uses host-aware helper instead of raw site option for URL selection. |
| projects/plugins/boost/app/modules/optimizations/minify/class-minify-common.php | Suppresses legacy notice on hosts that can’t serve static cache URLs. |
| projects/plugins/boost/jetpack-boost.php | Routes early bootstrap decision through the new strict, canonicalized router predicate. |
| projects/plugins/boost/app/class-jetpack-boost.php | Routes version-change cleanup through the shared forget helper to preserve correct invalidation signaling + marker cleanup. |
| projects/plugins/boost/app/admin/class-config.php | Documents that extending hosting-provider detection impacts concatenation delivery behavior. |
| projects/plugins/boost/tests/php/lib/minify/Static_Cache_Urls_Test.php | New unit tests covering host predicate, setup/tester behaviors, prefix canonicalization, router matrix, and forget-helper gating. |
| projects/plugins/boost/tests/php/lib/minify/Concatenate_Static_Cache_Urls_Test.php | New WorDBless regression tests asserting emitted URLs and rebuild behavior end-to-end for CSS/JS. |
| projects/plugins/boost/tests/php/lib/minify/Functions_Service_Test.php | Stabilizes unit tests by mocking blog ID needed by Host’s per-blog cache. |
| projects/plugins/boost/tests/php/Jetpack_Boost_Test.php | Adds upgrade-path tests for verdict drop announcements and migrated marker cleanup behavior. |
| projects/plugins/boost/tests/bootstrap.php | Adds str_starts_with() polyfill for the unit suite (non-WordPress runtime). |
| projects/plugins/boost/phpunit.9.xml.dist | Moves new WorDBless test into the with-wordpress suite and excludes from unit suite. |
| projects/plugins/boost/phpunit.11.xml.dist | Moves new WorDBless test into the with-wordpress suite and excludes from unit suite. |
| projects/plugins/boost/changelog/fix-boost-608-static-prefix-router | Changelog entry for static prefix routing fixes when constant lacks slashes. |
| projects/plugins/boost/changelog/fix-boost-608-static-minification-after-migration | Changelog entry for migration-safe delivery method selection and re-test behavior. |
| projects/plugins/boost/changelog/fix-boost-608-root-static-prefix | Changelog entry for guarding against root-prefix collisions + requiring concat payload. |
| projects/plugins/boost/changelog/fix-boost-608-remove-dead-router | Changelog entry for deprecating the unused legacy router function. |
| projects/plugins/boost/changelog/fix-boost-608-legacy-notice-placeholder | Changelog entry preventing legacy notice until a real test has occurred. |
The comment-to-code ratio on this branch got out of hand: 223 comment lines across the production files and 348 across the tests, most of it arguing a point three different ways. Rewrite them shorter. Every load-bearing claim survives - why the host is asked before the option, why the forget helper deletes rather than zeroes, why the marker is dropped with the verdict, why the router requires the ?? payload, why the predicate is an allowlist. What goes is repetition, the caller list that a grep already answers, and rationale that lives in the PR description instead. Production comments 223 -> 124, test comments 348 -> 291. No code changes: both suites green (222/581 unit, 120/546 with-wordpress), and phpcs and phan report the same findings as the parent commit.
The docblock said Atomic and WP Cloud answer wp-content requests in the web server. Measured against the site from the BOOST-608 report, that is true of .css and .js only: a missing .txt under wp-content still reaches WordPress and gets a WordPress 404 page, while a missing .min.css gets a bare nginx 404. The extension is the whole mechanism. Boost emits .min.css and .min.js, which is exactly what nginx claims, so loader.php never runs and the bundle is never rebuilt. Say that, so the next reader can check the claim with one curl.
An audit against the code found seven statements in this branch's comments that are wrong or overstated. Each is verified against the source: - The host predicate is not an allowlist that catches unknown providers. get_hosting_provider() returns 'other' as its fallback, so a host it does not name keeps the static cache. Only a new named return value opts one out. - Whether a missing .css or .js reaches WordPress on Atomic and WP Cloud is a per-site platform setting, not a platform-wide behaviour. Measured both ways on two live sites. Boost cannot read the setting, which is the real reason it declines to guess. - The bootstrap router runs while WordPress loads plugins, not before WordPress loads. It calls wp_unslash(). What has not run is the query and the theme. - The forget helper's marker drop is gated on the host check, so it is not 'outright'. The tester reads the marker directly because routing through the helper would delete the verdict it is about to write. - handle_version_change() clears and recreates the tester event only while a minify module is on, not on every upgrade. - The marker drop lives in the helper, so the tester paths do reach it. What is unique about the version-change path is that a migrated site reaches it with both minify modules off. - The prefix predicate is a suffix match, so a prefix of 'assets' claims pages whose path ends in /assets/, not every page beneath it. Comments only. Both suites green (222/581 unit, 120/546 with-wordpress).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes BOOST-608
Proposed changes
Concatenate JS/CSS links bundles from
wp-content/boost-cache/static/or through the/_jb_static/??fallback. The static form needs a host where WordPress sees the 404 for a missingwp-contentfile. The tester stores that answer injetpack_boost_static_minification, a site option that rides with the database. Migrate a site onto Atomic or WP Cloud and it keeps the old host's1, so bundle URLs 404 at the web server and pages render with no CSS or JS.jetpack_boost_minify_use_static_cache_urls(), which checks the host before the option. That is the fix. The check allowlists'other' === get_hosting_provider(), since a wrongtruerestores the outage.0that the legacy notice read as a measurement.jetpack_boost_minify_request_is_for_static_prefix()guards the early router. It canonicalizesJETPACK_BOOST_STATIC_PREFIXas the concatenators do and wants a??payload, soassetsworks,/assets/stays a page, and/counts as unset. The unusedjetpack_boost_minify_serve_concatenated()warns through_deprecated_function().Known limitations
/wp-contentcannot use the static cache at all, becauseloader.phpmatches a hardcoded path. That matches trunk; it needs its own issue.jetpack_boost_404_tester_last_runtravels with the database too.otherhosts can act on a stale1until the tester corrects it./loses its cached bundles until those pages re-render. Serving those URLs claims a site-root path pre-auth, so no alias is added.Host::is_atomic_platform()needs bothATOMIC_SITE_IDandATOMIC_CLIENT_ID. If a WP Cloud tenant class defines neither, this fix is inert there. Confirm on a live site.Related product discussion/links
#reprint: p1785266572764519-slack-C0AB91NNQBG. Hit during a live Reprint migration; the workaround was clearing the option by hand.Does this pull request change what data or activity we track or use?
No. Boost now deletes
jetpack_boost_static_minificationon Atomic/WP Cloud instead of leaving it stale.Testing instructions
Run
jp test php plugins/boost. The three test files cover the emitted URL, the predicate and router matrix, and the upgrade path.By hand, make Boost believe the site is on WP Cloud with a mu-plugin:
wp option update jetpack_boost_static_minification 1./_jb_static/??…, not the/wp-content/boost-cache/static/form trunk emits and WP Cloud 404s.wp option get jetpack_boost_static_minificationmust report the option gone, not0. Both go off first: the sharedMinify_Commonsubmodule re-activates only on its last active parent.wp cron event run jetpack_boost_404_tester_cron. The option must still be gone.wp cron event list | grep 404_testermust still show the daily event.1gives static cache URLs,0gives/_jb_static/??.For a custom prefix, define
JETPACK_BOOST_STATIC_PREFIXasassets,/assets,assets/and/assets/in turn, with the option set to0, then request an emitted stylesheet URL. All four must return CSS; on trunk the two slashless spellings 404. A page at/assets/must still render, and with the prefix set to/, so must/wp-admin/.