Skip to content

Fix/clean uses scheduled date 1107 (wp action-scheduler clean filtering by last_attempt_gmt instead of scheduled_date_gmt) #1348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MagicGeny wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

Fix/clean uses scheduled date 1107 (wp action-scheduler clean filtering by last_attempt_gmt instead of scheduled_date_gmt) #1348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e0f8e3
Fix default 'before' date in WP-CLI clean command #1343
MagicGeny Jul 12, 2026
cecad23
Fix: "clean" command works incorrectly #1107
MagicGeny Jul 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions classes/ActionScheduler_QueueCleaner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ public function delete_old_actions() {
*
* @return array Actions deleted.
*/
public function clean_actions( array $statuses_to_purge, DateTime $cutoff_date, $batch_size = null, $context = 'old' ) {
public function clean_actions( array $statuses_to_purge, DateTime $cutoff_date, $batch_size = null, $context = 'old', $date_type = 'modified' ) {
$batch_size = ! is_null( $batch_size ) ? $batch_size : $this->batch_size;
$cutoff = ! is_null( $cutoff_date ) ? $cutoff_date : as_get_datetime_object( $this->month_in_seconds . ' seconds ago' );
$lifespan = time() - $cutoff->getTimestamp();
Expand All @@ -198,6 +198,9 @@ public function clean_actions( array $statuses_to_purge, DateTime $cutoff_date,
// For inline cleanup during a queue run, the batch size should remain unchanged to avoid increasing the process footprint.
$is_scheduled_cleanup = doing_action( self::RUN_SCHEDULED_CLEANER_HOOK )
|| doing_action( self::CONTINUE_SCHEDULED_CLEANER_HOOK );
if ( $is_scheduled_cleanup ) {
$date_type = 'modified';
}
// 250 balances replication safety, backlog clearance speed, and claim slot duration on high-volume stores.
$iteration_batch_size = $is_scheduled_cleanup ? max( 250, $batch_size ) : $batch_size;
$iteration_unused_budget = 0;
Expand Down Expand Up @@ -231,15 +234,19 @@ static function( $a, $b ) {
$deleted_actions = array();
foreach ( $statuses_to_purge as $status ) {
$iteration_execution_budget = $iteration_batch_size + $iteration_unused_budget;
$actions_to_delete = $this->store->query_actions(
array(
'status' => $status,
'modified' => $cutoff,
'modified_compare' => '<=',
'per_page' => $iteration_execution_budget,
'orderby' => 'none',
)
$query = array(
'status' => $status,
'per_page' => $iteration_execution_budget,
'orderby' => 'none',
);
if ( 'date' === $date_type ) {
$query['date'] = $cutoff;
$query['date_compare'] = '<=';
} else {
$query['modified'] = $cutoff;
$query['modified_compare'] = '<=';
}
$actions_to_delete = $this->store->query_actions( $query );
$deleted_actions[] = $this->delete_actions( $actions_to_delete, $lifespan, $context );

$fetched_actions_count = count( $actions_to_delete );
Expand Down
4 changes: 2 additions & 2 deletions classes/WP_CLI/ActionScheduler_WPCLI_Clean_Command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function clean( $args, $assoc_args ) {
$batches = absint( \WP_CLI\Utils\get_flag_value( $assoc_args, 'batches', 0 ) );
$status = explode( ',', WP_CLI\Utils\get_flag_value( $assoc_args, 'status', '' ) );
$status = array_filter( array_map( 'trim', $status ) );
$before = \WP_CLI\Utils\get_flag_value( $assoc_args, 'before', '' );
$before = \WP_CLI\Utils\get_flag_value( $assoc_args, 'before', '31 days ago' );
$sleep = \WP_CLI\Utils\get_flag_value( $assoc_args, 'pause', 0 );

$batches_completed = 0;
Expand All @@ -58,7 +58,7 @@ public function clean( $args, $assoc_args ) {
sleep( $sleep );
}

$deleted = count( $cleaner->clean_actions( $status, $lifespan, null, 'CLI' ) );
$deleted = count( $cleaner->clean_actions( $status, $lifespan, null, 'CLI', 'date' ) );
if ( $deleted <= 0 ) {
break;
}
Expand Down
5 changes: 5 additions & 0 deletions classes/data-stores/ActionScheduler_DBLogger.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ public function init() {
* @param int $action_id Action ID.
*/
public function clear_deleted_action_logs( $action_id ) {
if ( defined( 'WP_CLI' ) && WP_CLI ) {
global $wpdb;
$wpdb->delete( $wpdb->actionscheduler_logs, array( 'action_id' => $action_id ), array( '%d' ) );
return;
}
$this->clear_deleted_action_logs_single_batch( $action_id, -1, 1, 4000 );
}

Expand Down
9 changes: 9 additions & 0 deletions classes/data-stores/ActionScheduler_DBStore.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,12 +872,21 @@ public function delete_action( $action_id ) {
*/
global $wpdb;

$claim_id = (int) $wpdb->get_var( $wpdb->prepare( "SELECT claim_id FROM {$wpdb->actionscheduler_actions} WHERE action_id = %d", $action_id ) );

$deleted = $wpdb->delete( $wpdb->actionscheduler_actions, array( 'action_id' => $action_id ), array( '%d' ) );
if ( empty( $deleted ) ) {
/* translators: %s is the action ID */
throw new \InvalidArgumentException( sprintf( __( 'Unidentified action %s: we were unable to delete this action. It may may have been deleted by another process.', 'action-scheduler' ), $action_id ) );
}
do_action( 'action_scheduler_deleted_action', $action_id );

if ( $claim_id > 0 ) {
$referenced = (int) $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(*) FROM {$wpdb->actionscheduler_actions} WHERE claim_id = %d", $claim_id ) );
if ( 0 === $referenced ) {
$wpdb->delete( $wpdb->actionscheduler_claims, array( 'claim_id' => $claim_id ), array( '%d' ) );
}
}
}

/**
Expand Down
20 changes: 20 additions & 0 deletions classes/data-stores/ActionScheduler_wpCommentLogger.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,24 @@ class ActionScheduler_wpCommentLogger extends ActionScheduler_Logger {
const AGENT = 'ActionScheduler';
const TYPE = 'action_log';

public function clear_deleted_action_logs( $action_id ) {
global $wpdb;
$wpdb->query(
$wpdb->prepare(
"DELETE cm FROM {$wpdb->commentmeta} cm INNER JOIN {$wpdb->comments} c ON c.comment_ID = cm.comment_id WHERE c.comment_post_ID = %d AND c.comment_type = %s",
$action_id,
self::TYPE
)
);
$wpdb->query(
$wpdb->prepare(
"DELETE FROM {$wpdb->comments} WHERE comment_post_ID = %d AND comment_type = %s",
$action_id,
self::TYPE
)
);
}

/**
* Create log entry.
*
Expand Down Expand Up @@ -255,6 +273,8 @@ public function init() {
add_action( 'action_scheduler_after_process_queue', array( $this, 'enable_comment_counting' ), 10, 0 );

parent::init();
add_action( 'action_scheduler_deleted_action', array( $this, 'clear_deleted_action_logs' ), 10, 1 );
add_action( 'action_scheduler_canceled_corrupted_action', array( $this, 'clear_deleted_action_logs' ), 10, 1 );

add_action( 'pre_get_comments', array( $this, 'filter_comment_queries' ), 10, 1 );
add_action( 'wp_count_comments', array( $this, 'filter_comment_count' ), 20, 2 ); // run after WC_Comments::wp_count_comments() to make sure we exclude order notes and action logs.
Expand Down
168 changes: 168 additions & 0 deletions tests/phpunit/runner/ActionScheduler_QueueCleaner_Test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,4 +586,172 @@ static function( $query ) {
remove_action( 'action_scheduler_run_actions_cleanup_hook', array( $cleaner, 'delete_old_actions' ) );
remove_action( 'action_scheduler_continue_actions_cleanup_hook', array( $cleaner, 'delete_old_actions' ) );
}

/**
* Helper to create a scenario with two completed actions scheduled 32 days ago,
* where Action B has a recent last_attempt_gmt to simulate a retried action.
*
* @return array{store: ActionScheduler_Store, action_a_id: int, action_b_id: int}
*/
private function create_cli_scenario() {
$store = ActionScheduler::store();
ActionScheduler_Callbacks::add_callbacks();
$scheduled_date_old = as_get_datetime_object( '-32 days' );

$action_a = new ActionScheduler_Action(
ActionScheduler_Callbacks::HOOK_WITH_CALLBACK,
array( md5( wp_rand() ) ),
new ActionScheduler_SimpleSchedule( $scheduled_date_old )
);
$action_a_id = $store->save_action( $action_a );
$store->mark_complete( $action_a_id );
$this->force_last_attempt_date( $action_a_id, $scheduled_date_old );

$action_b = new ActionScheduler_Action(
ActionScheduler_Callbacks::HOOK_WITH_CALLBACK,
array( md5( wp_rand() ) ),
new ActionScheduler_SimpleSchedule( $scheduled_date_old )
);
$action_b_id = $store->save_action( $action_b );
$store->mark_complete( $action_b_id );
$this->force_last_attempt_date( $action_b_id, as_get_datetime_object( '-1 hour' ) );

return array(
'store' => $store,
'action_a_id' => $action_a_id,
'action_b_id' => $action_b_id,
);
}

/**
* Regression test for #1107: clean_actions() with $date_type = 'date' deletes actions based on
* scheduled_date_gmt, so even actions with a recent last_attempt_gmt are correctly cleaned up.
*/
public function test_clean_actions_uses_scheduled_date_when_date_type_is_date() {
$scenario = $this->create_cli_scenario();
$store = $scenario['store'];
$action_a_id = $scenario['action_a_id'];
$action_b_id = $scenario['action_b_id'];

$cleaner = new ActionScheduler_QueueCleaner( $store );
$cutoff_date = as_get_datetime_object( '-31 days' );
$deleted = $cleaner->clean_actions(
array( ActionScheduler_Store::STATUS_COMPLETE ),
$cutoff_date,
null,
'CLI',
'date'
);

$this->assertContains( $action_a_id, $deleted, 'Action A (old modified) should be deleted.' );
$this->assertContains( $action_b_id, $deleted, 'Action B (recent modified) SHOULD be deleted when using scheduled date.' );
$this->assertCount( 2, $deleted, 'Both actions should be deleted when using date-based filtering.' );

$this->assertInstanceOf( ActionScheduler_NullAction::class, $store->fetch_action( $action_a_id ) );
$this->assertInstanceOf( ActionScheduler_NullAction::class, $store->fetch_action( $action_b_id ) );
}

/**
* Regression test for #1107: clean_actions() with $date_type = 'modified' skips
* actions with a recent last_attempt_gmt. This demonstrates the original bug.
*/
public function test_clean_actions_with_modified_date_misses_recently_touched_actions() {
$scenario = $this->create_cli_scenario();
$store = $scenario['store'];
$action_a_id = $scenario['action_a_id'];
$action_b_id = $scenario['action_b_id'];

$cleaner = new ActionScheduler_QueueCleaner( $store );
$cutoff_date = as_get_datetime_object( '-31 days' );
$deleted = $cleaner->clean_actions(
array( ActionScheduler_Store::STATUS_COMPLETE ),
$cutoff_date,
null,
'CLI',
'modified'
);

$this->assertContains( $action_a_id, $deleted, 'Action A (old both dates) should be deleted.' );
$this->assertNotContains( $action_b_id, $deleted, 'Action B (recent last_attempt) escapes deletion — this is the bug #1107.' );
$this->assertCount( 1, $deleted, 'Only Action A is deleted. Action B escapes modified-based filtering.' );
$this->assertNotInstanceOf( ActionScheduler_NullAction::class, $store->fetch_action( $action_b_id ) );
}

/**
* Scheduled (cron) cleanup always forces 'modified' date type, even if 'date' was requested externally.
*/
public function test_scheduled_cleanup_uses_modified_date() {
$store = ActionScheduler::store();
ActionScheduler_Callbacks::add_callbacks();

$action = new ActionScheduler_Action(
ActionScheduler_Callbacks::HOOK_WITH_CALLBACK,
array( md5( wp_rand() ) ),
new ActionScheduler_SimpleSchedule( as_get_datetime_object( '-32 days' ) )
);
$action_id = $store->save_action( $action );
$store->mark_complete( $action_id );
$this->force_last_attempt_date( $action_id, as_get_datetime_object( '-5 minutes' ) );

$cleaner = new ActionScheduler_QueueCleaner( $store );
$cleaner->register_cleaner_hooks();
do_action( 'action_scheduler_run_actions_cleanup_hook' );

$fetched_action = $store->fetch_action( $action_id );
$this->assertNotInstanceOf( ActionScheduler_NullAction::class, $fetched_action, 'Scheduled cleanup should NOT delete actions with recent last_attempt.' );
$this->assertInstanceOf( ActionScheduler_FinishedAction::class, $fetched_action, 'The action should still exist as a FinishedAction.' );
}

/**
* Verify orphan claims are cleaned up when deleting an action.
*/
public function test_orphan_claim_cleaned_on_action_delete() {
$store = ActionScheduler::store();
$schedule = new ActionScheduler_SimpleSchedule( as_get_datetime_object( '-32 days' ) );

$action_a_id = $store->save_action( new ActionScheduler_Action( 'hook_a', array(), $schedule ) );
$action_b_id = $store->save_action( new ActionScheduler_Action( 'hook_b', array(), $schedule ) );

$claim = $store->stake_claim( 10 );
$claim_id = $claim->get_id();

$store->delete_action( $action_a_id );

global $wpdb;
$claims_after_a = (int) $wpdb->get_var(
$wpdb->prepare( "SELECT COUNT(*) FROM {$wpdb->actionscheduler_claims} WHERE claim_id = %d", $claim_id )
);
$this->assertEquals( 1, $claims_after_a, 'Claim should still exist since action B references it.' );

$store->delete_action( $action_b_id );

$claims_after_b = (int) $wpdb->get_var(
$wpdb->prepare( "SELECT COUNT(*) FROM {$wpdb->actionscheduler_claims} WHERE claim_id = %d", $claim_id )
);
$this->assertEquals( 0, $claims_after_b, 'Orphan claim should be deleted when its last referencing action is deleted.' );
}

/**
* Force the last_attempt_gmt and last_attempt_local columns to a specific datetime.
*
* @param int $action_id Action ID.
* @param DateTime $date The date to set.
*/
private function force_last_attempt_date( $action_id, DateTime $date ) {
global $wpdb;
$date->setTimezone( new DateTimeZone( 'UTC' ) );
$gmt = $date->format( 'Y-m-d H:i:s' );
$local = get_date_from_gmt( $gmt );

$wpdb->update(
$wpdb->actionscheduler_actions,
array(
'last_attempt_gmt' => $gmt,
'last_attempt_local' => $local,
),
array( 'action_id' => $action_id ),
array( '%s', '%s' ),
array( '%d' )
);
}
}