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 60b3ff8 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.

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>

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.

…aller `wpdb::update()` returns false on a database error and 0 when no row changed. `ActionScheduler_DBStore::mark_failure()` now throws only on false, so the race (deleted or already failed by another process) is handled once for every caller and no caller reads `$wpdb->last_error`. One hunk replaces the two caller guards. 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 6 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.

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>

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 balanced review from Copilot September 18, 2026 06:51

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

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 18, 2026 06:56
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 .distignore Outdated

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

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

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

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

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

Copilot AI reviewed Sep 18, 2026

View reviewed changes

test: intercept only the UPDATE that sets status to failed

4dd6763

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

Alexia-Soare requested a balanced review from Copilot September 18, 2026 07:26

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

Copilot stopped reviewing on behalf of Alexia-Soare due to an error September 18, 2026 07:32
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

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

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

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!

pirate-bot Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

Alexia-Soare Sep 18, 2026

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!

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!

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!

Soare-Robert-Daniel Sep 18, 2026

Alexia-Soare commented Sep 17, 2026 •

edited

Loading

pirate-bot commented Sep 17, 2026 •

edited

Loading

pirate-bot left a comment •

edited

Loading