Site Health: Cache the full results and a timestamp in the status transient

[gziolo]

Trac ticket: https://core.trac.wordpress.org/ticket/65232

Note: This PR changed substantially based on review. The detail cache now stores a compact, locale-independent summary in a separate, non-autoloaded transient. Earlier commits and discussion reflect earlier approaches.

What & why

Trac #65232 needs a fast, cache-only Site Health summary that does not re-run the synchronous tests. The existing health-check-site-status-result transient stores only aggregate good/recommended/critical counts, so consumers that need per-test results currently have no cached source.

This PR adds a second cache containing each test's status and collection timestamp. A follow-up can expose it through the Abilities API and resolve human-readable labels at read time in the current locale.

Design

Two caches with separate roles

• health-check-site-status-result keeps its existing { good, recommended, critical } shape. It remains small and autoloaded for the admin-menu counter and Dashboard widget.
• health-check-site-status-detail stores per-test results and timestamps. It has an expiration, so the potentially larger payload is not autoloaded on every request.

The legacy counts transient is retained intentionally. The admin menu and Dashboard read it directly on hot paths without loading WP_Site_Health; deriving those values from the larger, non-autoloaded detail transient would require an additional option read and JSON decode.

Stored detail shape

{
  "results": {
    "<test>": {
      "status": "good|recommended|critical",
      "timestamp": 1715714399
    }
  },
  "timestamp": 1715714399
}

Counts are not stored in this transient. get_site_status_detail() derives them from results, so the returned counts and detailed results are always internally consistent.

The cache-level timestamp records the latest successful cache update, including an update with no results. Each result also has its own timestamp so independently collected entries can expire after one month when, for example, a plugin that registered a test is deactivated.

Both the stored payload and each result are validated against strict JSON schemas.

Only locale-independent data is cached

Each entry stores only the test identifier, status, and timestamp. Labels, descriptions, actions, and badges are intentionally omitted because they may contain translated or HTML content and the cache is shared across users and locales. Consumers resolve labels at read time in the current locale.

Writers and freshness

• The Site Health screen posts aggregate counts and per-test results as independent Ajax requests. Results include asynchronous tests that run in the browser.
• The weekly scheduled check refreshes aggregate counts and stores every successfully verified result as the freshest detail.
• If cron cannot perform an asynchronous test, its synthetic recommended / “A test is unavailable” result is reflected in the legacy aggregate counts but does not overwrite a verified detailed result collected by the browser. The verified result remains until another verified result replaces it or it expires.

Invalid or incomplete test results are ignored by the scheduled check and rejected by the detail-cache updater.

Canonical accessors

• get_site_status_counts() / set_site_status_counts() read and write the small legacy counts transient.
• get_site_status_detail() returns { results, counts, timestamp }, with counts derived at runtime.
• update_site_status_detail( $results ) validates, timestamps, merges, and prunes detailed results.

Backward compatibility

The existing counts transient keeps its original name and shape, so menu.php, the Dashboard widget, and the localized SiteHealth data continue to work unchanged. No WP_Site_Health load is added to the admin-menu path.

Testing

• tests/phpunit/tests/admin/wpSiteHealth.php covers the counts/detail split, malformed filtered results, verified-result freshness, unavailable async fallbacks, per-result expiry, strict cache shapes, and derived counts.
• tests/phpunit/tests/ajax/wpAjaxHealthCheckSiteStatusResult.php covers independent counts-only and results-only requests, sanitization, validation errors, and capability enforcement.
• tests/qunit/wp-admin/js/site-health.js covers collecting valid direct-test results into SiteHealth.site_status.results while ignoring results without reliable identifiers.

PHPCS, PHPStan, the focused Site Health PHPUnit and Ajax suites, and QUnit pass locally.

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code (Claude Opus 4.8), Codex
Used for: Development and validation; reviewed and verified by me.

🤖 Generated with Claude Code

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props gziolo, westonruter, wildworks.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[westonruter]

In addition to my comment about letting the ajax-stored result be preferred, here's a critical observation that Claude made: the lack of an $expiration means that the transient will be autoloaded.

Since it now includes all of the results which could potentially be large, this would run the risk of increasing the bucket of autoloaded options beyond the 1MB limit imposed by Memcached, right?

Full review from Claude

Confirmed the critical detail: set_transient() is called with no expiration ($expiration = 0), so without an external object cache the option is stored with $autoload = true (option.php:1547-1552). The full results blob is therefore autoloaded on every page load site-wide, not just on Site Health screens. That makes the size concern much more severe than the finders estimated.

I have enough verified context. Here are the findings, ranked most-severe first.

Code Review: PR #12181 — Cache Site Health full results + timestamp

Findings

1. Full results blob is autoloaded on every page request (perf regression) — class-wp-site-health.php:3492
set_transient( 'health-check-site-status-result', wp_json_encode( $site_status ) ) is called with no expiration. With no external object cache, set_transient() stores the option with autoload = true (option.php:1547-1552). Previously the value was ~60 bytes (three ints); now it's the entire $results array including every test's HTML description, actions, and badge. That payload is now loaded into memory on every front-end and admin request for every visitor, via wp_load_alloptions() — a site-wide cost paid to store data only the Site Health screen reads (and only the 3 counts from it). Passing an expiration, or storing under a non-autoloaded option, would avoid this.

2. Large results payload can exceed object-cache item limits → counter silently reverts to 0 — class-wp-site-health.php:3489
On installs with a persistent object cache (e.g. Memcached, default 1MB item limit), a site with many site_status_tests (plugins add them, with arbitrary HTML) can produce a JSON blob exceeding the backend's per-item limit. wp_cache_set() then silently fails, get_transient() returns false, and the admin-menu/dashboard counters fall back to 0/0/0 even though valid counts existed. No size cap or truncation is applied.

3. Race between scheduled cron and the AJAX counts update can discard freshly collected results — src/wp-admin/includes/ajax-actions.php:5488
The AJAX handler does a non-atomic read-modify-write: it reads the cached transient, splices in the POSTed counts, and re-writes. If wp_cron_scheduled_check() writes fresh results between the handler's get_transient() and set_transient() (admin loads Site Health while cron runs), the handler overwrites with the older (or absent) results it read earlier — silently dropping the detailed results the ticket exists to preserve, until the next weekly cron.

4. New status guard miscounts unknown/empty statuses and diverges from results count — class-wp-site-health.php:3471
The guard if ( ! is_array( $result ) || ! isset( $result['status'] ) ) continue; only checks that status is set. A result with status => '' or a custom value like 'info' still falls through to the final else and increments good, mislabeling it as a passing check. Separately, a result missing status was previously counted as good (old loop had no guard) but is now skipped entirely — so the cached results array length (e.g. 3) can no longer equal good + recommended + critical, which will surprise any consumer that assumes they match.

5. timestamp freshness mechanism is half-built — class-wp-site-health.php:3490
The comment says the timestamp lets freshness "be evaluated," but: the transient is set with no expiry, nothing in core ever compares the timestamp to now, and the AJAX handler preserves the old cron timestamp on every counts update (ajax-actions.php:5495-5497). So the stored timestamp reflects the last full cron run, not the last update — any future freshness check built on it would be subtly wrong, and the promised mechanism doesn't actually exist yet.

6. Payload schema duplicated across writers/readers with no canonical accessor — src/wp-admin/includes/ajax-actions.php:5477
The count-normalization array( 'good' => (int) ( $x['good'] ?? 0 ), 'recommended' => …, 'critical' => … ) is copied verbatim into ajax-actions.php:5477-5481 and class-wp-site-health.php:126-130, and the get_transient() + json_decode( …, true ) idiom is duplicated too. The transient's shape is now defined by convention across five sites (cron writer, ajax merge, enqueue reader, menu.php, dashboard.php). Adding a status bucket or renaming a key requires editing several hand-maintained literals; missing one yields counts that silently disagree between the menu counter, dashboard widget, and Site Health screen. A single read/write/normalize helper would centralize it.

7. Enqueue decodes the entire results blob just to read three integers — class-wp-site-health.php:117
On every Site Health and Dashboard page render, json_decode() parses the full cached payload (all HTML results) only to extract good/recommended/critical — the comment itself notes "only the aggregate counts are needed." The decode cost scales with the size/number of test results, for no benefit. (Mitigated if findings 1 & 2 are addressed by not storing results in the same hot transient.)

8. AJAX handler re-encodes the full results blob on every counts POST — src/wp-admin/includes/ajax-actions.php:5501
Each time an admin loads the Site Health Status tab, the JS posts the counts and this handler reads, decodes, and re-wp_json_encode()s the entire results payload back into the transient — repeated large serialization writes per page view, not just per weekly cron run (write amplification).

---

Note on a dropped candidate: finders flagged wp_json_encode( $site_status ) returning false (and storing false) on invalid UTF-8 in result HTML. I refuted it — wp_json_encode() runs _wp_json_sanity_check()/_wp_json_convert_string() to repair invalid UTF-8 and re-encode (functions.php:4411-4417), so the common case recovers; only a >512-depth structure would fail, which Site Health results never reach.

Findings 1–4 are the ones I'd block on; finding 1 in particular looks like a meaningful site-wide performance regression worth raising on the PR. Want me to post these as inline PR comments (/code-review --comment) or open a discussion on the specific lines?

[gziolo]

Thanks for the review! Reworked this substantially in 5bb9fee and updated the PR description. Key change: counts stay in the small autoloaded transient, full results move to a separate non-autoloaded detail cache — which also clears the race and write-amplification points. The Site Health screen is now authoritative for the detailed results (incl. async tests) with cron as a backstop, plus per-result timestamps.

Two design calls I'd value your take on before going further: the merge policy (1-week grace window vs. browser strictly outranking cron) and whether to cap the detail-cache size. Both noted in the description.

Disclosure: I used Claude Code and Codex to develop and validate these changes (reviewed and verified by me). 🙏

[westonruter]

Key change: counts stay in the small autoloaded transient, full results move to a separate non-autoloaded detail cache — which also clears the race and write-amplification points.

Related to this, my suggestion to reduce the number of stored keys to just test and status (and timestamp) would also alleviate the concern about the size of the option getting too big.

[gziolo]

Thanks for the thorough review! I have applied all the feedback in the latest commits. A quick summary:

• Cache only locale-independent data. Each entry now stores just the test name, status, and timestamp. The translated fields (label, description, actions, badge) are left out, since this cache is one shared site-wide option and those strings vary by the user's locale. This also keeps the option small.
• Renamed the parameter to $includes_async and reframed the related docblock around carrying the async tests rather than being authoritative.
• Tightened the cached-result types and removed the dead status guard in the counts helper.
• Seeded the JS results array from PHP, so the client no longer initializes it, and the upload guard checks for collected results.

One open question remains: the merge policy when the scheduled check runs after a screen visit. Storing the freshest result is simplest, but cron runs the async tests over loopback, which fails on some hosts and falls back to "unavailable". One option is to let cron overwrite only when it could actually verify the test, instead of the current one-week grace window. Happy to go either way.

I also revised the PR description so it reflects the current state of the code.

[gziolo]

@westonruter, would you mind doing another review pass to unblock progress on this PR?

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

tests/phpunit/tests/ajax/wpAjaxHealthCheckSiteStatusResult.php:83

• The helper uses PHP's assert() for runtime validation. Assertions can be disabled via configuration, which would turn this into a hard-to-diagnose TypeError if json_decode() fails. Use PHPUnit assertions instead so failures are consistently reported by the test runner.

		$response = json_decode( $this->_last_response, true );
		assert( is_array( $response ) );
		return $response;

src/wp-admin/includes/class-wp-site-health.php:3639

• This docblock includes "@todo This method is now not used." but the method is part of the new public API surface in this PR and is exercised by the new tests. The TODO is non-actionable and misleading; please remove it or replace it with a concrete task/ticket reference.

	 * @since 7.1.0
	 * @todo This method is now not used.
	 *

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

tests/phpunit/tests/ajax/wpAjaxHealthCheckSiteStatusResult.php:83

• Avoid using PHP's assert() in PHPUnit tests. Assertions can be disabled at runtime (zend.assertions=0), which would remove this safety check and could lead to harder-to-debug failures. Use $this->assertIsArray() (or $this->assertNotNull() etc.) so the test always validates the decoded response shape.

		$response = json_decode( $this->_last_response, true );
		assert( is_array( $response ) );
		return $response;

src/wp-admin/includes/class-wp-site-health.php:3639

• The @todo This method is now not used. line is misleading here: get_site_status_detail() is exercised by multiple PHPUnit tests in this PR and is also documented as a canonical accessor in the PR description. If there is no concrete follow-up work item, please remove the @todo (or replace it with an actionable, ticket-referenced todo).

	 * @since 7.1.0
	 * @todo This method is now not used.
	 *

src/wp-admin/includes/ajax-actions.php:5482

• The new inline TODO: comment isn’t tied to an actionable follow-up (ticket/issue), and it also conflicts a bit with the stated design goal of keeping the lightweight counts transient for hot paths. Consider removing it or converting it into an @todo that references the relevant ticket.

	// TODO: We don't need to keep this anymore. We can just obtain the counts at runtime from the stored results.

[gziolo]

Quick update on the review follow-ups completed this week:

• The detailed cache now stores only locale-independent per-test statuses and timestamps; counts are derived at runtime.
• Cron stores the freshest verified results, while synthetic “test unavailable” results affect aggregate counts without replacing verified browser results.
• JavaScript result collection was fixed to capture valid browser-run tests and ignore results without reliable identifiers.
• Quality improvements include stricter validation, clearer error handling and documentation, and expanded PHPUnit and QUnit coverage.

The remaining work is to explore using a single cache where aggregate counts are derived from the detailed results. This needs to account for the existing counts transient being small and autoloaded for the admin-menu and Dashboard hot paths, while the detailed transient is intentionally non-autoloaded.