fix: survive a failed action that another process already handled by Alexia-Soare · Pull Request #1381 · Codeinwp/visualizer

Alexia-Soare · 2026-09-17T13:10:47Z

Summary

The bundled Action Scheduler tried to mark an action failed that another process had already handled. The queue run then ended with an uncaught InvalidArgumentException: Unidentified action. Visualizer now supplies its own Action Scheduler store, which tolerates a row another process already deleted or failed.

Related: #1369

Note

The bug arrived with the Action Scheduler dependency in bc14d4b on version 4.0.0. It is an upstream defect, still present in Action Scheduler 4.2.0, tracked in woocommerce/action-scheduler#970.

What changed

Store replacement — Visualizer_ActionScheduler_Store extends ActionScheduler_DBStore and overrides mark_failure(). Zero changed rows means another process deleted the action or already marked it failed, so there is nothing to mark. A database error still throws, so real failures stay visible.
Registration — index.php answers the action_scheduler_store_class filter at priority 200, after Action Scheduler's own data controller at 100. It replaces only ActionScheduler_DBStore. Another plugin's store, the hybrid migration store, and the legacy post store, which does not have the problem, are left alone.
No vendor patch — the library is untouched, so a version bump needs no re-roll, and the fix applies to whichever copy of Action Scheduler is loaded on the site.
Test bootstrap — removes WordPress' _maybe_update_* hooks before the first test. The framework restores the hook snapshot of the first test after every test, and the AJAX test case only removes these hooks once per class. Any test file that sorted before test-ajax.php made the AJAX tests call api.wordpress.org.

Note

The exception fires whenever the UPDATE changes zero rows. That covers a deleted action, and also an action an overlapping cleaner already marked failed. Two cleaners can overlap because WP-Cron's queue run takes no lock; only the async runner does. The second case needs no deletion and is the more likely trigger on a real site.

The store is a singleton built on first use, so the filter is registered when the plugin loads, before anything asks Action Scheduler for a store.

Where the request used to die, and what decides now

flowchart LR
    A["Queue run starts"] --> B["Cleaner: mark stale<br/>in-progress actions failed"]
    A --> C["Runner: action throws,<br/>mark it failed"]
    B --> D{"UPDATE changed<br/>a row?"}
    C --> D
    D -- yes --> E["Continue queue run"]
    D -- no --> G{"Changed:<br/>database error?"}:::changed
    G -- yes --> F["Store throws,<br/>queue run ends"]
    G -- "no: deleted or<br/>already failed" --> E

    classDef changed fill:#9a6700,color:#fff,stroke:#5c3d00,stroke-width:3px,stroke-dasharray:6 3

Will affect visual aspect of the product

NO

Test instructions

Prerequisite: the database store must be active. The fix works with whichever copy of Action Scheduler is loaded. Check with:

wp eval 'echo ActionScheduler::plugin_path(""), " ", get_class(ActionScheduler::store()), "\n";'

Expect: the store is Visualizer_ActionScheduler_Store. If it is ActionScheduler_HybridStore, run wp action-scheduler run once, then check again.

Regression scenario. Open a MySQL session that stays open (Adminer, wp db cli). Insert a stale in-progress action, lock it, and delete it after 40 seconds, in one script:

INSERT INTO wp_actionscheduler_actions
  (hook, status, scheduled_date_gmt, scheduled_date_local, args, schedule, group_id, attempts, last_attempt_gmt, last_attempt_local, claim_id)
VALUES
  ('visualizer_schedule_refresh_db', 'in-progress',
   UTC_TIMESTAMP() - INTERVAL 2 HOUR, UTC_TIMESTAMP() - INTERVAL 2 HOUR, '[]', '', 0, 1,
   UTC_TIMESTAMP() - INTERVAL 2 HOUR, UTC_TIMESTAMP() - INTERVAL 2 HOUR, 0);
SET @id = LAST_INSERT_ID();
BEGIN;
SELECT * FROM wp_actionscheduler_actions WHERE action_id = @id FOR UPDATE;
SELECT SLEEP(40);
DELETE FROM wp_actionscheduler_actions WHERE action_id = @id;
COMMIT;

While the script sleeps, run the queue in a second terminal:
```
wp action-scheduler run
```
Expect: the command waits for the lock, then finishes normally. Before this change it ended with PHP Fatal error: Uncaught InvalidArgumentException: Unidentified action.
Repeat step 1 with the DELETE line replaced by UPDATE wp_actionscheduler_actions SET status = 'failed' WHERE action_id = @id;. Run step 2 again.

Expect: the command finishes normally. This is the overlapping-cleaner case.
Healthy path. Insert the same stale row without the lock script, then run wp action-scheduler run.

Expect: the row's status becomes failed.
Confirm the store is in use:
```
wp eval 'echo get_class( ActionScheduler::store() ), "\n";'
```
Expect: Visualizer_ActionScheduler_Store.

Check before Pull Request is ready:

I have written a test and included it in this PR
I have run all tests and they pass
The code passes when running the PHP CodeSniffer
Code meets WordPress Coding Standards for PHP, HTML, CSS and JS
Security and Sanitization requirements have been followed
I have assigned a reviewer or two to review this PR (if you're not sure who to assign, we can do this step for you)

🤖 Generated with Claude Code

Action Scheduler's `mark_failure()` throws "Unidentified action" when its UPDATE changes no row. That happens when another process deleted the action, and also when an overlapping cleaner already marked it failed: WP-Cron's queue run takes no lock, only the async runner does. Two callers let that exception escape and end the request with a fatal: the queue cleaner loop and the runner's action error path. The bundled copy is patched at `composer install` so both callers skip that action and continue, the same guard `delete_actions()` already uses. `composer-exit-on-patch-failure` makes a future Action Scheduler bump fail loudly instead of dropping the fix. Upstream 4.2.0 still throws; see woocommerce/action-scheduler#970. Refs: #1369 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

pirate-bot · 2026-09-17T13:13:50Z

Plugin build for fb00b81 is ready 🛎️!

Download build

The framework snapshots hooks at the first test of the run and restores that snapshot after every test. WP_Ajax_UnitTestCase removes the `_maybe_update_*` admin_init hooks once per class, which only holds when an AJAX class runs first. A test file that sorts before test-ajax.php put the hooks back into the snapshot, so every later AJAX test called api.wordpress.org and failed on the response. Remove the hooks in the bootstrap so the order of test files does not matter. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Pull request overview

Adds a Composer-applied patch plus regression tests to ensure Action Scheduler’s mark_failure() race conditions don’t abort queue processing when another process deletes or already-fails an action.

Changes:

Introduces a Composer patch to tolerate “no rows updated” scenarios when marking actions failed.
Adds WP unit tests reproducing the race (delete / already-failed / runner error path).
Updates test bootstrap to remove update-check hooks to avoid unintended outbound requests.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test-action-scheduler-mark-failure.php	New regression tests simulating concurrent updates/deletes during `mark_failures()` and runner error handling.
tests/bootstrap.php	Removes core update-check admin hooks early in test bootstrap to reduce external HTTP calls during tests.
patches/action-scheduler-1369-mark-failures.patch	Patch to Action Scheduler to ignore failures when `mark_failure()` affects zero rows due to races.
composer.json	Adds `cweagans/composer-patches` and config to apply the Action Scheduler patch.
.distignore	Excludes `patches/` and `vendor/cweagans` from build artifacts.

Suppressed comments (2)

patches/action-scheduler-1369-mark-failures.patch:1

Catching Exception here will also silently swallow unrelated/legitimate failures from mark_failure() (e.g., DB errors), which can hide real corruption or operational issues. Narrow the catch to the specific exception type thrown for the 'unidentified action / no rows updated' race (and/or verify the error condition), and rethrow anything else so genuine failures still surface.

--- a/classes/ActionScheduler_QueueCleaner.php

patches/action-scheduler-1369-mark-failures.patch:1

Catching Exception here will also silently swallow unrelated/legitimate failures from mark_failure() (e.g., DB errors), which can hide real corruption or operational issues. Narrow the catch to the specific exception type thrown for the 'unidentified action / no rows updated' race (and/or verify the error condition), and rethrow anything else so genuine failures still surface.

--- a/classes/ActionScheduler_QueueCleaner.php

Note

Copilot is running an experiment and ran this review at Lite.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The guard caught every Exception, so a database error inside `mark_failure()` was swallowed together with the race it targets. The store throws the same InvalidArgumentException for both; only `$wpdb->last_error` tells them apart. Catch that type only and rethrow when the database reported an error. Tests: one-shot guard in the query interceptor, so an injected UPDATE cannot re-enter it; a pattern that accepts quoted ids; a test that breaks the UPDATE and expects the exception to surface. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Note

Copilot is running an experiment and ran this review at Lite.

pirate-bot

🟡 Changes requested

The fix works and its tests fail on the base tree, but composer.lock has a stale content hash. Run composer update --lock and commit the lock file.

Validation details

Files reviewed: 6/6 changed files.
Patch application: composer install --prefer-dist on PHP 8.3.33 with Composer 2.10.3 applied patches/action-scheduler-1369-mark-failures.patch to Action Scheduler 3.9.3. php -l passed on both patched files.
New tests at HEAD: vendor/bin/phpunit --filter 'Test_Visualizer_Action_Scheduler_Mark_Failure::' on WordPress 7.1.0 test library, PHPUnit 9.6.34. Result: OK (4 tests, 6 assertions).
New tests on the base tree: the same test file on an unpatched pr-base checkout. Result: 3 errors with InvalidArgumentException: Unidentified action. The database error test passes on both, as intended.
Guard order: wpdb::query() applies the query filter, then flush() clears last_error, then runs the UPDATE. The catch reads the error of that UPDATE only.
Security pass: no trust boundary changed. The catch covers one throw site in ActionScheduler_DBStore::mark_failure().
Release exclusions: patches is the only path with that name in the plugin. No runtime code references vendor/cweagans.

Untested areas

patch binary on GitHub runners: composer-patches 1.7.3 shells out to patch for dist installs. The sandbox lacked it at first. Runner availability was not verified. A missing binary fails the install loudly because composer-exit-on-patch-failure is on.
Live queue run: the PR test steps with wp action-scheduler run and a locked MySQL session were not executed. The PHPUnit tests cover the same code paths.
PHPCS and PHPStan: left to CI.

🤖 Automated review · run code-review-agent_6aabf48134b1c4.54602197.

🤖 Review agent — review posted ✅ on 8bffd5bd · changes requested · 1 finding · 14 min

Run code-review-agent_6aabf48134b1c4.54602197 · trail

pirate-bot · 2026-09-17T14:23:17Z

        "This file is @generated automatically"
    ],
-    "content-hash": "881b5f99c0b72f47e79eb09bdac7fbea",
+    "content-hash": "54649940158be9c9d52224064f0f3be3",


✓ Verified in the sandbox. This content hash does not match the PR's composer.json. composer validate exits 2 on this branch and 0 on the base branch. Every composer install prints Warning: The lock file is not up to date with the latest changes in composer.json. The extra block is part of the hash, so composer.json changed after the lock was written.

Fix: run composer update --lock and commit composer.lock. In a copy, this changed only this line, to 28731c3ebcd9422db328f73b52e0883e.

Fixed: ran composer update --lock; only the content hash changed, to 28731c3ebcd9422db328f73b52e0883e. composer validate passes.

Resolved at 60b3ff8: the composer patch, composer.json and composer.lock changes are no longer part of this PR (git diff --name-status pr-base HEAD lists neither file). composer validate --no-check-publish exits 0 on HEAD (PHP 8.3.33). No further action needed on this thread.

composer.json changed after the lock was written, so `composer validate` failed and every install warned that the lock file was stale. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

patches/action-scheduler-1369-mark-failures.patch:1

This relies on $wpdb->last_error inside the catch to distinguish a real DB error from the intended race condition, but $wpdb->last_error can contain a stale value from an earlier, unrelated query. That can cause a false-positive rethrow and reintroduce the crash in unrelated scenarios. Clear (and optionally restore) $wpdb->last_error immediately before calling mark_failure(), then base the decision on whether that call set a new error.

--- a/classes/ActionScheduler_QueueCleaner.php

patches/action-scheduler-1369-mark-failures.patch:1

Same issue as in mark_failures(): $wpdb->last_error may be non-empty due to an earlier query, causing handle_action_error() to incorrectly throw while it should tolerate the 'already failed/deleted' race. Reset (and optionally restore) $wpdb->last_error immediately before attempting $this->store->mark_failure( $action_id ) so the check reflects only that operation.

--- a/classes/ActionScheduler_QueueCleaner.php

…he guard Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Suppressed comments (1)

patches/action-scheduler-1369-mark-failures.patch:1

The patch explicitly claims to tolerate the runner path when the action was deleted by another process (not just already-failed). The new tests cover deletion/already-failed races for the cleaner and already-failed for the runner, but there’s no test exercising the runner’s handle_action_error() behavior when the action row is deleted before mark_failure() runs. Add a regression test that deletes the action from actionscheduler_actions inside the throwing hook (before the exception) and asserts process_action() doesn’t fatally fail and the queue run continues.

--- a/classes/ActionScheduler_QueueCleaner.php

…d action Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Soare-Robert-Daniel · 2026-09-18T09:52:33Z

+--- a/classes/data-stores/ActionScheduler_DBStore.php
+++ b/classes/data-stores/ActionScheduler_DBStore.php
+@@ -1227,7 +1227,8 @@
+ 			array( '%s' ),
+ 			array( '%d' )
+ 		);
+-		if ( empty( $updated ) ) {
+		// Zero rows means another process deleted the action or already marked it failed: nothing left to mark.
+		if ( false === $updated ) {
+ 			/* translators: %s is the action ID */
+ 			throw new \InvalidArgumentException( sprintf( __( 'Unidentified action %s: we were unable to mark this action as having failed. It may may have been deleted by another process.', 'action-scheduler' ), $action_id ) );
+ 		}


can we do not another mechanism? This is not a good solution

Patching the bundled Action Scheduler needed a Composer plugin, a patch file kept in the repo, and a re-roll on every dependency bump. Action Scheduler resolves its store through `action_scheduler_store_class`. Visualizer now answers that filter with a subclass of the database store that tolerates zero changed rows in `mark_failure()`, and still throws when the UPDATE itself failed. The library stays untouched, so a version bump needs no work, and the fix applies to whichever copy of Action Scheduler is loaded. Other stores, including another plugin's, are left alone. Refs: #1369 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pirate-bot

🟢 Approved

The store subclass removes the Unidentified action fatal in the sandbox, and the shipped tests fail without it.

Validation details

Files reviewed: 5/5 changed files.
Setup: /repo/vendor held a patched Action Scheduler from an earlier run. Reinstalled woocommerce/action-scheduler 3.9.3 from the lock file with composer install on PHP 8.3.33.
New tests at HEAD: vendor/bin/phpunit --filter 'Test_Visualizer_Action_Scheduler_Mark_Failure::' on the WordPress 7.1 test library, PHPUnit 9.6.34. Result: OK (6 tests, 9 assertions).
Same tests on pr-base: 6 errors, Class "Visualizer_ActionScheduler_Store" not found.
Same tests with the plain ActionScheduler_DBStore: 4 errors with InvalidArgumentException: Unidentified action. The intercepts fire, and the subclass alone removes the failure.
Live store: after the migration action ran, wp eval 'echo get_class( ActionScheduler::store() );' printed Visualizer_ActionScheduler_Store. A stale in-progress row became failed after wp action-scheduler run.
Filter order: the store singleton is built on plugins_loaded priority 1. The filter is added at plugin file load, after the hooks at priority 100.
Upstream 4.2.0: mark_failure( $action_id ) keeps the same signature and single UPDATE, so the override stays compatible.
Security pass: no trust boundary changed. Only error visibility changes, guarded by $wpdb->last_error.

Untested areas

Locked-row regression steps: the two-terminal MySQL lock scenario from the PR body was not run. The PHPUnit variant covers the same code path.
Hybrid store window: before the Action Scheduler migration completes, the hybrid store is in use and the fix does not apply. This matches the stated intent.
Activation request: on the request that activates the plugin, Action Scheduler builds its store inside the require_once before the add_filter line. No queue runs in that request.
PHPCS and PHPStan: left to CI.

🤖 Automated review · run code-review-agent_6ab14089a53dc8.93415939.

🤖 Review agent — review posted ✅ on 60b3ff83 · approved · 0 findings · 12 min

Run code-review-agent_6ab14089a53dc8.93415939 · trail

pirate-bot

🟡 Changes requested

The store fix works in the sandbox. Add a test for the filter priority, and clean up the new test file as noted inline.

Validation details

Files reviewed: 5/5 changed files.
Setup: Reinstalled a clean Action Scheduler 3.9.3 from composer.lock on PHP 8.3.33. The local copy had patched queue files.
New tests at HEAD: OK (6 tests, 9 assertions) on the WordPress 7.1 test library.
Plain ActionScheduler_DBStore subclass: 4 tests fail with Unidentified action.
Override that swallows every exception: the database-error test fails.
Live site: ActionScheduler::store() is Visualizer_ActionScheduler_Store. A stale in-progress action became failed.
Action Scheduler 4.2.0: mark_failure( $action_id ) keeps the same signature.
Security: No trust boundary changed.

Untested areas

Migration window: Open index.php:249. While migration is incomplete, ActionScheduler_HybridStore uses ActionScheduler_DBStoreMigrator. The same race still throws there. This window ends after the first migration run. Confirm that this scope is intended.
Locked-row steps: The two-terminal MySQL steps from the PR body were not run.
PHPCS and PHPStan: Left to CI.

🤖 Automated review · run code-review-agent_6aba33b2f28199.32396841.

🤖 Review agent — review posted ✅ on 60b3ff83 · changes requested · 5 findings · 30 min

Run code-review-agent_6aba33b2f28199.32396841 · trail

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review effort: Lite
Findings: 3

Open (3)

Using $wpdb->last_error as the discriminator for whether to rethrow is relatively brittle because… · New
Adding the entire Action Scheduler vendor directory to scanDirectories can significantly slow… · New
update_option( 'action_scheduler_migration_status', 'complete' ) mutates global state that… · New

Resolved since last review (2)

The regexes don’t match backticked column names as emitted by wpdb (UPDATE ... SET status` = ...…
The error message contains a duplicated word ('may may'). Since this patch is already modifying the…

+		try {
+			parent::mark_failure( $action_id );
+		} catch ( InvalidArgumentException $e ) {
+			// The parent throws on zero changed rows. `wpdb::query()` clears
+			// `last_error` before each statement, so an error set here belongs
+			// to that UPDATE; without one the row was deleted or already failed.
+			if ( ! empty( $wpdb->last_error ) ) {
+				throw $e;
+			}


 		- %currentWorkingDirectory%/vendor/neitanod/forceutf8
 		- %currentWorkingDirectory%/vendor/openspout/openspout
 		- %currentWorkingDirectory%/vendor/codeinwp/themeisle-sdk
+		- %currentWorkingDirectory%/vendor/woocommerce/action-scheduler


+		update_option( 'action_scheduler_migration_status', 'complete' );
+
+		$this->assertSame( 'Visualizer_ActionScheduler_Store', apply_filters( 'action_scheduler_store_class', ActionScheduler_Store::DEFAULT_CLASS ) );


Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review effort: Lite
Findings: 5 · 2

Open (7)

This override bypasses the parent store’s $this->wpdb property and directly uses the global… · New
This test mutates the persistent action_scheduler_migration_status option but doesn’t restore it… · New
update_option( 'action_scheduler_migration_status', 'complete' ) mutates global state that…
Adding the entire Action Scheduler vendor directory to scanDirectories can significantly slow…
Using $wpdb->last_error as the discriminator for whether to rethrow is relatively brittle because…
The translator comment says the placeholder is an action ID, but the string uses %s. Since this… · New
The test adds an action callback but never removes it. Even with a unique hook name, leaving… · New

+	public function mark_failure( $action_id ) {
+		global $wpdb;
+
+		// Same UPDATE as the parent. Zero rows means the row was deleted or
+		// already failed; only `false` is a database error.
+		$updated = $wpdb->update(
+			$wpdb->actionscheduler_actions,
+			array( 'status' => self::STATUS_FAILED ),
+			array( 'action_id' => $action_id ),
+			array( '%s' ),
+			array( '%d' )
+		);


+		update_option( 'action_scheduler_migration_status', 'complete' );
+
+		$this->assertSame( 'Visualizer_ActionScheduler_Store', apply_filters( 'action_scheduler_store_class', ActionScheduler_Store::DEFAULT_CLASS ) );


+			/* translators: %s is the action ID */
+			throw new InvalidArgumentException( sprintf( __( 'Unable to mark action %s as failed.', 'visualizer' ), $action_id ) );


+
+		add_action(
+			$hook,
+			static function () use ( $wpdb, $action_id ) {
+				$wpdb->delete( $wpdb->actionscheduler_actions, array( 'action_id' => $action_id ) );
+				throw new RuntimeException( 'refresh failed' );
+			}
+		);
+
+		( new ActionScheduler_QueueRunner( $this->store ) )->process_action( $action_id, 'test' );


Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review effort: Lite
Findings: 8 · 3

Open (11)

Adding the entire Action Scheduler vendor directory to scanDirectories can significantly increase… · New
In seed_stale_running_action(), last_attempt_local is set to the same value as… · New
This test adds a hook callback but never removes it. Given the test framework behavior described in… · New
This test mutates the persistent action_scheduler_migration_status option but doesn’t restore it…
This override bypasses the parent store’s $this->wpdb property and directly uses the global…
update_option( 'action_scheduler_migration_status', 'complete' ) mutates global state that…
Adding the entire Action Scheduler vendor directory to scanDirectories can significantly slow…
Using $wpdb->last_error as the discriminator for whether to rethrow is relatively brittle because…
The regex interpolates ActionScheduler_Store::STATUS_FAILED without escaping. While the current… · New
The test adds an action callback but never removes it. Even with a unique hook name, leaving…
The translator comment says the placeholder is an action ID, but the string uses %s. Since this…

Alexia-Soare · 2026-09-28T11:43:05Z

 		- %currentWorkingDirectory%/vendor/neitanod/forceutf8
 		- %currentWorkingDirectory%/vendor/openspout/openspout
 		- %currentWorkingDirectory%/vendor/codeinwp/themeisle-sdk
+		- %currentWorkingDirectory%/vendor/woocommerce/action-scheduler


Not changing this. scanDirectories only loads the class names for discovery; PHPStan does not analyze or report errors in that code. The other vendor packages are listed the same way.

Alexia-Soare · 2026-09-28T11:43:07Z

+		$action_id = $this->store->save_action( new ActionScheduler_Action( 'visualizer_schedule_refresh_db', array(), new ActionScheduler_SimpleSchedule( as_get_datetime_object( '-2 hours' ) ) ) );
+		$gmt       = gmdate( 'Y-m-d H:i:s', time() - 2 * HOUR_IN_SECONDS );
+		$wpdb->update(
+			$wpdb->actionscheduler_actions,
+			array(
+				'status'             => ActionScheduler_Store::STATUS_RUNNING,
+				'last_attempt_gmt'   => $gmt,
+				'last_attempt_local' => $gmt,


Not needed. The queue cleaner finds stale actions by last_attempt_gmt only (ActionScheduler_DBStore filters modified on that column), and the tests run in UTC.

Alexia-Soare · 2026-09-28T11:43:08Z

+		add_action(
+			$hook,
+			static function () use ( $wpdb, $action_id ) {
+				$wpdb->delete( $wpdb->actionscheduler_actions, array( 'action_id' => $action_id ) );
+				throw new RuntimeException( 'refresh failed' );
+			}
+		);
+
+		( new ActionScheduler_QueueRunner( $this->store ) )->process_action( $action_id, 'test' );
+


Not needed. The comment in tests/bootstrap.php says the framework restores the hook snapshot after every test, so the callback is removed as soon as this test ends.

+		if ( 0 !== stripos( ltrim( $sql ), 'UPDATE' ) || false === strpos( $sql, $table ) ) {
+			return false;
+		}
+		if ( ! preg_match( '/status`?\s*=\s*\'' . ActionScheduler_Store::STATUS_FAILED . '\'/', $sql ) ) {


Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Copilot

Note

Copilot was unable to run its full agentic suite in this review.

Copilot review overview

Review effort: Lite
Findings: 10 · 5

Open (15)

This override hard-codes the actions table via $wpdb->actionscheduler_actions instead of using… · New
Adding the entire Action Scheduler vendor directory to scanDirectories can noticeably increase… · New
This test adds a hook callback but never removes it. Given the test framework behavior described in…
In seed_stale_running_action(), last_attempt_local is set to the same value as…
Adding the entire Action Scheduler vendor directory to scanDirectories can significantly increase…
This test mutates the persistent action_scheduler_migration_status option but doesn’t restore it…
This override bypasses the parent store’s $this->wpdb property and directly uses the global…
update_option( 'action_scheduler_migration_status', 'complete' ) mutates global state that…
Adding the entire Action Scheduler vendor directory to scanDirectories can significantly slow…
Using $wpdb->last_error as the discriminator for whether to rethrow is relatively brittle because…
This query filter intentionally becomes a no-op after the first interception, but it remains… · New
The docstring claims the matcher works “with or without … quotes”, but the status = ... regex… · New
The regex interpolates ActionScheduler_Store::STATUS_FAILED without escaping. While the current…
The test adds an action callback but never removes it. Even with a unique hook name, leaving…
The translator comment says the placeholder is an action ID, but the string uses %s. Since this…

Alexia-Soare · 2026-09-28T11:49:39Z

+		$updated = $wpdb->update(
+			$wpdb->actionscheduler_actions,
+			array( 'status' => self::STATUS_FAILED ),
+			array( 'action_id' => $action_id ),
+			array( '%s' ),
+			array( '%d' )
+		);


Not needed. The parent store has no table property; its own mark_failure() uses $wpdb->actionscheduler_actions, so this matches it.

Alexia-Soare · 2026-09-28T11:49:41Z

 		- %currentWorkingDirectory%/vendor/neitanod/forceutf8
 		- %currentWorkingDirectory%/vendor/openspout/openspout
 		- %currentWorkingDirectory%/vendor/codeinwp/themeisle-sdk
+		- %currentWorkingDirectory%/vendor/woocommerce/action-scheduler


Not changing this, same as the earlier thread: scanDirectories only loads the class names, PHPStan does not analyze that code.

Alexia-Soare · 2026-09-28T11:49:42Z

+		$filter = function ( $sql ) use ( $action_id, $table, $intercept, &$done ) {
+			if ( $done || ! $this->is_mark_failed_update( $sql, $table, $action_id ) ) {
+				return $sql;
+			}
+			$done = true;
+			return $intercept( $sql );
+		};
+		add_filter( 'query', $filter );
+		$this->filters_to_remove[] = $filter;


Not needed. The $done flag already makes the filter a no-op after the first match, and tear_down() removes it.

Alexia-Soare · 2026-09-28T11:49:44Z

+	/**
+	 * Whether `$sql` is the UPDATE that marks `$action_id` in `$table` failed.
+	 * Matches the SQL `wpdb::update()` builds with or without backticks and quotes.
+	 *
+	 * @param string $sql       SQL about to run.
+	 * @param string $table     Actions table name.
+	 * @param int    $action_id Action id.
+	 * @return bool
+	 */
+	private function is_mark_failed_update( $sql, $table, $action_id ) {
+		if ( 0 !== stripos( ltrim( $sql ), 'UPDATE' ) || false === strpos( $sql, $table ) ) {
+			return false;
+		}
+		if ( ! preg_match( '/status`?\s*=\s*\'' . preg_quote( ActionScheduler_Store::STATUS_FAILED, '/' ) . '\'/', $sql ) ) {
+			return false;
+		}


Fixed in fb00b81: the docstring now says only the action id can be bare or quoted.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Alexia-Soare requested a balanced review from Copilot September 17, 2026 13:40

Copilot started reviewing on behalf of Alexia-Soare September 17, 2026 13:41 View session

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 17, 2026 13:46
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread tests/test-action-scheduler-mark-failure.php Outdated

Alexia-Soare requested a balanced review from Copilot September 17, 2026 13:55

Copilot started reviewing on behalf of Alexia-Soare September 17, 2026 13:56 View session

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 17, 2026 14:00
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Comment thread patches/action-scheduler-1369-mark-failures.patch Outdated

Comment thread patches/action-scheduler-1369-mark-failures.patch Outdated

Comment thread tests/test-action-scheduler-mark-failure.php Outdated

Alexia-Soare requested a review from pirate-bot September 17, 2026 14:09

pirate-bot requested changes Sep 17, 2026

View reviewed changes

build: refresh composer.lock content hash

69af787

composer.json changed after the lock was written, so `composer validate` failed and every install warned that the lock file was stale. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Alexia-Soare requested a balanced review from Copilot September 18, 2026 06:02

Copilot started reviewing on behalf of Alexia-Soare September 18, 2026 06:07 View session

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 18, 2026 06:07
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread patches/action-scheduler-1369-mark-failures.patch Outdated

Comment thread patches/action-scheduler-1369-mark-failures.patch Outdated

Comment thread tests/test-action-scheduler-mark-failure.php Outdated

test: restore the previous suppress_errors() setting

ded4c25

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Alexia-Soare requested a balanced review from Copilot September 18, 2026 06:12

Copilot started reviewing on behalf of Alexia-Soare September 18, 2026 06:16 View session

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 18, 2026 06:17
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread patches/action-scheduler-1369-mark-failures.patch Outdated

Comment thread tests/test-action-scheduler-mark-failure.php Outdated

test: name the SQL predicate; note why last_error is trustworthy in t…

577ab24

…he guard Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Alexia-Soare requested a balanced review from Copilot September 18, 2026 06:33

Copilot started reviewing on behalf of Alexia-Soare September 18, 2026 06:38 View session

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 18, 2026 06:39
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot AI reviewed Sep 18, 2026

View reviewed changes

Comment thread tests/test-action-scheduler-mark-failure.php Outdated

Comment thread tests/test-action-scheduler-mark-failure.php Outdated

test: seed through the store API; cover the runner path with a delete…

7b06c8a

…d action Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Alexia-Soare requested a review from Soare-Robert-Daniel September 18, 2026 07:42

Alexia-Soare self-assigned this Sep 18, 2026

Soare-Robert-Daniel requested changes Sep 18, 2026

View reviewed changes

Soare-Robert-Daniel requested a review from pirate-bot September 21, 2026 14:34

pirate-bot approved these changes Sep 21, 2026

View reviewed changes

Soare-Robert-Daniel requested a review from pirate-bot September 28, 2026 09:30

pirate-bot requested changes Sep 28, 2026

View reviewed changes

test: cover the store filter priority and tidy the mark-failure tests

7af0208

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Alexia-Soare requested a balanced review from Copilot September 28, 2026 10:34

Copilot started reviewing on behalf of Alexia-Soare September 28, 2026 10:39 View session

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 28, 2026 10:39
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot AI reviewed Sep 28, 2026

View reviewed changes

fix: tell a database error from zero changed rows by the UPDATE result

dff8035

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Alexia-Soare requested a balanced review from Copilot September 28, 2026 10:49

Copilot started reviewing on behalf of Alexia-Soare September 28, 2026 10:53 View session

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 28, 2026 10:55
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot AI reviewed Sep 28, 2026

View reviewed changes

fix: format the action ID with %d in the failure message

dfab8d1

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Alexia-Soare requested a balanced review from Copilot September 28, 2026 11:13

Copilot started reviewing on behalf of Alexia-Soare September 28, 2026 11:17 View session

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 28, 2026 11:18
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot AI reviewed Sep 28, 2026

View reviewed changes

test: escape the status value in the UPDATE matcher

334fc19

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Alexia-Soare requested a balanced review from Copilot September 28, 2026 11:33

Copilot started reviewing on behalf of Alexia-Soare September 28, 2026 11:34 View session

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 28, 2026 11:38
An unexpected error occurred. For more details, see the detailed logs in GitHub Actions.

Copilot AI reviewed Sep 28, 2026

View reviewed changes

docs: say which parts of the UPDATE matcher accept quotes

fb00b81

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

pirate-bot mentioned this pull request Sep 28, 2026

Action Scheduler queue cleanup fatals with "Unidentified action" after concurrent deletion #1369

Open

		update_option( 'action_scheduler_migration_status', 'complete' );

		$this->assertSame( 'Visualizer_ActionScheduler_Store', apply_filters( 'action_scheduler_store_class', ActionScheduler_Store::DEFAULT_CLASS ) );

		/* translators: %s is the action ID */
		throw new InvalidArgumentException( sprintf( __( 'Unable to mark action %s as failed.', 'visualizer' ), $action_id ) );

Conversation

Alexia-Soare commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Where the request used to die, and what decides now

Will affect visual aspect of the product

Test instructions

Check before Pull Request is ready:

Uh oh!

pirate-bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pirate-bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

🟡 Changes requested

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pirate-bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

🟢 Approved

Uh oh!

pirate-bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

🟡 Changes requested

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Alexia-Soare commented Sep 17, 2026 •

edited

Loading

pirate-bot commented Sep 17, 2026 •

edited

Loading

pirate-bot left a comment •

edited

Loading

pirate-bot left a comment •

edited

Loading

pirate-bot left a comment •

edited

Loading