Skip to content

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

Open
MagicGeny wants to merge 2 commits into
woocommerce:trunkfrom
MagicGeny:fix/clean-uses-scheduled-date-1107
Open

Fix/clean uses scheduled date 1107 (wp action-scheduler clean filtering by last_attempt_gmt instead of scheduled_date_gmt)#1348
MagicGeny wants to merge 2 commits into
woocommerce:trunkfrom
MagicGeny:fix/clean-uses-scheduled-date-1107

Conversation

@MagicGeny

Copy link
Copy Markdown

Problem

The wp action-scheduler clean command was incorrectly filtering actions by their last modified date (last_attempt_gmt) instead of their scheduled date (scheduled_date_gmt).

The --before parameter in the CLI command is documented as referring to the scheduled date — e.g. --before='31 days ago' should delete actions scheduled more than 31 days ago. However, the query was using last_attempt_gmt, so any action that had been retried or modified recently would escape cleanup even if it was scheduled far beyond the retention period.

Example of the bug:

  • Action A: scheduled 60 days ago, last ran 60 days ago → correctly identified for deletion.
  • Action B: scheduled 60 days ago, but retried 1 hour ago (recent last_attempt_gmt) → incorrectly kept, even though it's well past the retention period.

This meant that over time, a growing backlog of old actions would accumulate in the database if they had ever been retried or re-scheduled after their initial run.

Root Cause

ActionScheduler_QueueCleaner::clean_actions() always queried actions using the modified field (last_attempt_gmt), regardless of who called it. Both the scheduled (cron) cleanup and the WP-CLI command used the same modified-based filtering.

The WP-CLI clean command should use the scheduled date (scheduled_date_gmt) since the --before parameter refers to when the action was scheduled to run, not when it last executed.

Changes

1. classes/ActionScheduler_QueueCleaner.php

  • Added $date_type parameter to clean_actions() with two modes:
    • 'modified' (default) — filters by last_attempt_gmt. Used by scheduled cron cleanup.
    • 'date' — filters by scheduled_date_gmt. Used by WP-CLI clean command.
  • Scheduled cleanup (delete_old_actions()) always forces $date_type = 'modified', preserving backward compatibility and the existing execution budget optimization.
  • Updated PHPDoc for the new parameter.

2. classes/WP_CLI/ActionScheduler_WPCLI_Clean_Command.php

  • The clean() method now passes 'date' as the 5th argument to $cleaner->clean_actions(), so the CLI correctly filters by scheduled date.

3. classes/data-stores/ActionScheduler_DBStore.php

  • When deleting an action, the store now checks the action's claim_id before deletion.
  • After deletion, if no other actions reference that claim, the orphan claim is automatically deleted from {$wpdb->actionscheduler_claims}.
  • This prevents orphaned claims from accumulating in the database.

4. classes/data-stores/ActionScheduler_DBLogger.php

  • In WP-CLI context (defined('WP_CLI') && WP_CLI), clear_deleted_action_logs() now performs a direct DELETE query instead of the batched approach using scheduled actions.
  • This avoids the problem of scheduling follow-up actions during CLI cleanup, which could cause the CLI process to exit before logs are fully purged.

5. classes/data-stores/ActionScheduler_wpCommentLogger.php

  • Added clear_deleted_action_logs() method that cleans up both commentmeta and comments records for deleted actions.
  • Registered via action_scheduler_deleted_action and action_scheduler_canceled_corrupted_action hooks.
  • Previously, the comment-based logger never cleaned up logs when actions were deleted.

Testing Instructions

Automated Tests (4 new regression tests in ActionScheduler_QueueCleaner_Test.php)

composer run test -- --filter=ActionScheduler_QueueCleaner_Test


The new tests are:

1. __`test_clean_actions_uses_scheduled_date_when_date_type_is_date`__ — Verifies that with `$date_type = 'date'`, both actions (even one with recent `last_attempt_gmt`) are correctly deleted based on scheduled date.
2. __`test_clean_actions_with_modified_date_misses_recently_touched_actions`__ — Demonstrates the original bug: filtering by `modified` causes recently-retried actions to escape cleanup.
3. __`test_scheduled_cleanup_uses_modified_date`__ — Verifies scheduled cron cleanup still uses `modified` date (backward compatibility).
4. __`test_orphan_claim_cleaned_on_action_delete`__ — Verifies orphan claims are cleaned up when their last referencing action is deleted.

#### Manual Test

1. Schedule two actions 32 days ago and mark them complete:

```php
as_schedule_single_action( strtotime('-32 days'), 'my_hook', array(), 'test' );
as_schedule_single_action( strtotime('-32 days'), 'my_hook', array(), 'test' );
  1. Manually update one action's last_attempt_gmt to simulate a recent retry:
UPDATE wp_actionscheduler_actions
SET last_attempt_gmt = UTC_TIMESTAMP(),
    last_attempt_local = NOW()
WHERE action_id = <second_action_id>;
  1. Run the clean command:
wp action-scheduler clean --before='31 days ago' --batch-size=100
  1. Before fix: Only the first action is deleted. The second action remains.
  2. After fix: Both actions are deleted.

Regression Test

Verify that scheduled (cron) cleanup still works as expected:

  • Create a completed action with a recent last_attempt_gmt (within the last hour).
  • Run wp cron event run action_scheduler_run_actions_cleanup_hook.
  • The action should not be deleted (scheduled cleanup uses modified date).

Related

@MagicGeny

Copy link
Copy Markdown
Author

Hi @barryhughes!

I have created this PR to address the issues with the wp action-scheduler clean command (#1107).

Since this bugfix relies on the default value fix from my previous task (#1343 / #1346), I have branched this PR from that work to ensure everything functions correctly together.

I have also written 4 new automated regression tests to cover these changes and ran them successfully in my local environment.

Could you please take a look when you have a moment? I would really appreciate your feedback on whether this approach looks good to you, or if we should merge the previous default-value fix first.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"clean" command works incorrectly

1 participant