From 76878822aaa93a5fc27dca7369f7ae02ee1eb011 Mon Sep 17 00:00:00 2001 From: The Original Barry Hughes <3594411+barryhughes@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:16:22 -0700 Subject: [PATCH 1/8] feat: model an unrecognized schedule as a recoverable placeholder A structurally-valid schedule that merely nests a class Action Scheduler does not recognize (an un-allow-listed, non-schedule class) is not evidently dangerous -- just unknown. Returning false for it, as the hardening does, makes the stores treat it as corrupt and the queue runner silently cancel the action, losing work a site operator might still need. Introduce ActionScheduler_UnrecognizedSchedule (a NullSchedule placeholder that records the unrecognized class names) and return it from the deserializer for the recoverable ($shadow_eligible) case, so the action stays a valid, displayable schedule. Structural rejections -- a top-level non-schedule, an unwalkable graph, or a throwing __wakeup -- have no legitimate form and remain corrupt (false). This is the foundation for routing such actions to a failed (reviewable, forced- re-runnable) state instead of cancelling them. Refs #1318 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ActionScheduler_ScheduleDeserializer.php | 10 +++++ .../ActionScheduler_UnrecognizedSchedule.php | 44 +++++++++++++++++++ ...tionScheduler_ScheduleUnserialize_Test.php | 40 ++++++++++++++--- 3 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 classes/schedules/ActionScheduler_UnrecognizedSchedule.php diff --git a/classes/ActionScheduler_ScheduleDeserializer.php b/classes/ActionScheduler_ScheduleDeserializer.php index ffd142b0..febe7b3b 100644 --- a/classes/ActionScheduler_ScheduleDeserializer.php +++ b/classes/ActionScheduler_ScheduleDeserializer.php @@ -305,6 +305,16 @@ protected function reject( $offending_class, $shadow_eligible ) { do_action( 'action_scheduler_unexpected_schedule_class', $offending_class, $this->outer_class, $this->potential_offenders, $rejected ); if ( $rejected ) { + // A recoverable ($shadow_eligible) rejection is a structurally-valid schedule that merely + // references an unrecognized class — not evidently dangerous, just unknown. Return it as an + // ActionScheduler_UnrecognizedSchedule placeholder so the action stays visible and can be + // routed to a failed state for operator review / forced re-run, rather than being treated as + // irretrievably corrupt (false) and silently cancelled. Structural rejections (a top-level + // non-schedule or an unwalkable graph) have no legitimate form and remain corrupt. + if ( $shadow_eligible ) { + return new ActionScheduler_UnrecognizedSchedule( $this->potential_offenders ); + } + return false; } diff --git a/classes/schedules/ActionScheduler_UnrecognizedSchedule.php b/classes/schedules/ActionScheduler_UnrecognizedSchedule.php new file mode 100644 index 00000000..9caf61bb --- /dev/null +++ b/classes/schedules/ActionScheduler_UnrecognizedSchedule.php @@ -0,0 +1,44 @@ +unrecognized_classes = array_values( array_unique( $unrecognized_classes ) ); + } + + /** + * The unrecognized class names, for display and logging. + * + * @return string[] + */ + public function get_unrecognized_classes() { + return $this->unrecognized_classes; + } +} diff --git a/tests/phpunit/jobstore/ActionScheduler_ScheduleUnserialize_Test.php b/tests/phpunit/jobstore/ActionScheduler_ScheduleUnserialize_Test.php index ecdbaeb1..dc7a7c07 100644 --- a/tests/phpunit/jobstore/ActionScheduler_ScheduleUnserialize_Test.php +++ b/tests/phpunit/jobstore/ActionScheduler_ScheduleUnserialize_Test.php @@ -370,10 +370,12 @@ function ( $offending, $outer, $unexpected, $rejected ) use ( &$captured ) { public function test_nested_allow_list_filter_is_honored_per_call() { $blob = serialize( new ActionScheduler_Test_Custom_Schedule_With_Property( time() + DAY_IN_SECONDS, new ActionScheduler_Test_Schedule_Helper( 42 ) ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize - // First call, no filter: the helper class is unexpected, so the blob is rejected. - $this->assertFalse( + // First call, no filter: the helper class is unexpected, so the blob yields the unrecognized + // placeholder (a valid schedule referencing an unknown class is recoverable, not corrupt). + $this->assertInstanceOf( + 'ActionScheduler_UnrecognizedSchedule', ActionScheduler_ScheduleDeserializer::unserialize( $blob ), - 'An un-allow-listed nested support class should be rejected by default.' + 'An un-allow-listed nested support class should yield an unrecognized-schedule placeholder.' ); // Second call, with the helper allow-listed via the filter: the blob is accepted. This also @@ -438,8 +440,9 @@ public function test_deserializer_instance_is_reusable() { array() ); - // First: a blob nesting a gadget (rejected, populating internal offender/seen state). - $this->assertFalse( $deserializer( $this->nested_gadget_blob() ) ); + // First: a blob nesting a gadget (rejected to the unrecognized placeholder, populating internal + // offender/seen state). + $this->assertInstanceOf( 'ActionScheduler_UnrecognizedSchedule', $deserializer( $this->nested_gadget_blob() ) ); // Then: a clean third party schedule must still deserialize correctly. $clean = serialize( new ActionScheduler_Test_Custom_Schedule( time() + HOUR_IN_SECONDS ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize @@ -458,7 +461,7 @@ public function test_constructor_nested_allow_list_is_used() { $blob = serialize( new ActionScheduler_Test_Custom_Schedule_With_Property( time() + DAY_IN_SECONDS, new ActionScheduler_Test_Schedule_Helper( 7 ) ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize $without = new ActionScheduler_ScheduleDeserializer( array(), array() ); - $this->assertFalse( $without( $blob ), 'Nested support classes absent from the allow-list must be rejected.' ); + $this->assertInstanceOf( 'ActionScheduler_UnrecognizedSchedule', $without( $blob ), 'Nested support classes absent from the allow-list yield an unrecognized-schedule placeholder.' ); $with = new ActionScheduler_ScheduleDeserializer( array(), @@ -548,7 +551,7 @@ public function test_gadget_nested_in_third_party_schedule_is_rejected_without_i $result = ActionScheduler_ScheduleDeserializer::unserialize( $blob ); - $this->assertFalse( $result ); + $this->assertInstanceOf( 'ActionScheduler_UnrecognizedSchedule', $result ); $this->assertFalse( ActionScheduler_Test_Evil_Gadget::$fired, 'A gadget nested in a third party schedule was instantiated.' @@ -586,4 +589,27 @@ public function test_tampered_blob_that_makes_a_trusted_class_throw_is_reported_ } $this->assertNotNull( $fetched, 'Fetching a tampered action fataled instead of being handled.' ); } + + /** + * A structurally-valid schedule that merely nests an unrecognized class is recoverable: it yields an + * ActionScheduler_UnrecognizedSchedule placeholder (carrying the offending class names) rather than + * being treated as irretrievably corrupt. A structural rejection (top-level non-schedule) stays + * corrupt (false). + */ + public function test_recoverable_schedule_yields_unrecognized_placeholder_but_structural_stays_corrupt() { + // Valid third party schedule nesting an unknown helper class -> recoverable. + $recoverable = serialize( new ActionScheduler_Test_Custom_Schedule_With_Property( time() + DAY_IN_SECONDS, new ActionScheduler_Test_Schedule_Helper( 1 ) ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $schedule = ActionScheduler_ScheduleDeserializer::unserialize( $recoverable ); + + $this->assertInstanceOf( 'ActionScheduler_UnrecognizedSchedule', $schedule ); + $this->assertContains( + 'ActionScheduler_Test_Schedule_Helper', + $schedule->get_unrecognized_classes(), + 'The placeholder should record the unrecognized class name for operator review.' + ); + + // A bare top-level non-schedule is a structural rejection, not recoverable -> stays corrupt. + $structural = serialize( new ActionScheduler_Test_Evil_Gadget() ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize + $this->assertFalse( ActionScheduler_ScheduleDeserializer::unserialize( $structural ) ); + } } From cb6813a4090e817ea02f24411d0e2cbe0eb39241 Mon Sep 17 00:00:00 2001 From: The Original Barry Hughes <3594411+barryhughes@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:22:53 -0700 Subject: [PATCH 2/8] feat: fail (not cancel) unrecognized-schedule actions, allow forced re-run The queue runner now marks an action failed -- for operator review -- when its schedule resolves to an ActionScheduler_UnrecognizedSchedule, instead of silently cancelling it as corrupt. We do not auto-run it, because its intended timing is unknown. Site operators who cannot change code still need to keep work flowing, so add force_run_action() (backed by a new $force flag on process_action()) which executes such an action's callback despite the unreadable schedule and completes it. A failed action would otherwise fetch as a non-executable FinishedAction, so for the duration of a forced run it is mapped to an executable action class. A new action_scheduler_unrecognized_schedule_action hook fires on the failure. Refs #1318 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ActionScheduler_Abstract_QueueRunner.php | 71 ++++++++++- tests/phpunit.xml.dist | 1 + ...ionScheduler_UnrecognizedSchedule_Test.php | 114 ++++++++++++++++++ 3 files changed, 184 insertions(+), 2 deletions(-) create mode 100644 tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php diff --git a/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php b/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php index 87fa61f9..c063c86d 100644 --- a/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php +++ b/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php @@ -60,7 +60,7 @@ public function __construct( ?ActionScheduler_Store $store = null, ?ActionSchedu * Generally, this should be capitalised and not localised as it's a proper noun. * @throws \Exception When error running action. */ - public function process_action( $action_id, $context = '' ) { + public function process_action( $action_id, $context = '', $force = false ) { // Temporarily override the error handler while we process the current action. // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_set_error_handler set_error_handler( @@ -89,7 +89,9 @@ function ( $type, $message ) { try { do_action( 'action_scheduler_before_execute', $action_id, $context ); - if ( ActionScheduler_Store::STATUS_PENDING !== $this->store->get_status( $action_id ) ) { + // A forced run (e.g. a site operator using the admin "Run" link) executes the action even + // if it is not pending, so it can flush work that is otherwise stuck. + if ( ! $force && ActionScheduler_Store::STATUS_PENDING !== $this->store->get_status( $action_id ) ) { $valid_action = false; do_action( 'action_scheduler_execution_ignored', $action_id, $context ); return; @@ -104,6 +106,15 @@ function ( $type, $message ) { return; } + // An action whose schedule could not be recognized is not run automatically: we cannot + // know when it was meant to run, so we mark it failed for operator review rather than + // cancelling it. A forced run overrides this and executes the action's callback anyway. + if ( ! $force && $action->get_schedule() instanceof ActionScheduler_UnrecognizedSchedule ) { + $valid_action = false; + $this->fail_unrecognized_action( $action_id ); + return; + } + $this->store->log_execution( $action_id ); $action->execute(); @@ -127,6 +138,34 @@ function ( $type, $message ) { } } + /** + * Force an action to run now, regardless of its current status. + * + * Intended for a site operator flushing stuck work from the admin list table — most usefully an + * action that was failed because its schedule references an unrecognized class, which cannot be run + * on the normal (automatic) path. A failed action would otherwise be fetched as a non-executable + * ActionScheduler_FinishedAction, so for the duration of this run we map it back to an executable + * action class. The action's callback runs; its (unreadable or valid) schedule is not consulted, so + * no next instance of a recurring series is scheduled. + * + * @param int $action_id Action ID. + * @param string $context Context in which the action is being run. + * @return void + */ + public function force_run_action( $action_id, $context = '' ) { + $as_executable = static function ( $action_class, $status ) { + return ActionScheduler_Store::STATUS_FAILED === $status ? 'ActionScheduler_Action' : $action_class; + }; + + add_filter( 'action_scheduler_stored_action_class', $as_executable, 10, 2 ); + + try { + $this->process_action( $action_id, $context, true ); + } finally { + remove_filter( 'action_scheduler_stored_action_class', $as_executable, 10 ); + } + } + /** * Fetches the action instance. On failure returns null instead of ActionScheduler_NullAction. * @@ -165,6 +204,34 @@ private function cancel_corrupted_action( int $action_id ) { ); } + /** + * Marks an action failed because its schedule references a class Action Scheduler does not + * recognize. Unlike a corrupt action, this is recoverable: an operator can review it and force it + * to run, or make the referenced class available (via the + * action_scheduler_allowed_nested_schedule_classes filter) and re-run it normally. + * + * @param int $action_id Action ID. + * @return void + */ + private function fail_unrecognized_action( int $action_id ) { + $this->store->mark_failure( $action_id ); + + /** + * Fires when an action is marked failed because its stored schedule references an unrecognized + * class, rather than being run or cancelled. + * + * @since 4.1.0 + * + * @param int $action_id Action ID. + */ + do_action( 'action_scheduler_unrecognized_schedule_action', $action_id ); + + ActionScheduler_Logger::instance()->log( + $action_id, + __( 'This action references a schedule that could not be recognized, so it has been marked as failed rather than run. Review it and, if it is safe, force it to run.', 'action-scheduler' ) + ); + } + /** * Marks actions as either having failed execution or failed validation, as appropriate. * diff --git a/tests/phpunit.xml.dist b/tests/phpunit.xml.dist index 721cbbf4..3830d4b5 100644 --- a/tests/phpunit.xml.dist +++ b/tests/phpunit.xml.dist @@ -18,6 +18,7 @@ ./phpunit/jobstore/ActionScheduler_DBStoreMigrator_Test.php ./phpunit/jobstore/ActionScheduler_DBStore_Test.php ./phpunit/jobstore/ActionScheduler_ScheduleUnserialize_Test.php + ./phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php ./phpunit/jobstore/ActionScheduler_HybridStore_Test.php ./phpunit/logging/ActionScheduler_DBLogger_Test.php diff --git a/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php new file mode 100644 index 00000000..9e0dc0a4 --- /dev/null +++ b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php @@ -0,0 +1,114 @@ +query( "DELETE FROM {$wpdb->actionscheduler_actions}" ); + parent::setUp(); + } + + /** + * Persist a real, due, pending action, then overwrite its stored schedule with a blob that is a + * valid outer schedule nesting an unrecognized class — which the deserializer resolves to an + * ActionScheduler_UnrecognizedSchedule. + * + * @param string $hook Hook to schedule. + * @return int Action ID. + */ + private function store_due_action_with_unrecognized_schedule( $hook ) { + global $wpdb; + + $store = new ActionScheduler_DBStore(); + $action = new ActionScheduler_Action( $hook, array(), new ActionScheduler_SimpleSchedule( as_get_datetime_object( '1 hour ago' ) ) ); + $action_id = $store->save_action( $action ); + + // A valid third party schedule (a placeholder in the safe parse, so nothing is instantiated) + // whose property holds a class Action Scheduler does not recognize. + $nul = chr( 0 ); + $prop = $nul . '*' . $nul . 'timestamp'; + $container = 'ActionScheduler_Test_Custom_Schedule'; + $unknown = 'O:14:"Some_Unknown_X":0:{}'; + $blob = 'O:' . strlen( $container ) . ':"' . $container . '":1:{s:' . strlen( $prop ) . ':"' . $prop . '";' . $unknown . '}'; + + $wpdb->update( + $wpdb->actionscheduler_actions, + array( 'schedule' => $blob ), + array( 'action_id' => $action_id ), + array( '%s' ), + array( '%d' ) + ); + + return $action_id; + } + + /** + * The deserializer resolves the tampered schedule to the placeholder, so the action fetches as a + * normal (displayable) action rather than a null action. + */ + public function test_action_fetches_with_unrecognized_schedule_placeholder() { + $store = new ActionScheduler_DBStore(); + $action_id = $this->store_due_action_with_unrecognized_schedule( 'as_test_unrecognized_fetch' ); + + $fetched = $store->fetch_action( $action_id ); + + $this->assertNotInstanceOf( 'ActionScheduler_NullAction', $fetched ); + $this->assertInstanceOf( 'ActionScheduler_UnrecognizedSchedule', $fetched->get_schedule() ); + } + + /** + * Automatic processing must mark an unrecognized-schedule action failed (for review) rather than + * running it or cancelling it. + */ + public function test_unrecognized_schedule_action_is_failed_not_run_or_cancelled() { + $hook = 'as_test_unrecognized_auto'; + $ran = 0; + add_action( $hook, function () use ( &$ran ) { ++$ran; } ); + + $store = new ActionScheduler_DBStore(); + $runner = ActionScheduler_Mocker::get_queue_runner( $store ); + $action_id = $this->store_due_action_with_unrecognized_schedule( $hook ); + + $runner->process_action( $action_id ); + + $this->assertSame( ActionScheduler_Store::STATUS_FAILED, $store->get_status( $action_id ), 'The action should be failed, not cancelled or run.' ); + $this->assertSame( 0, $ran, 'An unrecognized-schedule action must not run automatically.' ); + + remove_all_actions( $hook ); + } + + /** + * A forced run (as a site operator would trigger from the admin list table) executes the action's + * callback despite the unreadable schedule, flushing the work, and completes the action. + */ + public function test_forced_run_executes_unrecognized_schedule_action() { + $hook = 'as_test_unrecognized_forced'; + $ran = 0; + add_action( $hook, function () use ( &$ran ) { ++$ran; } ); + + $store = new ActionScheduler_DBStore(); + $runner = ActionScheduler_Mocker::get_queue_runner( $store ); + $action_id = $this->store_due_action_with_unrecognized_schedule( $hook ); + + // First, automatic processing marks it failed. + $runner->process_action( $action_id ); + $this->assertSame( ActionScheduler_Store::STATUS_FAILED, $store->get_status( $action_id ) ); + + // Then a forced run executes the callback and completes it. + $runner->force_run_action( $action_id, 'Test' ); + + $this->assertSame( 1, $ran, 'A forced run must execute the action callback.' ); + $this->assertSame( ActionScheduler_Store::STATUS_COMPLETE, $store->get_status( $action_id ) ); + + remove_all_actions( $hook ); + } +} From e3b55211438c2ddb58fc68120dbe33562c5ece9e Mon Sep 17 00:00:00 2001 From: The Original Barry Hughes <3594411+barryhughes@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:25:47 -0700 Subject: [PATCH 3/8] feat: force-run failed actions from the list table; label unrecognized schedules The admin "Run" row action now force-runs a failed action (previously a no-op, since a failed action is not pending), so an operator can retry a failed action or flush one whose schedule could not be recognized -- without needing to change code. Pending actions keep the normal run path. The Scheduled Date column renders an unrecognized schedule as "Unrecognized schedule" instead of falling through to the NullSchedule "async" label. Refs #1318 Co-Authored-By: Claude Opus 4.8 (1M context) --- classes/ActionScheduler_ListTable.php | 12 +++- ...ionScheduler_UnrecognizedSchedule_Test.php | 58 +++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/classes/ActionScheduler_ListTable.php b/classes/ActionScheduler_ListTable.php index 911d8c11..7218a982 100644 --- a/classes/ActionScheduler_ListTable.php +++ b/classes/ActionScheduler_ListTable.php @@ -469,6 +469,10 @@ protected function get_schedule_display_string( ActionScheduler_Schedule $schedu $schedule_display_string = ''; + if ( is_a( $schedule, 'ActionScheduler_UnrecognizedSchedule' ) ) { + return __( 'Unrecognized schedule', 'action-scheduler' ); + } + if ( is_a( $schedule, 'ActionScheduler_NullSchedule' ) ) { return __( 'async', 'action-scheduler' ); } @@ -570,7 +574,13 @@ protected function process_row_action( $action_id, $row_action_type ) { try { switch ( $row_action_type ) { case 'run': - $this->runner->process_action( $action_id, 'Admin List Table' ); + // A failed action is not pending, so it cannot be run on the normal path. Force it, so + // an operator can retry a failed action or flush one whose schedule was unrecognized. + if ( ActionScheduler_Store::STATUS_FAILED === $this->store->get_status( $action_id ) ) { + $this->runner->force_run_action( $action_id, 'Admin List Table' ); + } else { + $this->runner->process_action( $action_id, 'Admin List Table' ); + } break; case 'cancel': $this->store->cancel_action( $action_id ); diff --git a/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php index 9e0dc0a4..92226dd4 100644 --- a/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php +++ b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php @@ -111,4 +111,62 @@ public function test_forced_run_executes_unrecognized_schedule_action() { remove_all_actions( $hook ); } + + /** + * The admin list table renders an unrecognized schedule with a clear label rather than falling + * through to the NullSchedule "async" display. + */ + public function test_list_table_displays_unrecognized_schedule() { + $list_table = $this->make_list_table(); + $method = new ReflectionMethod( $list_table, 'get_schedule_display_string' ); + $method->setAccessible( true ); + + $this->assertSame( 'Unrecognized schedule', $method->invoke( $list_table, new ActionScheduler_UnrecognizedSchedule() ) ); + } + + /** + * The list table "Run" row action force-runs a failed action (previously impossible), so an + * operator can flush stuck work. + */ + public function test_list_table_run_force_runs_a_failed_action() { + $hook = 'as_test_unrecognized_listtable'; + $ran = 0; + add_action( $hook, function () use ( &$ran ) { ++$ran; } ); + + $store = new ActionScheduler_DBStore(); + $runner = ActionScheduler_Mocker::get_queue_runner( $store ); + $action_id = $this->store_due_action_with_unrecognized_schedule( $hook ); + + $runner->process_action( $action_id ); // Automatic pass marks it failed. + $this->assertSame( ActionScheduler_Store::STATUS_FAILED, $store->get_status( $action_id ) ); + + $list_table = $this->make_list_table( $store, $runner ); + $method = new ReflectionMethod( $list_table, 'process_row_action' ); + $method->setAccessible( true ); + $method->invoke( $list_table, $action_id, 'run' ); + + $this->assertSame( 1, $ran, 'The list table Run action should force-run a failed action.' ); + $this->assertSame( ActionScheduler_Store::STATUS_COMPLETE, $store->get_status( $action_id ) ); + + remove_all_actions( $hook ); + } + + /** + * Build a list table instance for tests, loading the WP_List_Table base if the admin context that + * normally provides it has not been loaded. + * + * @param ActionScheduler_Store|null $store Store to use. + * @param ActionScheduler_QueueRunner|null $runner Runner to use. + * @return ActionScheduler_ListTable + */ + private function make_list_table( $store = null, $runner = null ) { + if ( ! class_exists( 'WP_List_Table' ) ) { + require_once ABSPATH . 'wp-admin/includes/class-wp-list-table.php'; + } + + $store = $store ? $store : new ActionScheduler_DBStore(); + $runner = $runner ? $runner : ActionScheduler_Mocker::get_queue_runner( $store ); + + return new ActionScheduler_ListTable( $store, ActionScheduler_Logger::instance(), $runner ); + } } From e71d3e0b842c86d778fa35142fb01fde690a343a Mon Sep 17 00:00:00 2001 From: The Original Barry Hughes <3594411+barryhughes@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:29:34 -0700 Subject: [PATCH 4/8] feat: admin notice prompting review of unrecognized-schedule failures When the queue runner fails an action because its schedule references an unrecognized class, record a (non-autoloaded) flag. On admin pages, show a notice to users who can manage options, linking to the failed actions screen and explaining they can force-run an action that is safe. The notice renders until the operator dismisses it (a nonce-guarded dismissal clears the flag); a later failure sets it again. Refs #1318 Co-Authored-By: Claude Opus 4.8 (1M context) --- classes/ActionScheduler_AdminView.php | 88 +++++++++++++++++++ ...ionScheduler_UnrecognizedSchedule_Test.php | 48 ++++++++++ 2 files changed, 136 insertions(+) diff --git a/classes/ActionScheduler_AdminView.php b/classes/ActionScheduler_AdminView.php index cde70935..429d8c63 100644 --- a/classes/ActionScheduler_AdminView.php +++ b/classes/ActionScheduler_AdminView.php @@ -50,6 +50,10 @@ public static function instance() { * @codeCoverageIgnore */ public function init() { + // Registered unconditionally: the failure is recorded from the queue runner, which usually + // runs outside the admin (cron/async), so the recorder must not sit behind the is_admin() guard. + add_action( 'action_scheduler_unrecognized_schedule_action', array( $this, 'note_unrecognized_schedule_failure' ) ); + if ( is_admin() && ( ! defined( 'DOING_AJAX' ) || ! DOING_AJAX ) ) { if ( class_exists( 'WooCommerce' ) ) { @@ -60,6 +64,7 @@ public function init() { add_action( 'admin_menu', array( $this, 'register_menu' ) ); add_action( 'admin_notices', array( $this, 'maybe_check_pastdue_actions' ) ); + add_action( 'admin_notices', array( $this, 'maybe_show_unrecognized_schedule_notice' ) ); add_action( 'current_screen', array( $this, 'add_help_tabs' ) ); } } @@ -241,6 +246,89 @@ protected function check_pastdue_actions() { do_action( 'action_scheduler_pastdue_actions_extra_notices', $query_args ); } + /** + * Option flag set when one or more actions have been failed because their schedule referenced an + * unrecognized class. Drives the review notice below. + * + * @var string + */ + const UNRECOGNIZED_SCHEDULE_NOTICE_OPTION = 'action_scheduler_unrecognized_schedule_failures'; + + /** + * Record that an action was failed because its schedule referenced an unrecognized class, so an + * admin notice can prompt an operator to review it. + * + * Fired from the queue runner, which usually runs outside the admin (cron/async), so this makes no + * assumptions about admin context. The flag is not autoloaded. + */ + public function note_unrecognized_schedule_failure() { + if ( ! get_option( self::UNRECOGNIZED_SCHEDULE_NOTICE_OPTION ) ) { + update_option( self::UNRECOGNIZED_SCHEDULE_NOTICE_OPTION, 1, false ); + } + } + + /** + * Action: admin_notices + * + * Prompt operators to review actions failed because their schedule could not be recognized, and + * render until the operator dismisses the notice. + */ + public function maybe_show_unrecognized_schedule_notice() { + if ( ! current_user_can( 'manage_options' ) ) { + return; + } + + // Process an explicit dismissal. + if ( + isset( $_GET['as_dismiss_unrecognized_schedule'], $_GET['_asnonce'] ) + && wp_verify_nonce( sanitize_key( wp_unslash( $_GET['_asnonce'] ) ), 'as_dismiss_unrecognized_schedule' ) + ) { + delete_option( self::UNRECOGNIZED_SCHEDULE_NOTICE_OPTION ); + return; + } + + if ( ! get_option( self::UNRECOGNIZED_SCHEDULE_NOTICE_OPTION ) ) { + return; + } + + $failed_url = add_query_arg( + array( + 'page' => 'action-scheduler', + 'status' => ActionScheduler_Store::STATUS_FAILED, + 'order' => 'asc', + ), + admin_url( 'tools.php' ) + ); + + $dismiss_url = wp_nonce_url( + add_query_arg( 'as_dismiss_unrecognized_schedule', '1' ), + 'as_dismiss_unrecognized_schedule', + '_asnonce' + ); + + $message = wp_kses( + sprintf( + // translators: 1) link to the failed actions screen, 2) dismiss link. + __( 'Action Scheduler: one or more scheduled actions could not be run because their schedule references an unrecognized class, and have been marked failed for your review. If an action is safe to run, use its Run link to force it. Dismiss', 'action-scheduler' ), + esc_url( $failed_url ), + esc_url( $dismiss_url ) + ), + array( + 'strong' => array(), + 'em' => array(), + 'a' => array( 'href' => true ), + ) + ); + + wp_admin_notice( + $message, + array( + 'type' => 'warning', + 'paragraph_wrap' => true, + ) + ); + } + /** * Provide more information about the screen and its data in the help tab. */ diff --git a/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php index 92226dd4..f291d61a 100644 --- a/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php +++ b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php @@ -74,6 +74,9 @@ public function test_unrecognized_schedule_action_is_failed_not_run_or_cancelled $ran = 0; add_action( $hook, function () use ( &$ran ) { ++$ran; } ); + $notified = false; + add_action( 'action_scheduler_unrecognized_schedule_action', function () use ( &$notified ) { $notified = true; } ); + $store = new ActionScheduler_DBStore(); $runner = ActionScheduler_Mocker::get_queue_runner( $store ); $action_id = $this->store_due_action_with_unrecognized_schedule( $hook ); @@ -82,8 +85,10 @@ public function test_unrecognized_schedule_action_is_failed_not_run_or_cancelled $this->assertSame( ActionScheduler_Store::STATUS_FAILED, $store->get_status( $action_id ), 'The action should be failed, not cancelled or run.' ); $this->assertSame( 0, $ran, 'An unrecognized-schedule action must not run automatically.' ); + $this->assertTrue( $notified, 'The action_scheduler_unrecognized_schedule_action hook should fire.' ); remove_all_actions( $hook ); + remove_all_actions( 'action_scheduler_unrecognized_schedule_action' ); } /** @@ -151,6 +156,49 @@ public function test_list_table_run_force_runs_a_failed_action() { remove_all_actions( $hook ); } + /** + * The notice flag is set when an unrecognized-schedule failure is noted, and cleared by a valid + * dismissal request. + */ + public function test_notice_flag_is_set_and_dismissed() { + $admin = ActionScheduler_AdminView::instance(); + $option = ActionScheduler_AdminView::UNRECOGNIZED_SCHEDULE_NOTICE_OPTION; + delete_option( $option ); + + $admin->note_unrecognized_schedule_failure(); + $this->assertNotEmpty( get_option( $option ), 'Noting a failure should set the notice flag.' ); + + wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); + $_GET['as_dismiss_unrecognized_schedule'] = '1'; + $_GET['_asnonce'] = wp_create_nonce( 'as_dismiss_unrecognized_schedule' ); + + ob_start(); + $admin->maybe_show_unrecognized_schedule_notice(); + ob_end_clean(); + + $this->assertFalse( (bool) get_option( $option ), 'A valid dismissal should clear the notice flag.' ); + + unset( $_GET['as_dismiss_unrecognized_schedule'], $_GET['_asnonce'] ); + } + + /** + * The notice renders for an administrator while the flag is set, and links to the failed actions + * screen. + */ + public function test_notice_renders_for_admin_and_links_to_failed_actions() { + wp_set_current_user( self::factory()->user->create( array( 'role' => 'administrator' ) ) ); + update_option( ActionScheduler_AdminView::UNRECOGNIZED_SCHEDULE_NOTICE_OPTION, 1, false ); + + ob_start(); + ActionScheduler_AdminView::instance()->maybe_show_unrecognized_schedule_notice(); + $html = ob_get_clean(); + + $this->assertStringContainsString( 'unrecognized class', $html ); + $this->assertStringContainsString( 'status=failed', $html ); + + delete_option( ActionScheduler_AdminView::UNRECOGNIZED_SCHEDULE_NOTICE_OPTION ); + } + /** * Build a list table instance for tests, loading the WP_List_Table base if the admin context that * normally provides it has not been loaded. From 6042b861886936d49ff97bdc6aade5d8712fa279 Mon Sep 17 00:00:00 2001 From: The Original Barry Hughes <3594411+barryhughes@users.noreply.github.com> Date: Fri, 10 Jul 2026 18:39:35 -0700 Subject: [PATCH 5/8] fix: a forced run must not reschedule a recurring series Review follow-up. The list table routes every failed action to force_run_action(), including one that retains a valid recurring schedule. process_action() would then reach the tail reschedule and schedule another instance -- but the series already advanced when the action first ran, so this created a duplicate. Suppress the reschedule on any forced run (which also makes the code match force_run_action()'s documented contract). Refs #1318 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ActionScheduler_Abstract_QueueRunner.php | 8 ++++- ...ionScheduler_UnrecognizedSchedule_Test.php | 30 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php b/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php index c063c86d..14624d52 100644 --- a/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php +++ b/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php @@ -133,7 +133,10 @@ function ( $type, $message ) { restore_error_handler(); } - if ( isset( $action ) && is_a( $action, 'ActionScheduler_Action' ) && $action->get_schedule()->is_recurring() ) { + // A forced run flushes a single occurrence on an operator's request; it must not advance a + // recurring series. The next instance was already scheduled when the action first ran, so + // rescheduling here would create a duplicate. + if ( ! $force && isset( $action ) && is_a( $action, 'ActionScheduler_Action' ) && $action->get_schedule()->is_recurring() ) { $this->schedule_next_instance( $action, $action_id ); } } @@ -148,6 +151,9 @@ function ( $type, $message ) { * action class. The action's callback runs; its (unreadable or valid) schedule is not consulted, so * no next instance of a recurring series is scheduled. * + * This executes an action regardless of status, so callers are responsible for authorizing the + * request (the admin list table gates it behind a per-row nonce and the manage_options capability). + * * @param int $action_id Action ID. * @param string $context Context in which the action is being run. * @return void diff --git a/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php index f291d61a..0cb3e1a3 100644 --- a/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php +++ b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php @@ -199,6 +199,36 @@ public function test_notice_renders_for_admin_and_links_to_failed_actions() { delete_option( ActionScheduler_AdminView::UNRECOGNIZED_SCHEDULE_NOTICE_OPTION ); } + /** + * Force-running a failed recurring action (with a still-valid schedule) must not schedule a + * duplicate next instance — the series already advanced when the action first ran. + */ + public function test_forced_run_of_recurring_failed_action_does_not_reschedule() { + $hook = 'as_test_force_recurring'; + add_action( $hook, '__return_true' ); + + $store = new ActionScheduler_DBStore(); + $runner = ActionScheduler_Mocker::get_queue_runner( $store ); + + $schedule = new ActionScheduler_IntervalSchedule( as_get_datetime_object( '1 hour ago' ), HOUR_IN_SECONDS ); + $action = new ActionScheduler_Action( $hook, array(), $schedule ); + $action_id = $store->save_action( $action ); + $store->mark_failure( $action_id ); + + $pending_query = array( 'hook' => $hook, 'status' => ActionScheduler_Store::STATUS_PENDING ); + $this->assertSame( 0, (int) $store->query_actions( $pending_query, 'count' ) ); + + $runner->force_run_action( $action_id, 'Test' ); + + $this->assertSame( + 0, + (int) $store->query_actions( $pending_query, 'count' ), + 'A forced run of a failed recurring action must not schedule a duplicate next instance.' + ); + + remove_all_actions( $hook ); + } + /** * Build a list table instance for tests, loading the WP_List_Table base if the admin context that * normally provides it has not been loaded. From 16101ece40ea6e7b2c7b181c5f8712d0bc436905 Mon Sep 17 00:00:00 2001 From: The Original Barry Hughes <3594411+barryhughes@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:13:44 -0700 Subject: [PATCH 6/8] i18n: keep markup out of the unrecognized-schedule notice strings Translators should not need to understand HTML. Restructure the notice so every translatable string is plain text: the sentence carries only numbered placeholders, and the anchor text ("failed", "Dismiss") and label are separate plain strings. Markup is assembled in PHP and interpolated via sprintf, with each dynamic part escaped (esc_url/esc_html__). esc_html__() on the sentence also prevents a rogue translation from injecting markup, so wp_kses() -- which would instead allow whitelisted tags through -- is no longer needed. Refs #1318 Co-Authored-By: Claude Opus 4.8 (1M context) --- classes/ActionScheduler_AdminView.php | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/classes/ActionScheduler_AdminView.php b/classes/ActionScheduler_AdminView.php index 429d8c63..22483d7b 100644 --- a/classes/ActionScheduler_AdminView.php +++ b/classes/ActionScheduler_AdminView.php @@ -306,18 +306,19 @@ public function maybe_show_unrecognized_schedule_notice() { '_asnonce' ); - $message = wp_kses( - sprintf( - // translators: 1) link to the failed actions screen, 2) dismiss link. - __( 'Action Scheduler: one or more scheduled actions could not be run because their schedule references an unrecognized class, and have been marked failed for your review. If an action is safe to run, use its Run link to force it. Dismiss', 'action-scheduler' ), - esc_url( $failed_url ), - esc_url( $dismiss_url ) - ), - array( - 'strong' => array(), - 'em' => array(), - 'a' => array( 'href' => true ), - ) + // Markup is kept out of the translatable strings: each translatable piece is plain text, escaped + // here, then interpolated into developer-controlled HTML. esc_html__() on the sentence also means + // a rogue translation cannot inject markup, so wp_kses() is unnecessary. + $as_label = '' . esc_html__( 'Action Scheduler:', 'action-scheduler' ) . ''; + $failed_link = sprintf( '%s', esc_url( $failed_url ), esc_html__( 'failed', 'action-scheduler' ) ); + $dismiss_link = sprintf( '%s', esc_url( $dismiss_url ), esc_html__( 'Dismiss', 'action-scheduler' ) ); + + $message = sprintf( + /* translators: 1: a bold "Action Scheduler:" label, 2: a link reading "failed", 3: a link reading "Dismiss". */ + esc_html__( '%1$s one or more scheduled actions could not be run because their schedule references an unrecognized class, and have been marked %2$s for your review. If an action is safe to run, use its Run link to force it. %3$s', 'action-scheduler' ), + $as_label, + $failed_link, + $dismiss_link ); wp_admin_notice( From 92fb8eb163e6a574e60e8304d513cfb3c0842de7 Mon Sep 17 00:00:00 2001 From: The Original Barry Hughes <3594411+barryhughes@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:17:32 -0700 Subject: [PATCH 7/8] i18n: translate the notice as one whole sentence, tags as placeholders Splitting the anchor text into separate __() calls made the sentence a set of fragments, which translates poorly (word order, agreement and declension vary by language, and a lone "failed" has no context). Keep the entire sentence as a single translatable string and pull only the HTML tags out as opening/closing placeholder pairs, so the linked phrases stay in-sentence and can move as a unit. Escaping is unchanged: esc_html__() on the sentence still neutralizes rogue- translation markup, and each opening tag carries an esc_url()'d URL, so no wp_kses. Refs #1318 Co-Authored-By: Claude Opus 4.8 (1M context) --- classes/ActionScheduler_AdminView.php | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/classes/ActionScheduler_AdminView.php b/classes/ActionScheduler_AdminView.php index 22483d7b..d407ff52 100644 --- a/classes/ActionScheduler_AdminView.php +++ b/classes/ActionScheduler_AdminView.php @@ -306,19 +306,20 @@ public function maybe_show_unrecognized_schedule_notice() { '_asnonce' ); - // Markup is kept out of the translatable strings: each translatable piece is plain text, escaped - // here, then interpolated into developer-controlled HTML. esc_html__() on the sentence also means - // a rogue translation cannot inject markup, so wp_kses() is unnecessary. - $as_label = '' . esc_html__( 'Action Scheduler:', 'action-scheduler' ) . ''; - $failed_link = sprintf( '%s', esc_url( $failed_url ), esc_html__( 'failed', 'action-scheduler' ) ); - $dismiss_link = sprintf( '%s', esc_url( $dismiss_url ), esc_html__( 'Dismiss', 'action-scheduler' ) ); - + // The whole sentence is one translatable string, so translators keep full context and natural + // word order. Only the HTML tags are pulled out as placeholders (opening/closing pairs) -- tags + // need no translation. esc_html__() on the sentence neutralizes any markup a rogue translation + // might contain (the %n$s tokens survive it), and each opening tag carries an escaped URL, so + // wp_kses() is unnecessary. $message = sprintf( - /* translators: 1: a bold "Action Scheduler:" label, 2: a link reading "failed", 3: a link reading "Dismiss". */ - esc_html__( '%1$s one or more scheduled actions could not be run because their schedule references an unrecognized class, and have been marked %2$s for your review. If an action is safe to run, use its Run link to force it. %3$s', 'action-scheduler' ), - $as_label, - $failed_link, - $dismiss_link + /* translators: 1$s/2$s: opening/closing bold tags around "Action Scheduler:". 3$s/4$s: opening/closing tags of a link (around "failed") to the failed actions screen. 5$s/6$s: opening/closing tags of the dismiss link (around "Dismiss"). */ + esc_html__( '%1$sAction Scheduler:%2$s one or more scheduled actions could not be run because their schedule references an unrecognized class, and have been marked %3$sfailed%4$s for your review. If an action is safe to run, use its Run link to force it. %5$sDismiss%6$s', 'action-scheduler' ), + '', + '', + '', + '', + '', + '' ); wp_admin_notice( From 209f3fc2ca62651c048157fb43ea4236f37d2286 Mon Sep 17 00:00:00 2001 From: The Original Barry Hughes <3594411+barryhughes@users.noreply.github.com> Date: Fri, 10 Jul 2026 19:21:52 -0700 Subject: [PATCH 8/8] style: address phpcs feedback Document the new $force parameter on process_action(); add the missing short description to ActionScheduler_UnrecognizedSchedule::__construct(); expand the single-line test closures and the associative array to satisfy the WordPress coding standard; and annotate the deliberate $_GET cleanup in a test. Refs #1318 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ActionScheduler_Abstract_QueueRunner.php | 2 ++ .../ActionScheduler_UnrecognizedSchedule.php | 2 ++ ...ionScheduler_UnrecognizedSchedule_Test.php | 35 +++++++++++++++---- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php b/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php index 14624d52..4d177965 100644 --- a/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php +++ b/classes/abstracts/ActionScheduler_Abstract_QueueRunner.php @@ -58,6 +58,8 @@ public function __construct( ?ActionScheduler_Store $store = null, ?ActionSchedu * @param int $action_id The action ID to process. * @param string $context Optional identifier for the context in which this action is being processed, e.g. 'WP CLI' or 'WP Cron' * Generally, this should be capitalised and not localised as it's a proper noun. + * @param bool $force Run the action even if it is not pending, and skip the unrecognized-schedule + * guard. Used by force_run_action() so an operator can flush stuck work. * @throws \Exception When error running action. */ public function process_action( $action_id, $context = '', $force = false ) { diff --git a/classes/schedules/ActionScheduler_UnrecognizedSchedule.php b/classes/schedules/ActionScheduler_UnrecognizedSchedule.php index 9caf61bb..28177f16 100644 --- a/classes/schedules/ActionScheduler_UnrecognizedSchedule.php +++ b/classes/schedules/ActionScheduler_UnrecognizedSchedule.php @@ -26,6 +26,8 @@ class ActionScheduler_UnrecognizedSchedule extends ActionScheduler_NullSchedule protected $unrecognized_classes = array(); /** + * Construct. + * * @param string[] $unrecognized_classes Class names from the blob that could not be vetted. */ public function __construct( array $unrecognized_classes = array() ) { diff --git a/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php index 0cb3e1a3..c6020086 100644 --- a/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php +++ b/tests/phpunit/jobstore/ActionScheduler_UnrecognizedSchedule_Test.php @@ -72,10 +72,20 @@ public function test_action_fetches_with_unrecognized_schedule_placeholder() { public function test_unrecognized_schedule_action_is_failed_not_run_or_cancelled() { $hook = 'as_test_unrecognized_auto'; $ran = 0; - add_action( $hook, function () use ( &$ran ) { ++$ran; } ); + add_action( + $hook, + function () use ( &$ran ) { + ++$ran; + } + ); $notified = false; - add_action( 'action_scheduler_unrecognized_schedule_action', function () use ( &$notified ) { $notified = true; } ); + add_action( + 'action_scheduler_unrecognized_schedule_action', + function () use ( &$notified ) { + $notified = true; + } + ); $store = new ActionScheduler_DBStore(); $runner = ActionScheduler_Mocker::get_queue_runner( $store ); @@ -98,7 +108,12 @@ public function test_unrecognized_schedule_action_is_failed_not_run_or_cancelled public function test_forced_run_executes_unrecognized_schedule_action() { $hook = 'as_test_unrecognized_forced'; $ran = 0; - add_action( $hook, function () use ( &$ran ) { ++$ran; } ); + add_action( + $hook, + function () use ( &$ran ) { + ++$ran; + } + ); $store = new ActionScheduler_DBStore(); $runner = ActionScheduler_Mocker::get_queue_runner( $store ); @@ -136,7 +151,12 @@ public function test_list_table_displays_unrecognized_schedule() { public function test_list_table_run_force_runs_a_failed_action() { $hook = 'as_test_unrecognized_listtable'; $ran = 0; - add_action( $hook, function () use ( &$ran ) { ++$ran; } ); + add_action( + $hook, + function () use ( &$ran ) { + ++$ran; + } + ); $store = new ActionScheduler_DBStore(); $runner = ActionScheduler_Mocker::get_queue_runner( $store ); @@ -178,7 +198,7 @@ public function test_notice_flag_is_set_and_dismissed() { $this->assertFalse( (bool) get_option( $option ), 'A valid dismissal should clear the notice flag.' ); - unset( $_GET['as_dismiss_unrecognized_schedule'], $_GET['_asnonce'] ); + unset( $_GET['as_dismiss_unrecognized_schedule'], $_GET['_asnonce'] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- test cleanup of simulated request state. } /** @@ -215,7 +235,10 @@ public function test_forced_run_of_recurring_failed_action_does_not_reschedule() $action_id = $store->save_action( $action ); $store->mark_failure( $action_id ); - $pending_query = array( 'hook' => $hook, 'status' => ActionScheduler_Store::STATUS_PENDING ); + $pending_query = array( + 'hook' => $hook, + 'status' => ActionScheduler_Store::STATUS_PENDING, + ); $this->assertSame( 0, (int) $store->query_actions( $pending_query, 'count' ) ); $runner->force_run_action( $action_id, 'Test' );