Skip to content

fix(rucss): show admin notice when Used-CSS subfolder has no write permissions#8323

Open
Miraeld wants to merge 2 commits into
developfrom
fix/8268-show-admin-notice-or
Open

fix(rucss): show admin notice when Used-CSS subfolder has no write permissions#8323
Miraeld wants to merge 2 commits into
developfrom
fix/8268-show-admin-notice-or

Conversation

@Miraeld

@Miraeld Miraeld commented May 19, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #8268

When the Used-CSS feature creates a per-URL subfolder (e.g. wp-content/cache/used-css/example.com/path/) but that subfolder has incorrect write permissions, WP Rocket was silently failing — the CSS was not saved and no feedback was given to the admin. This PR detects that silent failure and surfaces it as an admin notice pointing the admin to the specific subfolder, consistent with the existing notice already shown when the base Used-CSS folder is not writable.

Type of change

Detailed scenario

What was tested

Automated (unit tests — 9 new tests, all passing):

  1. getNotWritableSubfolder — when no transient is set → returns empty string; when transient is set → returns path and deletes the transient.
  2. writeUsedCssSubfolderPermission — when write_file() fails and subfolder is not writable → transient is set; when write_file() fails but subfolder is writable → no transient set.
  3. noticeWritePermissions — covers all 5 branches: insufficient capability, RUCSS disabled, base folder not writable (existing notice), subfolder transient empty (no notice), subfolder transient set (new notice).

Full RUCSS unit suite: 64/64 pass — no regressions.
PHPCS: exit 0 on all changed files.

How to test

Prerequisites:

  • WP Rocket installed and activated
  • Used CSS (Remove Unused CSS) feature enabled in WP Rocket settings

Steps to reproduce the bug (before this fix) / validate the fix:

  1. Locate the Used-CSS cache directory: wp-content/cache/used-css/<host>/
  2. Create a subfolder for a URL you are about to visit, e.g. wp-content/cache/used-css/example.com/some-path/
  3. Remove write permissions on that subfolder: chmod 555 wp-content/cache/used-css/example.com/some-path/
  4. Trigger RUCSS processing for that URL (visit it, or use "Preload Cache" to enqueue the job, then run wp action-scheduler run)
  5. Go to WordPress Admin → any page

Expected result (with this fix): An admin notice is displayed identifying the non-writable subfolder and asking the admin to fix the permissions.

Expected result (without this fix / regression check): No notice is shown — silent failure.

Verify base-folder notice still works:

  1. Remove write permissions on the base folder: chmod 555 wp-content/cache/used-css/
  2. Trigger RUCSS and visit wp-admin
  3. Confirm the existing base-folder notice still appears correctly.

Affected Features & Quality Assurance Scope

  • Remove Unused CSS (RUCSS / Used-CSS) — core feature impacted. Specifically the file-write pipeline and the admin notices system for permission errors.
  • No impact on: cache, JS delay, lazy load, CDN, critical CSS, or any other WP Rocket feature.

Technical description

Documentation

Root cause: Filesystem::write_used_css() creates a per-URL subfolder via mkdir, then writes the gzipped CSS via WP_Filesystem::write_file(). If write_file() returns false (which happens when the subfolder is not writable), the method returned false silently. The existing notice_write_permissions() only checked the base folder (/used-css/), so the subfolder failure was invisible.

Solution — transient bridge:

The write failure occurs inside an Action Scheduler background job. The admin notice fires on admin_notices during a regular page load. A WordPress transient bridges these two async contexts (consistent with rocket_saas_processing used elsewhere in RUCSS).

Filesystem::write_used_css() (changed):

mkdir → gzencode → write_file
  └─ if write_file returns false:
       check is_writable($subdir)
         └─ if not writable: set_transient('rocket_rucss_subfolder_not_writable', $relative_path, HOUR_IN_SECONDS)
       return false

Filesystem::get_not_writable_subfolder() (new):

  • Reads and immediately deletes the transient (consumed once per failure).
  • Returns empty string if no transient is set.

UsedCSS::notice_write_permissions() (changed):

check user capability → check RUCSS enabled
  → check base folder writable? (no → existing notice, return)
  → get_not_writable_subfolder()
      → empty → return (no notice)
      → non-empty → show new notice with subfolder path

The notice message mirrors the existing base-folder notice, replacing the folder path with the specific subfolder path.

New dependencies

None.

Risks

  • Transient orphaning: If the background job fires but the admin never visits the backend, the transient expires after 1 hour — no permanent side-effect.
  • False negatives: If write fails for reasons other than permissions (e.g. disk full), is_writable() will return true and no transient is set — the failure remains silent for those edge cases, which is the same behavior as before this fix.
  • No performance risk: get_transient is a single option read, called only on admin_notices (admin page loads only).

Mandatory Checklist

Code validation

  • I validated all the Acceptance Criteria. If possible, provide screenshots or videos.
  • I triggered all changed lines of code at least once without new errors/warnings/notices.
  • I implemented built-in tests to cover the new/changed code.

Code style

  • I wrote a self-explanatory code about what it does.
  • I protected entry points against unexpected inputs.
  • I did not introduce unnecessary complexity.
  • Output messages (errors, notices, logs) are explicit enough for users to understand the issue and are actionnable.

Unticked items justification

N/A — all mandatory items are ticked.

Additional Checks

  • In the case of complex code, I wrote comments to explain it.
  • When possible, I prepared ways to observe the implemented system (logs, data, etc.).
  • I added error handling logic when using functions that could throw errors (HTTP/API request, filesystem, etc.)

Miraeld and others added 2 commits May 19, 2026 13:50
Adds full .aiassistant/ setup with issue workflow, QA engineer agent,
knowledge graph builder (880 PHP/JS nodes indexed), architecture and
compliance skills, and PHPCS specs. Adapted for wp-rocket: single
edition, WP_Rocket\ namespace, Subscriber/ServiceProvider/Container
patterns, wpm_apply_filters_typed enforcement, fix/enhancement/test
branch prefixes, and TDD testing guide.
…rmissions

When a Used-CSS subfolder is created but has incorrect write permissions,
write_used_css() now detects the permission failure, stores the subfolder
path in a transient, and notice_write_permissions() surfaces it as an
admin notice — consistent with how the base-folder write-permission notice
already works.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 duplication

Metric Results
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@Miraeld

Miraeld commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

QA Report — PR #8323 / Issue #8268

Validation Strategy

Code-analysis QA (no live browser testing required — this is a background job + admin notice flow). Reviewed all five changed/added files, then ran the new unit tests in isolation and the full RUCSS unit suite, and ran PHPCS on the two changed source files.


Test Results

New tests only (9 tests):

.........   9 / 9 (100%)
OK (9 tests, 32 assertions)   Exit 0

Full RUCSS unit suite (64 tests):

Tests: 64, Assertions: 196, Risky: 1   Exit 0

The single "risky" test is a pre-existing issue in Test_DisplayNoTableNotice (vfsStream PHP 8.4 deprecation notices printed to stdout) — unrelated to this PR.

PHPCS on changed source files:

Exit 0  (no violations)

Code Review Findings

Filesystem::write_used_css()

  • The permission check (is_writable) is correctly placed after write_file() fails, so no transient is set when the write succeeds. ✅
  • The path stored in the transient strips ABSPATH and trims slashes, matching the same pattern used for the base-folder notice. ✅
  • 1-hour TTL is reasonable; the transient is consumed (deleted) immediately on the first read, acting as a "flash message". ✅

Filesystem::get_not_writable_subfolder()

  • Clean read-and-delete pattern; returns '' on cache miss (strict false check). ✅
  • Return type hint string is correct. ✅

UsedCSS::notice_write_permissions()

  • Guard clauses (capability + feature enabled) preserved intact. ✅
  • Base-folder branch still early-returns before touching the transient — no regression. ✅
  • Subfolder branch calls rocket_notice_writing_permissions($subfolder) with the relative path, identical API to the base-folder call. ✅
  • if ( '' === $subfolder ) uses strict comparison — correct. ✅

Minor observation (non-blocking):
The base-folder test (testShouldShowBasePathNoticeWhenBaseFolderNotWritable) asserts rocket_notice_writing_permissions is called ->once() but does not assert the exact path argument. The subfolder test does assert the exact path ('wp-content/wpr-usedcss/1/a/b'). Both cases work correctly at runtime; the base-folder assertion could be tightened in a future pass but is not a defect.


Acceptance Criteria Validation

  • Admin notice shown when subfolder is not writablewrite_used_css() sets a transient when is_writable($subdir) is false; notice_write_permissions() reads it and calls rocket_notice_html() with error status.
  • Existing base-folder notice unaffected — base-folder path early-returns before any subfolder logic; existing behaviour preserved.
  • Tests cover new behaviour — 2 tests for get_not_writable_subfolder, 2 for the new write_used_css branch, 5 for all notice_write_permissions paths (including the new subfolder case).
  • PHPCS passes — exit 0 on both changed files.

Verdict

PASS ✅

The implementation is correct, complete, and consistent with existing RUCSS patterns. All 9 new tests pass, the full RUCSS suite (64 tests) is green, and PHPCS is clean. The acceptance criteria from issue #8268 are fully satisfied.


Notes

  • The transient is consumed on the first read, so if the admin does not load the dashboard within the 1-hour window after a failed write, the notice will be missed until the next failed write. This is acceptable behaviour for a permissions warning (and consistent with other WP transient-based notices).
  • The apply_filters() calls visible in UsedCSS.php are all pre-existing code, not introduced by this PR.

QA performed by GitHub Copilot CLI — code analysis only.

@Miraeld Miraeld marked this pull request as ready for review May 19, 2026 23:48
@Miraeld Miraeld self-assigned this May 20, 2026
@remyperona

Copy link
Copy Markdown
Contributor

In this one I don't think the solution, while it works, is the best one.

It should modify the RUCSS\Controller\UsedCSS::notice_write_permissions() to check for permissions on the specific folder associated with the site_id, instead of only the parent used_css.

That way the check is done earlier than when running RUCSS jobs, and it doesn't need to use a transient (which can expire) to show the issue.

Base automatically changed from chore/add-ai-assistant to develop June 2, 2026 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show admin notice or log message when Used-CSS subfolders do not have writing permissions

2 participants