Spec audit: turn specification.website findings into suggested tasks - #767
Draft
ilicfilip wants to merge 20 commits into
Draft
Spec audit: turn specification.website findings into suggested tasks#767ilicfilip wants to merge 20 commits into
ilicfilip wants to merge 20 commits into
Conversation
Introduce the audit layer that checks a site against specification.website. Defines the swappable Audit_Source contract + normalized finding schema (Audit_Runner), five deterministic PHP checks (doctype, lang, charset, robots.txt, sitemap) behind a filterable registry, a Local source that merges PHP checks with an optional AI pass (PHP wins on overlap), a Remote SaaS source stub for the future server-side engine, and Spec_Mcp_Client which drives WP 7.0's core AI client. Note: WP 7.0's AI client cannot act as an MCP client, so Spec_Mcp_Client feeds the spec checklist + HTML to wp_ai_client_prompt() instead. The whole AI path is guarded by is_available() and degrades to PHP-only checks; WP7-specific calls are marked TODO(wp7-verify) as they can't be exercised without a live WP 7.0 connector. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add the Spec_Audit data collector (caches the audit; runs the expensive checks/LLM only on cache refresh, never on admin_init) and the Spec_Audit task provider that turns failing findings into recommendations. The provider releases at most one task per window (default daily), overridable via progress_planner_spec_audit_max_tasks_per_window and _window filters; each failing rule maps to one durable task and is auto-completed when a re-audit shows it passing. Register both in their managers, and add a `wp prpl audit run` CLI command plus a progress_planner_run_spec_audit AJAX trigger for on-demand runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cover the deterministic PHP checks, Audit_Runner schema normalization/dedup, graceful degradation to PHP-only findings when the AI layer is unavailable, the per-window injection throttle and per-rule completion, and a shape-equality test asserting the local and remote sources produce identical finding shapes (the guard that keeps the phase-C and phase-B engines interchangeable). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Verified on a live WP 7.0 install: the AI builder (WP_AI_Client_Prompt_Builder) delegates SDK methods through __call, so the earlier method_exists() guards on using_max_tokens/as_json_response/is_supported_for_text_generation silently skipped those calls. Call them directly and gate availability on wp_supports_ai() plus is_supported_for_text_generation(). Confirmed end-to-end via `wp prpl audit run`: detects failing rules, injects one task, and the throttle blocks the second run; the AI path correctly reports unavailable when no provider is configured and degrades to PHP-only checks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the inject-time recheck I added in ba74aa5 called wp_remote_get from get_tasks_to_inject(), which runs on every admin_init. Each admin pageview then triggered 1+ loopback HTTP requests back into the same PHP-FPM pool serving the user — pinning workers and starving the whole pool until nginx returned 502s for unrelated Valet sites. Three structural changes prevent that class of bug entirely: 1. The Spec_Audit data collector's update_cache() is a no-op unless an explicit caller has opted in via Spec_Audit_Data_Collector::with_explicit_refresh(). The Data_Collector_Manager's admin_init sweep therefore cannot trigger the audit. Sanctioned callers: CLI command, cron hook, AJAX shutdown. 2. collect() never falls back to calculate_data() on cache miss — a missing cache returns []. is_specific_task_completed() now distinguishes "no cache" from "rule passed", so an object-cache flush can't mass-complete every audit task. 3. The AJAX "run now" handler defers the audit to shutdown and calls fastcgi_finish_request() so the user's worker is released to the pool before the outbound HTTP starts. A daily wp-cron hook also drives refreshes from a non-web context. Reverted is_still_failing()'s live recheck — it's the wrong place for HTTP. Kept the deferred throttle counter (pure-PHP, no HTTP). Updated tests: dropped the live-recheck test; added tests for the cache-empty completion guard and the no-explicit-refresh no-op. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The robots.txt check turned out to be the wrong rule for a suggested task: WordPress generates a virtual robots.txt automatically, and when it fails (e.g. status 404 with a real body, as seen on a Yoast + Woo install), the fix is at the nginx/Valet routing level — not something a WordPress user can address from wp-admin. Suggested tasks should be actionable inside WordPress. Replace with a meta-description check that operates on the homepage HTML we already fetch (zero extra outbound HTTP). The user-facing fix is real and in-scope: install/configure an SEO plugin or set a description in the theme. Yoast/RankMath both supply it out of the box, so the recommendation naturally guides users to a known good path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tasks are keyed by rule_id. When we rename, retire, or filter out a check (as just happened with robots-txt), the old task lives on in users' DBs forever — its completion logic only knew how to react to the rule still being audited. Two changes: 1. On injection, persist the finding's source as prpl_source meta so the provider can later tell deterministic (php-check) tasks from probabilistic (mcp-llm / saas) ones. Legacy tasks without the meta are backfilled to 'php-check' (the original starter set was all PHP checks), so existing installs self-heal on upgrade too. 2. In is_specific_task_completed(), short-circuit to "complete" for any php-check task whose rule_id is no longer present in the live Checks_Registry. Does NOT apply to LLM/SaaS tasks — their rule space is open-ended and a rule missing from one audit just means the model didn't mention it this run, not that it was retired. Pure in-memory work; no outbound HTTP. The existing cache-empty guard in is_specific_task_completed() still prevents mass-completion from an object-cache flush. Tests use synthetic rule IDs (rule-a etc.) that the registry never knew about, so the test setUp now registers no-op stub checks for them; the new retired-rule tests remove all audit_checks filters to simulate removal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three bugs blocked the AI path on a live WP 7.0 + Anthropic connector, found via foreground reflection probing rather than running the audit live: 1. JSON schema rejected. Anthropic's structured-output API requires `additionalProperties:false` on every `type:object`. Without it the call 400s and the WP_Error is swallowed, leaving no diagnostic. 2. Checklist URL was wrong. /mcp/ returns the HTML page describing the MCP server, not spec content — the model was fed 22KB of irrelevant HTML. Switched to /llms.txt, the canonical LLM-oriented Markdown index (~37KB) the spec publishes for exactly this purpose. 3. Errors were silent. run_prompt() returned null on any failure, and audit_url() caught Throwables and json_decode failures the same way. Added log_error() that writes to error_log under WP_DEBUG so future failures show up in debug.log without changing the public contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the architecture, decisions, what's verified vs not, open questions, and how to test. The top priority follow-up is using spec.website's canonical slugs as rule_ids so PHP and LLM findings dedupe naturally and doc_urls point at real spec pages — written up in detail at the bottom. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PHP checks were inventing slugs (html-doctype, html-lang-attribute,
charset-meta, xml-sitemap) and the LLM was inventing its own (doctype,
html-lang, meta-charset, etc.). They covered the same rules with
different identifiers, so the "PHP wins on overlap" dedupe never fired
and doc_urls were generic /specification.website/ pointers.
Adopt the spec's own URL slugs as canonical rule_ids:
html-doctype → doctype (/spec/foundations/doctype/)
html-lang-attribute → html-lang (/spec/foundations/html-lang/)
charset-meta → meta-charset (/spec/foundations/meta-charset/)
meta-description → meta-description (/spec/foundations/meta-description/)
xml-sitemap → xml-sitemaps (/spec/seo/xml-sitemaps/)
Also align categories to the spec ('foundations' for the HTML-baseline
checks, 'seo' for sitemaps). Each finding's doc_url now points at the
actual spec page so the "Why is this important?" link is genuinely
useful.
Update the AI prompt to instruct the model to use canonical slugs derived
from the spec URL pattern, with concrete examples. PHP and LLM findings
will now dedupe naturally where they cover the same rule.
Existing tasks with the old rule_ids will be auto-completed by the
self-heal logic (commit bbfaf44) on the next admin pageload — no manual
migration needed. Drop the canonical-slug TODO from the handoff doc and
renumber remaining open questions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Test on Playground |
Contributor
✅ Code Coverage Report
🎉 Great job maintaining/improving code coverage! 📊 File-level Coverage Changes (18 files)🆕 New Files
📈 Coverage Improved
📉 Coverage Decreased
ℹ️ About this report
|
is_specific_task_completed() treated "rule no longer reported in a populated audit" as completion for any source. Because the mcp-llm engine is non-deterministic, a rule simply being absent from a later audit run made its task self-complete even though the user fixed nothing — contradicting the documented design (only php-check tasks should complete on rule-absence; LLM/SaaS tasks complete only on an explicit pass). Guard the rule-absence branch on the php-check source. Add a regression test asserting an mcp-llm task is not completed on omission but is completed on an explicit pass, and clarify the existing php-check test. Update the handoff doc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
run_audit_now() returned only the tasks its own get_tasks_to_inject() call created. When the bootstrap inject_tasks() sweep already consumed the daily throttle slot earlier in the same request, the CLI command reported "0 task(s) injected" even though a task had been injected. Return pending_release_ids (all tasks injected this request) so the count reflects reality. Also fix the handoff's clean-state snippet: it iterated the cached get_tasks_by() result while delete_recommendation() flushed that cache group mid-loop, skipping tasks and leaving survivors. Snapshot the IDs with a raw get_posts() + provider tax_query instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two independent upstream breakages stack in this job: 1. `wp package install` fails with "Your github oauth token for github.com contains invalid characters". The stable WP-CLI phar bundles an old Composer that rejects the current GitHub Actions token format (hyphens) — composer#12076. setup-php exports the token via COMPOSER_AUTH, so pinning setup-php or clearing GITHUB_TOKEN does not help (both were tried in #766 and failed). The nightly phar bundles a Composer with relaxed validation, which fixed this step in #766. 2. The plugin check step then failed with "Environment not initialized. Run `wp-env start` first." plugin-check-action generated a .wp-env.json that loaded plugin-check from a download URL; on newer runner images wp-env exits 0 without initializing. #766 worked around it by pinning @wordpress/env to 11.5.0, which did not work and left that PR blocked. Upstream fixed (2) in plugin-check-action v1.1.7, so no @wordpress/env pin is needed — pinning to 11.5.0 against a fixed action would be counterproductive. v1.1.9 also picks up the v1.1.8 bundle-regression fix. Ref: WordPress/plugin-check-action#590 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… WP-CLI
Reverts the WP-CLI nightly step added in the previous commit and documents
why the surrounding pins exist.
The nightly was a May workaround for Composer rejecting the GitHub Actions
token ("contains invalid characters", composer#12076). Two things changed:
- composer#12076 was fixed in Composer 2.10.0 (2026-05-28, one day after
that debugging session) and setup-php now ships 2.10.2, so the stable
WP-CLI phar no longer hits the token error. Confirmed on CI run
31781466704: Composer reached dependency resolution, no token error.
- The nightly has since become wp-cli 3.0.0-alpha, and every released
dist-archive-command requires wp-cli ^2 / ^2.13, so `wp package install`
can no longer resolve against it. The nightly is now a dead end.
dist-archive-command stays on v3.1.0: it is the newest release accepting
wp-cli ^2. v3.2.x needs ^2.13, which has no stable release (latest is
2.12.0).
The remaining real fix is the action bump. v1.1.7 fixed the wp-env
silent-startup failure ("Environment not initialized") that blocked #766;
v1.1.9 picks up the v1.1.8 bundle-regression fix. Since that landed
upstream, the @wordpress/env 11.5.0 pin from #766 is not carried over.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
🔍 WordPress Plugin Check Report
📊 Report
|
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
518 |
Squiz.PHP.DiscouragedFunctions.Discouraged | The use of function set_time_limit() is discouraged |
📁 classes/suggested-tasks/providers/class-content-review.php (4 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
232 |
WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information. |
377 |
WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information. |
381 |
WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information. |
388 |
WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information. |
📁 classes/suggested-tasks/data-collector/class-unpublished-content.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
103 |
WordPressVIPMinimum.Performance.WPQueryParams.PostNotIn_post__not_in | Using exclusionary parameters, like post__not_in, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information. |
📁 classes/activities/class-query.php (2 warnings)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
71 |
PluginCheck.Security.DirectDB.UnescapedDBParameter | Unescaped parameter $table_name used in $wpdb->query()\n$table_name assigned unsafely at line 58. |
163 |
PluginCheck.Security.DirectDB.UnescapedDBParameter | Unescaped parameter $where_args used in $wpdb->get_results()\n$where_args assigned unsafely at line 153. |
📁 classes/suggested-tasks/data-collector/class-yoast-orphaned-content.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
111 |
PluginCheck.Security.DirectDB.UnescapedDBParameter | Unescaped parameter $query used in $wpdb->get_row()\n$query assigned unsafely at line 98. |
📁 classes/suggested-tasks/data-collector/class-terms-without-posts.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
120 |
PluginCheck.Security.DirectDB.UnescapedDBParameter | Unescaped parameter $query used in $wpdb->get_results()\n$query assigned unsafely at line 118. |
📁 classes/suggested-tasks/data-collector/class-terms-without-description.php (1 warning)
| 📍 Line | 🔖 Check | 💬 Message |
|---|---|---|
108 |
PluginCheck.Security.DirectDB.UnescapedDBParameter | Unescaped parameter $query used in $wpdb->get_results()\n$query assigned unsafely at line 106. |
🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check
Tested against the WP 7.1 RC ahead of its release. Clears the Plugin Check outdated_tested_upto_header error (readme declared 6.9). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Security workflow (symfonycorp/security-checker-action) failed on squizlabs/php_codesniffer 3.13.5: CVE-2026-67434, HIGH, OS command injection in the gitblame report via a crafted filename. Patched in 3.13.6. GHSA-hmqg-cxww-wqhq Dev-only, transitive (pulled by wpcs/yoastcs/phpcsextra et al). Every constraint already allows ^3.13.x, so this is a lockfile-only bump with no composer.json change. Composer also refreshed the packages locked alongside it: phpcodesniffer-composer-installer v1.2.1, phpcsutils 1.2.3, phpcsextra 1.5.1, wpcs 3.4.1. `composer check-cs` passes on the new set. Note this does not touch the 3 open composer/composer advisories (CVE-2026-59946/59947/59948) — a separate package, not flagged by this check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Composer package changes
|
…POSER_AUTH The "Your github oauth token for github.com contains invalid characters" failure is INTERMITTENT, not fixed. Proof: run 31782281682 (df71e84) passed, and a re-run of that identical commit minutes later failed at the same step with the same error. Composer 2.10.2 and wp-cli 2.12.0 in both runs, so the Composer version was never the deciding factor — whether the ephemeral token GitHub mints for a given run contains a rejected character is. This supersedes the claim in c96d1ca that composer#12076 being fixed in 2.10.0 made the stable phar safe. It did not; that run was luck. dist-archive-command is a public package, so this install needs no auth at all. Clear BOTH token sources: wp-cli reads GITHUB_TOKEN directly, and setup-php separately exports the same token via COMPOSER_AUTH. b0ec35d cleared only GITHUB_TOKEN, leaving COMPOSER_AUTH to reintroduce it — which is why that attempt failed and why single-variable fixes look like they work until the next unlucky token. Uses `unset` rather than env: '' because wp-cli writes the token whenever getenv() returns a string, and an empty string qualifies. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The unset-only fix in 3f52e7a was insufficient: run 31783342116 shows `unset GITHUB_TOKEN COMPOSER_AUTH` executing and the token error still occurring. So the credential is not only in the environment — setup-php also persists it into Composer's global auth config on the runner, which no env change can remove. Remove the stored credential (global github-oauth entry + auth.json, incl. wp-cli's own package home) before unsetting the env vars, so nothing can rewrite it. Also logs which auth locations are populated, so the next run identifies the real source instead of another guess. Still intermittent by nature: it only fails when the run's ephemeral token happens to contain a rejected character, so a single green run proves nothing here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Workflow: replace the diagnostic block with the one line that actually matters. Run 31784727273 proved GITHUB_TOKEN and COMPOSER_AUTH are already empty at that point and the rejected credential lives in ~/.composer/auth.json, so deleting that file is the whole fix — the unset and the extra rm targets were noise from narrowing the cause. Comments now state the verified mechanism rather than the intermediate guesses. `rm -f` exits 0 on a missing path, so no `|| true` is needed under `bash -e`. HANDOFF-spec-audit.md: purpose served (it says "Not pushed. No PR." while the branch is pushed with PR #767 open), and .distignore lists docs individually, so it was shipping inside the built plugin zip — Plugin Check was scanning it. Full content stays in git history from deb03b1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Adds a website-spec audit that runs against the site's public URL and turns each failing rule into a Progress Planner suggested task, throttled to 1/day. Two engines share all plugin-side task-mapping code:
https://specification.website/llms.txt. PHP wins on overlapping rule_ids.Designed so the audit engine can later move from the plugin to the progressplanner.com SaaS without touching task-creation code (phase B —
Remote_Audit_Sourceis a stub today, a regression test guards the contract).Key design points
fastcgi_finish_request()), or a dedicated cron hook. The data collector'supdate_cache()is a no-op unless an explicit caller has opted in. This caused FPM pool starvation during development and is now structurally prevented.prpl_source. If a PHP-check task'srule_idis no longer in the live registry, it auto-completes — so a future rule rename/retire doesn't strand orphan tasks. Legacy tasks without source meta backfill tophp-check.shutdownfor tasks that actually survived the request (so a same-request auto-completion doesn't burn the slot).doctype,html-lang,meta-charset,meta-description,xml-sitemaps). Each finding'sdoc_urlpoints at the real spec page.Full architecture, decisions, and open questions are in
HANDOFF-spec-audit.mdon this branch.Verified live
On a local WP 7.0 + Yoast + Woo + Anthropic Connector site (
planner.test):wp prpl audit runproduces ~5 PHP findings + ~10 LLM findings in ~18s.Not yet verified
[]untilprogressplanner.com/wp-json/progress-planner-saas/v1/auditexists).Test plan
HANDOFF-spec-audit.mdfor the why-behind-each-decision context.composer test(425/425 should pass — includes 27 spec-audit tests).composer phpstan(clean).wp prpl audit runon a fresh site — expect 1 task injected (the highest-severity failing rule).pass, next admin pageload auto-completes the task.mcp-llmsource findings appear inwp evaldump ofspec_audit_findings.Open follow-ups (handoff doc lists them in priority order)
https-tlsfor.test/.localURLs — small filter.Spec_Mcp_Client→Spec_Ai_Client(the "MCP" in the name was aspirational; core's AI client can't act as an MCP client).--dry-runflag on the CLI command.🤖 Generated with Claude Code