From f286ec04fd557623432212b62e38138a51391d3f Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 13:58:36 +0000 Subject: [PATCH 01/11] fix: retry branch reconciliation without re-execution --- ...kflow-action-scheduler-branch-executor.php | 85 +++++++++++++--- .../class-wp-agent-workflow-branch-store.php | 96 +++++++++++++++++++ ...s-wp-agent-workflow-request-controller.php | 2 +- .../class-wp-agent-workflow-scoped-drain.php | 1 + .../register-workflow-branch-executor.php | 17 +++- tests/workflow-as-branch-smoke.php | 21 +++- tests/workflow-request-controller-smoke.php | 2 +- tests/workflow-scoped-drain-smoke.php | 5 +- 8 files changed, 205 insertions(+), 24 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index 5d4c5d5..aaa8d78 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -57,6 +57,14 @@ final class WP_Agent_Workflow_Action_Scheduler_Branch_Executor implements WP_Age */ public const BRANCH_HOOK = 'wp_agent_workflow_branch_run'; + /** + * Reconcile-only retry hook. Its payload references a persisted terminal + * BranchResult, so this callback never executes branch steps. + * + * @since 0.7.0 + */ + public const RECONCILE_HOOK = 'wp_agent_workflow_branch_reconcile'; + /** * The resume action hook. When a reconcile observes all branches terminal it * enqueues ONE action under this hook rather than resuming inline; AS claims @@ -772,39 +780,90 @@ public static function run_branch_action( array $payload ): void { ), 'item' => null, ); - self::reconcile_branch_result( $payload, $branch_result ); + self::persist_and_reconcile_branch_result( $payload, $branch_result ); return; } $key = self::string_value( $descriptor['key'] ?? '' ); $branch_result = self::execute_branch( $descriptor, $key ); - self::reconcile_branch_result( $payload, $branch_result ); + self::persist_and_reconcile_branch_result( $payload, $branch_result ); } /** - * Reconcile a completed branch, re-enqueuing it when lock contention prevents - * the result from being recorded. Other errors are authoritative and are not - * retried. + * Persist and reconcile a completed branch result. Persistence happens before + * reconciliation so a lock-contention retry can survive process exit without + * executing the completed branch again. * - * @since 0.5.0 + * @since 0.7.0 * - * @param array $payload Original branch action payload. + * @param array $payload Branch action payload. * @param array $branch_result Terminal branch result. * @return void */ - private static function reconcile_branch_result( array $payload, array $branch_result ): void { - $run_id = self::string_value( $payload['run_id'] ?? '' ); - $handle_id = self::string_value( $payload['handle_id'] ?? '' ); - $result = agents_reconcile_workflow_branch( $run_id, $handle_id, $branch_result ); + private static function persist_and_reconcile_branch_result( array $payload, array $branch_result ): void { + $run_id = self::string_value( $payload['run_id'] ?? '' ); + $handle_id = self::string_value( $payload['handle_id'] ?? '' ); + $result_ref = WP_Agent_Workflow_Branch_Store::put_branch_result( $run_id, $handle_id, $branch_result ); + + self::reconcile_branch_result( $run_id, $handle_id, $result_ref, $branch_result ); + } + + /** + * The RECONCILE_HOOK callback. Rehydrates a persisted terminal result and + * retries only the recorder merge; branch execution is deliberately absent. + * + * @since 0.7.0 + * + * @param array $payload Action payload: { run_id, handle_id, result_ref }. + * @return void + */ + public static function run_reconcile_action( array $payload ): void { + $run_id = self::string_value( $payload['run_id'] ?? '' ); + $handle_id = self::string_value( $payload['handle_id'] ?? '' ); + $result_ref = self::string_value( $payload['result_ref'] ?? '' ); + if ( '' === $run_id || '' === $handle_id || '' === $result_ref ) { + return; + } + + $branch_result = WP_Agent_Workflow_Branch_Store::get_branch_result( $result_ref ); + if ( null === $branch_result ) { + throw new \RuntimeException( sprintf( 'Could not rehydrate the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); + } + + self::reconcile_branch_result( $run_id, $handle_id, $result_ref, $branch_result ); + } + + /** + * Reconcile a persisted terminal result, enqueueing another reconcile-only + * action when lock contention prevents it from being recorded. + * + * @param string $run_id Run id. + * @param string $handle_id Branch handle id. + * @param string $result_ref Durable terminal-result ref. + * @param array $branch_result Terminal BranchResult. + * @return void + */ + private static function reconcile_branch_result( string $run_id, string $handle_id, string $result_ref, array $branch_result ): void { + $result = agents_reconcile_workflow_branch( $run_id, $handle_id, $branch_result ); if ( ! is_wp_error( $result ) || 'agents_reconcile_lock_unavailable' !== $result->get_error_code() ) { return; } - $action_id = self::enqueue_async_action( self::BRANCH_HOOK, array( $payload ), self::group_for_run( $run_id ) ); + $action_id = self::enqueue_async_action( + self::RECONCILE_HOOK, + array( + array( + 'run_id' => $run_id, + 'handle_id' => $handle_id, + 'result_ref' => $result_ref, + ), + ), + self::group_for_run( $run_id ) + ); if ( $action_id <= 0 ) { - throw new \RuntimeException( sprintf( 'Could not re-enqueue branch `%s` for run `%s` after reconcile lock contention.', $handle_id, $run_id ) ); + throw new \RuntimeException( sprintf( 'Could not enqueue a reconcile retry for branch `%s` in run `%s` after lock contention.', $handle_id, $run_id ) ); } } diff --git a/src/Workflows/class-wp-agent-workflow-branch-store.php b/src/Workflows/class-wp-agent-workflow-branch-store.php index 60266fe..ad803e0 100644 --- a/src/Workflows/class-wp-agent-workflow-branch-store.php +++ b/src/Workflows/class-wp-agent-workflow-branch-store.php @@ -73,6 +73,13 @@ final class WP_Agent_Workflow_Branch_Store { */ private const CONTEXT_PREFIX = 'agents_wf_branch_ctx_'; + /** + * Option-name prefix for terminal branch results awaiting reconciliation. + * + * @since 0.7.0 + */ + private const RESULT_PREFIX = 'agents_wf_branch_result_'; + /** * Option-name prefix for the per-run index of branch ref keys, so * {@see self::forget_run()} can delete every branch row a run wrote without @@ -189,6 +196,79 @@ public static function get_branch( string $store_ref, string $context_ref ): ?ar return $descriptor; } + /** + * Persist a terminal branch result so reconciliation can be retried without + * executing the branch again. + * + * @since 0.7.0 + * + * @param string $run_id Run the branch belongs to. + * @param string $handle_id Branch handle id. + * @param array $branch_result Terminal BranchResult. + * @return string Opaque result ref carried by the reconcile action. + */ + public static function put_branch_result( string $run_id, string $handle_id, array $branch_result ): string { + if ( function_exists( 'apply_filters' ) ) { + /** + * Filter terminal branch-result persistence. Return a non-empty string ref + * to take over storage; return null to use the built-in option store. + * + * @since 0.7.0 + * + * @param string|null $ref No override by default. + * @param string $run_id Run id. + * @param string $handle_id Branch handle id. + * @param array $branch_result Terminal BranchResult. + */ + $override = apply_filters( 'wp_agent_workflow_branch_store_put_result', null, $run_id, $handle_id, $branch_result ); + if ( is_string( $override ) && '' !== $override ) { + return $override; + } + } + + $ref = self::RESULT_PREFIX . md5( $run_id . ':' . $handle_id ); + self::write_row( + $ref, + array( + 'run_id' => $run_id, + 'handle_id' => $handle_id, + 'branch_result' => $branch_result, + 'expires' => time() + self::TTL_SECONDS, + ) + ); + self::index_ref( $run_id, $ref ); + return $ref; + } + + /** + * Read a terminal branch result from its durable ref. + * + * @since 0.7.0 + * + * @param string $result_ref Opaque result ref. + * @return array|null Terminal BranchResult, or null when unavailable. + */ + public static function get_branch_result( string $result_ref ): ?array { + if ( function_exists( 'apply_filters' ) ) { + /** + * Filter terminal branch-result retrieval. Return an array to take over + * retrieval; return null to use the built-in option store. + * + * @since 0.7.0 + * + * @param array|null $branch_result No override by default. + * @param string $result_ref Opaque result ref. + */ + $override = apply_filters( 'wp_agent_workflow_branch_store_get_result', null, $result_ref ); + if ( is_array( $override ) ) { + return $override; + } + } + + $row = self::read_row( $result_ref ); + return is_array( $row['branch_result'] ?? null ) ? self::string_keyed_array( $row['branch_result'] ) : null; + } + /** * Delete every row a run wrote (its branch descriptors, its shared-context * row, and its index) once the run resolves. Same cleanup discipline the @@ -373,4 +453,20 @@ private static function filtered_forget_run( string $run_id ): bool { */ return (bool) apply_filters( 'wp_agent_workflow_branch_store_forget', false, $run_id ); } + + /** + * Normalize an array to string keys for static-analysis-safe public returns. + * + * @param array $value Value to normalize. + * @return array + */ + private static function string_keyed_array( array $value ): array { + $normalized = array(); + foreach ( $value as $key => $item ) { + if ( is_string( $key ) ) { + $normalized[ $key ] = $item; + } + } + return $normalized; + } } diff --git a/src/Workflows/class-wp-agent-workflow-request-controller.php b/src/Workflows/class-wp-agent-workflow-request-controller.php index 844289d..83fc002 100644 --- a/src/Workflows/class-wp-agent-workflow-request-controller.php +++ b/src/Workflows/class-wp-agent-workflow-request-controller.php @@ -430,7 +430,7 @@ private function release_lease( string $operation_id, string $token ): void { } ); } - /** Remove only this run's AS branch/resume actions; no shared group is touched. */ + /** Remove only this run's AS branch/reconcile/resume actions; no shared group is touched. */ private function cleanup_operation_actions( string $run_id ): void { if ( '' === $run_id || ! function_exists( 'as_unschedule_all_actions' ) ) { return; diff --git a/src/Workflows/class-wp-agent-workflow-scoped-drain.php b/src/Workflows/class-wp-agent-workflow-scoped-drain.php index 1ec140f..e48eca2 100644 --- a/src/Workflows/class-wp-agent-workflow-scoped-drain.php +++ b/src/Workflows/class-wp-agent-workflow-scoped-drain.php @@ -103,6 +103,7 @@ public static function is_available(): bool { public static function default_hooks(): array { return array( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK, + WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK, WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RESUME_HOOK, ); } diff --git a/src/Workflows/register-workflow-branch-executor.php b/src/Workflows/register-workflow-branch-executor.php index a3c84c2..e10e9f4 100644 --- a/src/Workflows/register-workflow-branch-executor.php +++ b/src/Workflows/register-workflow-branch-executor.php @@ -78,7 +78,20 @@ static function ( $payload = array() ): void { 1 ); -// 3a. Resume action: AS claimed it exactly once → re-check SUSPENDED → resume. +// 3a. Reconcile-only retry: read the persisted terminal result and merge it. +add_action( + WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK, + /** + * @param array $payload Action payload: { run_id, handle_id, result_ref }. + */ + static function ( $payload = array() ): void { + WP_Agent_Workflow_Action_Scheduler_Branch_Executor::run_reconcile_action( is_array( $payload ) ? $payload : array() ); + }, + 10, + 1 +); + +// 3b. Resume action: AS claimed it exactly once → re-check SUSPENDED → resume. add_action( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RESUME_HOOK, /** @@ -91,7 +104,7 @@ static function ( $payload = array() ): void { 1 ); -// 3b. Deferred-resume seam: enqueue a claimed RESUME action for AS-owned runs +// 3c. Deferred-resume seam: enqueue a claimed RESUME action for AS-owned runs // instead of resuming inline in the reconcile request. add_filter( 'wp_agent_workflow_resume_dispatch', diff --git a/tests/workflow-as-branch-smoke.php b/tests/workflow-as-branch-smoke.php index cf07fb8..b4f569b 100644 --- a/tests/workflow-as-branch-smoke.php +++ b/tests/workflow-as-branch-smoke.php @@ -323,7 +323,9 @@ function as_smoke_register_ability( string $name, \Closure $handler ): void { as_smoke_register_ability( 'demo/role-worker', static function ( array $input ): array { - return array( 'fragment' => strtoupper( (string) ( $input['label'] ?? 'X' ) ) ); + $label = (string) ( $input['label'] ?? 'X' ); + $GLOBALS['__role_worker_effects'][ $label ] = (int) ( $GLOBALS['__role_worker_effects'][ $label ] ?? 0 ) + 1; + return array( 'fragment' => strtoupper( $label ) ); } ); as_smoke_register_ability( @@ -465,8 +467,8 @@ function as_smoke_roles_spec(): WP_Agent_Workflow_Spec { smoke_assert( 2, count( $handles ), 'frame carries 2 sibling handles', $failures, $passes ); smoke_assert_true( is_int( $handles[0]['ref'] ?? null ) && $handles[0]['ref'] > 0, 'handle ref is the AS action id', $failures, $passes ); -// A contended reconcile must enqueue another branch action instead of silently -// completing the current action without recording its result. +// A contended reconcile must enqueue a reconcile-only action carrying a durable +// result ref. Draining that retry must not execute the branch effect again. $lock_attempts = 0; add_filter( 'wp_agent_workflow_reconcile_lock', @@ -481,13 +483,22 @@ static function ( $override, string $run_id, callable $critical ) use ( &$lock_a 3 ); $branch_count_before_retry = count( AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ) ); +$effect_count_before_retry = (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ); AS_Shim::fire( $branch_actions[0]['id'] ); $branch_actions_with_retry = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); -smoke_assert( $branch_count_before_retry + 1, count( $branch_actions_with_retry ), 'lock contention: branch action re-enqueued', $failures, $passes ); +smoke_assert( $branch_count_before_retry, count( $branch_actions_with_retry ), 'lock contention: completed branch action is not re-enqueued', $failures, $passes ); +smoke_assert( $effect_count_before_retry + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'lock contention: branch side effect executes once before retry', $failures, $passes ); smoke_assert( 0, count( $recorder->find( 'as-A' )->get_suspension()['completed'] ?? array() ), 'lock contention: result not recorded without lock', $failures, $passes ); -$retry_action = $branch_actions_with_retry[ count( $branch_actions_with_retry ) - 1 ]; +$reconcile_actions = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK ); +smoke_assert( 1, count( $reconcile_actions ), 'lock contention: reconcile-only retry enqueued', $failures, $passes ); +$retry_action = $reconcile_actions[0]; AS_Shim::fire( $retry_action['id'] ); smoke_assert( 1, count( $recorder->find( 'as-A' )->get_suspension()['completed'] ?? array() ), 'lock contention: queued retry records completion', $failures, $passes ); +smoke_assert( $effect_count_before_retry + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'lock contention: reconcile retry does not repeat branch side effect', $failures, $passes ); +$duplicate_retry_id = AS_Shim::enqueue( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK, $retry_action['args'], $retry_action['group'] ); +AS_Shim::fire( $duplicate_retry_id ); +smoke_assert( 1, count( $recorder->find( 'as-A' )->get_suspension()['completed'] ?? array() ), 'lock contention: duplicate reconcile delivery is an idempotent no-op', $failures, $passes ); +smoke_assert( $effect_count_before_retry + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'lock contention: duplicate reconcile delivery does not repeat the side effect', $failures, $passes ); remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); // TABLE-FREE: the frame lives in metadata._suspension, not a new table. diff --git a/tests/workflow-request-controller-smoke.php b/tests/workflow-request-controller-smoke.php index 19ea297..7476702 100644 --- a/tests/workflow-request-controller-smoke.php +++ b/tests/workflow-request-controller-smoke.php @@ -132,7 +132,7 @@ function controller_assert( bool $ok, string $name ): void { global $fails, $pas controller_assert( in_array( 'failed', $cleaned, true ) && in_array( 'cancelled', $cleaned, true ), 'failed and cancelled runs release terminal cleanup' ); controller_assert( array() === ( $controller->get( 'one' )['lease'] ?? null ), 'terminal lease is cleared' ); $one_group_cleanup = array_filter( $GLOBALS['controller_unscheduled'], static function ( array $call ) use ( $one ): bool { return 'agents-api-run-' . md5( $one['run_id'] ) === $call[2]; } ); -controller_assert( 2 === count( $one_group_cleanup ), 'terminal cleanup removes only the run-scoped branch and resume actions' ); +controller_assert( 3 === count( $one_group_cleanup ), 'terminal cleanup removes only the run-scoped branch, reconcile, and resume actions' ); controller_assert( array() === array_filter( $one_group_cleanup, static fn ( array $call ): bool => null !== $call[1] ), 'terminal cleanup matches every argument shape in the run-scoped group' ); $state = WP_Agent_Run_Control::state( 'controller-test' ); $state['runs']['two']['lease'] = array( 'token' => 'other-worker', 'worker_id' => 'worker-a', 'expires_at' => time() + 60 ); diff --git a/tests/workflow-scoped-drain-smoke.php b/tests/workflow-scoped-drain-smoke.php index 48b2b8b..4102ff3 100644 --- a/tests/workflow-scoped-drain-smoke.php +++ b/tests/workflow-scoped-drain-smoke.php @@ -315,15 +315,16 @@ function as_get_datetime_object( ?string $date_string = null, string $timezone = use AgentsAPI\AI\Workflows\WP_Agent_Workflow_Scoped_Drain; $branch_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK; +$reconcile_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK; $resume_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RESUME_HOOK; $group = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::GROUP; // The drain's default scope must be the executor's hooks + group (read, never // hardcoded), so this and the executor can never drift. smoke_assert( - array( $branch_hook, $resume_hook ), + array( $branch_hook, $reconcile_hook, $resume_hook ), WP_Agent_Workflow_Scoped_Drain::default_hooks(), - 'default_hooks() = executor BRANCH_HOOK + RESUME_HOOK', + 'default_hooks() includes executor branch, reconcile, and resume hooks', $failures, $passes ); From 9824d1e7b5b57543815575f32e278b97841fc5c7 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 17:14:18 +0000 Subject: [PATCH 02/11] fix: harden reconcile retry persistence --- ...kflow-action-scheduler-branch-executor.php | 74 +++++++-- .../class-wp-agent-workflow-branch-store.php | 141 ++++++++++-------- .../register-workflow-branch-executor.php | 2 +- tests/workflow-as-branch-smoke.php | 55 ++++++- tests/workflow-async-branch-payload-smoke.php | 70 +++++++++ 5 files changed, 264 insertions(+), 78 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index aaa8d78..2f67af9 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -802,11 +802,16 @@ public static function run_branch_action( array $payload ): void { * @return void */ private static function persist_and_reconcile_branch_result( array $payload, array $branch_result ): void { - $run_id = self::string_value( $payload['run_id'] ?? '' ); - $handle_id = self::string_value( $payload['handle_id'] ?? '' ); - $result_ref = WP_Agent_Workflow_Branch_Store::put_branch_result( $run_id, $handle_id, $branch_result ); + $run_id = self::string_value( $payload['run_id'] ?? '' ); + $handle_id = self::string_value( $payload['handle_id'] ?? '' ); + $store_ref = self::string_value( $payload['store_ref'] ?? '' ); + $context_ref = self::string_value( $payload['context_ref'] ?? '' ); + $result_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $run_id, $handle_id, $store_ref, $context_ref, $branch_result ); + if ( is_wp_error( $result_ref ) ) { + throw new \RuntimeException( $result_ref->get_error_message() ); + } - self::reconcile_branch_result( $run_id, $handle_id, $result_ref, $branch_result ); + self::reconcile_branch_result( $run_id, $handle_id, $result_ref, $context_ref, $branch_result, array(), false ); } /** @@ -815,23 +820,24 @@ private static function persist_and_reconcile_branch_result( array $payload, arr * * @since 0.7.0 * - * @param array $payload Action payload: { run_id, handle_id, result_ref }. + * @param array $payload Action payload: { run_id, handle_id, result_ref, context_ref }. * @return void */ public static function run_reconcile_action( array $payload ): void { - $run_id = self::string_value( $payload['run_id'] ?? '' ); - $handle_id = self::string_value( $payload['handle_id'] ?? '' ); - $result_ref = self::string_value( $payload['result_ref'] ?? '' ); + $run_id = self::string_value( $payload['run_id'] ?? '' ); + $handle_id = self::string_value( $payload['handle_id'] ?? '' ); + $result_ref = self::string_value( $payload['result_ref'] ?? '' ); + $context_ref = self::string_value( $payload['context_ref'] ?? '' ); if ( '' === $run_id || '' === $handle_id || '' === $result_ref ) { return; } - $branch_result = WP_Agent_Workflow_Branch_Store::get_branch_result( $result_ref ); - if ( null === $branch_result ) { + $receipt = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $result_ref, $context_ref ); + if ( null === $receipt ) { throw new \RuntimeException( sprintf( 'Could not rehydrate the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); } - self::reconcile_branch_result( $run_id, $handle_id, $result_ref, $branch_result ); + self::reconcile_branch_result( $run_id, $handle_id, $result_ref, $context_ref, $receipt['branch_result'], $receipt['continuation'], true ); } /** @@ -841,23 +847,59 @@ public static function run_reconcile_action( array $payload ): void { * @param string $run_id Run id. * @param string $handle_id Branch handle id. * @param string $result_ref Durable terminal-result ref. + * @param string $context_ref Shared-context ref for custom stores. * @param array $branch_result Terminal BranchResult. + * @param array $continuation Opaque reconcile continuation state. + * @param bool $is_retry Whether this is a reconcile-only retry. * @return void */ - private static function reconcile_branch_result( string $run_id, string $handle_id, string $result_ref, array $branch_result ): void { - $result = agents_reconcile_workflow_branch( $run_id, $handle_id, $branch_result ); + private static function reconcile_branch_result( string $run_id, string $handle_id, string $result_ref, string $context_ref, array $branch_result, array $continuation, bool $is_retry ): void { + $result = null; + if ( $is_retry ) { + /** + * Filter a reconcile-only retry before completed-handle redelivery. A + * reconcile implementation with a multi-phase authoritative continuation + * may consume the persisted opaque state and return its result here. + * Returning null falls back to idempotent branch-result redelivery. + * + * @since 0.7.0 + * + * @param mixed $result No override by default. + * @param string $run_id Run id. + * @param string $handle_id Branch handle id. + * @param array $branch_result Terminal BranchResult. + * @param array $continuation Opaque continuation state. + */ + $result = apply_filters( 'wp_agent_workflow_reconcile_retry', null, $run_id, $handle_id, $branch_result, $continuation ); + } + if ( null === $result ) { + $result = agents_reconcile_workflow_branch( $run_id, $handle_id, $branch_result ); + } if ( ! is_wp_error( $result ) || 'agents_reconcile_lock_unavailable' !== $result->get_error_code() ) { return; } + $error_data = $result->get_error_data(); + $next_continuation = is_array( $error_data ) && is_array( $error_data['reconcile_continuation'] ?? null ) + ? self::string_keyed_array( $error_data['reconcile_continuation'] ) + : $continuation; + if ( $next_continuation !== $continuation ) { + $next_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref, $branch_result, $next_continuation ); + if ( is_wp_error( $next_ref ) ) { + throw new \RuntimeException( $next_ref->get_error_message() ); + } + $result_ref = $next_ref; + } + $action_id = self::enqueue_async_action( self::RECONCILE_HOOK, array( array( - 'run_id' => $run_id, - 'handle_id' => $handle_id, - 'result_ref' => $result_ref, + 'run_id' => $run_id, + 'handle_id' => $handle_id, + 'result_ref' => $result_ref, + 'context_ref' => $context_ref, ), ), self::group_for_run( $run_id ) diff --git a/src/Workflows/class-wp-agent-workflow-branch-store.php b/src/Workflows/class-wp-agent-workflow-branch-store.php index ad803e0..5ae2276 100644 --- a/src/Workflows/class-wp-agent-workflow-branch-store.php +++ b/src/Workflows/class-wp-agent-workflow-branch-store.php @@ -73,12 +73,8 @@ final class WP_Agent_Workflow_Branch_Store { */ private const CONTEXT_PREFIX = 'agents_wf_branch_ctx_'; - /** - * Option-name prefix for terminal branch results awaiting reconciliation. - * - * @since 0.7.0 - */ - private const RESULT_PREFIX = 'agents_wf_branch_result_'; + /** Reconcile receipt key stored inside each branch descriptor. */ + private const RECONCILE_RECEIPT_KEY = '_agents_reconcile_receipt'; /** * Option-name prefix for the per-run index of branch ref keys, so @@ -197,76 +193,87 @@ public static function get_branch( string $store_ref, string $context_ref ): ?ar } /** - * Persist a terminal branch result so reconciliation can be retried without - * executing the branch again. + * Persist a terminal branch result and opaque continuation state inside the + * branch's existing descriptor row. Reusing that already-indexed per-branch + * row avoids a concurrent shared-index append during branch completion. * * @since 0.7.0 * * @param string $run_id Run the branch belongs to. * @param string $handle_id Branch handle id. + * @param string $store_ref Existing branch descriptor ref. + * @param string $context_ref Existing shared-context ref. * @param array $branch_result Terminal BranchResult. - * @return string Opaque result ref carried by the reconcile action. + * @param array $continuation Opaque reconcile continuation state. + * @return string|\WP_Error Durable receipt ref, or a hard persistence failure. */ - public static function put_branch_result( string $run_id, string $handle_id, array $branch_result ): string { - if ( function_exists( 'apply_filters' ) ) { - /** - * Filter terminal branch-result persistence. Return a non-empty string ref - * to take over storage; return null to use the built-in option store. - * - * @since 0.7.0 - * - * @param string|null $ref No override by default. - * @param string $run_id Run id. - * @param string $handle_id Branch handle id. - * @param array $branch_result Terminal BranchResult. - */ - $override = apply_filters( 'wp_agent_workflow_branch_store_put_result', null, $run_id, $handle_id, $branch_result ); - if ( is_string( $override ) && '' !== $override ) { - return $override; + public static function put_reconcile_receipt( string $run_id, string $handle_id, string $store_ref, string $context_ref, array $branch_result, array $continuation = array() ) { + $receipt = array( + 'branch_result' => $branch_result, + 'continuation' => $continuation, + ); + $row = self::read_row( $store_ref ); + + if ( null !== $row && is_array( $row['descriptor'] ?? null ) ) { + $descriptor = $row['descriptor']; + $descriptor[ self::RECONCILE_RECEIPT_KEY ] = $receipt; + $row['descriptor'] = $descriptor; + self::write_row( $store_ref, $row ); + + $persisted = self::read_row( $store_ref ); + $persisted_descriptor = null !== $persisted && is_array( $persisted['descriptor'] ?? null ) ? $persisted['descriptor'] : array(); + if ( $receipt === ( $persisted_descriptor[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ) { + return $store_ref; } + + return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'Could not durably persist the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); } - $ref = self::RESULT_PREFIX . md5( $run_id . ':' . $handle_id ); - self::write_row( - $ref, - array( - 'run_id' => $run_id, - 'handle_id' => $handle_id, - 'branch_result' => $branch_result, - 'expires' => time() + self::TTL_SECONDS, - ) - ); - self::index_ref( $run_id, $ref ); - return $ref; + // A non-local ref belongs to the consumer that supplied it through the + // existing branch-store filters. Require that same owner to persist and read + // the receipt; never fall back to an unexpected local option row. + $descriptor = self::filtered_get_branch( $store_ref, $context_ref ); + if ( null === $descriptor ) { + return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'The branch store that owns `%s` could not rehydrate it for terminal-result persistence.', $store_ref ) ); + } + $descriptor[ self::RECONCILE_RECEIPT_KEY ] = $receipt; + $result_ref = self::filtered_put_branch( $run_id, $handle_id, self::strip_shared_context( $descriptor ) ); + if ( null === $result_ref ) { + return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'The branch store that owns `%s` did not persist the terminal result.', $store_ref ) ); + } + + $persisted = self::filtered_get_branch( $result_ref, $context_ref ); + if ( null === $persisted || $receipt !== ( $persisted[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ) { + return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'The branch store could not verify the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); + } + + return $result_ref; } /** - * Read a terminal branch result from its durable ref. + * Read a durable reconcile receipt through the same built-in or custom store + * that owns the branch descriptor. * * @since 0.7.0 * - * @param string $result_ref Opaque result ref. - * @return array|null Terminal BranchResult, or null when unavailable. + * @param string $result_ref Opaque receipt ref. + * @param string $context_ref Existing shared-context ref. + * @return array{branch_result:array,continuation:array}|null */ - public static function get_branch_result( string $result_ref ): ?array { - if ( function_exists( 'apply_filters' ) ) { - /** - * Filter terminal branch-result retrieval. Return an array to take over - * retrieval; return null to use the built-in option store. - * - * @since 0.7.0 - * - * @param array|null $branch_result No override by default. - * @param string $result_ref Opaque result ref. - */ - $override = apply_filters( 'wp_agent_workflow_branch_store_get_result', null, $result_ref ); - if ( is_array( $override ) ) { - return $override; - } + public static function get_reconcile_receipt( string $result_ref, string $context_ref ): ?array { + $row = self::read_row( $result_ref ); + $descriptor = null !== $row && is_array( $row['descriptor'] ?? null ) + ? $row['descriptor'] + : self::filtered_get_branch( $result_ref, $context_ref ); + $receipt = is_array( $descriptor[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ? $descriptor[ self::RECONCILE_RECEIPT_KEY ] : array(); + if ( ! is_array( $receipt['branch_result'] ?? null ) ) { + return null; } - $row = self::read_row( $result_ref ); - return is_array( $row['branch_result'] ?? null ) ? self::string_keyed_array( $row['branch_result'] ) : null; + return array( + 'branch_result' => self::string_keyed_array( $receipt['branch_result'] ), + 'continuation' => is_array( $receipt['continuation'] ?? null ) ? self::string_keyed_array( $receipt['continuation'] ) : array(), + ); } /** @@ -306,12 +313,13 @@ public static function forget_run( string $run_id ): void { * * @param string $option Option name. * @param array $value Row value. - * @return void + * @return bool Whether WordPress reported that it changed the row. */ - private static function write_row( string $option, array $value ): void { + private static function write_row( string $option, array $value ): bool { if ( function_exists( 'update_option' ) ) { - update_option( $option, $value, false ); + return update_option( $option, $value, false ); } + return false; } /** @@ -469,4 +477,17 @@ private static function string_keyed_array( array $value ): array { } return $normalized; } + + /** + * Keep the existing custom-store put contract free of shared context copies. + * + * @param array $descriptor Branch descriptor. + * @return array + */ + private static function strip_shared_context( array $descriptor ): array { + if ( is_array( $descriptor['branch_vars'] ?? null ) && array_key_exists( 'context', $descriptor['branch_vars'] ) ) { + unset( $descriptor['branch_vars']['context'] ); + } + return $descriptor; + } } diff --git a/src/Workflows/register-workflow-branch-executor.php b/src/Workflows/register-workflow-branch-executor.php index e10e9f4..1eddf1e 100644 --- a/src/Workflows/register-workflow-branch-executor.php +++ b/src/Workflows/register-workflow-branch-executor.php @@ -82,7 +82,7 @@ static function ( $payload = array() ): void { add_action( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK, /** - * @param array $payload Action payload: { run_id, handle_id, result_ref }. + * @param array $payload Action payload: { run_id, handle_id, result_ref, context_ref }. */ static function ( $payload = array() ): void { WP_Agent_Workflow_Action_Scheduler_Branch_Executor::run_reconcile_action( is_array( $payload ) ? $payload : array() ); diff --git a/tests/workflow-as-branch-smoke.php b/tests/workflow-as-branch-smoke.php index b4f569b..4f10bec 100644 --- a/tests/workflow-as-branch-smoke.php +++ b/tests/workflow-as-branch-smoke.php @@ -73,6 +73,7 @@ public function execute( $input = null ) { $GLOBALS['__filters'] = array(); $GLOBALS['__abilities'] = array(); $GLOBALS['__options'] = array(); +$GLOBALS['__reject_option_write'] = ''; if ( ! function_exists( 'add_filter' ) ) { function add_filter( string $hook, callable $cb, int $priority = 10, int $accepted_args = 1 ): void { @@ -134,6 +135,9 @@ function get_option( string $option, $default = false ) { return $GLOBALS['__opt if ( ! function_exists( 'update_option' ) ) { function update_option( string $option, $value, $autoload = null ): bool { unset( $autoload ); + if ( $option === $GLOBALS['__reject_option_write'] ) { + return false; + } $GLOBALS['__options'][ $option ] = $value; return true; } @@ -475,7 +479,7 @@ function as_smoke_roles_spec(): WP_Agent_Workflow_Spec { static function ( $override, string $run_id, callable $critical ) use ( &$lock_attempts ) { unset( $override ); if ( 'as-A' === $run_id && 0 === $lock_attempts++ ) { - return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended', array( 'reconcile_continuation' => array( 'phase' => 'commit' ) ) ); } return $critical(); }, @@ -492,14 +496,27 @@ static function ( $override, string $run_id, callable $critical ) use ( &$lock_a $reconcile_actions = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK ); smoke_assert( 1, count( $reconcile_actions ), 'lock contention: reconcile-only retry enqueued', $failures, $passes ); $retry_action = $reconcile_actions[0]; +$retry_continuation = array(); +add_filter( + 'wp_agent_workflow_reconcile_retry', + static function ( $result, string $run_id, string $handle_id, array $branch_result, array $continuation ) use ( &$retry_continuation ) { + unset( $run_id, $handle_id, $branch_result ); + $retry_continuation = $continuation; + return $result; + }, + 10, + 5 +); AS_Shim::fire( $retry_action['id'] ); smoke_assert( 1, count( $recorder->find( 'as-A' )->get_suspension()['completed'] ?? array() ), 'lock contention: queued retry records completion', $failures, $passes ); smoke_assert( $effect_count_before_retry + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'lock contention: reconcile retry does not repeat branch side effect', $failures, $passes ); +smoke_assert( array( 'phase' => 'commit' ), $retry_continuation, 'lock contention: retry carries opaque authoritative continuation state', $failures, $passes ); $duplicate_retry_id = AS_Shim::enqueue( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK, $retry_action['args'], $retry_action['group'] ); AS_Shim::fire( $duplicate_retry_id ); smoke_assert( 1, count( $recorder->find( 'as-A' )->get_suspension()['completed'] ?? array() ), 'lock contention: duplicate reconcile delivery is an idempotent no-op', $failures, $passes ); smoke_assert( $effect_count_before_retry + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'lock contention: duplicate reconcile delivery does not repeat the side effect', $failures, $passes ); remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); +remove_all_filters( 'wp_agent_workflow_reconcile_retry' ); // TABLE-FREE: the frame lives in metadata._suspension, not a new table. smoke_assert_true( is_array( $recorder->find( 'as-A' )->get_suspension()['handles'] ?? null ), 'table-free: frame in metadata._suspension while suspended', $failures, $passes ); @@ -747,5 +764,41 @@ function as_smoke_two_fanout_spec(): WP_Agent_Workflow_Spec { smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUCCEEDED, $final5->get_status(), 'multi-fanout: second resume reaches terminal success', $failures, $passes ); smoke_assert( $group5, WP_Agent_Workflow_Action_Scheduler_Branch_Executor::group_for_run( 'as-two' ), 'multi-fanout: terminal run retains the same deterministic group identity', $failures, $passes ); +// A terminal result must be durably readable before reconciliation begins. If +// the branch-row update fails while the reconcile lock is also contended, the +// original action fails loudly and no unrecoverable reconcile retry is queued. +AS_Shim::reset(); +$recorder6 = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder6 ) { return $recorder6; } ); +$run6 = ( new WP_Agent_Workflow_Runner( $recorder6 ) )->run( as_smoke_roles_spec(), array(), array( 'run_id' => 'as-write-fail' ) ); +$branches6 = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +$payload6 = $branches6[0]['args'][0] ?? array(); +$GLOBALS['__reject_option_write'] = (string) ( $payload6['store_ref'] ?? '' ); +$lock_attempts6 = 0; +add_filter( + 'wp_agent_workflow_reconcile_lock', + static function () use ( &$lock_attempts6 ) { + ++$lock_attempts6; + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + }, + 10, + 3 +); +$effect_before6 = (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ); +$write_failed6 = false; +try { + AS_Shim::fire( $branches6[0]['id'] ); +} catch ( \RuntimeException $error ) { + $write_failed6 = str_contains( $error->getMessage(), 'durably persist' ); +} +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUSPENDED, $run6->get_status(), 'failed result write: fixture starts suspended', $failures, $passes ); +smoke_assert( true, $write_failed6, 'failed result write: branch action fails loudly before completing', $failures, $passes ); +smoke_assert( $effect_before6 + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'failed result write: branch side effect ran exactly once', $failures, $passes ); +smoke_assert( 0, $lock_attempts6, 'failed result write: reconcile is not attempted with an unreadable result', $failures, $passes ); +smoke_assert( 0, count( AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK ) ), 'failed result write: no stranded reconcile-only retry is queued', $failures, $passes ); +$GLOBALS['__reject_option_write'] = ''; +remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); + echo "Passed: {$passes}, Failed: " . count( $failures ) . "\n"; exit( count( $failures ) > 0 ? 1 : 0 ); diff --git a/tests/workflow-async-branch-payload-smoke.php b/tests/workflow-async-branch-payload-smoke.php index a6dbf11..57a0cf9 100644 --- a/tests/workflow-async-branch-payload-smoke.php +++ b/tests/workflow-async-branch-payload-smoke.php @@ -502,5 +502,75 @@ function payload_roles_spec(): WP_Agent_Workflow_Spec { } smoke_assert( 0, $leftover2, 'bug2: failed dispatch cleaned up its stored rows (no orphans)', $failures, $passes ); +// Concurrent terminal writers must not append separate refs through the shared +// run index. Restore a stale pre-result index snapshot to model a lost concurrent +// index update, then prove cleanup still removes every row because each receipt +// lives in its already-indexed descriptor. +$GLOBALS['__options'] = array(); +$cleanup_run = 'pay-concurrent-cleanup'; +$cleanup_a = WP_Agent_Workflow_Branch_Store::put_branch( $cleanup_run, 'a', array( 'run_id' => $cleanup_run, 'handle_id' => 'a', 'key' => 'a' ) ); +$cleanup_b = WP_Agent_Workflow_Branch_Store::put_branch( $cleanup_run, 'b', array( 'run_id' => $cleanup_run, 'handle_id' => 'b', 'key' => 'b' ) ); +$index_key = 'agents_wf_branch_index_' . md5( $cleanup_run ); +$stale_index = $GLOBALS['__options'][ $index_key ] ?? array(); +$cleanup_result = array( 'key' => 'a', 'status' => 'succeeded', 'output' => array( 'ok' => true ) ); +WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $cleanup_run, 'a', $cleanup_a, '', $cleanup_result ); +$cleanup_result['key'] = 'b'; +WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $cleanup_run, 'b', $cleanup_b, '', $cleanup_result ); +$GLOBALS['__options'][ $index_key ] = $stale_index; +WP_Agent_Workflow_Branch_Store::forget_run( $cleanup_run ); +$cleanup_leftover = array_filter( + array_keys( $GLOBALS['__options'] ), + static fn ( $option ): bool => str_starts_with( (string) $option, 'agents_wf_branch_' ) +); +smoke_assert( array(), array_values( $cleanup_leftover ), 'concurrent cleanup: stale shared index cannot orphan terminal result rows', $failures, $passes ); + +// A consumer-owned branch ref must keep terminal result persistence and cleanup +// in that same custom store. No local option fallback is permitted. +$GLOBALS['__options'] = array(); +$GLOBALS['__custom_branch_rows'] = array(); +add_filter( + 'wp_agent_workflow_branch_store_put', + static function ( $ref, string $run_id, string $handle_id, array $descriptor ) { + unset( $ref ); + $custom_ref = 'custom:' . md5( $run_id . ':' . $handle_id ); + $GLOBALS['__custom_branch_rows'][ $custom_ref ] = $descriptor; + return $custom_ref; + }, + 10, + 4 +); +add_filter( + 'wp_agent_workflow_branch_store_get', + static function ( $descriptor, string $store_ref ) { + unset( $descriptor ); + return $GLOBALS['__custom_branch_rows'][ $store_ref ] ?? null; + }, + 10, + 3 +); +add_filter( + 'wp_agent_workflow_branch_store_forget', + static function ( bool $handled, string $run_id ): bool { + unset( $handled, $run_id ); + $GLOBALS['__custom_branch_rows'] = array(); + return true; + }, + 10, + 2 +); +$custom_run = 'pay-custom-store'; +$custom_ref = WP_Agent_Workflow_Branch_Store::put_branch( $custom_run, 'custom-handle', array( 'run_id' => $custom_run, 'handle_id' => 'custom-handle', 'key' => 'custom' ) ); +$custom_result = array( 'key' => 'custom', 'status' => 'succeeded', 'output' => array( 'owned' => true ) ); +$custom_receipt_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $custom_run, 'custom-handle', $custom_ref, '', $custom_result ); +$custom_receipt = is_wp_error( $custom_receipt_ref ) ? null : WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_receipt_ref, '' ); +smoke_assert( $custom_ref, $custom_receipt_ref, 'custom store: existing put filter owns terminal result persistence', $failures, $passes ); +smoke_assert( $custom_result, $custom_receipt['branch_result'] ?? null, 'custom store: existing get filter rehydrates the terminal result', $failures, $passes ); +smoke_assert( array(), $GLOBALS['__options'], 'custom store: terminal persistence creates no local option fallback', $failures, $passes ); +WP_Agent_Workflow_Branch_Store::forget_run( $custom_run ); +smoke_assert( array(), $GLOBALS['__custom_branch_rows'], 'custom store: existing forget filter cleans terminal result state', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_branch_store_put' ); +remove_all_filters( 'wp_agent_workflow_branch_store_get' ); +remove_all_filters( 'wp_agent_workflow_branch_store_forget' ); + echo "Passed: {$passes}, Failed: " . count( $failures ) . "\n"; exit( count( $failures ) > 0 ? 1 : 0 ); From 00055f3e1de2173f87bb026a1151a4f1ca04a194 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 17:24:43 +0000 Subject: [PATCH 03/11] fix: reconcile missing branch descriptors --- ...kflow-action-scheduler-branch-executor.php | 28 ++++++- .../class-wp-agent-workflow-branch-store.php | 79 +++++++++++++++++-- tests/workflow-as-branch-smoke.php | 52 ++++++++++++ tests/workflow-async-branch-payload-smoke.php | 15 ++++ 4 files changed, 168 insertions(+), 6 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index 2f67af9..0b21125 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -834,6 +834,9 @@ public static function run_reconcile_action( array $payload ): void { $receipt = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $result_ref, $context_ref ); if ( null === $receipt ) { + if ( self::is_branch_reconciled( $run_id, $handle_id ) ) { + return; + } throw new \RuntimeException( sprintf( 'Could not rehydrate the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); } @@ -876,7 +879,11 @@ private static function reconcile_branch_result( string $run_id, string $handle_ $result = agents_reconcile_workflow_branch( $run_id, $handle_id, $branch_result ); } - if ( ! is_wp_error( $result ) || 'agents_reconcile_lock_unavailable' !== $result->get_error_code() ) { + if ( ! is_wp_error( $result ) ) { + WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref ); + return; + } + if ( 'agents_reconcile_lock_unavailable' !== $result->get_error_code() ) { return; } @@ -909,6 +916,25 @@ private static function reconcile_branch_result( string $run_id, string $handle_ } } + /** Whether the authoritative run already recorded this branch completion. */ + private static function is_branch_reconciled( string $run_id, string $handle_id ): bool { + $recorder = agents_workflow_resolve_recorder(); + if ( null === $recorder ) { + return false; + } + $result = $recorder->find( $run_id ); + if ( null === $result ) { + return false; + } + if ( ! $result->is_suspended() ) { + return true; + } + + $suspension = $result->get_suspension(); + $completed = is_array( $suspension['completed'] ?? null ) ? $suspension['completed'] : array(); + return isset( $completed[ $handle_id ] ); + } + /** * Run one branch's nested steps through the shared runner and normalize the * outcome into a BranchResult ({ key, status, output, steps, error, item }). diff --git a/src/Workflows/class-wp-agent-workflow-branch-store.php b/src/Workflows/class-wp-agent-workflow-branch-store.php index 5ae2276..751eafc 100644 --- a/src/Workflows/class-wp-agent-workflow-branch-store.php +++ b/src/Workflows/class-wp-agent-workflow-branch-store.php @@ -115,7 +115,7 @@ public static function put_branch( string $run_id, string $handle_id, array $des return $override; } - $ref = self::BRANCH_PREFIX . md5( $run_id . ':' . $handle_id ); + $ref = self::branch_ref( $run_id, $handle_id ); self::write_row( $ref, array( @@ -229,12 +229,36 @@ public static function put_reconcile_receipt( string $run_id, string $handle_id, return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'Could not durably persist the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); } + // A missing or expired built-in descriptor still has a deterministic ref. + // Persist a receipt-only row at that exact ref: no shared-index append, and + // successful reconciliation can delete the row directly. + if ( self::branch_ref( $run_id, $handle_id ) === $store_ref ) { + self::write_row( + $store_ref, + array( + 'run_id' => $run_id, + 'handle_id' => $handle_id, + self::RECONCILE_RECEIPT_KEY => $receipt, + 'expires' => time() + self::TTL_SECONDS, + ) + ); + $persisted = self::read_row( $store_ref ); + if ( $receipt === ( $persisted[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ) { + return $store_ref; + } + + return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'Could not durably persist the terminal result for missing branch `%s` in run `%s`.', $handle_id, $run_id ) ); + } + // A non-local ref belongs to the consumer that supplied it through the // existing branch-store filters. Require that same owner to persist and read // the receipt; never fall back to an unexpected local option row. $descriptor = self::filtered_get_branch( $store_ref, $context_ref ); if ( null === $descriptor ) { - return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'The branch store that owns `%s` could not rehydrate it for terminal-result persistence.', $store_ref ) ); + $descriptor = array( + 'run_id' => $run_id, + 'handle_id' => $handle_id, + ); } $descriptor[ self::RECONCILE_RECEIPT_KEY ] = $receipt; $result_ref = self::filtered_put_branch( $run_id, $handle_id, self::strip_shared_context( $descriptor ) ); @@ -261,11 +285,15 @@ public static function put_reconcile_receipt( string $run_id, string $handle_id, * @return array{branch_result:array,continuation:array}|null */ public static function get_reconcile_receipt( string $result_ref, string $context_ref ): ?array { - $row = self::read_row( $result_ref ); - $descriptor = null !== $row && is_array( $row['descriptor'] ?? null ) + $row = self::read_row( $result_ref ); + if ( null !== $row && is_array( $row[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ) { + $receipt = $row[ self::RECONCILE_RECEIPT_KEY ]; + } else { + $descriptor = null !== $row && is_array( $row['descriptor'] ?? null ) ? $row['descriptor'] : self::filtered_get_branch( $result_ref, $context_ref ); - $receipt = is_array( $descriptor[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ? $descriptor[ self::RECONCILE_RECEIPT_KEY ] : array(); + $receipt = is_array( $descriptor[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ? $descriptor[ self::RECONCILE_RECEIPT_KEY ] : array(); + } if ( ! is_array( $receipt['branch_result'] ?? null ) ) { return null; } @@ -276,6 +304,39 @@ public static function get_reconcile_receipt( string $result_ref, string $contex ); } + /** + * Delete one successfully reconciled receipt without touching sibling refs. + * + * @since 0.7.0 + * + * @param string $run_id Run id. + * @param string $handle_id Branch handle id. + * @param string $result_ref Receipt ref. + * @param string $context_ref Shared-context ref for custom stores. + * @return void + */ + public static function forget_reconcile_receipt( string $run_id, string $handle_id, string $result_ref, string $context_ref ): void { + $row = self::read_row( $result_ref ); + if ( null !== $row ) { + if ( is_array( $row['descriptor'] ?? null ) ) { + $descriptor = $row['descriptor']; + unset( $descriptor[ self::RECONCILE_RECEIPT_KEY ] ); + $row['descriptor'] = $descriptor; + self::write_row( $result_ref, $row ); + } elseif ( self::branch_ref( $run_id, $handle_id ) === $result_ref && function_exists( 'delete_option' ) ) { + delete_option( $result_ref ); + } + return; + } + + $descriptor = self::filtered_get_branch( $result_ref, $context_ref ); + if ( null === $descriptor ) { + return; + } + unset( $descriptor[ self::RECONCILE_RECEIPT_KEY ] ); + self::filtered_put_branch( $run_id, $handle_id, self::strip_shared_context( $descriptor ) ); + } + /** * Delete every row a run wrote (its branch descriptors, its shared-context * row, and its index) once the run resolves. Same cleanup discipline the @@ -340,6 +401,9 @@ private static function read_row( string $option ): ?array { } $expires = is_numeric( $row['expires'] ?? null ) ? (int) $row['expires'] : 0; if ( $expires > 0 && $expires <= time() ) { + if ( function_exists( 'delete_option' ) ) { + delete_option( $option ); + } return null; } @@ -377,6 +441,11 @@ private static function index_ref( string $run_id, string $ref ): void { } } + /** Return the deterministic built-in descriptor ref for one branch. */ + private static function branch_ref( string $run_id, string $handle_id ): string { + return self::BRANCH_PREFIX . md5( $run_id . ':' . $handle_id ); + } + /** * Offer a consumer's store the chance to persist a branch descriptor. A * filter that returns a non-empty string ref owns persistence for this diff --git a/tests/workflow-as-branch-smoke.php b/tests/workflow-as-branch-smoke.php index 4f10bec..092ae21 100644 --- a/tests/workflow-as-branch-smoke.php +++ b/tests/workflow-as-branch-smoke.php @@ -800,5 +800,57 @@ static function () use ( &$lock_attempts6 ) { $GLOBALS['__reject_option_write'] = ''; remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); +// Missing and expired descriptors still reconcile a durable terminal failure. +// The fallback receipt uses the deterministic descriptor ref itself, survives +// one contended attempt, and is deleted immediately after the retry records it. +foreach ( array( 'missing', 'expired' ) as $descriptor_state ) { + AS_Shim::reset(); + $GLOBALS['__options'] = array(); + $edge_run_id = 'as-descriptor-' . $descriptor_state; + $edge_recorder = new AS_Smoke_Recorder(); + remove_all_filters( 'wp_agent_workflow_run_recorder' ); + add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $edge_recorder ) { return $edge_recorder; } ); + $edge_run = ( new WP_Agent_Workflow_Runner( $edge_recorder ) )->run( as_smoke_failing_spec(), array(), array( 'run_id' => $edge_run_id ) ); + $edge_branches = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); + $edge_payload = $edge_branches[0]['args'][0] ?? array(); + $edge_store_ref = (string) ( $edge_payload['store_ref'] ?? '' ); + if ( 'missing' === $descriptor_state ) { + delete_option( $edge_store_ref ); + } else { + $GLOBALS['__options'][ $edge_store_ref ]['expires'] = time() - 1; + $expired_descriptor = \AgentsAPI\AI\Workflows\WP_Agent_Workflow_Branch_Store::get_branch( $edge_store_ref, (string) ( $edge_payload['context_ref'] ?? '' ) ); + smoke_assert( null, $expired_descriptor, 'expired descriptor: expired payload is unavailable', $failures, $passes ); + smoke_assert( false, array_key_exists( $edge_store_ref, $GLOBALS['__options'] ), 'expired descriptor: expired option deletes itself on read', $failures, $passes ); + } + + $edge_lock_attempts = 0; + add_filter( + 'wp_agent_workflow_reconcile_lock', + static function ( $override, string $run_id, callable $critical ) use ( &$edge_lock_attempts, $edge_run_id ) { + unset( $override ); + if ( $edge_run_id === $run_id && 0 === $edge_lock_attempts++ ) { + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + } + return $critical(); + }, + 10, + 3 + ); + AS_Shim::fire( $edge_branches[0]['id'] ); + $edge_retries = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK ); + $edge_receipt = \AgentsAPI\AI\Workflows\WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $edge_store_ref, (string) ( $edge_payload['context_ref'] ?? '' ) ); + smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUSPENDED, $edge_run->get_status(), $descriptor_state . ' descriptor: fixture starts suspended', $failures, $passes ); + smoke_assert( 1, count( $edge_retries ), $descriptor_state . ' descriptor: lock contention queues reconcile-only retry', $failures, $passes ); + smoke_assert( 'workflow_branch_descriptor_missing', $edge_receipt['branch_result']['error']['code'] ?? '', $descriptor_state . ' descriptor: receipt durably carries terminal missing-descriptor failure', $failures, $passes ); + AS_Shim::fire( $edge_retries[0]['id'] ); + smoke_assert( false, array_key_exists( $edge_store_ref, $GLOBALS['__options'] ), $descriptor_state . ' descriptor: successful reconcile deletes exact receipt-only row', $failures, $passes ); + $edge_resumes = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RESUME_HOOK ); + AS_Shim::fire( $edge_resumes[0]['id'] ); + $edge_final = $edge_recorder->find( $edge_run_id ); + smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_FAILED, $edge_final->get_status(), $descriptor_state . ' descriptor: reconcile-only retry reaches terminal workflow failure', $failures, $passes ); + smoke_assert( 'workflow_parallel_required_branch_failed', $edge_final->get_error()['code'] ?? '', $descriptor_state . ' descriptor: terminal failure preserves required-branch semantics', $failures, $passes ); + remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); +} + echo "Passed: {$passes}, Failed: " . count( $failures ) . "\n"; exit( count( $failures ) > 0 ? 1 : 0 ); diff --git a/tests/workflow-async-branch-payload-smoke.php b/tests/workflow-async-branch-payload-smoke.php index 57a0cf9..304191f 100644 --- a/tests/workflow-async-branch-payload-smoke.php +++ b/tests/workflow-async-branch-payload-smoke.php @@ -568,6 +568,21 @@ static function ( bool $handled, string $run_id ): bool { smoke_assert( array(), $GLOBALS['__options'], 'custom store: terminal persistence creates no local option fallback', $failures, $passes ); WP_Agent_Workflow_Branch_Store::forget_run( $custom_run ); smoke_assert( array(), $GLOBALS['__custom_branch_rows'], 'custom store: existing forget filter cleans terminal result state', $failures, $passes ); +$custom_missing_run = 'pay-custom-store-missing'; +$custom_missing_ref = WP_Agent_Workflow_Branch_Store::put_branch( $custom_missing_run, 'missing-handle', array( 'run_id' => $custom_missing_run, 'handle_id' => 'missing-handle', 'key' => 'missing' ) ); +unset( $GLOBALS['__custom_branch_rows'][ $custom_missing_ref ] ); +$custom_missing_result = array( 'key' => '', 'status' => 'failed', 'output' => null, 'error' => array( 'code' => 'workflow_branch_descriptor_missing' ) ); +$custom_missing_receipt_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $custom_missing_run, 'missing-handle', $custom_missing_ref, '', $custom_missing_result ); +$custom_missing_receipt = is_wp_error( $custom_missing_receipt_ref ) ? null : WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_missing_receipt_ref, '' ); +smoke_assert( $custom_missing_ref, $custom_missing_receipt_ref, 'custom store: owner filter persists a receipt-only missing descriptor', $failures, $passes ); +smoke_assert( 'workflow_branch_descriptor_missing', $custom_missing_receipt['branch_result']['error']['code'] ?? '', 'custom store: owner filter rehydrates receipt-only failure', $failures, $passes ); +smoke_assert( array(), $GLOBALS['__options'], 'custom store: missing-descriptor receipt still creates no local fallback', $failures, $passes ); +WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $custom_missing_run, 'missing-handle', $custom_missing_ref, '' ); +$custom_missing_after_reconcile = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_missing_ref, '' ); +smoke_assert( null, $custom_missing_after_reconcile, 'custom store: successful reconcile removes the exact receipt through owner filters', $failures, $passes ); +smoke_assert( array(), $GLOBALS['__options'], 'custom store: exact receipt cleanup creates no local option fallback', $failures, $passes ); +WP_Agent_Workflow_Branch_Store::forget_run( $custom_missing_run ); +smoke_assert( array(), $GLOBALS['__custom_branch_rows'], 'custom store: forget filter cleans receipt-only missing descriptor', $failures, $passes ); remove_all_filters( 'wp_agent_workflow_branch_store_put' ); remove_all_filters( 'wp_agent_workflow_branch_store_get' ); remove_all_filters( 'wp_agent_workflow_branch_store_forget' ); From 70df62b71c09366cc2082cc2464598a4515ba6b5 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 17:38:27 +0000 Subject: [PATCH 04/11] fix: keep reconcile retries claimable --- ...kflow-action-scheduler-branch-executor.php | 30 +++++++++++++++++ .../class-wp-agent-workflow-branch-store.php | 25 +++++++-------- .../register-workflow-branch-executor.php | 26 +++++++-------- tests/workflow-async-branch-payload-smoke.php | 32 +++++++++++++++++-- ...workflow-branch-concurrency-gate-smoke.php | 24 ++++++++++++++ 5 files changed, 108 insertions(+), 29 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index 4046776..d65f686 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -426,6 +426,36 @@ public static function resume_inflight_count(): int { return is_array( $ids ) ? count( $ids ) : 0; } + /** + * Count reconcile-only retries still pending or in progress. These quick + * continuation actions need additive claim headroom for the same reason as a + * resume: once the original branch action completes, branch inflight can be + * zero while an unrelated long-lived claim keeps AS's default gate closed. + * + * @since 0.7.0 + * + * @return int In-flight reconcile-action count. + */ + public static function reconcile_inflight_count(): int { + if ( ! function_exists( 'as_get_scheduled_actions' ) || ! class_exists( '\ActionScheduler_Store' ) ) { + return 0; + } + + $ids = as_get_scheduled_actions( + array( + 'hook' => self::RECONCILE_HOOK, + 'status' => array( + \ActionScheduler_Store::STATUS_PENDING, + \ActionScheduler_Store::STATUS_RUNNING, + ), + 'per_page' => self::MAX_BRANCH_CONCURRENCY, + ), + 'ids' + ); + + return is_array( $ids ) ? count( $ids ) : 0; + } + /** * Trigger Action Scheduler's async-request queue runner: fire N CONCURRENT * loopback HTTP requests to admin-ajax.php so AS spawns N separate native-PHP diff --git a/src/Workflows/class-wp-agent-workflow-branch-store.php b/src/Workflows/class-wp-agent-workflow-branch-store.php index ae31fd9..8515c8c 100644 --- a/src/Workflows/class-wp-agent-workflow-branch-store.php +++ b/src/Workflows/class-wp-agent-workflow-branch-store.php @@ -399,25 +399,24 @@ public static function get_reconcile_receipt( string $result_ref, string $contex * @return void */ public static function forget_reconcile_receipt( string $run_id, string $handle_id, string $result_ref, string $context_ref ): void { + unset( $context_ref ); $row = self::read_row( $result_ref ); - if ( null !== $row ) { - if ( is_array( $row['descriptor'] ?? null ) ) { - $descriptor = $row['descriptor']; - unset( $descriptor[ self::RECONCILE_RECEIPT_KEY ] ); - $row['descriptor'] = $descriptor; - self::write_row( $result_ref, $row ); - } elseif ( self::branch_ref( $run_id, $handle_id ) === $result_ref && function_exists( 'delete_option' ) ) { - delete_option( $result_ref ); - } + if ( null === $row ) { return; } - $descriptor = self::filtered_get_branch( $result_ref, $context_ref ); - if ( null === $descriptor ) { + // Descriptor-backed and consumer-owned receipts belong to run-level cleanup. + // Never rewrite them here: a resume may have already called forget_run() + // after this read, and a write would resurrect the terminal run's storage. + if ( is_array( $row['descriptor'] ?? null ) ) { return; } - unset( $descriptor[ self::RECONCILE_RECEIPT_KEY ] ); - self::filtered_put_branch( $run_id, $handle_id, self::strip_shared_context( $descriptor ) ); + + // Receipt-only missing-descriptor rows are safe to delete exactly. Deletion + // remains harmless if terminal cleanup won the race first. + if ( self::branch_ref( $run_id, $handle_id ) === $result_ref && function_exists( 'delete_option' ) ) { + delete_option( $result_ref ); + } } /** diff --git a/src/Workflows/register-workflow-branch-executor.php b/src/Workflows/register-workflow-branch-executor.php index 1eddf1e..389c29a 100644 --- a/src/Workflows/register-workflow-branch-executor.php +++ b/src/Workflows/register-workflow-branch-executor.php @@ -243,10 +243,11 @@ static function ( $deferred, $run_id, $executor_id, $result ) { * @return int */ static function ( $batches ) { - $incoming = is_numeric( $batches ) ? (int) $batches : 1; - $branches = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::branch_inflight_count(); - $resumes = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::resume_inflight_count(); - if ( $branches < 1 && $resumes < 1 ) { + $incoming = is_numeric( $batches ) ? (int) $batches : 1; + $branches = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::branch_inflight_count(); + $resumes = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::resume_inflight_count(); + $reconciles = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::reconcile_inflight_count(); + if ( $branches < 1 && $resumes < 1 && $reconciles < 1 ) { return $incoming; } @@ -257,18 +258,17 @@ static function ( $batches ) { // parallel-branch behavior. $branch_ceiling = max( $incoming, min( $branches, $max ) ); - // Resume headroom is ADDITIVE on top of the branch ceiling — the resume needs - // a slot BEYOND the branches and beyond any UNRELATED claims that are already + // Continuation headroom is ADDITIVE on top of the branch ceiling — resume and + // reconcile-only actions need slots BEYOND branches and UNRELATED claims already // consuming the branch ceiling. If it merely matched the ceiling it would be - // starved: AS's has_maximum_concurrent_batches() compares the GLOBAL claim - // count against the ceiling, so a lone due resume with even one unrelated claim + // starved: AS's has_maximum_concurrent_batches() compares the GLOBAL claim count + // against the ceiling, so a lone due continuation with one unrelated claim // outstanding would sit at claim_count >= ceiling and never be admitted. Adding - // the resume count lifts the ceiling above the outstanding claims so the WP-Cron - // runner is admitted and claims the resume. Bounded (a fan-out has one resume; - // concurrent fan-outs are bounded by MAX) so the raise stays sane. - $resume_headroom = min( $resumes, $max ); + // each bounded count lifts the ceiling so the WP-Cron runner can claim it. + $resume_headroom = min( $resumes, $max ); + $reconcile_headroom = min( $reconciles, $max ); - return $branch_ceiling + $resume_headroom; + return $branch_ceiling + $resume_headroom + $reconcile_headroom; }, 100 ); diff --git a/tests/workflow-async-branch-payload-smoke.php b/tests/workflow-async-branch-payload-smoke.php index 97e2e08..7782287 100644 --- a/tests/workflow-async-branch-payload-smoke.php +++ b/tests/workflow-async-branch-payload-smoke.php @@ -64,6 +64,7 @@ public function execute( $input = null ) { $GLOBALS['__filters'] = array(); $GLOBALS['__abilities'] = array(); $GLOBALS['__options'] = array(); +$GLOBALS['__get_option_after_read'] = null; if ( ! function_exists( 'add_filter' ) ) { function add_filter( string $hook, callable $cb, int $priority = 10, int $accepted_args = 1 ): void { @@ -121,7 +122,11 @@ function current_user_can( $cap ): bool { unset( $cap ); return true; } } if ( ! function_exists( 'get_option' ) ) { function get_option( string $option, $default = false ) { - return array_key_exists( $option, $GLOBALS['__options'] ) ? $GLOBALS['__options'][ $option ] : $default; + $value = array_key_exists( $option, $GLOBALS['__options'] ) ? $GLOBALS['__options'][ $option ] : $default; + if ( is_callable( $GLOBALS['__get_option_after_read'] ) ) { + call_user_func( $GLOBALS['__get_option_after_read'], $option ); + } + return $value; } } if ( ! function_exists( 'add_option' ) ) { @@ -580,6 +585,27 @@ function payload_roles_spec(): WP_Agent_Workflow_Spec { ); smoke_assert( array(), array_values( $cleanup_leftover ), 'concurrent cleanup: stale shared index cannot orphan terminal result rows', $failures, $passes ); +// Terminal run cleanup can interleave after receipt cleanup reads a descriptor. +// The per-receipt path must never write that stale row back and resurrect it. +$GLOBALS['__options'] = array(); +$resurrection_run = 'pay-cleanup-interleave'; +$resurrection_ref = WP_Agent_Workflow_Branch_Store::put_branch( $resurrection_run, 'branch', array( 'run_id' => $resurrection_run, 'handle_id' => 'branch', 'key' => 'branch' ) ); +$resurrection_result = array( 'key' => 'branch', 'status' => 'succeeded', 'output' => array( 'ok' => true ) ); +WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $resurrection_run, 'branch', $resurrection_ref, '', $resurrection_result ); +$GLOBALS['__get_option_after_read'] = static function ( string $option ) use ( $resurrection_run, $resurrection_ref ): void { + if ( $resurrection_ref !== $option ) { + return; + } + $GLOBALS['__get_option_after_read'] = null; + WP_Agent_Workflow_Branch_Store::forget_run( $resurrection_run ); +}; +WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $resurrection_run, 'branch', $resurrection_ref, '' ); +$resurrection_leftover = array_filter( + array_keys( $GLOBALS['__options'] ), + static fn ( $option ): bool => str_starts_with( (string) $option, 'agents_wf_branch_' ) +); +smoke_assert( array(), array_values( $resurrection_leftover ), 'receipt cleanup race: terminal forget cannot be followed by descriptor resurrection', $failures, $passes ); + // A consumer-owned branch ref must keep terminal result persistence and cleanup // in that same custom store. No local option fallback is permitted. $GLOBALS['__options'] = array(); @@ -635,8 +661,8 @@ static function ( bool $handled, string $run_id ): bool { smoke_assert( array(), $GLOBALS['__options'], 'custom store: missing-descriptor receipt still creates no local fallback', $failures, $passes ); WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $custom_missing_run, 'missing-handle', $custom_missing_ref, '' ); $custom_missing_after_reconcile = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_missing_ref, '' ); -smoke_assert( null, $custom_missing_after_reconcile, 'custom store: successful reconcile removes the exact receipt through owner filters', $failures, $passes ); -smoke_assert( array(), $GLOBALS['__options'], 'custom store: exact receipt cleanup creates no local option fallback', $failures, $passes ); +smoke_assert( $custom_missing_result, $custom_missing_after_reconcile['branch_result'] ?? null, 'custom store: run-level owner cleanup retains receipt until terminal forget', $failures, $passes ); +smoke_assert( array(), $GLOBALS['__options'], 'custom store: deferred receipt cleanup creates no local option fallback', $failures, $passes ); WP_Agent_Workflow_Branch_Store::forget_run( $custom_missing_run ); smoke_assert( array(), $GLOBALS['__custom_branch_rows'], 'custom store: forget filter cleans receipt-only missing descriptor', $failures, $passes ); remove_all_filters( 'wp_agent_workflow_branch_store_put' ); diff --git a/tests/workflow-branch-concurrency-gate-smoke.php b/tests/workflow-branch-concurrency-gate-smoke.php index ee31b78..a33c332 100644 --- a/tests/workflow-branch-concurrency-gate-smoke.php +++ b/tests/workflow-branch-concurrency-gate-smoke.php @@ -165,6 +165,7 @@ function smoke_assert( $expected, $actual, string $name, array &$failures, int & use AgentsAPI\AI\Workflows\WP_Agent_Workflow_Action_Scheduler_Branch_Executor; $branch_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK; +$reconcile_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK; $resume_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RESUME_HOOK; $max = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::MAX_BRANCH_CONCURRENCY; @@ -183,6 +184,7 @@ function smoke_assert( $expected, $actual, string $name, array &$failures, int & AS_Query_Shim::reset(); smoke_assert( 0, WP_Agent_Workflow_Action_Scheduler_Branch_Executor::branch_inflight_count(), 'inflight=0 when no branch actions exist', $failures, $passes ); +smoke_assert( 0, WP_Agent_Workflow_Action_Scheduler_Branch_Executor::reconcile_inflight_count(), 'reconcile_inflight=0 when no retries exist', $failures, $passes ); smoke_assert( 1, $concurrent_batches(), 'no fan-out: concurrent_batches passes through AS default (1)', $failures, $passes ); smoke_assert( 25, $batch_size(), 'no fan-out: batch_size passes through AS default (25)', $failures, $passes ); @@ -321,5 +323,27 @@ function smoke_assert( $expected, $actual, string $name, array &$failures, int & smoke_assert( 0, WP_Agent_Workflow_Action_Scheduler_Branch_Executor::resume_inflight_count(), 'no fan-out: resume_inflight=0', $failures, $passes ); smoke_assert( 1, $concurrent_batches(), 'no fan-out: no resume slot added, stock ceiling (1)', $failures, $passes ); +// ═════════════════════════════════════════════════════════════════════════════ +// 10. RECONCILE-RETRY STARVATION. The original branch action is complete, so its +// inflight count is zero. One unrelated long-lived claim would close AS's +// default ceiling of 1; the pending reconcile retry needs one additive slot. +// ═════════════════════════════════════════════════════════════════════════════ + +AS_Query_Shim::reset(); +AS_Query_Shim::add( 'unrelated_long_action', ActionScheduler_Store::STATUS_RUNNING, 1 ); +AS_Query_Shim::add( $reconcile_hook, ActionScheduler_Store::STATUS_PENDING, 1 ); +$unrelated_claims = 1; +smoke_assert( 0, WP_Agent_Workflow_Action_Scheduler_Branch_Executor::branch_inflight_count(), 'reconcile starvation: original branch is no longer in flight', $failures, $passes ); +smoke_assert( 1, WP_Agent_Workflow_Action_Scheduler_Branch_Executor::reconcile_inflight_count(), 'reconcile starvation: pending retry counts as in flight', $failures, $passes ); +smoke_assert( 2, $concurrent_batches(), 'reconcile starvation: ceiling adds one retry slot above the default', $failures, $passes ); +smoke_assert( true, $concurrent_batches() > $unrelated_claims, 'reconcile starvation: unrelated long claim cannot block retry until reaping', $failures, $passes ); +smoke_assert( 25, $batch_size(), 'reconcile-only retry does not pin branch batch size', $failures, $passes ); + +AS_Query_Shim::reset(); +AS_Query_Shim::add( $reconcile_hook, ActionScheduler_Store::STATUS_PENDING, $max + 5 ); +AS_Query_Shim::add( $reconcile_hook, ActionScheduler_Store::STATUS_RUNNING, $max + 5 ); +smoke_assert( $max, WP_Agent_Workflow_Action_Scheduler_Branch_Executor::reconcile_inflight_count(), 'reconcile headroom count is bounded by MAX_BRANCH_CONCURRENCY', $failures, $passes ); +smoke_assert( $max + 1, $concurrent_batches(), 'reconcile headroom raise remains bounded', $failures, $passes ); + echo "Passed: {$passes}, Failed: " . count( $failures ) . "\n"; exit( count( $failures ) > 0 ? 1 : 0 ); From 5e6c49114ff9a297bde48b76535cba18871a94c8 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 17:49:30 +0000 Subject: [PATCH 05/11] fix: persist only contended reconcile receipts --- ...kflow-action-scheduler-branch-executor.php | 40 ++++++------ .../class-wp-agent-workflow-branch-store.php | 61 +++++-------------- tests/workflow-as-branch-smoke.php | 45 ++++++++++++-- tests/workflow-async-branch-payload-smoke.php | 42 ++++++++----- 4 files changed, 105 insertions(+), 83 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index d65f686..2492b10 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -829,6 +829,16 @@ public static function run_branch_action( array $payload ): void { } } + // A previous attempt may have completed branch effects and persisted a + // reconcile receipt before its retry enqueue failed. Resume from that receipt + // before reading the descriptor so retrying the branch action cannot repeat + // external effects. + $receipt = '' !== $store_ref ? WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $store_ref, $context_ref ) : null; + if ( null !== $receipt ) { + self::reconcile_branch_result( $run_id, $handle_id, $store_ref, $context_ref, $receipt['branch_result'], $receipt['continuation'], true ); + return; + } + // Rehydrate the full self-contained descriptor from the branch store using // the lightweight ref the AS args carried. The store re-seats the run-scoped // shared context into branch_vars.context, so the branch runs against the @@ -852,20 +862,19 @@ public static function run_branch_action( array $payload ): void { ), 'item' => null, ); - self::persist_and_reconcile_branch_result( $payload, $branch_result ); + self::reconcile_branch_action_result( $payload, $branch_result ); return; } $key = self::string_value( $descriptor['key'] ?? '' ); $branch_result = self::execute_branch( $descriptor, $key ); - self::persist_and_reconcile_branch_result( $payload, $branch_result ); + self::reconcile_branch_action_result( $payload, $branch_result ); } /** - * Persist and reconcile a completed branch result. Persistence happens before - * reconciliation so a lock-contention retry can survive process exit without - * executing the completed branch again. + * Reconcile a completed branch result directly from memory. Only lock + * contention persists a retry receipt, minimizing post-terminal writes. * * @since 0.7.0 * @@ -873,17 +882,13 @@ public static function run_branch_action( array $payload ): void { * @param array $branch_result Terminal branch result. * @return void */ - private static function persist_and_reconcile_branch_result( array $payload, array $branch_result ): void { + private static function reconcile_branch_action_result( array $payload, array $branch_result ): void { $run_id = self::string_value( $payload['run_id'] ?? '' ); $handle_id = self::string_value( $payload['handle_id'] ?? '' ); $store_ref = self::string_value( $payload['store_ref'] ?? '' ); $context_ref = self::string_value( $payload['context_ref'] ?? '' ); - $result_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $run_id, $handle_id, $store_ref, $context_ref, $branch_result ); - if ( is_wp_error( $result_ref ) ) { - throw new \RuntimeException( $result_ref->get_error_message() ); - } - self::reconcile_branch_result( $run_id, $handle_id, $result_ref, $context_ref, $branch_result, array(), false ); + self::reconcile_branch_result( $run_id, $handle_id, $store_ref, $context_ref, $branch_result, array(), false ); } /** @@ -953,6 +958,9 @@ private static function reconcile_branch_result( string $run_id, string $handle_ if ( ! is_wp_error( $result ) ) { WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref ); + if ( is_object( $result ) && method_exists( $result, 'is_suspended' ) && ! $result->is_suspended() ) { + WP_Agent_Workflow_Branch_Store::forget_run( $run_id ); + } return; } if ( 'agents_reconcile_lock_unavailable' !== $result->get_error_code() ) { @@ -963,13 +971,11 @@ private static function reconcile_branch_result( string $run_id, string $handle_ $next_continuation = is_array( $error_data ) && is_array( $error_data['reconcile_continuation'] ?? null ) ? self::string_keyed_array( $error_data['reconcile_continuation'] ) : $continuation; - if ( $next_continuation !== $continuation ) { - $next_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref, $branch_result, $next_continuation ); - if ( is_wp_error( $next_ref ) ) { - throw new \RuntimeException( $next_ref->get_error_message() ); - } - $result_ref = $next_ref; + $next_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref, $branch_result, $next_continuation ); + if ( is_wp_error( $next_ref ) ) { + throw new \RuntimeException( $next_ref->get_error_message() ); } + $result_ref = $next_ref; $action_id = self::enqueue_async_action( self::RECONCILE_HOOK, diff --git a/src/Workflows/class-wp-agent-workflow-branch-store.php b/src/Workflows/class-wp-agent-workflow-branch-store.php index 8515c8c..db1700e 100644 --- a/src/Workflows/class-wp-agent-workflow-branch-store.php +++ b/src/Workflows/class-wp-agent-workflow-branch-store.php @@ -276,9 +276,10 @@ public static function get_branch( string $store_ref, string $context_ref ): ?ar } /** - * Persist a terminal branch result and opaque continuation state inside the - * branch's existing descriptor row. Reusing that already-indexed per-branch - * row avoids a concurrent shared-index append during branch completion. + * Persist a terminal branch result and opaque continuation state as a + * receipt-only record. This deliberately replaces neither a stale descriptor + * snapshot nor the shared run index, so terminal cleanup cannot be followed by + * descriptor resurrection. * * @since 0.7.0 * @@ -295,26 +296,7 @@ public static function put_reconcile_receipt( string $run_id, string $handle_id, 'branch_result' => $branch_result, 'continuation' => $continuation, ); - $row = self::read_row( $store_ref ); - - if ( null !== $row && is_array( $row['descriptor'] ?? null ) ) { - $descriptor = $row['descriptor']; - $descriptor[ self::RECONCILE_RECEIPT_KEY ] = $receipt; - $row['descriptor'] = $descriptor; - self::write_row( $store_ref, $row ); - - $persisted = self::read_row( $store_ref ); - $persisted_descriptor = null !== $persisted && is_array( $persisted['descriptor'] ?? null ) ? $persisted['descriptor'] : array(); - if ( $receipt === ( $persisted_descriptor[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ) { - return $store_ref; - } - return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'Could not durably persist the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); - } - - // A missing or expired built-in descriptor still has a deterministic ref. - // Persist a receipt-only row at that exact ref: no shared-index append, and - // successful reconciliation can delete the row directly. if ( self::branch_ref( $run_id, $handle_id ) === $store_ref ) { self::write_row( $store_ref, @@ -330,21 +312,18 @@ public static function put_reconcile_receipt( string $run_id, string $handle_id, return $store_ref; } - return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'Could not durably persist the terminal result for missing branch `%s` in run `%s`.', $handle_id, $run_id ) ); + return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'Could not durably persist the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); } - // A non-local ref belongs to the consumer that supplied it through the - // existing branch-store filters. Require that same owner to persist and read - // the receipt; never fall back to an unexpected local option row. - $descriptor = self::filtered_get_branch( $store_ref, $context_ref ); - if ( null === $descriptor ) { - $descriptor = array( - 'run_id' => $run_id, - 'handle_id' => $handle_id, - ); - } - $descriptor[ self::RECONCILE_RECEIPT_KEY ] = $receipt; - $result_ref = self::filtered_put_branch( $run_id, $handle_id, self::strip_shared_context( $descriptor ) ); + // A non-local ref belongs to a consumer store. Persist only the receipt shape + // through its existing put/get contract; never rehydrate and rewrite a stale + // descriptor, and never fall back to local options. + $receipt_record = array( + 'run_id' => $run_id, + 'handle_id' => $handle_id, + self::RECONCILE_RECEIPT_KEY => $receipt, + ); + $result_ref = self::filtered_put_branch( $run_id, $handle_id, $receipt_record ); if ( null === $result_ref ) { return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'The branch store that owns `%s` did not persist the terminal result.', $store_ref ) ); } @@ -643,16 +622,4 @@ private static function string_keyed_array( array $value ): array { return $normalized; } - /** - * Keep the existing custom-store put contract free of shared context copies. - * - * @param array $descriptor Branch descriptor. - * @return array - */ - private static function strip_shared_context( array $descriptor ): array { - if ( is_array( $descriptor['branch_vars'] ?? null ) && array_key_exists( 'context', $descriptor['branch_vars'] ) ) { - unset( $descriptor['branch_vars']['context'] ); - } - return $descriptor; - } } diff --git a/tests/workflow-as-branch-smoke.php b/tests/workflow-as-branch-smoke.php index ea11901..9d3a1e3 100644 --- a/tests/workflow-as-branch-smoke.php +++ b/tests/workflow-as-branch-smoke.php @@ -785,9 +785,8 @@ function as_smoke_two_fanout_spec(): WP_Agent_Workflow_Spec { smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUCCEEDED, $final5->get_status(), 'multi-fanout: second resume reaches terminal success', $failures, $passes ); smoke_assert( $group5, WP_Agent_Workflow_Action_Scheduler_Branch_Executor::group_for_run( 'as-two' ), 'multi-fanout: terminal run retains the same deterministic group identity', $failures, $passes ); -// A terminal result must be durably readable before reconciliation begins. If -// the branch-row update fails while the reconcile lock is also contended, the -// original action fails loudly and no unrecoverable reconcile retry is queued. +// A contended result is persisted only after the in-memory reconcile attempt. If +// that receipt write fails, the action fails loudly and queues no unreadable retry. AS_Shim::reset(); $recorder6 = new AS_Smoke_Recorder(); remove_all_filters( 'wp_agent_workflow_run_recorder' ); @@ -816,7 +815,7 @@ static function () use ( &$lock_attempts6 ) { smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUSPENDED, $run6->get_status(), 'failed result write: fixture starts suspended', $failures, $passes ); smoke_assert( true, $write_failed6, 'failed result write: branch action fails loudly before completing', $failures, $passes ); smoke_assert( $effect_before6 + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'failed result write: branch side effect ran exactly once', $failures, $passes ); -smoke_assert( 0, $lock_attempts6, 'failed result write: reconcile is not attempted with an unreadable result', $failures, $passes ); +smoke_assert( 1, $lock_attempts6, 'failed result write: in-memory reconcile is attempted before receipt persistence', $failures, $passes ); smoke_assert( 0, count( AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK ) ), 'failed result write: no stranded reconcile-only retry is queued', $failures, $passes ); $GLOBALS['__reject_option_write'] = ''; remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); @@ -873,5 +872,43 @@ static function ( $override, string $run_id, callable $critical ) use ( &$edge_l remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); } +// If receipt persistence succeeds but enqueueing RECONCILE_HOOK fails, retrying +// the original branch payload must discover the receipt before executing effects. +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$recorder7 = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder7 ) { return $recorder7; } ); +( new WP_Agent_Workflow_Runner( $recorder7 ) )->run( as_smoke_roles_spec(), array(), array( 'run_id' => 'as-enqueue-fail' ) ); +$branches7 = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +$payload7 = $branches7[0]['args'][0] ?? array(); +$lock_attempts7 = 0; +add_filter( + 'wp_agent_workflow_reconcile_lock', + static function ( $override, string $run_id, callable $critical ) use ( &$lock_attempts7 ) { + unset( $override ); + if ( 'as-enqueue-fail' === $run_id && 0 === $lock_attempts7++ ) { + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + } + return $critical(); + }, + 10, + 3 +); +$effect_before7 = (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ); +AS_Shim::$reject_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK; +$enqueue_failed7 = false; +try { + AS_Shim::fire( $branches7[0]['id'] ); +} catch ( \RuntimeException $error ) { + $enqueue_failed7 = str_contains( $error->getMessage(), 'enqueue a reconcile retry' ); +} +AS_Shim::$reject_hook = ''; +WP_Agent_Workflow_Action_Scheduler_Branch_Executor::run_branch_action( $payload7 ); +smoke_assert( true, $enqueue_failed7, 'failed retry enqueue: original branch action fails loudly after persisting receipt', $failures, $passes ); +smoke_assert( $effect_before7 + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'failed retry enqueue: retried branch payload does not repeat side effect', $failures, $passes ); +smoke_assert( 1, count( $recorder7->find( 'as-enqueue-fail' )->get_suspension()['completed'] ?? array() ), 'failed retry enqueue: retried branch payload reconciles persisted result', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); + echo "Passed: {$passes}, Failed: " . count( $failures ) . "\n"; exit( count( $failures ) > 0 ? 1 : 0 ); diff --git a/tests/workflow-async-branch-payload-smoke.php b/tests/workflow-async-branch-payload-smoke.php index 7782287..b710941 100644 --- a/tests/workflow-async-branch-payload-smoke.php +++ b/tests/workflow-async-branch-payload-smoke.php @@ -64,7 +64,7 @@ public function execute( $input = null ) { $GLOBALS['__filters'] = array(); $GLOBALS['__abilities'] = array(); $GLOBALS['__options'] = array(); -$GLOBALS['__get_option_after_read'] = null; +$GLOBALS['__update_option_before_write'] = null; if ( ! function_exists( 'add_filter' ) ) { function add_filter( string $hook, callable $cb, int $priority = 10, int $accepted_args = 1 ): void { @@ -122,11 +122,7 @@ function current_user_can( $cap ): bool { unset( $cap ); return true; } } if ( ! function_exists( 'get_option' ) ) { function get_option( string $option, $default = false ) { - $value = array_key_exists( $option, $GLOBALS['__options'] ) ? $GLOBALS['__options'][ $option ] : $default; - if ( is_callable( $GLOBALS['__get_option_after_read'] ) ) { - call_user_func( $GLOBALS['__get_option_after_read'], $option ); - } - return $value; + return array_key_exists( $option, $GLOBALS['__options'] ) ? $GLOBALS['__options'][ $option ] : $default; } } if ( ! function_exists( 'add_option' ) ) { @@ -142,6 +138,9 @@ function add_option( string $option, $value = '', $deprecated = '', $autoload = if ( ! function_exists( 'update_option' ) ) { function update_option( string $option, $value, $autoload = null ): bool { unset( $autoload ); + if ( is_callable( $GLOBALS['__update_option_before_write'] ) ) { + call_user_func( $GLOBALS['__update_option_before_write'], $option, $value ); + } $GLOBALS['__options'][ $option ] = $value; return true; } @@ -585,26 +584,39 @@ function payload_roles_spec(): WP_Agent_Workflow_Spec { ); smoke_assert( array(), array_values( $cleanup_leftover ), 'concurrent cleanup: stale shared index cannot orphan terminal result rows', $failures, $passes ); -// Terminal run cleanup can interleave after receipt cleanup reads a descriptor. -// The per-receipt path must never write that stale row back and resurrect it. +// Terminal cleanup can win immediately before a contended branch persists its +// receipt. The stale writer may create a receipt-only row afterward, but a +// terminal retry must discover it without effects and clean it deterministically. $GLOBALS['__options'] = array(); -$resurrection_run = 'pay-cleanup-interleave'; -$resurrection_ref = WP_Agent_Workflow_Branch_Store::put_branch( $resurrection_run, 'branch', array( 'run_id' => $resurrection_run, 'handle_id' => 'branch', 'key' => 'branch' ) ); +$resurrection_run = 'pay-A'; +$resurrection_ref = WP_Agent_Workflow_Branch_Store::put_branch( $resurrection_run, 'race-handle', array( 'run_id' => $resurrection_run, 'handle_id' => 'race-handle', 'key' => 'branch' ) ); $resurrection_result = array( 'key' => 'branch', 'status' => 'succeeded', 'output' => array( 'ok' => true ) ); -WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $resurrection_run, 'branch', $resurrection_ref, '', $resurrection_result ); -$GLOBALS['__get_option_after_read'] = static function ( string $option ) use ( $resurrection_run, $resurrection_ref ): void { +$GLOBALS['__update_option_before_write'] = static function ( string $option ) use ( $resurrection_run, $resurrection_ref ): void { if ( $resurrection_ref !== $option ) { return; } - $GLOBALS['__get_option_after_read'] = null; + $GLOBALS['__update_option_before_write'] = null; WP_Agent_Workflow_Branch_Store::forget_run( $resurrection_run ); }; -WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $resurrection_run, 'branch', $resurrection_ref, '' ); +WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $resurrection_run, 'race-handle', $resurrection_ref, '', $resurrection_result ); +smoke_assert( false, isset( $GLOBALS['__options'][ $resurrection_ref ]['descriptor'] ), 'receipt put race: stale writer cannot recreate descriptor state', $failures, $passes ); +$GLOBALS['__branch_effects'] = 1; +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder ) { return $recorder; } ); +WP_Agent_Workflow_Action_Scheduler_Branch_Executor::run_reconcile_action( + array( + 'run_id' => $resurrection_run, + 'handle_id' => 'race-handle', + 'result_ref' => $resurrection_ref, + 'context_ref' => '', + ) +); $resurrection_leftover = array_filter( array_keys( $GLOBALS['__options'] ), static fn ( $option ): bool => str_starts_with( (string) $option, 'agents_wf_branch_' ) ); -smoke_assert( array(), array_values( $resurrection_leftover ), 'receipt cleanup race: terminal forget cannot be followed by descriptor resurrection', $failures, $passes ); +smoke_assert( array(), array_values( $resurrection_leftover ), 'receipt put race: terminal retry removes row written after forget_run', $failures, $passes ); +smoke_assert( 1, $GLOBALS['__branch_effects'], 'receipt put race: terminal retry does not repeat branch effects', $failures, $passes ); // A consumer-owned branch ref must keep terminal result persistence and cleanup // in that same custom store. No local option fallback is permitted. From a07b3d41458475cc1433ea3a3b1a54430cddebff Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 18:09:53 +0000 Subject: [PATCH 06/11] fix: recover failed reconcile continuations --- ...kflow-action-scheduler-branch-executor.php | 155 ++++++++++++++---- .../class-wp-agent-workflow-branch-store.php | 129 ++++++++++++--- .../register-workflow-branch-executor.php | 25 +++ stubs/action-scheduler-classes.php | 16 ++ tests/workflow-as-branch-smoke.php | 82 +++++++-- tests/workflow-async-branch-payload-smoke.php | 115 +++++++++++-- 6 files changed, 449 insertions(+), 73 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index 2492b10..8530bb5 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -214,12 +214,15 @@ public function dispatch( array $branches, array $context ) { $descriptor = self::strip_shared_context( $descriptor ); // Offload the descriptor to the store; the AS args carry only the ref. - $store_ref = WP_Agent_Workflow_Branch_Store::put_branch( $run_id, $handle_id, $descriptor ); + $stored = WP_Agent_Workflow_Branch_Store::put_branch_with_provenance( $run_id, $handle_id, $descriptor ); + $store_ref = $stored['ref']; + $store_backend = $stored['backend']; $payload = array( 'run_id' => $run_id, 'handle_id' => $handle_id, 'store_ref' => $store_ref, + 'store_backend' => $store_backend, 'context_ref' => $context_ref, 'admission_token' => $admission_token, ); @@ -801,13 +804,14 @@ public function collect( array $handles ): array { * * @since 0.5.0 * - * @param array $payload Action payload: { run_id, handle_id, store_ref, context_ref, admission_token }. + * @param array $payload Action payload: { run_id, handle_id, store_ref, store_backend, context_ref, admission_token }. * @return void */ public static function run_branch_action( array $payload ): void { $run_id = self::string_value( $payload['run_id'] ?? '' ); $handle_id = self::string_value( $payload['handle_id'] ?? '' ); $store_ref = self::string_value( $payload['store_ref'] ?? '' ); + $store_backend = self::string_value( $payload['store_backend'] ?? WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY ); $context_ref = self::string_value( $payload['context_ref'] ?? '' ); $admission_token = self::string_value( $payload['admission_token'] ?? '' ); @@ -833,9 +837,9 @@ public static function run_branch_action( array $payload ): void { // reconcile receipt before its retry enqueue failed. Resume from that receipt // before reading the descriptor so retrying the branch action cannot repeat // external effects. - $receipt = '' !== $store_ref ? WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $store_ref, $context_ref ) : null; + $receipt = '' !== $store_ref ? WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $store_ref, $context_ref, $store_backend ) : null; if ( null !== $receipt ) { - self::reconcile_branch_result( $run_id, $handle_id, $store_ref, $context_ref, $receipt['branch_result'], $receipt['continuation'], true ); + self::reconcile_branch_result( $run_id, $handle_id, $store_ref, $context_ref, $store_backend, $receipt['branch_result'], $receipt['continuation'], true ); return; } @@ -883,12 +887,13 @@ public static function run_branch_action( array $payload ): void { * @return void */ private static function reconcile_branch_action_result( array $payload, array $branch_result ): void { - $run_id = self::string_value( $payload['run_id'] ?? '' ); - $handle_id = self::string_value( $payload['handle_id'] ?? '' ); - $store_ref = self::string_value( $payload['store_ref'] ?? '' ); - $context_ref = self::string_value( $payload['context_ref'] ?? '' ); + $run_id = self::string_value( $payload['run_id'] ?? '' ); + $handle_id = self::string_value( $payload['handle_id'] ?? '' ); + $store_ref = self::string_value( $payload['store_ref'] ?? '' ); + $store_backend = self::string_value( $payload['store_backend'] ?? WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY ); + $context_ref = self::string_value( $payload['context_ref'] ?? '' ); - self::reconcile_branch_result( $run_id, $handle_id, $store_ref, $context_ref, $branch_result, array(), false ); + self::reconcile_branch_result( $run_id, $handle_id, $store_ref, $context_ref, $store_backend, $branch_result, array(), false ); } /** @@ -897,27 +902,28 @@ private static function reconcile_branch_action_result( array $payload, array $b * * @since 0.7.0 * - * @param array $payload Action payload: { run_id, handle_id, result_ref, context_ref }. - * @return void + * @param array $payload Action payload: { run_id, handle_id, result_ref, store_backend, context_ref }. + * @return bool Whether reconciliation completed or a durable continuation exists. */ - public static function run_reconcile_action( array $payload ): void { - $run_id = self::string_value( $payload['run_id'] ?? '' ); - $handle_id = self::string_value( $payload['handle_id'] ?? '' ); - $result_ref = self::string_value( $payload['result_ref'] ?? '' ); - $context_ref = self::string_value( $payload['context_ref'] ?? '' ); + public static function run_reconcile_action( array $payload ): bool { + $run_id = self::string_value( $payload['run_id'] ?? '' ); + $handle_id = self::string_value( $payload['handle_id'] ?? '' ); + $result_ref = self::string_value( $payload['result_ref'] ?? '' ); + $store_backend = self::string_value( $payload['store_backend'] ?? WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY ); + $context_ref = self::string_value( $payload['context_ref'] ?? '' ); if ( '' === $run_id || '' === $handle_id || '' === $result_ref ) { - return; + return false; } - $receipt = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $result_ref, $context_ref ); + $receipt = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $result_ref, $context_ref, $store_backend ); if ( null === $receipt ) { if ( self::is_branch_reconciled( $run_id, $handle_id ) ) { - return; + return true; } throw new \RuntimeException( sprintf( 'Could not rehydrate the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); } - self::reconcile_branch_result( $run_id, $handle_id, $result_ref, $context_ref, $receipt['branch_result'], $receipt['continuation'], true ); + return self::reconcile_branch_result( $run_id, $handle_id, $result_ref, $context_ref, $store_backend, $receipt['branch_result'], $receipt['continuation'], true ); } /** @@ -928,12 +934,13 @@ public static function run_reconcile_action( array $payload ): void { * @param string $handle_id Branch handle id. * @param string $result_ref Durable terminal-result ref. * @param string $context_ref Shared-context ref for custom stores. + * @param string $store_backend Explicit descriptor backend provenance. * @param array $branch_result Terminal BranchResult. * @param array $continuation Opaque reconcile continuation state. * @param bool $is_retry Whether this is a reconcile-only retry. - * @return void + * @return bool Whether reconciliation completed or a durable continuation exists. */ - private static function reconcile_branch_result( string $run_id, string $handle_id, string $result_ref, string $context_ref, array $branch_result, array $continuation, bool $is_retry ): void { + private static function reconcile_branch_result( string $run_id, string $handle_id, string $result_ref, string $context_ref, string $store_backend, array $branch_result, array $continuation, bool $is_retry ): bool { $result = null; if ( $is_retry ) { /** @@ -957,21 +964,21 @@ private static function reconcile_branch_result( string $run_id, string $handle_ } if ( ! is_wp_error( $result ) ) { - WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref ); + WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref, $store_backend ); if ( is_object( $result ) && method_exists( $result, 'is_suspended' ) && ! $result->is_suspended() ) { WP_Agent_Workflow_Branch_Store::forget_run( $run_id ); } - return; + return true; } if ( 'agents_reconcile_lock_unavailable' !== $result->get_error_code() ) { - return; + return false; } $error_data = $result->get_error_data(); $next_continuation = is_array( $error_data ) && is_array( $error_data['reconcile_continuation'] ?? null ) ? self::string_keyed_array( $error_data['reconcile_continuation'] ) : $continuation; - $next_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref, $branch_result, $next_continuation ); + $next_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref, $store_backend, $branch_result, $next_continuation ); if ( is_wp_error( $next_ref ) ) { throw new \RuntimeException( $next_ref->get_error_message() ); } @@ -981,10 +988,11 @@ private static function reconcile_branch_result( string $run_id, string $handle_ self::RECONCILE_HOOK, array( array( - 'run_id' => $run_id, - 'handle_id' => $handle_id, - 'result_ref' => $result_ref, - 'context_ref' => $context_ref, + 'run_id' => $run_id, + 'handle_id' => $handle_id, + 'result_ref' => $result_ref, + 'store_backend' => $store_backend, + 'context_ref' => $context_ref, ), ), self::group_for_run( $run_id ) @@ -992,6 +1000,93 @@ private static function reconcile_branch_result( string $run_id, string $handle_ if ( $action_id <= 0 ) { throw new \RuntimeException( sprintf( 'Could not enqueue a reconcile retry for branch `%s` in run `%s` after lock contention.', $handle_id, $run_id ) ); } + return true; + } + + /** + * Recover a failed branch action without invoking its effects again. + * + * @param int|string $action_id Failed Action Scheduler action id. + * @param mixed $failure Failure exception or timeout metadata. + */ + public static function recover_failed_action( $action_id, $failure = null ): void { + if ( ! class_exists( '\ActionScheduler_Store' ) ) { + return; + } + try { + $action = \ActionScheduler_Store::instance()->fetch_action( $action_id ); + } catch ( \Throwable $error ) { + unset( $error ); + return; + } + if ( self::BRANCH_HOOK !== $action->get_hook() ) { + return; + } + $args = $action->get_args(); + $payload = is_array( $args[0] ?? null ) ? $args[0] : array(); + $run_id = self::string_value( $payload['run_id'] ?? '' ); + $handle_id = self::string_value( $payload['handle_id'] ?? '' ); + $result_ref = self::string_value( $payload['store_ref'] ?? '' ); + $context_ref = self::string_value( $payload['context_ref'] ?? '' ); + $store_backend = self::string_value( $payload['store_backend'] ?? WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY ); + $receipt = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $result_ref, $context_ref, $store_backend ); + if ( '' === $run_id || '' === $handle_id ) { + return; + } + if ( null === $receipt ) { + if ( WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY !== $store_backend ) { + $failure_message = $failure instanceof \Throwable ? $failure->getMessage() : 'The branch failed before a durable reconcile continuation could be established.'; + self::fail_reconcile_recovery( $run_id, $handle_id, $failure_message ); + } + return; + } + + $retry_payload = array( + 'run_id' => $run_id, + 'handle_id' => $handle_id, + 'result_ref' => $result_ref, + 'context_ref' => $context_ref, + 'store_backend' => $store_backend, + ); + $recovery_id = self::enqueue_async_action( self::RECONCILE_HOOK, array( $retry_payload ), self::group_for_run( $run_id ) ); + if ( $recovery_id > 0 ) { + return; + } + + try { + if ( self::run_reconcile_action( $retry_payload ) ) { + return; + } + } catch ( \Throwable $error ) { + $message = $error->getMessage(); + } + + self::fail_reconcile_recovery( $run_id, $handle_id, isset( $message ) ? $message : 'No durable reconcile continuation could be established.' ); + } + + /** Mark a stranded suspended run terminal when every recovery path failed. */ + private static function fail_reconcile_recovery( string $run_id, string $handle_id, string $message ): void { + $recorder = agents_workflow_resolve_recorder(); + $result = null !== $recorder ? $recorder->find( $run_id ) : null; + if ( null === $recorder || null === $result || ! $result->is_suspended() ) { + return; + } + $metadata = $result->get_metadata(); + unset( $metadata['_suspension'] ); + $terminal = $result->with( + array( + 'status' => WP_Agent_Workflow_Run_Result::STATUS_FAILED, + 'error' => array( + 'code' => 'workflow_branch_reconcile_recovery_failed', + 'message' => sprintf( 'Could not recover reconciliation for branch `%s`: %s', $handle_id, $message ), + ), + 'ended_at' => time(), + 'metadata' => $metadata, + ) + ); + $recorder->update( $terminal ); + \AgentsAPI\AI\WP_Agent_Run_Control::finish_run( WP_Agent_Workflow_Runner::RUN_CONTROL_STORE, $run_id, \AgentsAPI\AI\WP_Agent_Run_Control::STATUS_FAILED ); + WP_Agent_Workflow_Branch_Store::forget_run( $run_id ); } /** Whether the authoritative run already recorded this branch completion. */ diff --git a/src/Workflows/class-wp-agent-workflow-branch-store.php b/src/Workflows/class-wp-agent-workflow-branch-store.php index db1700e..d049581 100644 --- a/src/Workflows/class-wp-agent-workflow-branch-store.php +++ b/src/Workflows/class-wp-agent-workflow-branch-store.php @@ -55,6 +55,10 @@ */ final class WP_Agent_Workflow_Branch_Store { + public const BACKEND_BUILTIN = 'builtin'; + public const BACKEND_CUSTOM = 'custom'; + public const BACKEND_LEGACY = 'legacy'; + /** * Option-name prefix for a per-branch descriptor row. The run id and handle * id are folded into the key so rows never collide across runs/branches and @@ -193,9 +197,22 @@ public static function admission_status( string $token ): string { * @return string The store ref (the option name) placed in the AS args. */ public static function put_branch( string $run_id, string $handle_id, array $descriptor ): string { + $stored = self::put_branch_with_provenance( $run_id, $handle_id, $descriptor ); + return $stored['ref']; + } + + /** + * Persist a descriptor and report which backend owns its opaque ref. + * + * @param string $run_id Run id. + * @param string $handle_id Branch handle id. + * @param array $descriptor Branch descriptor. + * @return array{ref:string,backend:string} + */ + public static function put_branch_with_provenance( string $run_id, string $handle_id, array $descriptor ): array { $override = self::filtered_put_branch( $run_id, $handle_id, $descriptor ); if ( is_string( $override ) ) { - return $override; + return array( 'ref' => $override, 'backend' => self::BACKEND_CUSTOM ); } $ref = self::branch_ref( $run_id, $handle_id ); @@ -209,7 +226,7 @@ public static function put_branch( string $run_id, string $handle_id, array $des ) ); self::index_ref( $run_id, $ref ); - return $ref; + return array( 'ref' => $ref, 'backend' => self::BACKEND_BUILTIN ); } /** @@ -287,17 +304,18 @@ public static function get_branch( string $store_ref, string $context_ref ): ?ar * @param string $handle_id Branch handle id. * @param string $store_ref Existing branch descriptor ref. * @param string $context_ref Existing shared-context ref. + * @param string $backend Explicit descriptor backend provenance. * @param array $branch_result Terminal BranchResult. * @param array $continuation Opaque reconcile continuation state. * @return string|\WP_Error Durable receipt ref, or a hard persistence failure. */ - public static function put_reconcile_receipt( string $run_id, string $handle_id, string $store_ref, string $context_ref, array $branch_result, array $continuation = array() ) { + public static function put_reconcile_receipt( string $run_id, string $handle_id, string $store_ref, string $context_ref, string $backend, array $branch_result, array $continuation = array() ) { $receipt = array( 'branch_result' => $branch_result, 'continuation' => $continuation, ); - if ( self::branch_ref( $run_id, $handle_id ) === $store_ref ) { + if ( self::BACKEND_BUILTIN === $backend ) { self::write_row( $store_ref, array( @@ -315,21 +333,17 @@ public static function put_reconcile_receipt( string $run_id, string $handle_id, return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'Could not durably persist the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); } - // A non-local ref belongs to a consumer store. Persist only the receipt shape - // through its existing put/get contract; never rehydrate and rewrite a stale - // descriptor, and never fall back to local options. - $receipt_record = array( - 'run_id' => $run_id, - 'handle_id' => $handle_id, - self::RECONCILE_RECEIPT_KEY => $receipt, - ); - $result_ref = self::filtered_put_branch( $run_id, $handle_id, $receipt_record ); + if ( self::BACKEND_CUSTOM !== $backend ) { + return new \WP_Error( 'workflow_branch_receipt_backend_unknown', sprintf( 'Branch `%s` in run `%s` has no explicit receipt backend provenance.', $handle_id, $run_id ) ); + } + + $result_ref = self::filtered_put_receipt( $store_ref, $run_id, $handle_id, $receipt ); if ( null === $result_ref ) { - return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'The branch store that owns `%s` did not persist the terminal result.', $store_ref ) ); + return new \WP_Error( 'workflow_branch_receipt_backend_unsupported', sprintf( 'The custom branch store that owns `%s` does not provide durable reconcile receipt persistence.', $store_ref ) ); } - $persisted = self::filtered_get_branch( $result_ref, $context_ref ); - if ( null === $persisted || $receipt !== ( $persisted[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ) { + $persisted = self::filtered_get_receipt( $result_ref, $context_ref ); + if ( null === $persisted || $receipt !== $persisted ) { return new \WP_Error( 'workflow_branch_result_persistence_failed', sprintf( 'The branch store could not verify the terminal result for branch `%s` in run `%s`.', $handle_id, $run_id ) ); } @@ -344,9 +358,23 @@ public static function put_reconcile_receipt( string $run_id, string $handle_id, * * @param string $result_ref Opaque receipt ref. * @param string $context_ref Existing shared-context ref. + * @param string $backend Explicit descriptor backend provenance. * @return array{branch_result:array,continuation:array}|null */ - public static function get_reconcile_receipt( string $result_ref, string $context_ref ): ?array { + public static function get_reconcile_receipt( string $result_ref, string $context_ref, string $backend ): ?array { + if ( self::BACKEND_CUSTOM === $backend ) { + $receipt = self::filtered_get_receipt( $result_ref, $context_ref ); + if ( null === $receipt || ! is_array( $receipt['branch_result'] ?? null ) ) { + return null; + } + return array( + 'branch_result' => self::string_keyed_array( $receipt['branch_result'] ), + 'continuation' => is_array( $receipt['continuation'] ?? null ) ? self::string_keyed_array( $receipt['continuation'] ) : array(), + ); + } + if ( self::BACKEND_BUILTIN !== $backend ) { + return null; + } $row = self::read_row( $result_ref ); if ( null !== $row && is_array( $row[ self::RECONCILE_RECEIPT_KEY ] ?? null ) ) { $receipt = $row[ self::RECONCILE_RECEIPT_KEY ]; @@ -375,10 +403,17 @@ public static function get_reconcile_receipt( string $result_ref, string $contex * @param string $handle_id Branch handle id. * @param string $result_ref Receipt ref. * @param string $context_ref Shared-context ref for custom stores. + * @param string $backend Explicit descriptor backend provenance. * @return void */ - public static function forget_reconcile_receipt( string $run_id, string $handle_id, string $result_ref, string $context_ref ): void { - unset( $context_ref ); + public static function forget_reconcile_receipt( string $run_id, string $handle_id, string $result_ref, string $context_ref, string $backend ): void { + if ( self::BACKEND_CUSTOM === $backend ) { + self::filtered_forget_receipt( $result_ref, $run_id, $handle_id, $context_ref ); + return; + } + if ( self::BACKEND_BUILTIN !== $backend ) { + return; + } $row = self::read_row( $result_ref ); if ( null === $row ) { return; @@ -581,6 +616,62 @@ private static function filtered_get_branch( string $store_ref, string $context_ return is_array( $descriptor ) ? $descriptor : null; } + /** @param array $receipt */ + private static function filtered_put_receipt( string $store_ref, string $run_id, string $handle_id, array $receipt ): ?string { + if ( ! function_exists( 'apply_filters' ) ) { + return null; + } + /** + * Persist a reconcile receipt in the custom backend that owns a branch ref. + * + * @since 0.7.0 + * + * @param string|null $receipt_ref No implementation by default. + * @param string $store_ref Owning descriptor ref. + * @param string $run_id Run id. + * @param string $handle_id Handle id. + * @param array $receipt Receipt payload. + */ + $receipt_ref = apply_filters( 'wp_agent_workflow_branch_receipt_put', null, $store_ref, $run_id, $handle_id, $receipt ); + return is_string( $receipt_ref ) && '' !== $receipt_ref ? $receipt_ref : null; + } + + /** @return array|null */ + private static function filtered_get_receipt( string $receipt_ref, string $context_ref ): ?array { + if ( ! function_exists( 'apply_filters' ) ) { + return null; + } + /** + * Read a reconcile receipt from a custom branch backend. + * + * @since 0.7.0 + * + * @param array|null $receipt No implementation by default. + * @param string $receipt_ref Receipt ref. + * @param string $context_ref Shared-context ref. + */ + $receipt = apply_filters( 'wp_agent_workflow_branch_receipt_get', null, $receipt_ref, $context_ref ); + return is_array( $receipt ) ? $receipt : null; + } + + private static function filtered_forget_receipt( string $receipt_ref, string $run_id, string $handle_id, string $context_ref ): bool { + if ( ! function_exists( 'apply_filters' ) ) { + return false; + } + /** + * Delete one reconcile receipt from a custom branch backend. + * + * @since 0.7.0 + * + * @param bool $handled No implementation by default. + * @param string $receipt_ref Receipt ref. + * @param string $run_id Run id. + * @param string $handle_id Handle id. + * @param string $context_ref Shared-context ref. + */ + return (bool) apply_filters( 'wp_agent_workflow_branch_receipt_delete', false, $receipt_ref, $run_id, $handle_id, $context_ref ); + } + /** * Offer a consumer's store the chance to release a run's payload rows. * diff --git a/src/Workflows/register-workflow-branch-executor.php b/src/Workflows/register-workflow-branch-executor.php index 389c29a..a3954f7 100644 --- a/src/Workflows/register-workflow-branch-executor.php +++ b/src/Workflows/register-workflow-branch-executor.php @@ -78,6 +78,31 @@ static function ( $payload = array() ): void { 1 ); +// Recover branch actions that failed after persisting a terminal receipt (most +// importantly: RECONCILE_HOOK enqueue returned 0 and the callback threw). The +// failed action remains fetchable from AS, so recovery can inspect its payload +// and continue reconciliation without invoking branch effects again. +add_action( + 'action_scheduler_failed_execution', + static function ( $action_id, $error = null ): void { + if ( is_int( $action_id ) || is_string( $action_id ) ) { + WP_Agent_Workflow_Action_Scheduler_Branch_Executor::recover_failed_action( $action_id, $error ); + } + }, + 20, + 2 +); +add_action( + 'action_scheduler_failed_action', + static function ( $action_id, $error = null ): void { + if ( is_int( $action_id ) || is_string( $action_id ) ) { + WP_Agent_Workflow_Action_Scheduler_Branch_Executor::recover_failed_action( $action_id, $error ); + } + }, + 20, + 2 +); + // 3a. Reconcile-only retry: read the persisted terminal result and merge it. add_action( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK, diff --git a/stubs/action-scheduler-classes.php b/stubs/action-scheduler-classes.php index efabe06..b36536b 100644 --- a/stubs/action-scheduler-classes.php +++ b/stubs/action-scheduler-classes.php @@ -41,6 +41,11 @@ public static function instance(): ActionScheduler_Store { throw new \RuntimeException( 'stub' ); } + /** @param int|string $action_id Action id. */ + public function fetch_action( $action_id ): ActionScheduler_Action { + throw new \RuntimeException( 'stub' ); + } + /** * @param int $max_actions Maximum actions to claim. * @param \DateTime|null $before_date Claim actions scheduled before this date. @@ -57,6 +62,17 @@ public function release_claim( ActionScheduler_ActionClaim $claim ): void {} public function cancel_action( $action_id ): void {} } +class ActionScheduler_Action { + public function get_hook(): string { + return ''; + } + + /** @return array */ + public function get_args(): array { + return array(); + } +} + /** * Action Scheduler's queue runner (returned by ActionScheduler::runner()). */ diff --git a/tests/workflow-as-branch-smoke.php b/tests/workflow-as-branch-smoke.php index 9d3a1e3..5492a64 100644 --- a/tests/workflow-as-branch-smoke.php +++ b/tests/workflow-as-branch-smoke.php @@ -222,6 +222,45 @@ public static function fire( int $id ): bool { } return false; } + + public static function fire_with_failure_lifecycle( int $id ): bool { + try { + return self::fire( $id ); + } catch ( \Throwable $error ) { + do_action( 'action_scheduler_failed_execution', $id, $error, 'test' ); + return false; + } + } + + public static function action_for( int $id ): ?AS_Shim_Action { + foreach ( self::$queue as $action ) { + if ( $action['id'] === $id ) { + return new AS_Shim_Action( $action['hook'], $action['args'] ); + } + } + return null; + } +} + +final class AS_Shim_Action { + public function __construct( private string $hook, private array $args ) {} + public function get_hook(): string { return $this->hook; } + public function get_args(): array { return $this->args; } +} + +if ( ! class_exists( 'ActionScheduler_Store' ) ) { + class ActionScheduler_Store { + public const STATUS_PENDING = 'pending'; + public const STATUS_RUNNING = 'in-progress'; + public static function instance(): self { return new self(); } + public function fetch_action( $action_id ): AS_Shim_Action { + $action = AS_Shim::action_for( (int) $action_id ); + if ( null === $action ) { + throw new RuntimeException( 'Action not found.' ); + } + return $action; + } + } } if ( ! function_exists( 'as_enqueue_async_action' ) ) { @@ -858,7 +897,7 @@ static function ( $override, string $run_id, callable $critical ) use ( &$edge_l ); AS_Shim::fire( $edge_branches[0]['id'] ); $edge_retries = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK ); - $edge_receipt = \AgentsAPI\AI\Workflows\WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $edge_store_ref, (string) ( $edge_payload['context_ref'] ?? '' ) ); + $edge_receipt = \AgentsAPI\AI\Workflows\WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $edge_store_ref, (string) ( $edge_payload['context_ref'] ?? '' ), (string) ( $edge_payload['store_backend'] ?? '' ) ); smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUSPENDED, $edge_run->get_status(), $descriptor_state . ' descriptor: fixture starts suspended', $failures, $passes ); smoke_assert( 1, count( $edge_retries ), $descriptor_state . ' descriptor: lock contention queues reconcile-only retry', $failures, $passes ); smoke_assert( 'workflow_branch_descriptor_missing', $edge_receipt['branch_result']['error']['code'] ?? '', $descriptor_state . ' descriptor: receipt durably carries terminal missing-descriptor failure', $failures, $passes ); @@ -897,17 +936,38 @@ static function ( $override, string $run_id, callable $critical ) use ( &$lock_a ); $effect_before7 = (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ); AS_Shim::$reject_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK; -$enqueue_failed7 = false; -try { - AS_Shim::fire( $branches7[0]['id'] ); -} catch ( \RuntimeException $error ) { - $enqueue_failed7 = str_contains( $error->getMessage(), 'enqueue a reconcile retry' ); -} +$failure_lifecycle7 = ! AS_Shim::fire_with_failure_lifecycle( $branches7[0]['id'] ); +AS_Shim::$reject_hook = ''; +smoke_assert( true, $failure_lifecycle7, 'failed retry enqueue: Action Scheduler failure lifecycle handles original action failure', $failures, $passes ); +smoke_assert( $effect_before7 + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'failed retry enqueue: failed-action recovery does not repeat side effect', $failures, $passes ); +smoke_assert( 1, count( $recorder7->find( 'as-enqueue-fail' )->get_suspension()['completed'] ?? array() ), 'failed retry enqueue: failed-action callback reconciles persisted result', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); + +// If both durable enqueue and inline reconciliation remain unavailable, the +// failed-action callback must terminate the run instead of leaving it suspended. +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$recorder8 = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder8 ) { return $recorder8; } ); +( new WP_Agent_Workflow_Runner( $recorder8 ) )->run( as_smoke_roles_spec(), array(), array( 'run_id' => 'as-recovery-terminal' ) ); +$branches8 = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +add_filter( + 'wp_agent_workflow_reconcile_lock', + static function () { + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + }, + 10, + 3 +); +$effect_before8 = (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ); +AS_Shim::$reject_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK; +AS_Shim::fire_with_failure_lifecycle( $branches8[0]['id'] ); AS_Shim::$reject_hook = ''; -WP_Agent_Workflow_Action_Scheduler_Branch_Executor::run_branch_action( $payload7 ); -smoke_assert( true, $enqueue_failed7, 'failed retry enqueue: original branch action fails loudly after persisting receipt', $failures, $passes ); -smoke_assert( $effect_before7 + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'failed retry enqueue: retried branch payload does not repeat side effect', $failures, $passes ); -smoke_assert( 1, count( $recorder7->find( 'as-enqueue-fail' )->get_suspension()['completed'] ?? array() ), 'failed retry enqueue: retried branch payload reconciles persisted result', $failures, $passes ); +$terminal8 = $recorder8->find( 'as-recovery-terminal' ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_FAILED, $terminal8->get_status(), 'failed-action recovery: run fails terminal when no continuation can be established', $failures, $passes ); +smoke_assert( 'workflow_branch_reconcile_recovery_failed', $terminal8->get_error()['code'] ?? '', 'failed-action recovery: terminal failure uses stable recovery code', $failures, $passes ); +smoke_assert( $effect_before8 + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'failed-action recovery: terminal fallback does not repeat branch effects', $failures, $passes ); remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); echo "Passed: {$passes}, Failed: " . count( $failures ) . "\n"; diff --git a/tests/workflow-async-branch-payload-smoke.php b/tests/workflow-async-branch-payload-smoke.php index b710941..515cf6b 100644 --- a/tests/workflow-async-branch-payload-smoke.php +++ b/tests/workflow-async-branch-payload-smoke.php @@ -573,9 +573,9 @@ function payload_roles_spec(): WP_Agent_Workflow_Spec { $index_key = 'agents_wf_branch_index_' . md5( $cleanup_run ); $stale_index = $GLOBALS['__options'][ $index_key ] ?? array(); $cleanup_result = array( 'key' => 'a', 'status' => 'succeeded', 'output' => array( 'ok' => true ) ); -WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $cleanup_run, 'a', $cleanup_a, '', $cleanup_result ); +WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $cleanup_run, 'a', $cleanup_a, '', WP_Agent_Workflow_Branch_Store::BACKEND_BUILTIN, $cleanup_result ); $cleanup_result['key'] = 'b'; -WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $cleanup_run, 'b', $cleanup_b, '', $cleanup_result ); +WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $cleanup_run, 'b', $cleanup_b, '', WP_Agent_Workflow_Branch_Store::BACKEND_BUILTIN, $cleanup_result ); $GLOBALS['__options'][ $index_key ] = $stale_index; WP_Agent_Workflow_Branch_Store::forget_run( $cleanup_run ); $cleanup_leftover = array_filter( @@ -598,7 +598,7 @@ function payload_roles_spec(): WP_Agent_Workflow_Spec { $GLOBALS['__update_option_before_write'] = null; WP_Agent_Workflow_Branch_Store::forget_run( $resurrection_run ); }; -WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $resurrection_run, 'race-handle', $resurrection_ref, '', $resurrection_result ); +WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $resurrection_run, 'race-handle', $resurrection_ref, '', WP_Agent_Workflow_Branch_Store::BACKEND_BUILTIN, $resurrection_result ); smoke_assert( false, isset( $GLOBALS['__options'][ $resurrection_ref ]['descriptor'] ), 'receipt put race: stale writer cannot recreate descriptor state', $failures, $passes ); $GLOBALS['__branch_effects'] = 1; remove_all_filters( 'wp_agent_workflow_run_recorder' ); @@ -609,6 +609,7 @@ function payload_roles_spec(): WP_Agent_Workflow_Spec { 'handle_id' => 'race-handle', 'result_ref' => $resurrection_ref, 'context_ref' => '', + 'store_backend' => WP_Agent_Workflow_Branch_Store::BACKEND_BUILTIN, ) ); $resurrection_leftover = array_filter( @@ -622,6 +623,7 @@ function payload_roles_spec(): WP_Agent_Workflow_Spec { // in that same custom store. No local option fallback is permitted. $GLOBALS['__options'] = array(); $GLOBALS['__custom_branch_rows'] = array(); +$GLOBALS['__custom_receipts'] = array(); add_filter( 'wp_agent_workflow_branch_store_put', static function ( $ref, string $run_id, string $handle_id, array $descriptor ) { @@ -647,18 +649,48 @@ static function ( $descriptor, string $store_ref ) { static function ( bool $handled, string $run_id ): bool { unset( $handled, $run_id ); $GLOBALS['__custom_branch_rows'] = array(); + $GLOBALS['__custom_receipts'] = array(); return true; }, 10, 2 ); +add_filter( + 'wp_agent_workflow_branch_receipt_put', + static function ( $receipt_ref, string $store_ref, string $run_id, string $handle_id, array $receipt ) { + unset( $receipt_ref, $run_id, $handle_id ); + $GLOBALS['__custom_receipts'][ $store_ref ] = $receipt; + return $store_ref; + }, + 10, + 5 +); +add_filter( + 'wp_agent_workflow_branch_receipt_get', + static function ( $receipt, string $receipt_ref ) { + unset( $receipt ); + return $GLOBALS['__custom_receipts'][ $receipt_ref ] ?? null; + }, + 10, + 3 +); +add_filter( + 'wp_agent_workflow_branch_receipt_delete', + static function ( bool $handled, string $receipt_ref ): bool { + unset( $handled ); + unset( $GLOBALS['__custom_receipts'][ $receipt_ref ] ); + return true; + }, + 10, + 5 +); $custom_run = 'pay-custom-store'; $custom_ref = WP_Agent_Workflow_Branch_Store::put_branch( $custom_run, 'custom-handle', array( 'run_id' => $custom_run, 'handle_id' => 'custom-handle', 'key' => 'custom' ) ); $custom_result = array( 'key' => 'custom', 'status' => 'succeeded', 'output' => array( 'owned' => true ) ); -$custom_receipt_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $custom_run, 'custom-handle', $custom_ref, '', $custom_result ); -$custom_receipt = is_wp_error( $custom_receipt_ref ) ? null : WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_receipt_ref, '' ); -smoke_assert( $custom_ref, $custom_receipt_ref, 'custom store: existing put filter owns terminal result persistence', $failures, $passes ); -smoke_assert( $custom_result, $custom_receipt['branch_result'] ?? null, 'custom store: existing get filter rehydrates the terminal result', $failures, $passes ); +$custom_receipt_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $custom_run, 'custom-handle', $custom_ref, '', WP_Agent_Workflow_Branch_Store::BACKEND_CUSTOM, $custom_result ); +$custom_receipt = is_wp_error( $custom_receipt_ref ) ? null : WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_receipt_ref, '', WP_Agent_Workflow_Branch_Store::BACKEND_CUSTOM ); +smoke_assert( $custom_ref, $custom_receipt_ref, 'custom store: dedicated put filter owns terminal result persistence', $failures, $passes ); +smoke_assert( $custom_result, $custom_receipt['branch_result'] ?? null, 'custom store: dedicated get filter rehydrates the terminal result', $failures, $passes ); smoke_assert( array(), $GLOBALS['__options'], 'custom store: terminal persistence creates no local option fallback', $failures, $passes ); WP_Agent_Workflow_Branch_Store::forget_run( $custom_run ); smoke_assert( array(), $GLOBALS['__custom_branch_rows'], 'custom store: existing forget filter cleans terminal result state', $failures, $passes ); @@ -666,20 +698,77 @@ static function ( bool $handled, string $run_id ): bool { $custom_missing_ref = WP_Agent_Workflow_Branch_Store::put_branch( $custom_missing_run, 'missing-handle', array( 'run_id' => $custom_missing_run, 'handle_id' => 'missing-handle', 'key' => 'missing' ) ); unset( $GLOBALS['__custom_branch_rows'][ $custom_missing_ref ] ); $custom_missing_result = array( 'key' => '', 'status' => 'failed', 'output' => null, 'error' => array( 'code' => 'workflow_branch_descriptor_missing' ) ); -$custom_missing_receipt_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $custom_missing_run, 'missing-handle', $custom_missing_ref, '', $custom_missing_result ); -$custom_missing_receipt = is_wp_error( $custom_missing_receipt_ref ) ? null : WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_missing_receipt_ref, '' ); +$custom_missing_receipt_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $custom_missing_run, 'missing-handle', $custom_missing_ref, '', WP_Agent_Workflow_Branch_Store::BACKEND_CUSTOM, $custom_missing_result ); +$custom_missing_receipt = is_wp_error( $custom_missing_receipt_ref ) ? null : WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_missing_receipt_ref, '', WP_Agent_Workflow_Branch_Store::BACKEND_CUSTOM ); smoke_assert( $custom_missing_ref, $custom_missing_receipt_ref, 'custom store: owner filter persists a receipt-only missing descriptor', $failures, $passes ); smoke_assert( 'workflow_branch_descriptor_missing', $custom_missing_receipt['branch_result']['error']['code'] ?? '', 'custom store: owner filter rehydrates receipt-only failure', $failures, $passes ); smoke_assert( array(), $GLOBALS['__options'], 'custom store: missing-descriptor receipt still creates no local fallback', $failures, $passes ); -WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $custom_missing_run, 'missing-handle', $custom_missing_ref, '' ); -$custom_missing_after_reconcile = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_missing_ref, '' ); -smoke_assert( $custom_missing_result, $custom_missing_after_reconcile['branch_result'] ?? null, 'custom store: run-level owner cleanup retains receipt until terminal forget', $failures, $passes ); -smoke_assert( array(), $GLOBALS['__options'], 'custom store: deferred receipt cleanup creates no local option fallback', $failures, $passes ); +WP_Agent_Workflow_Branch_Store::forget_reconcile_receipt( $custom_missing_run, 'missing-handle', $custom_missing_ref, '', WP_Agent_Workflow_Branch_Store::BACKEND_CUSTOM ); +$custom_missing_after_reconcile = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $custom_missing_ref, '', WP_Agent_Workflow_Branch_Store::BACKEND_CUSTOM ); +smoke_assert( null, $custom_missing_after_reconcile, 'custom store: dedicated receipt delete removes successful continuation', $failures, $passes ); +smoke_assert( array(), $GLOBALS['__options'], 'custom store: dedicated receipt cleanup creates no local option fallback', $failures, $passes ); WP_Agent_Workflow_Branch_Store::forget_run( $custom_missing_run ); smoke_assert( array(), $GLOBALS['__custom_branch_rows'], 'custom store: forget filter cleans receipt-only missing descriptor', $failures, $passes ); remove_all_filters( 'wp_agent_workflow_branch_store_put' ); remove_all_filters( 'wp_agent_workflow_branch_store_get' ); remove_all_filters( 'wp_agent_workflow_branch_store_forget' ); +remove_all_filters( 'wp_agent_workflow_branch_receipt_put' ); +remove_all_filters( 'wp_agent_workflow_branch_receipt_get' ); +remove_all_filters( 'wp_agent_workflow_branch_receipt_delete' ); + +// Opaque ref text cannot determine backend ownership. A custom descriptor store +// may return the exact built-in-looking ref while receipts remain on a separate, +// explicitly registered contract. +$GLOBALS['__options'] = array(); +$GLOBALS['__lookalike_descriptors'] = array(); +$GLOBALS['__lookalike_receipts'] = array(); +$GLOBALS['__lookalike_descriptor_writes'] = 0; +$lookalike_run = 'pay-lookalike-custom'; +$lookalike_handle = 'lookalike-handle'; +$lookalike_ref = 'agents_wf_branch_' . md5( $lookalike_run . ':' . $lookalike_handle ); +add_filter( + 'wp_agent_workflow_branch_store_put', + static function ( $ref, string $run_id, string $handle_id, array $descriptor ) use ( $lookalike_ref ) { + unset( $ref, $run_id, $handle_id ); + ++$GLOBALS['__lookalike_descriptor_writes']; + $GLOBALS['__lookalike_descriptors'][ $lookalike_ref ] = $descriptor; + return $lookalike_ref; + }, + 10, + 4 +); +add_filter( + 'wp_agent_workflow_branch_receipt_put', + static function ( $receipt_ref, string $store_ref, string $run_id, string $handle_id, array $receipt ) { + unset( $receipt_ref, $run_id, $handle_id ); + $GLOBALS['__lookalike_receipts'][ $store_ref ] = $receipt; + return $store_ref; + }, + 10, + 5 +); +add_filter( + 'wp_agent_workflow_branch_receipt_get', + static function ( $receipt, string $receipt_ref ) { + unset( $receipt ); + return $GLOBALS['__lookalike_receipts'][ $receipt_ref ] ?? null; + }, + 10, + 3 +); +$lookalike_stored = WP_Agent_Workflow_Branch_Store::put_branch_with_provenance( $lookalike_run, $lookalike_handle, array( 'run_id' => $lookalike_run, 'handle_id' => $lookalike_handle ) ); +$lookalike_result = array( 'key' => 'lookalike', 'status' => 'succeeded', 'output' => array( 'ok' => true ) ); +$lookalike_receipt_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $lookalike_run, $lookalike_handle, $lookalike_stored['ref'], '', $lookalike_stored['backend'], $lookalike_result ); +smoke_assert( WP_Agent_Workflow_Branch_Store::BACKEND_CUSTOM, $lookalike_stored['backend'], 'lookalike custom store: provenance is explicit despite built-in-looking ref', $failures, $passes ); +smoke_assert( 1, $GLOBALS['__lookalike_descriptor_writes'], 'lookalike custom store: receipt persistence does not call descriptor put filter', $failures, $passes ); +smoke_assert( $lookalike_ref, $lookalike_receipt_ref, 'lookalike custom store: dedicated receipt contract owns persistence', $failures, $passes ); +smoke_assert( false, array_key_exists( $lookalike_ref, $GLOBALS['__options'] ), 'lookalike custom store: no local option fallback', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_branch_receipt_put' ); +$unsupported_receipt = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $lookalike_run, $lookalike_handle, $lookalike_stored['ref'], '', $lookalike_stored['backend'], $lookalike_result ); +smoke_assert( 'workflow_branch_receipt_backend_unsupported', is_wp_error( $unsupported_receipt ) ? $unsupported_receipt->get_error_code() : '', 'lookalike custom store: missing receipt contract fails clearly', $failures, $passes ); +smoke_assert( false, array_key_exists( $lookalike_ref, $GLOBALS['__options'] ), 'lookalike custom store: unsupported receipts still do not fall back locally', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_branch_store_put' ); +remove_all_filters( 'wp_agent_workflow_branch_receipt_get' ); // A payload queued by the previous release has no admission token. It must // retain its pre-upgrade execution behavior instead of being silently fenced. From 56392d90d90c02a23d5693df26b99dfe5a7ae5a3 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 18:21:34 +0000 Subject: [PATCH 07/11] fix: bridge reconcile payload compatibility --- ...kflow-action-scheduler-branch-executor.php | 67 +++++++- .../class-wp-agent-workflow-branch-store.php | 28 ++++ tests/workflow-as-branch-smoke.php | 152 ++++++++++++++++++ 3 files changed, 239 insertions(+), 8 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index 8530bb5..9c6a8b0 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -811,7 +811,7 @@ public static function run_branch_action( array $payload ): void { $run_id = self::string_value( $payload['run_id'] ?? '' ); $handle_id = self::string_value( $payload['handle_id'] ?? '' ); $store_ref = self::string_value( $payload['store_ref'] ?? '' ); - $store_backend = self::string_value( $payload['store_backend'] ?? WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY ); + $store_backend = self::payload_store_backend( $payload ); $context_ref = self::string_value( $payload['context_ref'] ?? '' ); $admission_token = self::string_value( $payload['admission_token'] ?? '' ); @@ -837,9 +837,10 @@ public static function run_branch_action( array $payload ): void { // reconcile receipt before its retry enqueue failed. Resume from that receipt // before reading the descriptor so retrying the branch action cannot repeat // external effects. - $receipt = '' !== $store_ref ? WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $store_ref, $context_ref, $store_backend ) : null; + $receipt_ref = WP_Agent_Workflow_Branch_Store::locate_reconcile_receipt( $store_ref, $run_id, $handle_id, $context_ref, $store_backend ); + $receipt = '' !== $receipt_ref ? WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $receipt_ref, $context_ref, $store_backend ) : null; if ( null !== $receipt ) { - self::reconcile_branch_result( $run_id, $handle_id, $store_ref, $context_ref, $store_backend, $receipt['branch_result'], $receipt['continuation'], true ); + self::reconcile_branch_result( $run_id, $handle_id, $receipt_ref, $context_ref, $store_backend, $receipt['branch_result'], $receipt['continuation'], true ); return; } @@ -890,7 +891,7 @@ private static function reconcile_branch_action_result( array $payload, array $b $run_id = self::string_value( $payload['run_id'] ?? '' ); $handle_id = self::string_value( $payload['handle_id'] ?? '' ); $store_ref = self::string_value( $payload['store_ref'] ?? '' ); - $store_backend = self::string_value( $payload['store_backend'] ?? WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY ); + $store_backend = self::payload_store_backend( $payload ); $context_ref = self::string_value( $payload['context_ref'] ?? '' ); self::reconcile_branch_result( $run_id, $handle_id, $store_ref, $context_ref, $store_backend, $branch_result, array(), false ); @@ -909,7 +910,7 @@ public static function run_reconcile_action( array $payload ): bool { $run_id = self::string_value( $payload['run_id'] ?? '' ); $handle_id = self::string_value( $payload['handle_id'] ?? '' ); $result_ref = self::string_value( $payload['result_ref'] ?? '' ); - $store_backend = self::string_value( $payload['store_backend'] ?? WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY ); + $store_backend = self::payload_store_backend( $payload ); $context_ref = self::string_value( $payload['context_ref'] ?? '' ); if ( '' === $run_id || '' === $handle_id || '' === $result_ref ) { return false; @@ -978,6 +979,9 @@ private static function reconcile_branch_result( string $run_id, string $handle_ $next_continuation = is_array( $error_data ) && is_array( $error_data['reconcile_continuation'] ?? null ) ? self::string_keyed_array( $error_data['reconcile_continuation'] ) : $continuation; + if ( WP_Agent_Workflow_Branch_Store::BACKEND_TRANSITION === $store_backend ) { + return self::continue_transitional_reconcile( $run_id, $handle_id, $branch_result, $next_continuation ); + } $next_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref, $store_backend, $branch_result, $next_continuation ); if ( is_wp_error( $next_ref ) ) { throw new \RuntimeException( $next_ref->get_error_message() ); @@ -1028,8 +1032,9 @@ public static function recover_failed_action( $action_id, $failure = null ): voi $handle_id = self::string_value( $payload['handle_id'] ?? '' ); $result_ref = self::string_value( $payload['store_ref'] ?? '' ); $context_ref = self::string_value( $payload['context_ref'] ?? '' ); - $store_backend = self::string_value( $payload['store_backend'] ?? WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY ); - $receipt = WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $result_ref, $context_ref, $store_backend ); + $store_backend = self::payload_store_backend( $payload ); + $receipt_ref = WP_Agent_Workflow_Branch_Store::locate_reconcile_receipt( $result_ref, $run_id, $handle_id, $context_ref, $store_backend ); + $receipt = '' !== $receipt_ref ? WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $receipt_ref, $context_ref, $store_backend ) : null; if ( '' === $run_id || '' === $handle_id ) { return; } @@ -1044,7 +1049,7 @@ public static function recover_failed_action( $action_id, $failure = null ): voi $retry_payload = array( 'run_id' => $run_id, 'handle_id' => $handle_id, - 'result_ref' => $result_ref, + 'result_ref' => $receipt_ref, 'context_ref' => $context_ref, 'store_backend' => $store_backend, ); @@ -1089,6 +1094,52 @@ private static function fail_reconcile_recovery( string $run_id, string $handle_ WP_Agent_Workflow_Branch_Store::forget_run( $run_id ); } + /** + * Continue a #535-era token-bearing payload without guessing its backend. + * + * @param array $branch_result Terminal BranchResult. + * @param array $continuation Opaque reconcile continuation. + */ + private static function continue_transitional_reconcile( string $run_id, string $handle_id, array $branch_result, array $continuation ): bool { + for ( $attempt = 0; $attempt < 3; ++$attempt ) { + $result = apply_filters( 'wp_agent_workflow_reconcile_retry', null, $run_id, $handle_id, $branch_result, $continuation ); + if ( null === $result ) { + $result = agents_reconcile_workflow_branch( $run_id, $handle_id, $branch_result ); + } + if ( ! is_wp_error( $result ) ) { + if ( is_object( $result ) && method_exists( $result, 'is_suspended' ) && ! $result->is_suspended() ) { + WP_Agent_Workflow_Branch_Store::forget_run( $run_id ); + } + return true; + } + if ( 'agents_reconcile_lock_unavailable' !== $result->get_error_code() ) { + self::fail_reconcile_recovery( $run_id, $handle_id, $result->get_error_message() ); + return true; + } + $error_data = $result->get_error_data(); + if ( is_array( $error_data ) && is_array( $error_data['reconcile_continuation'] ?? null ) ) { + $continuation = self::string_keyed_array( $error_data['reconcile_continuation'] ); + } + } + + self::fail_reconcile_recovery( $run_id, $handle_id, 'Transitional reconcile contention exceeded its bounded retry budget.' ); + return true; + } + + /** + * Resolve explicit payload provenance, including the #535 transition shape. + * + * @param array $payload Branch action payload. + */ + private static function payload_store_backend( array $payload ): string { + if ( array_key_exists( 'store_backend', $payload ) ) { + return self::string_value( $payload['store_backend'] ); + } + return array_key_exists( 'admission_token', $payload ) + ? WP_Agent_Workflow_Branch_Store::BACKEND_TRANSITION + : WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY; + } + /** Whether the authoritative run already recorded this branch completion. */ private static function is_branch_reconciled( string $run_id, string $handle_id ): bool { $recorder = agents_workflow_resolve_recorder(); diff --git a/src/Workflows/class-wp-agent-workflow-branch-store.php b/src/Workflows/class-wp-agent-workflow-branch-store.php index d049581..c866756 100644 --- a/src/Workflows/class-wp-agent-workflow-branch-store.php +++ b/src/Workflows/class-wp-agent-workflow-branch-store.php @@ -58,6 +58,7 @@ final class WP_Agent_Workflow_Branch_Store { public const BACKEND_BUILTIN = 'builtin'; public const BACKEND_CUSTOM = 'custom'; public const BACKEND_LEGACY = 'legacy'; + public const BACKEND_TRANSITION = 'transition'; /** * Option-name prefix for a per-branch descriptor row. The run id and handle @@ -394,6 +395,33 @@ public static function get_reconcile_receipt( string $result_ref, string $contex ); } + /** + * Resolve a receipt ref from explicit backend ownership and descriptor identity. + * + * @return string Empty when no receipt is discoverable. + */ + public static function locate_reconcile_receipt( string $store_ref, string $run_id, string $handle_id, string $context_ref, string $backend ): string { + if ( self::BACKEND_BUILTIN === $backend ) { + return $store_ref; + } + if ( self::BACKEND_CUSTOM !== $backend || ! function_exists( 'apply_filters' ) ) { + return ''; + } + /** + * Locate a custom reconcile receipt by its owning descriptor identity. + * + * @since 0.7.0 + * + * @param string $receipt_ref No locator by default. + * @param string $store_ref Descriptor ref. + * @param string $run_id Run id. + * @param string $handle_id Handle id. + * @param string $context_ref Shared-context ref. + */ + $receipt_ref = apply_filters( 'wp_agent_workflow_branch_receipt_locate', '', $store_ref, $run_id, $handle_id, $context_ref ); + return $receipt_ref; + } + /** * Delete one successfully reconciled receipt without touching sibling refs. * diff --git a/tests/workflow-as-branch-smoke.php b/tests/workflow-as-branch-smoke.php index 5492a64..9093285 100644 --- a/tests/workflow-as-branch-smoke.php +++ b/tests/workflow-as-branch-smoke.php @@ -752,6 +752,25 @@ function as_smoke_failing_spec(): WP_Agent_Workflow_Spec { ); } +function as_smoke_single_branch_spec( string $label ): WP_Agent_Workflow_Spec { + return WP_Agent_Workflow_Spec::from_array( + array( + 'id' => 'demo/as-single-' . $label, + 'steps' => array( + array( + 'id' => 'scatter', + 'type' => 'parallel', + 'as' => 'item', + 'items' => array( array( 'label' => $label ) ), + 'steps' => array( + array( 'id' => 'work', 'type' => 'ability', 'ability' => 'demo/role-worker', 'args' => array( 'label' => $label ) ), + ), + ), + ), + ) + ); +} + AS_Shim::reset(); $recorder4 = new AS_Smoke_Recorder(); remove_all_filters( 'wp_agent_workflow_run_recorder' ); @@ -970,5 +989,138 @@ static function () { smoke_assert( $effect_before8 + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'failed-action recovery: terminal fallback does not repeat branch effects', $failures, $passes ); remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); +// #535 payloads carry an admission token but predate explicit backend provenance. +// They continue reconciliation synchronously without guessing receipt ownership. +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$recorder9 = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder9 ) { return $recorder9; } ); +( new WP_Agent_Workflow_Runner( $recorder9 ) )->run( as_smoke_single_branch_spec( 'transition' ), array(), array( 'run_id' => 'as-transition' ) ); +$branches9 = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +foreach ( AS_Shim::$queue as $index => $queued9 ) { + if ( $queued9['id'] === $branches9[0]['id'] ) { + unset( AS_Shim::$queue[ $index ]['args'][0]['store_backend'] ); + } +} +$transition_attempts9 = 0; +add_filter( + 'wp_agent_workflow_reconcile_lock', + static function ( $override, string $run_id, callable $critical ) use ( &$transition_attempts9 ) { + unset( $override ); + if ( 'as-transition' === $run_id && 0 === $transition_attempts9++ ) { + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + } + return $critical(); + }, + 10, + 3 +); +$transition_effect_before9 = (int) ( $GLOBALS['__role_worker_effects']['transition'] ?? 0 ); +AS_Shim::fire( $branches9[0]['id'] ); +$resumes9 = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RESUME_HOOK ); +AS_Shim::fire( $resumes9[0]['id'] ); +$transition_final9 = $recorder9->find( 'as-transition' ); +smoke_assert( true, isset( $branches9[0]['args'][0]['admission_token'] ), '#535 transition: payload retains admission token', $failures, $passes ); +smoke_assert( 0, count( AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK ) ), '#535 transition: contention uses no backend-dependent receipt action', $failures, $passes ); +smoke_assert( $transition_effect_before9 + 1, (int) ( $GLOBALS['__role_worker_effects']['transition'] ?? 0 ), '#535 transition: branch effects execute exactly once', $failures, $passes ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUCCEEDED, $transition_final9->get_status(), '#535 transition: bounded synchronous continuation reaches terminal success', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); + +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$recorder10 = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder10 ) { return $recorder10; } ); +( new WP_Agent_Workflow_Runner( $recorder10 ) )->run( as_smoke_single_branch_spec( 'transition-fail' ), array(), array( 'run_id' => 'as-transition-fail' ) ); +$branches10 = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +foreach ( AS_Shim::$queue as $index => $queued10 ) { + if ( $queued10['id'] === $branches10[0]['id'] ) { + unset( AS_Shim::$queue[ $index ]['args'][0]['store_backend'] ); + } +} +add_filter( 'wp_agent_workflow_reconcile_lock', static function () { return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); }, 10, 3 ); +$transition_effect_before10 = (int) ( $GLOBALS['__role_worker_effects']['transition-fail'] ?? 0 ); +AS_Shim::fire( $branches10[0]['id'] ); +$transition_final10 = $recorder10->find( 'as-transition-fail' ); +smoke_assert( $transition_effect_before10 + 1, (int) ( $GLOBALS['__role_worker_effects']['transition-fail'] ?? 0 ), '#535 transition failure: bounded attempts do not repeat effects', $failures, $passes ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_FAILED, $transition_final10->get_status(), '#535 transition failure: exhausted continuation reaches terminal failure', $failures, $passes ); +smoke_assert( 'workflow_branch_reconcile_recovery_failed', $transition_final10->get_error()['code'] ?? '', '#535 transition failure: stable terminal recovery code', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); + +// A custom receipt ref may differ from its descriptor ref. The failed-action +// lifecycle recovers it through the dedicated descriptor-identity locator. +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$GLOBALS['__distinct_descriptors'] = array(); +$GLOBALS['__distinct_receipts'] = array(); +$GLOBALS['__distinct_locators'] = array(); +$GLOBALS['__distinct_descriptor_writes'] = 0; +add_filter( + 'wp_agent_workflow_branch_store_put', + static function ( $ref, string $run_id, string $handle_id, array $descriptor ) { + unset( $ref, $run_id ); + ++$GLOBALS['__distinct_descriptor_writes']; + $descriptor_ref = 'custom-descriptor:' . $handle_id; + $GLOBALS['__distinct_descriptors'][ $descriptor_ref ] = $descriptor; + return $descriptor_ref; + }, + 10, + 4 +); +add_filter( 'wp_agent_workflow_branch_store_get', static function ( $descriptor, string $store_ref ) { unset( $descriptor ); return $GLOBALS['__distinct_descriptors'][ $store_ref ] ?? null; }, 10, 3 ); +add_filter( + 'wp_agent_workflow_branch_receipt_put', + static function ( $receipt_ref, string $store_ref, string $run_id, string $handle_id, array $receipt ) { + unset( $receipt_ref, $run_id ); + $distinct_ref = 'custom-receipt:' . $handle_id; + $GLOBALS['__distinct_receipts'][ $distinct_ref ] = $receipt; + $GLOBALS['__distinct_locators'][ $store_ref ] = $distinct_ref; + return $distinct_ref; + }, + 10, + 5 +); +add_filter( 'wp_agent_workflow_branch_receipt_get', static function ( $receipt, string $receipt_ref ) { unset( $receipt ); return $GLOBALS['__distinct_receipts'][ $receipt_ref ] ?? null; }, 10, 3 ); +add_filter( 'wp_agent_workflow_branch_receipt_locate', static function ( string $receipt_ref, string $store_ref ): string { unset( $receipt_ref ); return $GLOBALS['__distinct_locators'][ $store_ref ] ?? ''; }, 10, 5 ); +add_filter( 'wp_agent_workflow_branch_receipt_delete', static function ( bool $handled, string $receipt_ref ): bool { unset( $handled ); unset( $GLOBALS['__distinct_receipts'][ $receipt_ref ] ); return true; }, 10, 5 ); +add_filter( 'wp_agent_workflow_branch_store_forget', static function (): bool { $GLOBALS['__distinct_descriptors'] = array(); $GLOBALS['__distinct_receipts'] = array(); $GLOBALS['__distinct_locators'] = array(); return true; }, 10, 2 ); +$recorder11 = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder11 ) { return $recorder11; } ); +( new WP_Agent_Workflow_Runner( $recorder11 ) )->run( as_smoke_single_branch_spec( 'distinct' ), array(), array( 'run_id' => 'as-distinct' ) ); +$branches11 = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +$distinct_attempts11 = 0; +add_filter( + 'wp_agent_workflow_reconcile_lock', + static function ( $override, string $run_id, callable $critical ) use ( &$distinct_attempts11 ) { + unset( $override ); + if ( 'as-distinct' === $run_id && 0 === $distinct_attempts11++ ) { + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + } + return $critical(); + }, + 10, + 3 +); +$distinct_effect_before11 = (int) ( $GLOBALS['__role_worker_effects']['distinct'] ?? 0 ); +AS_Shim::$reject_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK; +AS_Shim::fire_with_failure_lifecycle( $branches11[0]['id'] ); +AS_Shim::$reject_hook = ''; +$resumes11 = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RESUME_HOOK ); +AS_Shim::fire( $resumes11[0]['id'] ); +$distinct_final11 = $recorder11->find( 'as-distinct' ); +smoke_assert( 1, $GLOBALS['__distinct_descriptor_writes'], 'distinct custom receipt: recovery never writes through descriptor contract', $failures, $passes ); +smoke_assert( $distinct_effect_before11 + 1, (int) ( $GLOBALS['__role_worker_effects']['distinct'] ?? 0 ), 'distinct custom receipt: failed-action recovery preserves exactly-one effects', $failures, $passes ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUCCEEDED, $distinct_final11->get_status(), 'distinct custom receipt: locator recovers distinct ref to terminal success', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_branch_store_put' ); +remove_all_filters( 'wp_agent_workflow_branch_store_get' ); +remove_all_filters( 'wp_agent_workflow_branch_receipt_put' ); +remove_all_filters( 'wp_agent_workflow_branch_receipt_get' ); +remove_all_filters( 'wp_agent_workflow_branch_receipt_locate' ); +remove_all_filters( 'wp_agent_workflow_branch_receipt_delete' ); +remove_all_filters( 'wp_agent_workflow_branch_store_forget' ); +remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); + echo "Passed: {$passes}, Failed: " . count( $failures ) . "\n"; exit( count( $failures ) > 0 ? 1 : 0 ); From dd1b6e18089ccd3996f61d97956df51e33d22cae Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 18:28:08 +0000 Subject: [PATCH 08/11] fix: complete reconcile recovery lifecycle --- ...kflow-action-scheduler-branch-executor.php | 20 ++-- tests/workflow-as-branch-smoke.php | 105 ++++++++++++++++++ 2 files changed, 117 insertions(+), 8 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index 9c6a8b0..5cb38dc 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -979,8 +979,8 @@ private static function reconcile_branch_result( string $run_id, string $handle_ $next_continuation = is_array( $error_data ) && is_array( $error_data['reconcile_continuation'] ?? null ) ? self::string_keyed_array( $error_data['reconcile_continuation'] ) : $continuation; - if ( WP_Agent_Workflow_Branch_Store::BACKEND_TRANSITION === $store_backend ) { - return self::continue_transitional_reconcile( $run_id, $handle_id, $branch_result, $next_continuation ); + if ( in_array( $store_backend, array( WP_Agent_Workflow_Branch_Store::BACKEND_TRANSITION, WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY ), true ) ) { + return self::continue_compatibility_reconcile( $run_id, $handle_id, $branch_result, $next_continuation ); } $next_ref = WP_Agent_Workflow_Branch_Store::put_reconcile_receipt( $run_id, $handle_id, $result_ref, $context_ref, $store_backend, $branch_result, $next_continuation ); if ( is_wp_error( $next_ref ) ) { @@ -1023,17 +1023,20 @@ public static function recover_failed_action( $action_id, $failure = null ): voi unset( $error ); return; } - if ( self::BRANCH_HOOK !== $action->get_hook() ) { + $failed_hook = $action->get_hook(); + if ( self::BRANCH_HOOK !== $failed_hook && self::RECONCILE_HOOK !== $failed_hook ) { return; } $args = $action->get_args(); $payload = is_array( $args[0] ?? null ) ? $args[0] : array(); $run_id = self::string_value( $payload['run_id'] ?? '' ); $handle_id = self::string_value( $payload['handle_id'] ?? '' ); - $result_ref = self::string_value( $payload['store_ref'] ?? '' ); + $result_ref = self::string_value( self::RECONCILE_HOOK === $failed_hook ? ( $payload['result_ref'] ?? '' ) : ( $payload['store_ref'] ?? '' ) ); $context_ref = self::string_value( $payload['context_ref'] ?? '' ); $store_backend = self::payload_store_backend( $payload ); - $receipt_ref = WP_Agent_Workflow_Branch_Store::locate_reconcile_receipt( $result_ref, $run_id, $handle_id, $context_ref, $store_backend ); + $receipt_ref = self::RECONCILE_HOOK === $failed_hook + ? $result_ref + : WP_Agent_Workflow_Branch_Store::locate_reconcile_receipt( $result_ref, $run_id, $handle_id, $context_ref, $store_backend ); $receipt = '' !== $receipt_ref ? WP_Agent_Workflow_Branch_Store::get_reconcile_receipt( $receipt_ref, $context_ref, $store_backend ) : null; if ( '' === $run_id || '' === $handle_id ) { return; @@ -1091,16 +1094,17 @@ private static function fail_reconcile_recovery( string $run_id, string $handle_ ); $recorder->update( $terminal ); \AgentsAPI\AI\WP_Agent_Run_Control::finish_run( WP_Agent_Workflow_Runner::RUN_CONTROL_STORE, $run_id, \AgentsAPI\AI\WP_Agent_Run_Control::STATUS_FAILED ); + do_action( 'wp_agent_workflow_run_completed', $terminal, $run_id ); WP_Agent_Workflow_Branch_Store::forget_run( $run_id ); } /** - * Continue a #535-era token-bearing payload without guessing its backend. + * Continue a pre-provenance payload without guessing its backend. * * @param array $branch_result Terminal BranchResult. * @param array $continuation Opaque reconcile continuation. */ - private static function continue_transitional_reconcile( string $run_id, string $handle_id, array $branch_result, array $continuation ): bool { + private static function continue_compatibility_reconcile( string $run_id, string $handle_id, array $branch_result, array $continuation ): bool { for ( $attempt = 0; $attempt < 3; ++$attempt ) { $result = apply_filters( 'wp_agent_workflow_reconcile_retry', null, $run_id, $handle_id, $branch_result, $continuation ); if ( null === $result ) { @@ -1122,7 +1126,7 @@ private static function continue_transitional_reconcile( string $run_id, string } } - self::fail_reconcile_recovery( $run_id, $handle_id, 'Transitional reconcile contention exceeded its bounded retry budget.' ); + self::fail_reconcile_recovery( $run_id, $handle_id, 'Compatibility reconcile contention exceeded its bounded retry budget.' ); return true; } diff --git a/tests/workflow-as-branch-smoke.php b/tests/workflow-as-branch-smoke.php index 9093285..9bb14ed 100644 --- a/tests/workflow-as-branch-smoke.php +++ b/tests/workflow-as-branch-smoke.php @@ -980,6 +980,23 @@ static function () { 3 ); $effect_before8 = (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ); +$completion_events8 = array(); +$forget_calls8 = 0; +add_action( + 'wp_agent_workflow_run_completed', + static function ( $result, string $run_id ) use ( &$completion_events8 ): void { + $completion_events8[] = array( 'result' => $result, 'run_id' => $run_id ); + }, + 10, + 2 +); +add_filter( + 'wp_agent_workflow_branch_store_forget', + static function ( bool $handled ) use ( &$forget_calls8 ): bool { + ++$forget_calls8; + return $handled; + } +); AS_Shim::$reject_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK; AS_Shim::fire_with_failure_lifecycle( $branches8[0]['id'] ); AS_Shim::$reject_hook = ''; @@ -987,6 +1004,44 @@ static function () { smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_FAILED, $terminal8->get_status(), 'failed-action recovery: run fails terminal when no continuation can be established', $failures, $passes ); smoke_assert( 'workflow_branch_reconcile_recovery_failed', $terminal8->get_error()['code'] ?? '', 'failed-action recovery: terminal failure uses stable recovery code', $failures, $passes ); smoke_assert( $effect_before8 + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'failed-action recovery: terminal fallback does not repeat branch effects', $failures, $passes ); +smoke_assert( 1, count( $completion_events8 ), 'failed-action recovery: forced terminal path fires completion funnel exactly once', $failures, $passes ); +smoke_assert( 'as-recovery-terminal', $completion_events8[0]['run_id'] ?? '', 'failed-action recovery: completion observer receives run id', $failures, $passes ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_FAILED, isset( $completion_events8[0]['result'] ) ? $completion_events8[0]['result']->get_status() : '', 'failed-action recovery: completion observer receives terminal result', $failures, $passes ); +smoke_assert( 1, $forget_calls8, 'failed-action recovery: run-scoped branch cleanup executes exactly once', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); +remove_all_filters( 'wp_agent_workflow_run_completed' ); +remove_all_filters( 'wp_agent_workflow_branch_store_forget' ); + +// A RECONCILE_HOOK action can itself fail while handing off another contended +// attempt. Its failed-action lifecycle must recover the receipt directly. +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$recorder_reconcile_failure = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder_reconcile_failure ) { return $recorder_reconcile_failure; } ); +( new WP_Agent_Workflow_Runner( $recorder_reconcile_failure ) )->run( as_smoke_roles_spec(), array(), array( 'run_id' => 'as-reconcile-action-fail' ) ); +$reconcile_failure_branches = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +$reconcile_failure_attempts = 0; +add_filter( + 'wp_agent_workflow_reconcile_lock', + static function ( $override, string $run_id, callable $critical ) use ( &$reconcile_failure_attempts ) { + unset( $override ); + if ( 'as-reconcile-action-fail' === $run_id && $reconcile_failure_attempts++ < 2 ) { + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + } + return $critical(); + }, + 10, + 3 +); +$reconcile_failure_effect_before = (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ); +AS_Shim::fire( $reconcile_failure_branches[0]['id'] ); +$failed_reconcile_actions = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK ); +AS_Shim::$reject_hook = WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RECONCILE_HOOK; +AS_Shim::fire_with_failure_lifecycle( $failed_reconcile_actions[0]['id'] ); +AS_Shim::$reject_hook = ''; +smoke_assert( $reconcile_failure_effect_before + 1, (int) ( $GLOBALS['__role_worker_effects']['head'] ?? 0 ), 'failed reconcile action: recovery does not repeat branch effects', $failures, $passes ); +smoke_assert( 1, count( $recorder_reconcile_failure->find( 'as-reconcile-action-fail' )->get_suspension()['completed'] ?? array() ), 'failed reconcile action: failed-action lifecycle recovers durable receipt', $failures, $passes ); remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); // #535 payloads carry an admission token but predate explicit backend provenance. @@ -1048,6 +1103,56 @@ static function ( $override, string $run_id, callable $critical ) use ( &$transi smoke_assert( 'workflow_branch_reconcile_recovery_failed', $transition_final10->get_error()['code'] ?? '', '#535 transition failure: stable terminal recovery code', $failures, $passes ); remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); +// Pre-#535 payloads have neither admission_token nor store_backend. They use the +// same bounded no-reexecution continuation and still reach the completion funnel. +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$recorder_tokenless = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder_tokenless ) { return $recorder_tokenless; } ); +( new WP_Agent_Workflow_Runner( $recorder_tokenless ) )->run( as_smoke_single_branch_spec( 'tokenless' ), array(), array( 'run_id' => 'as-tokenless' ) ); +$tokenless_branches = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +foreach ( AS_Shim::$queue as $index => $queued_tokenless ) { + if ( $queued_tokenless['id'] === $tokenless_branches[0]['id'] ) { + unset( AS_Shim::$queue[ $index ]['args'][0]['store_backend'], AS_Shim::$queue[ $index ]['args'][0]['admission_token'] ); + } +} +$tokenless_attempts = 0; +add_filter( + 'wp_agent_workflow_reconcile_lock', + static function ( $override, string $run_id, callable $critical ) use ( &$tokenless_attempts ) { + unset( $override ); + if ( 'as-tokenless' === $run_id && 0 === $tokenless_attempts++ ) { + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + } + return $critical(); + }, + 10, + 3 +); +$tokenless_completions = 0; +add_action( + 'wp_agent_workflow_run_completed', + static function ( $result, string $run_id ) use ( &$tokenless_completions ): void { + unset( $result ); + if ( 'as-tokenless' === $run_id ) { + ++$tokenless_completions; + } + }, + 10, + 2 +); +$tokenless_effect_before = (int) ( $GLOBALS['__role_worker_effects']['tokenless'] ?? 0 ); +AS_Shim::fire( $tokenless_branches[0]['id'] ); +$tokenless_resumes = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RESUME_HOOK ); +AS_Shim::fire( $tokenless_resumes[0]['id'] ); +$tokenless_final = $recorder_tokenless->find( 'as-tokenless' ); +smoke_assert( $tokenless_effect_before + 1, (int) ( $GLOBALS['__role_worker_effects']['tokenless'] ?? 0 ), 'tokenless compatibility: contention does not repeat branch effects', $failures, $passes ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUCCEEDED, $tokenless_final->get_status(), 'tokenless compatibility: bounded continuation reaches terminal success', $failures, $passes ); +smoke_assert( 1, $tokenless_completions, 'tokenless compatibility: completion funnel fires exactly once', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); +remove_all_filters( 'wp_agent_workflow_run_completed' ); + // A custom receipt ref may differ from its descriptor ref. The failed-action // lifecycle recovers it through the dedicated descriptor-identity locator. AS_Shim::reset(); From 2e07b1ca1dd802cdeb14898c9cb13427b1c7c3dc Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 18:40:04 +0000 Subject: [PATCH 09/11] fix: serialize reconcile terminal recovery --- ...kflow-action-scheduler-branch-executor.php | 59 ++++++++++++------- tests/workflow-as-branch-smoke.php | 58 ++++++++++++++++++ 2 files changed, 95 insertions(+), 22 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index 5cb38dc..870f1ef 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -1042,10 +1042,9 @@ public static function recover_failed_action( $action_id, $failure = null ): voi return; } if ( null === $receipt ) { - if ( WP_Agent_Workflow_Branch_Store::BACKEND_LEGACY !== $store_backend ) { - $failure_message = $failure instanceof \Throwable ? $failure->getMessage() : 'The branch failed before a durable reconcile continuation could be established.'; - self::fail_reconcile_recovery( $run_id, $handle_id, $failure_message ); - } + $failure_message = $failure instanceof \Throwable ? $failure->getMessage() : 'The branch failed before a durable reconcile continuation could be established.'; + $failure_code = self::BRANCH_HOOK === $failed_hook ? 'workflow_branch_execution_uncertain' : 'workflow_branch_reconcile_recovery_failed'; + self::fail_reconcile_recovery( $run_id, $handle_id, $failure_message, $failure_code ); return; } @@ -1072,30 +1071,46 @@ public static function recover_failed_action( $action_id, $failure = null ): voi self::fail_reconcile_recovery( $run_id, $handle_id, isset( $message ) ? $message : 'No durable reconcile continuation could be established.' ); } - /** Mark a stranded suspended run terminal when every recovery path failed. */ - private static function fail_reconcile_recovery( string $run_id, string $handle_id, string $message ): void { + /** Elect and publish one terminal failure when every recovery path failed. */ + private static function fail_reconcile_recovery( string $run_id, string $handle_id, string $message, string $code = 'workflow_branch_reconcile_recovery_failed' ): bool { $recorder = agents_workflow_resolve_recorder(); - $result = null !== $recorder ? $recorder->find( $run_id ) : null; - if ( null === $recorder || null === $result || ! $result->is_suspended() ) { - return; + if ( null === $recorder ) { + return false; } - $metadata = $result->get_metadata(); - unset( $metadata['_suspension'] ); - $terminal = $result->with( - array( - 'status' => WP_Agent_Workflow_Run_Result::STATUS_FAILED, - 'error' => array( - 'code' => 'workflow_branch_reconcile_recovery_failed', - 'message' => sprintf( 'Could not recover reconciliation for branch `%s`: %s', $handle_id, $message ), - ), - 'ended_at' => time(), - 'metadata' => $metadata, - ) + + $transition = WP_Agent_Workflow_Reconcile_Lock::with_lock( + $run_id, + static function () use ( $recorder, $run_id, $handle_id, $message, $code ) { + $result = $recorder->find( $run_id ); + if ( null === $result || ! $result->is_suspended() ) { + return array( 'won' => false, 'terminal' => null ); + } + $metadata = $result->get_metadata(); + unset( $metadata['_suspension'] ); + $terminal = $result->with( + array( + 'status' => WP_Agent_Workflow_Run_Result::STATUS_FAILED, + 'error' => array( + 'code' => $code, + 'message' => sprintf( 'Could not recover branch `%s`: %s', $handle_id, $message ), + ), + 'ended_at' => time(), + 'metadata' => $metadata, + ) + ); + $updated = $recorder->update( $terminal ); + return is_wp_error( $updated ) ? $updated : array( 'won' => true, 'terminal' => $terminal ); + }, ); - $recorder->update( $terminal ); + + if ( ! is_array( $transition ) || empty( $transition['won'] ) ) { + return false; + } + $terminal = $transition['terminal']; \AgentsAPI\AI\WP_Agent_Run_Control::finish_run( WP_Agent_Workflow_Runner::RUN_CONTROL_STORE, $run_id, \AgentsAPI\AI\WP_Agent_Run_Control::STATUS_FAILED ); do_action( 'wp_agent_workflow_run_completed', $terminal, $run_id ); WP_Agent_Workflow_Branch_Store::forget_run( $run_id ); + return true; } /** diff --git a/tests/workflow-as-branch-smoke.php b/tests/workflow-as-branch-smoke.php index 9bb14ed..7da7a08 100644 --- a/tests/workflow-as-branch-smoke.php +++ b/tests/workflow-as-branch-smoke.php @@ -321,12 +321,14 @@ function smoke_assert_true( $actual, string $name, array &$failures, int &$passe final class AS_Smoke_Recorder implements WP_Agent_Workflow_Run_Recorder { /** @var array> */ public array $rows = array(); + public int $updates = 0; public function start( WP_Agent_Workflow_Run_Result $result ) { $this->rows[ $result->get_run_id() ] = $result->to_array(); return $result->get_run_id(); } public function update( WP_Agent_Workflow_Run_Result $result ) { + ++$this->updates; $this->rows[ $result->get_run_id() ] = $result->to_array(); return true; } @@ -1227,5 +1229,61 @@ static function ( $override, string $run_id, callable $critical ) use ( &$distin remove_all_filters( 'wp_agent_workflow_branch_store_forget' ); remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); +// A tokenless branch action can fail before writing a receipt. Effects are +// uncertain, so recovery terminalizes without rerunning the branch. A duplicate +// failure callback arriving during completion publication must lose the locked +// terminal election and emit no second completion or cleanup. +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$recorder_legacy_failure = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder_legacy_failure ) { return $recorder_legacy_failure; } ); +( new WP_Agent_Workflow_Runner( $recorder_legacy_failure ) )->run( as_smoke_single_branch_spec( 'legacy-failure' ), array(), array( 'run_id' => 'as-legacy-failure' ) ); +$legacy_failure_branches = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +$legacy_failure_action_id = $legacy_failure_branches[0]['id']; +foreach ( AS_Shim::$queue as $index => $queued_legacy_failure ) { + if ( $queued_legacy_failure['id'] === $legacy_failure_action_id ) { + unset( AS_Shim::$queue[ $index ]['args'][0]['store_backend'], AS_Shim::$queue[ $index ]['args'][0]['admission_token'] ); + } +} +$legacy_failure_completions = 0; +$legacy_failure_cleanups = 0; +$legacy_duplicate_sent = false; +add_action( + 'wp_agent_workflow_run_completed', + static function ( $result, string $run_id ) use ( &$legacy_failure_completions, &$legacy_duplicate_sent, $legacy_failure_action_id ): void { + unset( $result ); + if ( 'as-legacy-failure' !== $run_id ) { + return; + } + ++$legacy_failure_completions; + if ( ! $legacy_duplicate_sent ) { + $legacy_duplicate_sent = true; + do_action( 'action_scheduler_failed_execution', $legacy_failure_action_id, new RuntimeException( 'concurrent duplicate failure' ), 'test' ); + } + }, + 10, + 2 +); +add_filter( + 'wp_agent_workflow_branch_store_forget', + static function ( bool $handled ) use ( &$legacy_failure_cleanups ): bool { + ++$legacy_failure_cleanups; + return $handled; + } +); +$legacy_failure_updates_before = $recorder_legacy_failure->updates; +$legacy_failure_effect_before = (int) ( $GLOBALS['__role_worker_effects']['legacy-failure'] ?? 0 ); +do_action( 'action_scheduler_failed_execution', $legacy_failure_action_id, new RuntimeException( 'worker outcome unknown' ), 'test' ); +$legacy_failure_final = $recorder_legacy_failure->find( 'as-legacy-failure' ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_FAILED, $legacy_failure_final->get_status(), 'legacy failed action: receipt-less failure terminalizes honestly', $failures, $passes ); +smoke_assert( 'workflow_branch_execution_uncertain', $legacy_failure_final->get_error()['code'] ?? '', 'legacy failed action: terminal error records uncertain execution', $failures, $passes ); +smoke_assert( $legacy_failure_effect_before, (int) ( $GLOBALS['__role_worker_effects']['legacy-failure'] ?? 0 ), 'legacy failed action: recovery never reruns branch effects', $failures, $passes ); +smoke_assert( 1, $recorder_legacy_failure->updates - $legacy_failure_updates_before, 'duplicate failure race: one locked recorder terminal transition wins', $failures, $passes ); +smoke_assert( 1, $legacy_failure_completions, 'duplicate failure race: completion funnel fires once', $failures, $passes ); +smoke_assert( 1, $legacy_failure_cleanups, 'duplicate failure race: run-scoped cleanup executes once', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_run_completed' ); +remove_all_filters( 'wp_agent_workflow_branch_store_forget' ); + echo "Passed: {$passes}, Failed: " . count( $failures ) . "\n"; exit( count( $failures ) > 0 ? 1 : 0 ); From f5d5f7d1425b62737ae933e694aaf0da8a1c13ad Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 18:49:18 +0000 Subject: [PATCH 10/11] fix: guard reconcile recovery terminal election --- ...kflow-action-scheduler-branch-executor.php | 5 +- tests/workflow-as-branch-smoke.php | 84 ++++++++++++++++++- 2 files changed, 85 insertions(+), 4 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index 870f1ef..b2a1e42 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -1042,6 +1042,9 @@ public static function recover_failed_action( $action_id, $failure = null ): voi return; } if ( null === $receipt ) { + if ( self::is_branch_reconciled( $run_id, $handle_id ) ) { + return; + } $failure_message = $failure instanceof \Throwable ? $failure->getMessage() : 'The branch failed before a durable reconcile continuation could be established.'; $failure_code = self::BRANCH_HOOK === $failed_hook ? 'workflow_branch_execution_uncertain' : 'workflow_branch_reconcile_recovery_failed'; self::fail_reconcile_recovery( $run_id, $handle_id, $failure_message, $failure_code ); @@ -1078,7 +1081,7 @@ private static function fail_reconcile_recovery( string $run_id, string $handle_ return false; } - $transition = WP_Agent_Workflow_Reconcile_Lock::with_lock( + $transition = agents_workflow_reconcile_with_lock( $run_id, static function () use ( $recorder, $run_id, $handle_id, $message, $code ) { $result = $recorder->find( $run_id ); diff --git a/tests/workflow-as-branch-smoke.php b/tests/workflow-as-branch-smoke.php index 7da7a08..2157129 100644 --- a/tests/workflow-as-branch-smoke.php +++ b/tests/workflow-as-branch-smoke.php @@ -973,10 +973,15 @@ static function ( $override, string $run_id, callable $critical ) use ( &$lock_a add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder8 ) { return $recorder8; } ); ( new WP_Agent_Workflow_Runner( $recorder8 ) )->run( as_smoke_roles_spec(), array(), array( 'run_id' => 'as-recovery-terminal' ) ); $branches8 = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +$recovery_lock_attempts8 = 0; add_filter( 'wp_agent_workflow_reconcile_lock', - static function () { - return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + static function ( $override, string $run_id, callable $critical ) use ( &$recovery_lock_attempts8 ) { + unset( $override ); + if ( 'as-recovery-terminal' === $run_id && $recovery_lock_attempts8++ < 2 ) { + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + } + return $critical(); }, 10, 3 @@ -1096,7 +1101,19 @@ static function ( $override, string $run_id, callable $critical ) use ( &$transi unset( AS_Shim::$queue[ $index ]['args'][0]['store_backend'] ); } } -add_filter( 'wp_agent_workflow_reconcile_lock', static function () { return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); }, 10, 3 ); +$transition_lock_attempts10 = 0; +add_filter( + 'wp_agent_workflow_reconcile_lock', + static function ( $override, string $run_id, callable $critical ) use ( &$transition_lock_attempts10 ) { + unset( $override ); + if ( 'as-transition-fail' === $run_id && $transition_lock_attempts10++ < 4 ) { + return new WP_Error( 'agents_reconcile_lock_unavailable', 'contended' ); + } + return $critical(); + }, + 10, + 3 +); $transition_effect_before10 = (int) ( $GLOBALS['__role_worker_effects']['transition-fail'] ?? 0 ); AS_Shim::fire( $branches10[0]['id'] ); $transition_final10 = $recorder10->find( 'as-transition-fail' ); @@ -1229,6 +1246,67 @@ static function ( $override, string $run_id, callable $critical ) use ( &$distin remove_all_filters( 'wp_agent_workflow_branch_store_forget' ); remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); +// A late failed callback can arrive after reconciliation removed the receipt but +// before the queued resume runs. The authoritative completed handle wins. +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$recorder_late_failure = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder_late_failure ) { return $recorder_late_failure; } ); +( new WP_Agent_Workflow_Runner( $recorder_late_failure ) )->run( as_smoke_single_branch_spec( 'late-failure' ), array(), array( 'run_id' => 'as-late-failure' ) ); +$late_failure_branches = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +$late_failure_completions = 0; +add_action( + 'wp_agent_workflow_run_completed', + static function ( $result, string $run_id ) use ( &$late_failure_completions ): void { + unset( $result ); + if ( 'as-late-failure' === $run_id ) { + ++$late_failure_completions; + } + }, + 10, + 2 +); +AS_Shim::fire( $late_failure_branches[0]['id'] ); +$late_failure_resumes = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::RESUME_HOOK ); +do_action( 'action_scheduler_failed_execution', $late_failure_branches[0]['id'], new RuntimeException( 'late timeout callback' ), 'test' ); +$late_failure_mid = $recorder_late_failure->find( 'as-late-failure' ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUSPENDED, $late_failure_mid->get_status(), 'late failed callback: reconciled handle preserves suspended run for queued resume', $failures, $passes ); +smoke_assert( 1, count( $late_failure_resumes ), 'late failed callback: queued resume remains authoritative', $failures, $passes ); +AS_Shim::fire( $late_failure_resumes[0]['id'] ); +$late_failure_final = $recorder_late_failure->find( 'as-late-failure' ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_SUCCEEDED, $late_failure_final->get_status(), 'late failed callback: queued resume reaches success', $failures, $passes ); +smoke_assert( 1, $late_failure_completions, 'late failed callback: completion funnel fires once', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_run_completed' ); + +// Forced terminal election must use the pluggable reconcile-lock contract. +AS_Shim::reset(); +$GLOBALS['__options'] = array(); +$recorder_custom_terminal_lock = new AS_Smoke_Recorder(); +remove_all_filters( 'wp_agent_workflow_run_recorder' ); +add_filter( 'wp_agent_workflow_run_recorder', static function () use ( $recorder_custom_terminal_lock ) { return $recorder_custom_terminal_lock; } ); +( new WP_Agent_Workflow_Runner( $recorder_custom_terminal_lock ) )->run( as_smoke_single_branch_spec( 'custom-terminal-lock' ), array(), array( 'run_id' => 'as-custom-terminal-lock' ) ); +$custom_terminal_lock_branches = AS_Shim::actions_for( WP_Agent_Workflow_Action_Scheduler_Branch_Executor::BRANCH_HOOK ); +$custom_terminal_lock_calls = 0; +add_filter( + 'wp_agent_workflow_reconcile_lock', + static function ( $override, string $run_id, callable $critical ) use ( &$custom_terminal_lock_calls ) { + unset( $override ); + if ( 'as-custom-terminal-lock' === $run_id ) { + ++$custom_terminal_lock_calls; + return $critical(); + } + return null; + }, + 10, + 3 +); +do_action( 'action_scheduler_failed_execution', $custom_terminal_lock_branches[0]['id'], new RuntimeException( 'uncertain worker outcome' ), 'test' ); +$custom_terminal_lock_final = $recorder_custom_terminal_lock->find( 'as-custom-terminal-lock' ); +smoke_assert( 1, $custom_terminal_lock_calls, 'custom terminal lock: pluggable reconcile lock serializes election', $failures, $passes ); +smoke_assert( WP_Agent_Workflow_Run_Result::STATUS_FAILED, $custom_terminal_lock_final->get_status(), 'custom terminal lock: winner commits terminal failure', $failures, $passes ); +remove_all_filters( 'wp_agent_workflow_reconcile_lock' ); + // A tokenless branch action can fail before writing a receipt. Effects are // uncertain, so recovery terminalizes without rerunning the branch. A duplicate // failure callback arriving during completion publication must lose the locked From c06c16ce9f6c8f0ff289f8f6fc6d101a4068b162 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 18:55:09 +0000 Subject: [PATCH 11/11] fix: propagate reconcile recovery contention --- ...s-wp-agent-workflow-action-scheduler-branch-executor.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php index b2a1e42..0664d8a 100644 --- a/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php +++ b/src/Workflows/class-wp-agent-workflow-action-scheduler-branch-executor.php @@ -1135,8 +1135,7 @@ private static function continue_compatibility_reconcile( string $run_id, string return true; } if ( 'agents_reconcile_lock_unavailable' !== $result->get_error_code() ) { - self::fail_reconcile_recovery( $run_id, $handle_id, $result->get_error_message() ); - return true; + return self::fail_reconcile_recovery( $run_id, $handle_id, $result->get_error_message() ); } $error_data = $result->get_error_data(); if ( is_array( $error_data ) && is_array( $error_data['reconcile_continuation'] ?? null ) ) { @@ -1144,8 +1143,7 @@ private static function continue_compatibility_reconcile( string $run_id, string } } - self::fail_reconcile_recovery( $run_id, $handle_id, 'Compatibility reconcile contention exceeded its bounded retry budget.' ); - return true; + return self::fail_reconcile_recovery( $run_id, $handle_id, 'Compatibility reconcile contention exceeded its bounded retry budget.' ); } /**