Plugin Directory: Let reviewers block a release from being served#728
Draft
obenland wants to merge 3 commits into
Draft
Plugin Directory: Let reviewers block a release from being served#728obenland wants to merge 3 commits into
obenland wants to merge 3 commits into
Conversation
The test environment was missing three things the development environment already had, which between them made whole classes of test impossible: - `update_source` was never created, so anything touching the update API's table failed. - `wp-content/env-bin` was not mapped, so `database-tables.sql` was not reachable from the test container to import. - `PLUGINS_TABLE_PREFIX` was undefined, so `API\Base::load_routes()` fatalled and no `plugins/v1` route was registered. Shared verbatim with WordPress#717; whichever of the two PRs lands second should drop this commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UNnLdnE7etihGNvsWY1qGE
Reviewers could already skip the release cooldown, but had no way to hold a release back short of closing the whole plugin. Add a Block control to the Controls metabox, alongside the force-release one and sharing a single reason field, so a version still inside its cooldown can be held out of `update_source` until it's force-released. - API_Update_Updater::block_release() is the counterpart to force_release(): it records a `release_block` on the release row, and a single gate in update_single_plugin() honours it, so both the deferred cooldown cron and the backup reconciliation cron keep the version out of `update_source`. - A version that's already live can't be un-shipped, so a block on one is refused rather than recorded as a hold that never takes effect. - force_release() clears the block and notes the override in the audit log. Holding a version back means update_single_plugin() can't rewrite the row at all, since every version-specific column describes the held version. That was already true of the cooldown, and it means a status change never reached sites while one was in flight: closing a plugin whose next version was deferred left `update_source` serving the current version with `available = 1`. A block makes that unbounded rather than a few hours, so sync_availability() now writes the availability and closure fields on their own and leaves the served version alone. Covered by Reviewer_Release_Block_Test, which exercises the block through the same path the metabox uses, including closing and re-opening a plugin while a version is held. Its fixture lives in Release_Block_Test_Case, resolved by a small autoloader in the test bootstrap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ndings. Address correctness and feedback gaps in the release-block feature: - Flush the update/info caches when syncing availability, so a status change on a plugin with a held version reaches sites instead of waiting out the cache TTL. Extracted into flush_update_caches(). - Make sync_availability() return a bool, tell a failed lookup apart from no row, bail rather than overwrite on unreadable meta, and check the update. Both held-path early returns propagate it so cron_trigger()'s retry works again. - Guard block_release() against the deferred serve racing it: confirm the hold persisted, then roll it back if the version was served in the meantime, so no block is recorded against a live version. Refuse a second block rather than merging it into the first. Audit-log the outcome, not the intent. - Report every save-handler outcome to the reviewer via an admin notice (success, stale version, missing reason, already-served, failure), instead of a silent no-op that reads like success. - Key the metabox display and Block button on whether the version is actually served, not on the cooldown window, so the controls survive the gap between cooldown expiry and the deferred write. - Suppress the committer-facing cooldown notices while a version is held, rather than promising a serve time that no longer applies. Tests: scope the fixture's cleanup to its own slug (no more shared-table TRUNCATE), uniquify the reviewer login, give serve() a meta argument, fix the vacuous scheduled-hook assertion, and cover meta preservation, the double-block refusal, the metabox render gates, and the new notices. Co-Authored-By: Claude Opus 4.8 (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
Reviewers could already skip the release cooldown, but had no way to hold a release back short
of closing the whole plugin. This adds a Block control to the Controls metabox, alongside
the force-release one and sharing a single reason field, so a version still inside its cooldown
can be held out of
update_source— the previously served version keeps being served — until areviewer force-releases it.
This is the plumbing half of #720, which is now split in two. The scan-driven half is #729.
How it works
API_Update_Updater::block_release(), the counterpart toforce_release(),records a
release_blockon the release row. A single gate inupdate_single_plugin()honours it, so both the deferred cooldown cron and the backup reconciliation cron keep the
version out of
update_source.A block on one is refused rather than recorded as a hold that never takes effect.
is recorded in the audit log.
force-release control, and the Block button appears only while a version is still cooling
down.
The
sync_availability()fixHolding a version back means
update_single_plugin()can't rewrite the row at all — everyversion-specific column describes the held version, not the served one.
That was already true of the release cooldown, and it means a status change never reached
sites while one was in flight: closing a plugin whose next version was deferred left
update_sourceserving the current version withavailable = 1. A block makes that unboundedrather than a few hours, which is what surfaced it.
sync_availability()now writes the availability and closure fields on their own and leaves theserved version alone. Worth reviewing on its own terms — it fixes live behaviour that predates
this feature.
Tests
Reviewer_Release_Block_Testcovers the block through the same path the metabox uses: the hold,the audit notes, the refusals, the metabox save handler including its capability check, and
closing/re-opening a plugin while a version is held. Its fixture lives in
Release_Block_Test_Case, resolved by a small autoloader in the test bootstrap so the sharedbases don't need requiring in dependency order.
170 tests pass locally against
environments/plugin-directory.Notes for reviewers
c1edf5b11. Whichever ofPlugin Directory: Serve a release immediately when Gandalf reports no findings #717 / this / Plugin Directory: Block a release when a security scan reports a high risk score #729 lands second should drop it.
API_Update_Updater::get_served_version()is the same helper Plugin Directory: Serve a release immediately when Gandalf reports no findings #717 extracts.manual reviewer action.
🤖 Generated with Claude Code