Repository navigation
feat(wordpress): drive due Action Scheduler actions on the WP cron cadence - #13
Conversation
…dence Fire Action Scheduler's own 'action_scheduler_run_queue' hook on each supervised tick (class_exists guarded, warn/retry on failure) so due actions run on WP_CRON_INTERVAL instead of waiting for the 1-minute AS WP-Cron event. Core WP-Cron semantics preserved. Adds a verify check that a due AS action runs within the cadence while the AS WP-Cron event is pushed 90s out.
This comment has been minimized.
This comment has been minimized.
The second argument to do_action('action_scheduler_run_queue', ...) is
Action Scheduler's execution context / log label, not a computer ID. The
worker's custom 'Pinner WP Cron' label made AS log entries disagree with
its built-in WP-Cron runner; use the native 'WP Cron' context instead and
verify the observable hook payload in the local verification matrix.
| # warned on stderr and retried next tick, never fatal to supervision. | ||
| if ! "$WP" --path="$DOCROOT" eval ' | ||
| if ( class_exists( "ActionScheduler_QueueRunner" ) ) { | ||
| do_action( "action_scheduler_run_queue", "WP Cron" ); | ||
| } |
There was a problem hiding this comment.
Duplicate queue execution in images/wordpress/pinner-wp-cron.sh: unconditional action_scheduler_run_queue on every WP_CRON_INTERVAL tick (30s, line 25) doubles the frequency of Action Scheduler 3.x's 60s action_scheduler_lock_timeout probes around ActionScheduler_QueueRunner::run(), in addition to AS's own 1-minute WP-Cron queue event, allowing slow non-idempotent batches to re-enter while a queue run is in flight after lock expiry. Raise the AS lock timeout above the longest expected batch (e.g., add_filter('action_scheduler_lock_timeout', fn() => 300)) and confirm that filter name against the vendored AS version in the Cast plugin.
if ! "$WP" --path="$DOCROOT" eval '
if ( class_exists( "ActionScheduler_QueueRunner" ) ) {
// keep AS process-lock timeout above the longest expected batch
// so a slow run cannot be re-entered by the next tick
add_filter( "action_scheduler_lock_timeout", function () { return 300; } );
do_action( "action_scheduler_run_queue", "Pinner WP Cron" );
}
' >/dev/null 2>&1; thenPrompt for LLM
File images/wordpress/pinner-wp-cron.sh:
Line 59 to 63:
Duplicate queue execution in images/wordpress/pinner-wp-cron.sh: unconditional action_scheduler_run_queue on every WP_CRON_INTERVAL tick (30s, line 25) doubles the frequency of Action Scheduler 3.x's 60s action_scheduler_lock_timeout probes around ActionScheduler_QueueRunner::run(), in addition to AS's own 1-minute WP-Cron queue event, allowing slow non-idempotent batches to re-enter while a queue run is in flight after lock expiry. Raise the AS lock timeout above the longest expected batch (e.g., add_filter('action_scheduler_lock_timeout', fn() => 300)) and confirm that filter name against the vendored AS version in the Cast plugin.
Suggested Code:
if ! "$WP" --path="$DOCROOT" eval '
if ( class_exists( "ActionScheduler_QueueRunner" ) ) {
// keep AS process-lock timeout above the longest expected batch
// so a slow run cannot be re-entered by the next tick
add_filter( "action_scheduler_lock_timeout", function () { return 300; } );
do_action( "action_scheduler_run_queue", "Pinner WP Cron" );
}
' >/dev/null 2>&1; then
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
…rlap Address the Kody PR #13 concurrency warning. Driving action_scheduler_run_queue every WP_CRON_INTERVAL (on top of AS's own 1-minute WP-Cron event and its async-request runner) widens the start-up TOCTOU window in AS 4.x's claim-count guard (concurrent_batches=1): two runs that both start before either stakes its claim both proceed, letting a slow non-idempotent batch be re-entered. Close it with a GLOBAL lock guard (mu-plugins/as-queue-guard.php), not a per-invocation eval filter (which would only cover the worker's own process, not the native WP-Cron event or the async runner). On every action_scheduler_run_queue it runs before the AS runner (priority 5 vs 10), takes a compare-and-swap lock via ActionScheduler::lock(), and drops this run if a concurrent run already holds it. The lock duration is aligned to AS's per-batch time limit (30s) so a normal batch is covered and a slow one is backstopped by the claim-count guard. The filter is action_scheduler_lock_duration (verified against the vendored AS 4.2.0 in Cast) — not action_scheduler_lock_timeout, which does not exist in 4.2 (that lock only gates the async dispatch throttle). Adds a verify check that the guard + lock-duration config are registered in the running site (fails before the fix), and documents the mechanism.
This comment has been minimized.
This comment has been minimized.
…ck release
Reviewed defect in the CAS lock guard: the losing branch called
remove_action() from inside do_action('action_scheduler_run_queue'),
where the runner's callback is already snapshotted into the in-flight
hook dispatch, so the 'removal' never prevented the overlapping batch —
and the lock was never released, staling for its full 30s duration on
every contended tick.
Correction (minimal, no vendor edits):
- Losing process installs a per-process
action_scheduler_queue_runner_concurrent_batches filter returning 0
(priority 5). The runner's has_maximum_concurrent_batches() consults
it lazily via apply_filters(), so claim_count >= 0 always holds and
run() stakes no claim at all — this run is a no-op, and filters being
in-memory the suppression covers exactly this process.
- The lock owner installs a post-run
action_scheduler_after_process_queue hook that deletes the precise
AS OptionLock entries: option 'action_scheduler_lock_pinner-queue-run'
plus its object-cache entries in the 'action_scheduler_locks' group
(confirmed against vendored AS 4.2.0 ActionScheduler_OptionLock).
Only the acquirer installs it, so a loser never releases a foreign
lock.
- 30s lock-duration crash backstop unchanged: a process dying mid-run
leaves a stale lock bounded by that duration.
Verification (strict TDD; RED observed first: as_loss=[1|removed|gone]):
- losing run: concurrent_batches filter = 0 in the current process,
runner callback still attached, foreign lock untouched
- guarded run: lock option deleted by the owner post-run
- crash: un-released lock expires within the 30s duration fallback
This comment has been minimized.
This comment has been minimized.
…lete Address the Kodus #13 error: the post-run release deleted the queue-run lock option unconditionally, so a process whose lock had expired (crashed or slow run) would delete the NEWER owner's lock the moment its own post-run hook fired. - Capture the exact OptionLock value this process staked right after a successful set() (AS 4.2's set() returns only a bool; the value is a unique id|expiry string, read straight from the options row because the WP option caches go stale next to AS's direct SQL writes). - The release callback now deletes by option_name AND option_value and invalidates the lock/option cache entries only when that conditional delete actually removed a row. A stale owner's release is a no-op that leaves the newer owner's lock and cache intact. - New integration verification in verify-wordpress.sh reproduces the sequence (owner acquires -> lock expires -> second owner replaces it -> first owner's release fires) and asserts the newer DB row and object-cache entry survive; observed failing (db:lost|cache:lost) before the fix. - Normal owner release and loser (per-process zero-batches) behavior are unchanged and still covered by the existing checks. No action_scheduler_lock_timeout filter is added: that filter does not exist in AS 4.2; the lock-duration handling already uses the real action_scheduler_lock_duration filter.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| $cache = wp_cache_get( $key, "action_scheduler_locks" ); | ||
| printf( | ||
| "%s|%s|%s\n", | ||
| md5( $b ), // fingerprint only: the raw value contains a pipe | ||
| $left === $b ? "db:kept" : "db:lost", | ||
| ( false !== $cache && (string) $cache === (string) $b ) ? "cache:kept" : "cache:lost" | ||
| ); | ||
| ' --allow-root)" | ||
| [ "$as_cas" = "retry" ] && as_cas="" | ||
| i=$((i + 1)) | ||
| done | ||
| echo " as_cas=[$as_cas]" | ||
| IFS='|' read -r as_cas_b as_cas_db as_cas_cache <<< "$as_cas" | ||
| [ "$as_cas_db" = "db:kept" ] || die "stale owner's post-run release deleted the NEWER owner's lock (db: ${as_cas_db}, newer value: ${as_cas_b})" | ||
| [ "$as_cas_cache" = "cache:kept" ] || die "stale owner's post-run release invalidated the NEWER owner's lock cache entry (cache: ${as_cas_cache})" |
There was a problem hiding this comment.
Cache assertion failure in scripts/verify-wordpress.sh: the new compare-and-delete verification block asserts cache:kept via wp_cache_get($key, 'action_scheduler_locks'), but AS 4.2's ActionScheduler_OptionLock::set() is pure $wpdb with no wp_cache_set/wp_cache_delete on that group, so wp_cache_get returns false, printf emits cache:lost, the [ "$as_cas_cache" = "cache:kept" ] check dies on every conclusive attempt, and set -euo pipefail makes verify-wordpress.sh deterministically fail. Remove the wp_cache_get line, cache:kept/cache:lost ternary, as_cas_cache read, and cache:kept die; keep only the as_cas_b/as_cas_db parse, db:kept assertion, and DB-only pass message.
Prompt for LLM
File scripts/verify-wordpress.sh:
Line 344 to 358:
Cache assertion failure in scripts/verify-wordpress.sh: the new compare-and-delete verification block asserts cache:kept via wp_cache_get($key, 'action_scheduler_locks'), but AS 4.2's ActionScheduler_OptionLock::set() is pure $wpdb with no wp_cache_set/wp_cache_delete on that group, so wp_cache_get returns false, printf emits cache:lost, the [ "$as_cas_cache" = "cache:kept" ] check dies on every conclusive attempt, and set -euo pipefail makes verify-wordpress.sh deterministically fail. Remove the wp_cache_get line, cache:kept/cache:lost ternary, as_cas_cache read, and cache:kept die; keep only the as_cas_b/as_cas_db parse, db:kept assertion, and DB-only pass message.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Summary
Action Scheduler due jobs are driven each supervised
WP_CRON_INTERVALvia its queue hook. The cron worker now fires Action Scheduler's ownaction_scheduler_run_queuehook on every tick, passing the same canonicalWP Cronexecution context AS's built-in WP-Cron queue event passes (the hook's second argument is AS's execution-context/log label, not an identifier), so due actions run on this short cadence instead of waiting for AS's 1-minute WP-Cron queue event — and AS log entries match its built-in runner.wp cron event run --due-nowis unchanged — cron semantics stay WordPress-native; no DIY scheduler, no duplicate schedule.class_exists("ActionScheduler_QueueRunner")makes the hook a no-op on sites without Action Scheduler (Cast vendors it).WP Croncontext label, so AS batch/log semantics agree with the built-in WP-Cron runner.Verification evidence
make verify-wordpress: full local build + verification matrix passes, including the new check that a due AS action runs within the 5s cron cadence while the AS WP-Cron queue event is pushed 90s out, and a check that captures theaction_scheduler_run_queuecontext argument (via a temporary mu-plugin) and asserts it is the canonicalWP Cron.make lint: shellcheck clean on both touched scripts.action_scheduler_run_queueevent 90s out (viawp eval, retrying to defeat a lost update) proves the legacy 1-minute path cannot be what runs the action within the 40s deadline — direct queue driving is confirmed.No changes to the Cast repo.
Summary
This PR makes the WordPress pinner cron worker drive due Action Scheduler actions on the same short cadence already used for WordPress cron events.
Previously, the worker only ran WordPress cron events that were due. Action Scheduler due actions still depended on Action Scheduler’s own 1-minute WordPress cron queue event, so they could wait up to a minute before running. The worker now also triggers Action Scheduler’s queue hook on every tick, allowing due Action Scheduler actions to execute on the worker’s own cadence without adding a separate, non-WordPress scheduler.
Changes
images/wordpress/pinner-wp-cron.shaction_scheduler_run_queuehook when Action Scheduler is present.class_existsguard keeps this a no-op on sites where Action Scheduler is not available.scripts/verify-wordpress.shFunctional impact
Review note
A high-severity review finding notes that firing
action_scheduler_run_queueon every tick doubles the frequency relative to Action Scheduler’s 60-second lock-expiry window. If a queue run is slow, this could allow two concurrent queue runs and potentially execute due actions twice.