Skip to content

Conversation

@westonruter
Copy link
Member

@westonruter westonruter commented Feb 3, 2026

This improves the robustness of page cache detection in Site Health by refining the response header validation and expanding support for common caching layers.

Logic & Header Improvements

  • Stricter Cache Hit Matching: The $cache_hit_callback now uses a word-boundary regular expression (/\bhit\b/i) instead of a simple substring check. This prevents false positives from header values like shit.
  • X-SRCache-Store-Status Correction: Fixed the validation for this header to check for the correct value STORE (the module's indicator for a successful cache storage) instead of HIT.
  • Varnish Support: Added detection for the X-Varnish header, specifically identifying a hit when the header contains two request IDs (e.g., 123 456), as documented in the Varnish FAQ.
  • Standard Headers: Restored Last-Modified, ETag, and Via as valid indicators of a caching layer, while ensuring they are handled as existence-based detections or validated where appropriate.
  • Documentation: Added detailed docblocks and source links for caching headers (e.g., srcache, Varnish) and explicitly listed supported generic proxies (Squid, Go, Fastly, LiteSpeed) in comments.

PHPUnit Test Enhancements

  • Comprehensive Test Cases: Added new test scenarios to Tests_Admin_wpSiteHealth to verify:
    • X-Varnish hits vs. misses.
    • X-SRCache-Store-Status (STORE) and X-SRCache-Fetch-Status (HIT).
    • Prevention of false positives from substrings (e.g., x-cache: shit).
    • Robustness of standard headers like ETag, Last-Modified, and Via.
  • Improved Typing: Updated the test suite with strict parameter and return types for data providers and test methods, aligning with modern PHP standards and resolving PHPStan reported issues.

Code Quality

  • Applied consistent formatting to class-wp-site-health.php and the test suite using composer format to ensure adherence to WordPress Coding Standards.

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

Use of AI Tooling

The above PR description was drafted by Gemini, and Gemini CLI was involved in authoring some commits (as attributed via co-authored-by).


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

westonruter and others added 4 commits February 3, 2026 09:27
This commit updates the page cache detection in Site Health by:
- Using a stricter regex (\bhit\b) for detecting cache hits in response headers, preventing false positives (e.g., 'shit').
- Adding support for the X-Varnish header, identifying a cache hit when it contains two request IDs.
- Adding documentation and source links for the supported headers.
- Adding PHPUnit test cases to verify the new detection logic.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
The x-srcache-store-status header uses 'STORE' or 'BYPASS', not 'HIT' or 'MISS'. This update ensures that 'STORE' is correctly identified as a positive page cache indicator.

Also includes:
- Documentation for srcache headers.
- Explicit list of generic caching proxies (Squid, Go, Fastly, LiteSpeed) in comments.
- Test cases for x-srcache-store-status and x-srcache-fetch-status.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@github-actions
Copy link

github-actions bot commented Feb 3, 2026

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 westonruter.

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

@westonruter
Copy link
Member Author

Gemini review:

I have completed the review of the changes committed to the trac-64370 branch.

Commit Context

The changes aim to improve the robustness of page cache detection in WordPress Site Health. Key improvements include:

  • Using stricter regex (\bhit\b) to prevent false positives from header values like "shit".
  • Adding support for the X-Varnish header by detecting the specific hit format (two request IDs).
  • Correcting x-srcache-store-status detection to look for STORE instead of HIT.
  • Improving documentation and PHPStan typing for the detection methods and tests.

Code Review

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

  • Logic Improvement: The move from str_contains to preg_match( '/\bhit\b/i', ... ) is an excellent fix for substring-related false positives.
  • Varnish Header: The x-varnish callback correctly implements the detection of a hit by checking for two space-separated digits, matching Varnish's debugging header behavior.
  • SRCache Correction: Updating x-srcache-store-status to check for STORE is correct as per the module's documentation.
  • Coding Standards:
    • The use of array shape return types (array<string, ?callable>) in docblocks aligns with current project goals for PHPStan level 10 compatibility.
    • The addition of the : array return type hint is compatible with PHP 7.4.
  • Nitpick: The via header is restored but set to null (existence check). While standard, many proxies use Via without caching. However, given the context of other headers, this is a reasonable heuristic for WordPress core.

tests/phpunit/tests/admin/wpSiteHealth.php

  • Type Safety: The addition of strict types to properties and method signatures (private WP_Site_Health $instance;, test_get_page_cache( array $responses, ... )) is a great improvement for the test suite's reliability.
  • PHPStan Resolution: The return type for data_get_page_cache has been meticulously defined to match the complex array structure returned, which should resolve the reported PHPStan errors.
  • Test Coverage:
    • New cases for false-positive-hit-in-word, varnish-header, and srcache variants provide thorough coverage for the logic changes.
    • The last-modified test case uses a realistic date string, which is better than the previous "hit" value.
  • Consistency: The change from null to false for good_basic_auth in the data provider, coupled with the signature change to bool, ensures type consistency.

Summary

The changes are high quality, follow WordPress coding standards, and significantly improve the reliability of Site Health's page cache detection. No regressions or standards violations were identified.

Recommendation: Ready for merge.

@github-actions
Copy link

github-actions bot commented Feb 3, 2026

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

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • 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 westonruter requested a review from dmsnell February 3, 2026 18:59
'x-cache-disabled' => static function ( $header_value ) {
return ( 'on' !== strtolower( $header_value ) );
},
'x-srcache-store-status' => $cache_hit_callback,
Copy link
Member Author

Choose a reason for hiding this comment

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

This was incorrect. Now fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant