Skip to content

Address the WordPress.org review: scope the admin notices, fix the duplicate-install fatal - #11

Merged
henry-mosh merged 4 commits into
mainfrom
feat/citecue-plugin-review-85c8bb
Aug 12, 2026
Merged

Address the WordPress.org review: scope the admin notices, fix the duplicate-install fatal#11
henry-mosh merged 4 commits into
mainfrom
feat/citecue-plugin-review-85c8bb

Conversation

@henry-mosh

Copy link
Copy Markdown
Collaborator

Addresses the WordPress.org pre-review that pended the plugin submission, plus two defects a Codex review of the first commit turned up.

The review's finding: guideline 11

Plugins should not hijack the admin dashboard. Upgrade prompts, notices, alerts, and the like must be limited in scope and used with moderation.

Two of the plugin's three notices printed on every admin screen, and neither could be dismissed: the rejected-API-key error, which persists until someone fixes the key, and the duplicate-install error, which persists until someone deletes a directory. Neither asks for anything that can be done from the media library.

All three are now confined to the Plugins list and Settings → CiteCue — the only screens where their advice is actionable. The settings screen's status card already restates every one of those conditions, so a notice nobody sees costs no information. The reconnect prompt additionally gets a permanent dismissal (nonce-guarded, per user, per site), since it is advice rather than an error.

I swept for the other guideline 11 patterns and found none: no activation redirect, no dashboard widget, no admin-bar node, no footer text, no top-level menu, no upsell copy anywhere in the codebase.

A fatal error the duplicate-install guard never caught

The guard added in 1.0.1 has never once fired in the case its own comment describes, and the case is a fatal error rather than a stray notice.

citecue/ only ever held v1.0.0, which predates the guard and so defines the constants and runs its requires unconditionally. activate_plugin() sorts active_plugins before storing it, and - sorts before /, so citecue-ai-auto-fix/citecue.php is always included first. The guard is therefore checked by the copy that never loses the race, while the copy that does lose it — 1.0.0 — has no guard to check and redeclares every class.

Since 1.0.0 cannot stand aside, this copy now does. When it finds citecue/citecue.php active with its file still on disk, it registers a Plugins-screen notice naming the folder to delete and returns without loading anything. The site runs on 1.0.0: the worse version, but a running one.

The file_exists() check is load-bearing — it mirrors the identical test core applies in wp_get_active_and_valid_plugins(). Without it, a stale active_plugins entry for a deleted folder would strand the site on a copy that is not installed, running neither. The original guard stays for every other duplicate directory (GitHub's wordpress-plugin-main/, say), which does load second and can stand down.

Dismissals were network-wide on multisite

The usermeta table is shared across a whole multisite network, while needs_seo_head_reconnect() reads a per-site option. Stored as user meta, one administrator dismissing the prompt on one site silenced a still-true prompt on every other site in the network. Now stored through update_user_option(), which prefixes the key with the current blog's; uninstall deletes the prefixed key.

Readme

The short description was 167 characters, over the 150 Plugin Check allows — not mentioned in the review, but it would have failed the tooling they link to. Now 144.

The review also classified the plugin as "LLMs.txt Generation" and warned about that crowded category. llms.txt is one of four features and the plugin does not generate the file — CiteCue does, the plugin serves it. The description now leads with the part only a plugin can do (choosing per request which version of a page WordPress returns) and adds a section spelling that out. Tags drop llms.txt for ai-crawlers.

Testing

340 tests pass in both the core and WooCommerce suites, up from 318 — 22 new, covering notice scoping across six unrelated screens, dismissal persistence and per-user isolation, nonce enforcement on the dismiss handler, blog-scoped storage, the legacy stand-down, the stale-entry case, and uninstall cleanup.

Each new test was confirmed to fail against the unfixed code rather than passing vacuously. PHPCS is clean under the project ruleset and under a WordPress-Extra ruleset approximating Plugin Check's own sniffs. I checked against the vendored WordPress source that get_option(), is_multisite(), WP_PLUGIN_DIR and plugin_basename() are all available at wp-settings.php:574 where plugins are included — a wrong assumption there would fatal every request.

Note for the reviewer

Version stays 1.1.1 across both commits: nothing has been published, so this is still one unreleased submission rather than two.

🤖 Generated with Claude Code

henry-idingo and others added 4 commits August 11, 2026 09:58
The directory's pre-review flagged guideline 11 — notices limited in scope
and used with moderation — and it was right to. Two of the three notices
printed on every admin screen there is: the rejected-key error, which
persists until someone fixes the key, and the duplicate-install error,
which persists until someone deletes a directory. Neither asks for
anything that can be done from the media library.

All three are now confined to the Plugins list and the CiteCue settings
screen, which are the only places their advice is actionable, and the
settings screen's status card states the same conditions a second time —
so a notice nobody sees costs no information.

The reconnect prompt goes further and can be dismissed for good, per
user. It is advice rather than an error, and a message an administrator
has read and decided against should stop arriving. Uninstall clears the
dismissal rows with everything else.

The readme's summary was 167 characters, over the 150 the directory
allows, and led with llms.txt — which is one of four features and is
generated by CiteCue, not here. It now leads with the part that only a
plugin can do: choosing per request which version of a page WordPress
returns.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex review of the previous commit found two defects.

The duplicate-install guard never fires in the case its own comment
describes. citecue/ only ever held 1.0.0, which predates the guard and
so defines the constants and runs its requires unconditionally, and
activate_plugin() sorts active_plugins, where '-' sorts before '/' — so
citecue-ai-auto-fix/ is always included first and 1.0.0 is always the
copy doing the redeclaring. The guard is checked by the copy that never
loses the race, and the copy that does lose it has no guard to check.
The outcome on such a site is not a stray notice, it is a fatal error.

So this copy now stands aside when it finds citecue/ active with its file
still on disk, mirroring the file_exists() test core itself applies, so a
stale active_plugins entry for a deleted folder cannot strand the site on
a copy that is not installed. The site runs on 1.0.0 — the worse version,
but a running one — and the notice names the folder to delete. The
original guard stays for every other duplicate directory, which does load
second and can stand down.

Dismissals moved from user meta to user options. The usermeta table is
network-wide on multisite while needs_seo_head_reconnect() reads a
per-site option, so one administrator dismissing the prompt on one site
silenced a still-true prompt on every other site in the network.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two multisite defects from the Codex review of the last commit, both of
which the single-site test suite was structurally unable to see.

WordPress fires admin_notices and network_admin_notices in mutually
exclusive branches, so neither duplicate-install warning appeared
anywhere in Network Admin — including Plugins, which is the only screen a
network-activated copy can be deactivated from. Both now register for
both hooks and accept the plugins-network screen alongside plugins. The
screen check is duplicated between the two callbacks rather than shared
through a helper: this is the one file that can legitimately be included
twice, and a named function in it is a redeclaration waiting to happen.

Uninstall reached one site's dismissal rows. usermeta is a single table
for the whole network while WordPress includes uninstall.php once, so
every other site's rows survived the plugin's own deletion in a table
that outlives the sites that wrote to them. It now walks get_sites() and
deletes each site's prefixed key.

The suite now also runs as a network, in CI and through
`composer test:multisite`. Both bugs above were multisite-only, which is
enough evidence that a single-site run is not the whole story. Doing so
turned up a third thing: the duplicate-notice tests were signing in as a
site administrator, who on multisite does not hold activate_plugins —
it maps through manage_network_plugins — so they would have gone on
passing while asserting nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"The entry is stale" means two things — active_plugins lists the copy,
and the file is not there — and the test said only the first, inheriting
the second from whatever the plugins directory happened to hold. A run
that died before tear_down left the directory behind and the next run
failed there instead of where the problem was.

install_legacy_copy() now clears the directory on both paths and asserts
the absence on the one that depends on it, so the test carries its own
premise. The directory it removes can only be its own leftover: under the
test suite WP_PLUGIN_DIR is the vendored WordPress, which ships with no
plugins at all.

Found by CodeRabbit, which read the helper more carefully than I did.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@henry-mosh
henry-mosh merged commit 43d8527 into main Aug 12, 2026
8 checks passed
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