fix(routines): hourly WP-Cron reconcile repair and one atomic shared option lock

agents-api.php

@@ -347,6 +347,7 @@
 require_once AGENTS_API_PATH . 'src/Workflows/register-action-scheduler-listener.php';
 require_once AGENTS_API_PATH . 'src/Routines/class-wp-agent-routine.php';
 require_once AGENTS_API_PATH . 'src/Routines/interface-wp-agent-routine-backend.php';
+require_once AGENTS_API_PATH . 'src/Routines/class-wp-agent-routine-option-lock.php';
 require_once AGENTS_API_PATH . 'src/Routines/class-wp-agent-routine-registry.php';
 require_once AGENTS_API_PATH . 'src/Routines/class-wp-agent-routine-action-scheduler-bridge.php';
 require_once AGENTS_API_PATH . 'src/Routines/register-routines.php';

docs/channels-workflows-operations.md

@@ -424,11 +424,11 @@ Failure reporting is uniform: a missing ability (`ability_missing`), a denied pe
 
 Durable scheduling sits behind a contract: `WP_Agent_Routine_Backend` (in `src/Routines/`) is the only scheduling surface the registry consumes — availability, register/repair/unregister, pause/resume, run-now, pause-state and generation reads, and `pending_by_routine()` / `cancel()` for reconcile (handles are opaque ints). `register()` is the cheap declaration entry point; `repair()` is the authoritative check-and-repair that never trusts recorded state. The registry resolves one backend per request through the `wp_agent_routine_backend` filter; the default is the Action Scheduler bridge (`WP_Agent_Routine_Action_Scheduler_Bridge`) when Action Scheduler is present, and `null` otherwise. With no backend, routines are still registered and their lifecycle hooks still fire, but nothing is scheduled and `reconcile()` reports a `_scheduler` error. Consumers replace the backend by filtering in their own `WP_Agent_Routine_Backend` implementation.
 
-`register()` is idempotent **and cheap to call on every boot, even for hundreds of persisted routines — and the cheap path touches Action Scheduler not at all**. The bridge keeps one persisted record of what it last scheduled: a single non-autoloaded option (`agents_routine_schedules`) mapping routine id → `['signature' => string, 'action_id' => int]`, where the signature is exactly what the matcher compares — trigger type plus interval seconds or cron expression. When a declaration's signature matches its record, `register()` returns before issuing a single `as_get_scheduled_actions()` query or `fetch_action()` call; reading the record costs one `get_option()` per request, independent of routine count. Only a missing or changed signature reaches the store, serialized through a short single-flight per-routine lock (`agents_routine_schedule_lock_<id>`, `add_option()`-based, 30-second stale takeover) so two concurrent processes can never both check-then-schedule — the race that used to mint duplicate chains; a losing caller no-ops. A consumer that must re-declare all of its routines on every request (the in-memory registry does not persist across requests) no longer pays any per-routine scheduler I/O once every record matches.
+`register()` is idempotent **and cheap to call on every boot, even for hundreds of persisted routines — and the cheap path touches Action Scheduler not at all**. The bridge keeps one persisted record of what it last scheduled: a single non-autoloaded option (`agents_routine_schedules`) mapping routine id → `['signature' => string, 'action_id' => int]`, where the signature is exactly what the matcher compares — trigger type plus interval seconds or cron expression. When a declaration's signature matches its record, `register()` returns before issuing a single `as_get_scheduled_actions()` query or `fetch_action()` call; reading the record costs one `get_option()` per request, independent of routine count. Only a missing or changed signature reaches the store, serialized through a short single-flight per-routine lock (`agents_routine_schedule_lock_<id>`, the same shared atomic `$wpdb` compare-and-set as the reconcile lock — see the shared lock helper below — with a 30-second stale takeover) so two concurrent processes can never both check-then-schedule — the race that used to mint duplicate chains; a losing caller no-ops. A consumer that must re-declare all of its routines on every request (the in-memory registry does not persist across requests) no longer pays any per-routine scheduler I/O once every record matches.
 
 Whenever a `register()` or reconcile call does reach Action Scheduler, it enforces **one chain per routine**: among the routine's pending recurring actions whose recurrence matches, exactly the lowest-id chain that is live (at or above the routine's current watermark) is kept and every other pending recurring action for that routine is cancelled by id — healing duplicate chains left by earlier versions or crashed requests. An action already claimed by an in-flight worker is not part of the pending set (Action Scheduler marks claimed actions in-progress), so a collapse never breaks a running worker. When no live matching chain exists (missing schedule, changed interval/expression, or a chain that is entirely below the current watermark), a fresh chain is scheduled, the generation/watermark fence is advanced, and the record is rewritten.
 
-The trade-off is deliberate: plain declarations trust the record, so an out-of-band deletion (pruned store, restored backup, manually deleted row) is invisible to them; `WP_Agent_Routine_Registry::reconcile()` — via the backend contract's `repair( $routine )` method, which never trusts recorded state — is what repairs that drift.
+The trade-off is deliberate: plain declarations trust the record, so an out-of-band deletion (pruned store, restored backup, manually deleted row) is invisible to them. A lost chain is restored within one hour by the WP-Cron routine repair (below), and on demand by `WP_Agent_Routine_Registry::reconcile()` — via the backend contract's `repair( $routine )` method, which never trusts recorded state.
 
 ### Routine generation + watermark fencing
 
@@ -444,12 +444,20 @@ Earlier versions stamped a generation onto every stored action individually, in
 
 Routines registered with the same interval would all fire in the same second. `WP_Agent_Routine` accepts `stagger => bool|int` (default `true` for interval routines, `false` for cron expressions, where the expression already *is* the slot; an int is an explicit max window in seconds). `WP_Agent_Routine::stagger_offset()` computes `crc32( 'agents_routine_stagger_' . $id ) % min( interval, max_window )`, capped by `WP_Agent_Routine::MAX_STAGGER_SECONDS` (one hour). The bridge adds the offset to the first-run timestamp. The offset depends only on the routine id, so re-registration always lands the routine back in the same slot — and, since the offset only affects the first-run timestamp and never the recurrence itself, it never causes an unchanged re-register to be mistaken for a schedule mismatch.
 
+### Hourly WP-Cron routine repair
+
+Agents API declares ONE WordPress cron event on `init`: hook `wp_agent_routines_reconcile`, recurrence `hourly` (in `src/Routines/register-routine-bridge-sync.php`, alongside the declaration listeners). The declaration is guarded by `wp_next_scheduled()`, so any number of `init` fires schedule exactly one event, and the guard itself reads the autoloaded `cron` option — it costs no queries per request. Firing the event runs `WP_Agent_Routine_Registry::reconcile()` in apply mode (the lock-protected path) and fires `wp_agent_routines_reconcile_completed` with the reconcile report (`enqueued`/`removed`/`unchanged`/`errors`) so consumers can log or alert on it.
+
+This is the scheduled counterpart to the record fast path above: since declarations trust the persisted schedule record, a chain lost out of band (cancelled in the Action Scheduler admin UI, a pruned or restored table) stays lost until something authoritative runs. The repair lives in WP-Cron rather than Action Scheduler on purpose — losing Action Scheduler chains cannot also take out the mechanism that restores them — and it runs through `$wpdb`, not the options API, so a stale or cached-away lock row can never wedge it. There is deliberately no unschedule-on-deactivation hook and no self-repair machinery for the event itself: agents-api ships far more often as a Composer-embedded library inside a consumer plugin than as its own activatable plugin, so there is no deactivation path that would fire for most real installs, and a leftover event after removal is inert.
+
 ### Routine reconcile
 
 `WP_Agent_Routine_Registry::reconcile( array $opts = [] )` repairs drift between the registry and the Action Scheduler store. For every registered, non-paused routine it calls the backend's authoritative `repair()`: pending-action coverage is verified against the real store by logical identity (never against the schedule record), duplicate matching chains are collapsed to one live chain, a missing or entirely-stale schedule is re-enqueued, and the schedule record is (re)written so later declarations fast-path again; pending routine actions whose logical `routine_id` is not registered (or is durably paused) are unscheduled as orphans. It returns `array( 'enqueued' => [ids], 'removed' => [ids], 'unchanged' => [ids], 'errors' => [id => message] )`; `$opts['dry_run']` reports the same shape without writing. `pending_by_routine()` (the one bulk read reconcile uses) always forces a fresh fetch of the Action Scheduler store — reconcile's entire purpose is authoritative drift detection, so trusting recorded state would defeat the point; that fresh read also refreshes the request-scoped map `repair()` walks, so a reconcile run pays exactly one bulk read.
 
 The run is serialized through a `$wpdb`-level compare-and-set lock (`agents_routine_reconcile_lock`; a single `INSERT ... ON DUPLICATE KEY UPDATE` against `wp_options`) that never trusts `get_option()`/the options cache for the decision — only the real DB row. A lock older than five minutes is treated as stale and taken over atomically in the same query, and the lock is always released (a direct `DELETE`, again bypassing the cache) in a `finally`. This closes a production failure mode where a killed request left the lock option cached (e.g. in Redis) with no backing DB row: `add_option()` always failed because the cache said the option existed, the stale check read that same cached value, and `delete_option()` found nothing in the DB to delete — the lock stayed stuck "held" until the cache entry was manually cleared. Reading and writing exclusively through `$wpdb`, with an explicit `wp_cache_delete()` after every write, means a stale cache entry with no backing row can never block acquisition.
 
+Both routine locks — this one and the bridge's per-routine schedule lock — share one internal helper, `AgentsAPI\AI\Routines\WP_Agent_Routine_Option_Lock` (`acquire( $option_name, $ttl )` / `release( $option_name )`): one atomic query decides insert, stale takeover, or "someone fresh holds it", so two processes that both observe the same stale lock cannot both take it over — only the first write changes the row and the second reports zero affected rows.
+
 Pause state is durable: the bridge maintains the `agents_routine_paused` option so reconcile can distinguish "unscheduled on purpose" from "missing by drift" across requests, even though the registry itself stays in-memory. `unregister()` and `pause()` also drop the routine's entry from the `agents_routine_schedules` record, so a later `register()`/`resume()` cannot fast-path on a signature whose action no longer exists.
 
 The `agents/reconcile-routines` ability (`show_in_rest: true`, annotations `destructive: true, idempotent: true`) exposes the same operation to ability consumers: input `{ dry_run?: bool }`, output the reconcile report, permission `current_user_can( 'manage_options' )` filterable via `agents_reconcile_routines_permission`.

src/Routines/class-wp-agent-routine-action-scheduler-bridge.php

@@ -811,39 +811,22 @@ private function cancel_superseded( int $keep_id, array $existing, bool $recurri
 	 * schedule step must not run concurrently in two processes, or both see
 	 * "no matching chain" and both schedule one (the duplicate-chain race).
 	 *
-	 * An `add_option()`-based lock: the insert succeeds only when no lock row
-	 * exists, so acquisition is atomic in MySQL against `option_name`'s
-	 * unique key. A lock older than {@see SCHEDULE_LOCK_TTL} is stale (its
-	 * holder crashed mid-schedule) and is taken over. With no option layer at
-	 * all (pure-PHP harnesses) callers are already single-process and the
-	 * lock is a no-op success.
+	 * Delegates to the shared routines option lock: one atomic `$wpdb`
+	 * compare-and-set that inserts the lock row when absent and takes over a
+	 * lock older than {@see SCHEDULE_LOCK_TTL} in the same query — never
+	 * through the options API, whose cache could report a lock as held with
+	 * no backing DB row (or hand a stale takeover to two processes at once,
+	 * the flaw of the plain `update_option()` takeover this replaces).
 	 */
 	private function acquire_schedule_lock( string $routine_id ): bool {
-		if ( ! self::has_option_layer() ) {
-			return true;
-		}
-
-		$name = self::SCHEDULE_LOCK_OPTION_PREFIX . $routine_id;
-		if ( add_option( $name, time(), '', false ) ) {
-			return true;
-		}
-
-		$held_at = get_option( $name, 0 );
-		if ( is_numeric( $held_at ) && ( time() - (int) $held_at ) < self::SCHEDULE_LOCK_TTL ) {
-			return false; // Freshly held by another process.
-		}
-
-		// Stale (or unreadable): take over so a crashed holder cannot strand
-		// the routine unscheduled.
-		update_option( $name, time(), false );
-		return true;
+		return WP_Agent_Routine_Option_Lock::acquire(
+			self::SCHEDULE_LOCK_OPTION_PREFIX . $routine_id,
+			self::SCHEDULE_LOCK_TTL
+		);
 	}
 
 	private function release_schedule_lock( string $routine_id ): void {
-		if ( ! self::has_option_layer() ) {
-			return;
-		}
-		delete_option( self::SCHEDULE_LOCK_OPTION_PREFIX . $routine_id );
+		WP_Agent_Routine_Option_Lock::release( self::SCHEDULE_LOCK_OPTION_PREFIX . $routine_id );
 	}
 
 	/**

src/Routines/class-wp-agent-routine-option-lock.php

@@ -0,0 +1,130 @@
+<?php
+/**
+ * Atomic, TTL-based option lock shared by the routines subsystem.
+ *
+ * One small internal helper for the two single-flight paths in `src/Routines/`:
+ * {@see WP_Agent_Routine_Registry}'s whole-registry reconcile lock and the
+ * Action Scheduler bridge's per-routine schedule lock. Both need the same
+ * guarantee — at most one process inside the critical section at a time,
+ * with a crashed holder's lock taken over after a TTL — and both must make
+ * the decision atomically against the real `wp_options` row.
+ *
+ * @package AgentsAPI
+ * @since   0.17.0
+ */
+
+namespace AgentsAPI\AI\Routines;
+
+defined( 'ABSPATH' ) || exit;
+
+final class WP_Agent_Routine_Option_Lock {
+
+	/**
+	 * Acquire a named lock via a single atomic `$wpdb` compare-and-set —
+	 * `INSERT ... ON DUPLICATE KEY UPDATE` against `wp_options` directly,
+	 * never through `get_option()`/`add_option()`.
+	 *
+	 * This closes a production failure mode: a killed request can leave a
+	 * lock option as a persistent object-cache entry (e.g. Redis) with no
+	 * backing DB row. `add_option()` then always fails (the cache says the
+	 * option exists), a stale check reading through the cache sees the same
+	 * cached value, and `delete_option()` finds nothing in the DB to delete
+	 * — the lock stays stuck "held" indefinitely. Reading and writing
+	 * exclusively through `$wpdb` — and never consulting
+	 * `get_option()`/the options cache for the decision — means a stale
+	 * cache entry with no DB row cannot block acquisition: MySQL's
+	 * `ON DUPLICATE KEY` path only ever sees the real row (or its absence).
+	 *
+	 * The single query handles all three cases atomically against
+	 * `option_name`'s unique key:
+	 *  - no row exists: plain insert, 1 affected row;
+	 *  - row exists and is stale (older than `$ttl`): `ON DUPLICATE KEY
+	 *    UPDATE` overwrites it, MySQL reports 2 affected rows for a changed
+	 *    row — and because the takeover itself is the atomic compare-and-set,
+	 *    two processes that both observe the same stale lock cannot both take
+	 *    it over: only the first write changes the row, and the second sees a
+	 *    fresh value and reports 0 affected rows;
+	 *  - row exists and is fresh: the `IF()` leaves the value unchanged,
+	 *    MySQL reports 0 affected rows for a no-op update.
+	 *
+	 * `$wpdb->query()` returns the raw affected-rows count for a non-SELECT
+	 * query, so `> 0` is "we now hold the lock" and `0` is "someone else
+	 * holds it, and it is not stale".
+	 *
+	 * When there is no `$wpdb` layer at all (pure-PHP test harnesses) the
+	 * caller is already single-process and the lock is a no-op success.
+	 *
+	 * @since 0.17.0
+	 *
+	 * @param string $option_name wp_options row holding the lock timestamp.
+	 * @param int    $ttl         Seconds after which a held lock is treated
+	 *                            as stale and may be taken over.
+	 * @return bool True when the lock is now held by this process.
+	 */
+	public static function acquire( string $option_name, int $ttl ): bool {
+		global $wpdb;
+		if ( ! $wpdb instanceof \wpdb ) {
+			return true;
+		}
+
+		$now          = time();
+		$stale_before = $now - $ttl;
+
+		$query = $wpdb->prepare(
+			'INSERT INTO %i (option_name, option_value, autoload)
+			VALUES (%s, %s, %s)
+			ON DUPLICATE KEY UPDATE option_value = IF( CAST( option_value AS UNSIGNED ) <= %d, VALUES(option_value), option_value )',
+			$wpdb->options,
+			$option_name,
+			(string) $now,
+			'no',
+			$stale_before
+		);
+		if ( ! is_string( $query ) ) {
+			return false;
+		}
+
+		// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- $query is built by $wpdb->prepare() above.
+		$affected = $wpdb->query( $query );
+
+		self::flush_cache( $option_name );
+
+		return is_int( $affected ) && $affected > 0;
+	}
+
+	/**
+	 * Release a named lock previously taken by {@see acquire()}: delete the
+	 * backing row directly (bypassing the options cache) and evict any cache
+	 * entry, so a later read through `get_option()` never observes a value
+	 * the DB no longer has.
+	 *
+	 * @since 0.17.0
+	 *
+	 * @param string $option_name wp_options row holding the lock timestamp.
+	 */
+	public static function release( string $option_name ): void {
+		global $wpdb;
+		if ( ! $wpdb instanceof \wpdb ) {
+			return;
+		}
+
+		$query = $wpdb->prepare( 'DELETE FROM %i WHERE option_name = %s', $wpdb->options, $option_name );
+		if ( is_string( $query ) ) {
+			// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- $query is built by $wpdb->prepare() above.
+			$wpdb->query( $query );
+		}
+
+		self::flush_cache( $option_name );
+	}
+
+	/**
+	 * Explicitly evict the lock option from the object cache after every
+	 * direct `$wpdb` write, so anything that does happen to read it via
+	 * `get_option()` never observes a value the DB no longer has.
+	 */
+	private static function flush_cache( string $option_name ): void {
+		if ( function_exists( 'wp_cache_delete' ) ) {
+			wp_cache_delete( $option_name, 'options' );
+		}
+	}
+}