From ccec3801dd2f72c3638bef0d7e9cd3f3836c3edd Mon Sep 17 00:00:00 2001 From: Miguel Lezama Date: Mon, 3 Aug 2026 11:34:05 -0300 Subject: [PATCH 1/7] Security: fail closed when the conversation loop body throws `WP_Agent_Conversation_Loop::run()` wrapped only the turn-runner call in a try/catch that finalizes the run to STATUS_FAILED, persists the transcript, and emits a `failed` event. The rest of the outer loop body -- post-turn checks, tool-call mediation, message construction, the runtime-tool store, and the caller-supplied `should_continue` continuation policy -- ran under an outer try whose only companion was a `finally` that released the transcript lock. There was no catch. An unguarded `\Throwable` from that region (e.g. `WP_Agent_Tool_Call::normalize`, message construction, the runtime-tool store, or a throwing `should_continue`) escaped `run()` with the run stuck in STATUS_RUNNING forever: no `failed` event, no persisted transcript, and the transcript lock as the only thing the `finally` cleaned up. Fail closed. Factor the existing failure finalization (emit `failed`, finish_run -> FAILED, persist transcript, return the normalized failure result) into a shared `finalize_loop_failure()` helper, and add an outer catch that runs it for any unguarded throw from the loop body, matching the turn-runner boundary exactly. The `finally` still releases the lock. The deliberate turn-runner contract violation (non-array return) is finalized in place and re-thrown to the caller as before; the guard detects it by object identity so it keeps escaping instead of being converted into a structured failure. Adds tests/conversation-loop-fail-closed-smoke.php: a throw from the outer body finalizes to FAILED, emits a `failed` event, and calls the transcript persister (fails without this change); the normal continuation path still completes cleanly; and the non-array contract violation still escapes. Co-Authored-By: Claude Opus 4.8 (1M context) --- composer.json | 1 + .../class-wp-agent-conversation-loop.php | 136 ++++++++++++++-- tests/conversation-loop-fail-closed-smoke.php | 151 ++++++++++++++++++ 3 files changed, 271 insertions(+), 17 deletions(-) create mode 100644 tests/conversation-loop-fail-closed-smoke.php diff --git a/composer.json b/composer.json index 8b864bf..1380e2b 100644 --- a/composer.json +++ b/composer.json @@ -88,6 +88,7 @@ "php tests/citation-metadata-smoke.php", "php tests/context-registry-smoke.php", "php tests/conversation-loop-smoke.php", + "php tests/conversation-loop-fail-closed-smoke.php", "php tests/provider-turn-adapter-smoke.php", "php tests/default-provider-turn-adapter-smoke.php", "php tests/agents-chat-ability-smoke.php", diff --git a/src/Runtime/class-wp-agent-conversation-loop.php b/src/Runtime/class-wp-agent-conversation-loop.php index d7e650c..eb9d5f4 100644 --- a/src/Runtime/class-wp-agent-conversation-loop.php +++ b/src/Runtime/class-wp-agent-conversation-loop.php @@ -185,6 +185,12 @@ public static function run( array $messages, ?callable $turn_runner = null, arra } } + // Tracks a deliberate turn-runner contract violation (non-array return), + // which is finalized in place and re-thrown to the caller. The fail-closed + // guard below must let this specific throw keep escaping rather than + // convert it into a structured failure result. + $loop_contract_error = null; + try { for ( $turn = 1; $turn <= $max_turns; ++$turn ) { $wall_clock_exceeded = self::check_wall_clock_budget( $budgets, $wall_clock_started_at, $wall_clock_initial, $on_event ); @@ -218,12 +224,8 @@ public static function run( array $messages, ?callable $turn_runner = null, arra try { $result = call_user_func( $turn_runner, $messages, $turn_context ); } catch ( \Throwable $error ) { - self::emit_event( $on_event, 'failed', array( - 'turn' => $turn, - 'error' => $error->getMessage(), - ) ); - - $failure_result = self::failure_result( + return self::finalize_loop_failure( + $on_event, $messages, $tool_results, $tool_events, @@ -232,23 +234,21 @@ public static function run( array $messages, ?callable $turn_runner = null, arra $error, $turn, $total_usage, - $request_metadata + $request_metadata, + $run_id, + $lock_session_id, + $run_workspace, + $transcript_persister, + $options ); - - if ( '' !== $run_id && '' !== $lock_session_id ) { - WP_Agent_Chat_Run_Control::finish_run( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); - } - - self::persist_transcript( $transcript_persister, $messages, $options, $failure_result ); - return $failure_result; } if ( ! is_array( $result ) ) { - $error = new \InvalidArgumentException( 'invalid_agent_conversation_loop: turn runner must return an array' ); + $loop_contract_error = new \InvalidArgumentException( 'invalid_agent_conversation_loop: turn runner must return an array' ); self::emit_event( $on_event, 'failed', array( 'turn' => $turn, - 'error' => $error->getMessage(), + 'error' => $loop_contract_error->getMessage(), ) ); self::persist_transcript( $transcript_persister, $messages, $options, array( @@ -259,7 +259,7 @@ public static function run( array $messages, ?callable $turn_runner = null, arra 'events' => $events, ) ); - throw $error; + throw $loop_contract_error; } $interrupt = self::check_runtime_cancellation( $run_id, $lock_session_id, $turn_context, $on_event, $run_workspace, $run_owner ); @@ -557,6 +557,37 @@ public static function run( array $messages, ?callable $turn_runner = null, arra ) ); return $final_result; + } catch ( \Throwable $error ) { + // A deliberate turn-runner contract violation (non-array return) is + // finalized in place and re-thrown to the caller; keep it escaping. + if ( null !== $loop_contract_error && $error === $loop_contract_error ) { + throw $error; + } + + // Fail closed. An unguarded throw from anywhere in the loop body -- + // compaction/summarizer, tool-call normalization, message construction, + // mediation, or the runtime-tool store -- would otherwise escape run() + // leaving the run stuck in STATUS_RUNNING with no `failed` event and no + // persisted transcript. Finalize identically to the turn-runner boundary + // (finish_run -> FAILED, persist transcript, emit `failed`) so the + // `finally` below still releases the transcript lock. + return self::finalize_loop_failure( + $on_event, + $messages, + $tool_results, + $tool_events, + $tool_audit_events, + $events, + $error, + $turns_run, + $total_usage, + $request_metadata, + $run_id, + $lock_session_id, + $run_workspace, + $transcript_persister, + $options + ); } finally { if ( null !== $transcript_lock && null !== $lock_token && '' !== $lock_session_id ) { try { @@ -1090,6 +1121,77 @@ public static function mediate_tool_calls( ); } + /** + * Fail closed: finalize a run that threw and return the structured failure result. + * + * Shared by the turn-runner boundary and the outer loop-body guard so an + * unguarded `\Throwable` from anywhere in the loop finalizes identically: + * emit the `failed` event, mark run-control FAILED, persist the transcript, + * and return the normalized failure result. Without this, a throw from + * compaction, tool-call normalization, message construction, mediation, or + * the runtime-tool store would escape `run()` and leave the run stuck in + * STATUS_RUNNING with no `failed` event and no persisted transcript. + * + * @param callable|null $on_event Event sink. + * @param array> $messages Current transcript messages. + * @param array> $tool_results Accumulated tool execution results. + * @param array $tool_events Accumulated canonical tool events. + * @param array> $tool_audit_events Accumulated audit events. + * @param array> $events Accumulated loop events. + * @param \Throwable $error Runtime error. + * @param int $turn Current turn. + * @param array $usage Accumulated usage. + * @param array $request_metadata Latest request metadata. + * @param string $run_id Run identifier, or '' when run control is disabled. + * @param string $lock_session_id Session id, or '' when run control is disabled. + * @param \AgentsAPI\Core\Workspace\WP_Agent_Workspace_Scope|null $run_workspace Run workspace scope. + * @param WP_Agent_Transcript_Persister|null $transcript_persister Transcript persister. + * @param array $options Loop options. + * @return array Normalized conversation result. + */ + private static function finalize_loop_failure( + ?callable $on_event, + array $messages, + array $tool_results, + array $tool_events, + array $tool_audit_events, + array $events, + \Throwable $error, + int $turn, + array $usage, + array $request_metadata, + string $run_id, + string $lock_session_id, + ?\AgentsAPI\Core\Workspace\WP_Agent_Workspace_Scope $run_workspace, + ?WP_Agent_Transcript_Persister $transcript_persister, + array $options + ): array { + self::emit_event( $on_event, 'failed', array( + 'turn' => $turn, + 'error' => $error->getMessage(), + ) ); + + $failure_result = self::failure_result( + $messages, + $tool_results, + $tool_events, + $tool_audit_events, + $events, + $error, + $turn, + $usage, + $request_metadata + ); + + if ( '' !== $run_id && '' !== $lock_session_id ) { + WP_Agent_Chat_Run_Control::finish_run( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); + } + + self::persist_transcript( $transcript_persister, $messages, $options, $failure_result ); + + return $failure_result; + } + /** * Build a structured runtime failure result without forcing callers to rebuild loop state. * diff --git a/tests/conversation-loop-fail-closed-smoke.php b/tests/conversation-loop-fail-closed-smoke.php new file mode 100644 index 0000000..4a05a0c --- /dev/null +++ b/tests/conversation-loop-fail-closed-smoke.php @@ -0,0 +1,151 @@ +log = &$log; + } + + public function persist( array $messages, AgentsAPI\AI\WP_Agent_Conversation_Request $request, array $result ): string { + unset( $request ); + $this->log[] = array( + 'message_count' => count( $messages ), + 'status' => $result['status'] ?? '', + ); + + return 'transcript-' . count( $this->log ); + } +}; + +// A well-behaved caller-managed turn runner that always yields one assistant turn. +$turn_runner = static function ( array $messages ): array { + $messages[] = AgentsAPI\AI\WP_Agent_Message::text( 'assistant', 'working' ); + return array( + 'messages' => $messages, + 'tool_execution_results' => array(), + ); +}; + +echo "\n[1] An unguarded throw from the loop body fails closed instead of escaping:\n"; +$persist_log = array(); +$events = array(); +$loop_id = 'fail-closed-run-1'; +$escaped_error = null; + +try { + $result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'hello' ) ), + $turn_runner, + array( + 'run_id' => $loop_id, + 'transcript_session_id' => 'fail-closed-session-1', + 'max_turns' => 3, + 'should_continue' => static function (): bool { + // A caller-owned continuation policy that throws inside the outer + // loop body, past the turn-runner try/catch boundary. + throw new \RuntimeException( 'continuation policy exploded' ); + }, + 'transcript_persister' => $persister, + 'on_event' => static function ( string $event, array $payload ) use ( &$events ): void { + $events[] = array( 'event' => $event, 'payload' => $payload ); + }, + ) + ); +} catch ( \Throwable $error ) { + // Before the fix the throw escapes run() entirely; capture it so the + // remaining assertions can report the regression instead of fataling. + $escaped_error = $error; + $result = array(); +} + +$failed_events = array_values( array_filter( $events, static fn( array $e ): bool => 'failed' === $e['event'] ) ); + +agents_api_smoke_assert_equals( null, $escaped_error, 'unguarded loop-body throw does not escape run()', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $result['status'] ?? '', 'loop returns a structured failed result instead of hanging in RUNNING', $failures, $passes ); +agents_api_smoke_assert_equals( 'continuation policy exploded', $result['failure']['message'] ?? '', 'structured failure preserves the thrown message', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $failed_events ), 'a failed lifecycle event is emitted', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $persist_log ), 'transcript persister is called on the fail-closed path', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $persist_log[0]['status'] ?? '', 'persister receives the failed result', $failures, $passes ); + +echo "\n[2] The normal (non-throwing) continuation path still completes cleanly:\n"; +$persist_log = array(); +$events = array(); + +$ok_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'hello' ) ), + $turn_runner, + array( + 'run_id' => 'fail-closed-run-2', + 'transcript_session_id' => 'fail-closed-session-2', + 'max_turns' => 3, + 'should_continue' => static function ( array $turn_result, array $context ): bool { + unset( $turn_result ); + // Stop after the first turn -- a well-behaved continuation policy. + return 1 > $context['turn']; + }, + 'transcript_persister' => $persister, + 'on_event' => static function ( string $event, array $payload ) use ( &$events ): void { + $events[] = array( 'event' => $event, 'payload' => $payload ); + }, + ) +); + +$ok_failed_events = array_values( array_filter( $events, static fn( array $e ): bool => 'failed' === $e['event'] ) ); +$ok_completed_events = array_values( array_filter( $events, static fn( array $e ): bool => 'completed' === $e['event'] ) ); + +agents_api_smoke_assert_equals( true, $ok_result['completed'] ?? null, 'legitimate flow completes when the continuation policy does not throw', $failures, $passes ); +agents_api_smoke_assert_equals( false, 'failed' === ( $ok_result['status'] ?? '' ), 'normal path is not marked failed', $failures, $passes ); +agents_api_smoke_assert_equals( 0, count( $ok_failed_events ), 'no failed event is emitted on the normal path', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $ok_completed_events ), 'a completed event is emitted on the normal path', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $persist_log ), 'persister is called once on the normal path', $failures, $passes ); + +echo "\n[3] The turn-runner contract violation (non-array return) still escapes to the caller:\n"; +$threw = false; +try { + AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'hello' ) ), + static function (): string { + return 'not an array'; + } + ); +} catch ( InvalidArgumentException $e ) { + $threw = str_starts_with( $e->getMessage(), 'invalid_agent_conversation_loop:' ); +} +agents_api_smoke_assert_equals( true, $threw, 'deliberate contract violation is not swallowed by the fail-closed guard', $failures, $passes ); + +agents_api_smoke_finish( 'Agents API conversation loop fail-closed', $failures, $passes ); From 49b4a51fbff31db0273574f9725720239fbc4920 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 21:57:34 +0000 Subject: [PATCH 2/7] fix: fail closed conversation loop policy errors --- .../class-wp-agent-conversation-loop.php | 180 +++++++++++------ .../class-wp-agent-tool-mediation-runner.php | 6 +- tests/conversation-loop-fail-closed-smoke.php | 190 +++++++++++++++++- 3 files changed, 313 insertions(+), 63 deletions(-) diff --git a/src/Runtime/class-wp-agent-conversation-loop.php b/src/Runtime/class-wp-agent-conversation-loop.php index c005541..f134450 100644 --- a/src/Runtime/class-wp-agent-conversation-loop.php +++ b/src/Runtime/class-wp-agent-conversation-loop.php @@ -262,18 +262,23 @@ public static function run( array $messages, ?callable $turn_runner = null, arra if ( ! is_array( $result ) ) { $loop_contract_error = new \InvalidArgumentException( 'invalid_agent_conversation_loop: turn runner must return an array' ); - self::emit_event( $on_event, 'failed', array( - 'turn' => $turn, - 'error' => $loop_contract_error->getMessage(), - ) ); - - self::persist_transcript( $transcript_persister, $messages, $options, array( - 'messages' => $messages, - 'tool_execution_results' => $tool_results, - 'tool_events' => $tool_events, - 'tool_audit_events' => $tool_audit_events, - 'events' => $events, - ) ); + self::finalize_loop_failure( + $on_event, + $messages, + $tool_results, + $tool_events, + $tool_audit_events, + $events, + $loop_contract_error, + $turn, + $total_usage, + $request_metadata, + $run_id, + $lock_session_id, + $run_workspace, + $transcript_persister, + $options + ); throw $loop_contract_error; } @@ -314,7 +319,7 @@ public static function run( array $messages, ?callable $turn_runner = null, arra ) ); if ( '' !== $run_id && '' !== $lock_session_id ) { - WP_Agent_Chat_Run_Control::finish_run( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); + self::finish_run_or_throw( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); } self::persist_transcript( $transcript_persister, $messages, $options, $failure_result ); @@ -352,25 +357,38 @@ public static function run( array $messages, ?callable $turn_runner = null, arra break; } - $mediation_result = WP_Agent_Tool_Mediation_Runner::run( - $messages, - self::normalize_assoc_array( $result ), - $tool_executor, - $tool_declarations, - array( - 'completion_policy' => $completion_policy, - 'turn_context' => $turn_context, - 'turn' => $turn, - 'on_event' => $on_event, - 'budgets' => $budgets, - 'identical_failure_tracker' => $failure_tracker, - 'tool_result_truncator' => $result_truncator, - 'pre_tool_mediator' => $pre_tool_mediator, - 'prior_tool_results' => $tool_results, - 'post_tool_result_diagnostics' => $post_tool_diagnostics, - 'runtime_tool_request_store' => $runtime_tool_store, - ) - ); + $mediation_checkpoint = array(); + try { + $mediation_result = WP_Agent_Tool_Mediation_Runner::run( + $messages, + self::normalize_assoc_array( $result ), + $tool_executor, + $tool_declarations, + array( + 'completion_policy' => $completion_policy, + 'turn_context' => $turn_context, + 'turn' => $turn, + 'on_event' => $on_event, + 'budgets' => $budgets, + 'identical_failure_tracker' => $failure_tracker, + 'tool_result_truncator' => $result_truncator, + 'pre_tool_mediator' => $pre_tool_mediator, + 'prior_tool_results' => $tool_results, + 'post_tool_result_diagnostics' => $post_tool_diagnostics, + 'runtime_tool_request_store' => $runtime_tool_store, + ), + $mediation_checkpoint + ); + } catch ( \Throwable $error ) { + if ( ! empty( $mediation_checkpoint ) ) { + $messages = self::normalize_messages( is_array( $mediation_checkpoint['messages'] ?? null ) ? $mediation_checkpoint['messages'] : array() ); + $tool_results = array_merge( $tool_results, self::normalize_array_list( $mediation_checkpoint['tool_execution_results'] ?? array() ) ); + $tool_events = array_merge( $tool_events, self::normalize_array_list( $mediation_checkpoint['tool_events'] ?? array() ) ); + $tool_audit_events = array_merge( $tool_audit_events, self::normalize_array_list( $mediation_checkpoint['tool_audit_events'] ?? array() ) ); + $events = array_merge( $events, self::normalize_events( $mediation_checkpoint['events'] ?? array() ) ); + } + throw $error; + } $messages = $mediation_result['messages']; $tool_results = array_merge( $tool_results, $mediation_result['tool_execution_results'] ); @@ -566,11 +584,7 @@ public static function run( array $messages, ?callable $turn_runner = null, arra $final_result = self::normalize_conversation_result( $final_result_data ); if ( '' !== $run_id && '' !== $lock_session_id ) { - $finished = WP_Agent_Chat_Run_Control::finish_run( $run_id, WP_Agent_Run_Outcome::run_control_status( $final_result ), $run_workspace ); - if ( is_wp_error( $finished ) ) { - self::emit_event( $on_event, 'failed', array( 'error' => $finished->get_error_message() ) ); - return self::run_control_failure_result( $messages, $finished ); - } + self::finish_run_or_throw( $run_id, WP_Agent_Run_Outcome::run_control_status( $final_result ), $run_workspace ); } self::persist_transcript( $transcript_persister, $messages, $options, $final_result ); @@ -583,6 +597,10 @@ public static function run( array $messages, ?callable $turn_runner = null, arra return $final_result; } catch ( \Throwable $error ) { + if ( $error instanceof WP_Agent_Run_Control_Store_Exception ) { + throw $error; + } + // A deliberate turn-runner contract violation (non-array return) is // finalized in place and re-thrown to the caller; keep it escaping. if ( null !== $loop_contract_error && $error === $loop_contract_error ) { @@ -699,6 +717,7 @@ public static function run_conversation( array $messages, array $tool_declaratio * @param array> $prior_tool_results Prior mediated tool results. * @param callable|null $post_tool_diagnostics Optional post-result diagnostics callback. * @param WP_Agent_Runtime_Tool_Request_Store|null $runtime_tool_store Optional durable runtime tool request store. + * @param array|null $checkpoint Out: latest canonical mediation state. * @return array{messages: array>, tool_execution_results: array>, tool_events: array>, tool_audit_events: array>, events: array>, conversation_complete: bool, exceeded_budget: string|null, approval_required: array|null, runtime_tool_pending: array|null, spin_signatures: array} */ public static function mediate_tool_calls( @@ -716,7 +735,8 @@ public static function mediate_tool_calls( ?callable $pre_tool_mediator = null, array $prior_tool_results = array(), ?callable $post_tool_diagnostics = null, - ?WP_Agent_Runtime_Tool_Request_Store $runtime_tool_store = null + ?WP_Agent_Runtime_Tool_Request_Store $runtime_tool_store = null, + ?array &$checkpoint = null ): array { $core = new WP_Agent_Tool_Execution_Core(); @@ -737,6 +757,13 @@ public static function mediate_tool_calls( $exceeded_budget = null; $approval_required = null; $runtime_tool_pending = null; + $checkpoint = self::mediation_checkpoint( + $messages, + $tool_execution_results, + $tool_events, + $tool_audit_events, + $events + ); // If the turn runner returned text content, add it as an assistant message. if ( isset( $result['content'] ) && is_string( $result['content'] ) && '' !== $result['content'] ) { @@ -801,6 +828,13 @@ public static function mediate_tool_calls( 'parameters_redacted' => true, ) ); + $checkpoint = self::mediation_checkpoint( + $messages, + $tool_execution_results, + $tool_events, + $tool_audit_events, + $events + ); $mediator_complete = false; $mediation_context = array( 'messages' => $messages, @@ -1022,6 +1056,13 @@ public static function mediate_tool_calls( $exec_result, array( 'tool_call_id' => $tool_call_id ) ); + $checkpoint = self::mediation_checkpoint( + $messages, + $tool_execution_results, + $tool_events, + $tool_audit_events, + $events + ); $nudge = self::check_identical_failure_tracker( $failure_tracker, @@ -1146,6 +1187,26 @@ public static function mediate_tool_calls( ); } + /** + * Snapshot durable mediation state before caller-owned policy runs. + * + * @param array> $messages Current transcript. + * @param array> $tool_results Current tool results. + * @param array> $tool_events Current tool events. + * @param array> $tool_audit_events Current tool audit events. + * @param array> $events Current loop events. + * @return array{messages: array>, tool_execution_results: array>, tool_events: array>, tool_audit_events: array>, events: array>} + */ + private static function mediation_checkpoint( array $messages, array $tool_results, array $tool_events, array $tool_audit_events, array $events ): array { + return array( + 'messages' => $messages, + 'tool_execution_results' => $tool_results, + 'tool_events' => $tool_events, + 'tool_audit_events' => $tool_audit_events, + 'events' => $events, + ); + } + /** * Fail closed: finalize a run that threw and return the structured failure result. * @@ -1208,15 +1269,25 @@ private static function finalize_loop_failure( $request_metadata ); + self::persist_transcript( $transcript_persister, $messages, $options, $failure_result ); + if ( '' !== $run_id && '' !== $lock_session_id ) { - WP_Agent_Chat_Run_Control::finish_run( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); + self::finish_run_or_throw( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); } - self::persist_transcript( $transcript_persister, $messages, $options, $failure_result ); - return $failure_result; } + /** + * Finalize run control without hiding retryable storage failures. + */ + private static function finish_run_or_throw( string $run_id, string $status, ?\AgentsAPI\Core\Workspace\WP_Agent_Workspace_Scope $workspace ): void { + $finished = WP_Agent_Chat_Run_Control::finish_run( $run_id, $status, $workspace ); + if ( is_wp_error( $finished ) ) { + throw new WP_Agent_Run_Control_Store_Exception( $finished->get_error_message() ); + } + } + /** * Build a structured runtime failure result without forcing callers to rebuild loop state. * @@ -1371,9 +1442,9 @@ private static function maybe_truncate_tool_result( ?WP_Agent_Tool_Result_Trunca /** * Invoke and normalize the optional pre-tool mediation decision callback. * - * The mediator is a synchronous, storage-free product policy seam. Invalid or - * throwing callbacks fall back to `proceed` so the default execution path is - * preserved unless the mediator explicitly returns a supported decision. + * The mediator is a synchronous, storage-free product policy seam. Invalid + * decisions retain the current decision, while exceptions propagate so policy + * failures can never authorize tool execution by substituting `proceed`. * * @param callable|null $mediator Optional pre-tool mediator. * @param array $context Tool mediation context. @@ -1388,24 +1459,15 @@ private static function pre_tool_mediation_decision( ?callable $mediator, array $decision = $proceed; if ( null !== $mediator ) { - try { - $decision = self::normalize_pre_tool_mediation_decision( call_user_func( $mediator, $context ), $context, $proceed ); - } catch ( \Throwable $error ) { - unset( $error ); - $decision = $proceed; - } + $decision = self::normalize_pre_tool_mediation_decision( call_user_func( $mediator, $context ), $context, $proceed ); } if ( function_exists( 'apply_filters' ) ) { - try { - $decision = self::normalize_pre_tool_mediation_decision( - apply_filters( 'agents_api_pre_tool_call_decision', $decision, $context ), - $context, - $decision - ); - } catch ( \Throwable $error ) { - unset( $error ); - } + $decision = self::normalize_pre_tool_mediation_decision( + apply_filters( 'agents_api_pre_tool_call_decision', $decision, $context ), + $context, + $decision + ); } return $decision; diff --git a/src/Runtime/class-wp-agent-tool-mediation-runner.php b/src/Runtime/class-wp-agent-tool-mediation-runner.php index 16df3e1..f40205b 100644 --- a/src/Runtime/class-wp-agent-tool-mediation-runner.php +++ b/src/Runtime/class-wp-agent-tool-mediation-runner.php @@ -26,9 +26,10 @@ class WP_Agent_Tool_Mediation_Runner { * @param WP_Agent_Tool_Executor $executor Tool executor adapter. * @param array> $declarations Tool declarations keyed by name. * @param array $options Execution policy and observers. + * @param array|null $checkpoint Out: latest canonical mediation state. * @return array{messages: array>, tool_execution_results: array>, tool_events: array>, tool_audit_events: array>, events: array>, conversation_complete: bool, exceeded_budget: string|null, approval_required: array|null, runtime_tool_pending: array|null, spin_signatures: array} */ - public static function run( array $transcript, array $turn_result, WP_Agent_Tool_Executor $executor, array $declarations, array $options = array() ): array { + public static function run( array $transcript, array $turn_result, WP_Agent_Tool_Executor $executor, array $declarations, array $options = array(), ?array &$checkpoint = null ): array { $turn_context = isset( $options['turn_context'] ) && is_array( $options['turn_context'] ) ? self::normalize_assoc_array( $options['turn_context'] ) : array(); $turn = isset( $options['turn'] ) && is_int( $options['turn'] ) ? $options['turn'] : 1; $completion_policy = $options['completion_policy'] ?? null; @@ -52,7 +53,8 @@ public static function run( array $transcript, array $turn_result, WP_Agent_Tool is_callable( $options['pre_tool_mediator'] ?? null ) ? $options['pre_tool_mediator'] : null, isset( $options['prior_tool_results'] ) && is_array( $options['prior_tool_results'] ) ? self::normalize_array_list( $options['prior_tool_results'] ) : array(), is_callable( $options['post_tool_result_diagnostics'] ?? null ) ? $options['post_tool_result_diagnostics'] : null, - $runtime_tool_store instanceof WP_Agent_Runtime_Tool_Request_Store ? $runtime_tool_store : null + $runtime_tool_store instanceof WP_Agent_Runtime_Tool_Request_Store ? $runtime_tool_store : null, + $checkpoint ); } diff --git a/tests/conversation-loop-fail-closed-smoke.php b/tests/conversation-loop-fail-closed-smoke.php index 4a05a0c..6a5cc67 100644 --- a/tests/conversation-loop-fail-closed-smoke.php +++ b/tests/conversation-loop-fail-closed-smoke.php @@ -30,6 +30,8 @@ require_once __DIR__ . '/agents-api-smoke-helpers.php'; agents_api_smoke_require_module(); +require_once __DIR__ . '/class-agents-api-memory-atomic-run-control-store.php'; +AgentsAPI\AI\WP_Agent_Run_Control::set_store( new Agents_API_Memory_Atomic_Run_Control_Store() ); // A transcript persister that records whether it was called and with what status. $persist_log = array(); @@ -46,6 +48,7 @@ public function persist( array $messages, AgentsAPI\AI\WP_Agent_Conversation_Req $this->log[] = array( 'message_count' => count( $messages ), 'status' => $result['status'] ?? '', + 'result' => $result, ); return 'transcript-' . count( $this->log ); @@ -101,6 +104,7 @@ public function persist( array $messages, AgentsAPI\AI\WP_Agent_Conversation_Req agents_api_smoke_assert_equals( 1, count( $failed_events ), 'a failed lifecycle event is emitted', $failures, $passes ); agents_api_smoke_assert_equals( 1, count( $persist_log ), 'transcript persister is called on the fail-closed path', $failures, $passes ); agents_api_smoke_assert_equals( 'failed', $persist_log[0]['status'] ?? '', 'persister receives the failed result', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( $loop_id )['status'] ?? '', 'should_continue failure durably finalizes run control', $failures, $passes ); echo "\n[2] The normal (non-throwing) continuation path still completes cleanly:\n"; $persist_log = array(); @@ -134,18 +138,200 @@ public function persist( array $messages, AgentsAPI\AI\WP_Agent_Conversation_Req agents_api_smoke_assert_equals( 1, count( $ok_completed_events ), 'a completed event is emitted on the normal path', $failures, $passes ); agents_api_smoke_assert_equals( 1, count( $persist_log ), 'persister is called once on the normal path', $failures, $passes ); -echo "\n[3] The turn-runner contract violation (non-array return) still escapes to the caller:\n"; +echo "\n[3] Policy exceptions fail closed before tool execution:\n"; +$policy_executor = new class() implements AgentsAPI\AI\Tools\WP_Agent_Tool_Executor { + public int $calls = 0; + public function executeWP_Agent_Tool_Call( array $tool_call, array $tool_definition, array $context = array() ): array { + unset( $tool_definition, $context ); + ++$this->calls; + return array( 'success' => true, 'tool_name' => $tool_call['tool_name'], 'result' => array( 'ok' => true ) ); + } +}; +$policy_tools = array( + 'write/item' => array( + 'name' => 'write/item', + 'source' => 'test', + 'description' => 'Write one item.', + 'parameters' => array( 'type' => 'object', 'properties' => array() ), + 'executor' => 'test', + ), +); +$tool_turn = static function ( array $messages ): array { + return array( + 'messages' => $messages, + 'tool_calls' => array( array( 'id' => 'policy-call', 'name' => 'write/item', 'parameters' => array() ) ), + ); +}; + +$persist_log = array(); +$mediator_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'write' ) ), + $tool_turn, + array( + 'run_id' => 'fail-closed-mediator', + 'transcript_session_id' => 'fail-closed-mediator-session', + 'tool_executor' => $policy_executor, + 'tool_declarations' => $policy_tools, + 'pre_tool_mediator' => static function (): array { + throw new RuntimeException( 'mediator unavailable' ); + }, + 'transcript_persister' => $persister, + ) +); +agents_api_smoke_assert_equals( 'failed', $mediator_result['status'] ?? '', 'throwing mediator fails the run closed', $failures, $passes ); +agents_api_smoke_assert_equals( 0, $policy_executor->calls, 'throwing mediator never substitutes proceed', $failures, $passes ); + +add_filter( + 'agents_api_pre_tool_call_decision', + static function ( array $decision, array $context ): array { + if ( 'gate-call' === ( $context['tool_call_id'] ?? '' ) ) { + throw new RuntimeException( 'gate unavailable' ); + } + return $decision; + }, + 10, + 2 +); +$gate_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'write' ) ), + static function ( array $messages ): array { + return array( 'messages' => $messages, 'tool_calls' => array( array( 'id' => 'gate-call', 'name' => 'write/item', 'parameters' => array() ) ) ); + }, + array( + 'run_id' => 'fail-closed-gate', + 'transcript_session_id' => 'fail-closed-gate-session', + 'tool_executor' => $policy_executor, + 'tool_declarations' => $policy_tools, + 'transcript_persister' => $persister, + ) +); +agents_api_smoke_assert_equals( 'failed', $gate_result['status'] ?? '', 'throwing policy filter fails the run closed', $failures, $passes ); +agents_api_smoke_assert_equals( 0, $policy_executor->calls, 'throwing policy filter never executes the tool', $failures, $passes ); + +echo "\n[4] Runtime-tool store exceptions fail the run and persist the transcript:\n"; +$runtime_store = new class() implements AgentsAPI\AI\WP_Agent_Runtime_Tool_Request_Store { + public function create( array $request ): void { unset( $request ); throw new RuntimeException( 'runtime store unavailable' ); } + public function get( string $request_id ): ?array { unset( $request_id ); return null; } + public function complete( string $request_id, array $result ): void { unset( $request_id, $result ); } + public function timeout( string $request_id ): void { unset( $request_id ); } + public function recent_pending( array $query = array() ): array { unset( $query ); return array(); } +}; +$pending_executor = new class() implements AgentsAPI\AI\Tools\WP_Agent_Tool_Executor { + public function executeWP_Agent_Tool_Call( array $tool_call, array $tool_definition, array $context = array() ): array { + unset( $tool_definition, $context ); + return array( + 'success' => false, + 'tool_name' => $tool_call['tool_name'], + 'status' => AgentsAPI\AI\WP_Agent_Runtime_Tool_Request::STATUS_PENDING, + 'runtime_tool_request' => array( 'tool_name' => $tool_call['tool_name'], 'tool_call_id' => $tool_call['id'], 'parameters' => array() ), + ); + } +}; +$persist_log = array(); +$runtime_store_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'external tool' ) ), + $tool_turn, + array( + 'run_id' => 'fail-closed-runtime-store', + 'transcript_session_id' => 'fail-closed-runtime-store-session', + 'tool_executor' => $pending_executor, + 'tool_declarations' => $policy_tools, + 'runtime_tool_request_store' => $runtime_store, + 'transcript_persister' => $persister, + ) +); +agents_api_smoke_assert_equals( 'failed', $runtime_store_result['status'] ?? '', 'runtime-tool storage throw returns a failed result', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-runtime-store' )['status'] ?? '', 'runtime-tool storage throw durably finalizes run control', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $persist_log ), 'runtime-tool storage throw persists the failed transcript', $failures, $passes ); + +echo "\n[5] Completed tool effects remain in the failed audit when later policy throws:\n"; +$policy_executor->calls = 0; +$persist_log = array(); +$throwing_policy = new class() implements AgentsAPI\AI\WP_Agent_Conversation_Completion_Policy { + public function recordToolResult( string $tool_name, ?array $tool_def, array $tool_result, array $runtime_context, int $turn_count ): AgentsAPI\AI\WP_Agent_Conversation_Completion_Decision { + unset( $tool_name, $tool_def, $tool_result, $runtime_context, $turn_count ); + throw new RuntimeException( 'completion policy unavailable' ); + } +}; +$post_tool_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'write once' ) ), + $tool_turn, + array( + 'run_id' => 'fail-closed-post-tool', + 'transcript_session_id' => 'fail-closed-post-tool-session', + 'tool_executor' => $policy_executor, + 'tool_declarations' => $policy_tools, + 'completion_policy' => $throwing_policy, + 'transcript_persister' => $persister, + ) +); +agents_api_smoke_assert_equals( 1, $policy_executor->calls, 'side-effecting tool executes exactly once', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $post_tool_result['status'] ?? '', 'later completion-policy throw produces a terminal failure', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $post_tool_result['tool_execution_results'] ?? array() ), 'failed result retains completed tool execution', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $post_tool_result['tool_audit_events'] ?? array() ), 'failed result retains completed tool audit event', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $persist_log[0]['result']['tool_execution_results'] ?? array() ), 'persisted transcript retains completed tool effect', $failures, $passes ); + +echo "\n[6] The turn-runner contract violation finalizes run control and still escapes:\n"; $threw = false; +$persist_log = array(); try { AgentsAPI\AI\WP_Agent_Conversation_Loop::run( array( array( 'role' => 'user', 'content' => 'hello' ) ), static function (): string { return 'not an array'; - } + }, + array( + 'run_id' => 'fail-closed-contract', + 'transcript_session_id' => 'fail-closed-contract-session', + 'transcript_persister' => $persister, + ) ); } catch ( InvalidArgumentException $e ) { $threw = str_starts_with( $e->getMessage(), 'invalid_agent_conversation_loop:' ); } agents_api_smoke_assert_equals( true, $threw, 'deliberate contract violation is not swallowed by the fail-closed guard', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-contract' )['status'] ?? '', 'contract violation does not strand running run control', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $persist_log[0]['status'] ?? '', 'contract violation persists a canonical failed result', $failures, $passes ); + +echo "\n[7] Run-control finalization storage failures remain retryable and visible:\n"; +if ( ! class_exists( 'WP_Error' ) ) { + class WP_Error { + public function __construct( private string $code = '', private string $message = '' ) {} + public function get_error_code(): string { return $this->code; } + public function get_error_message(): string { return $this->message; } + } +} +if ( ! class_exists( 'wpdb' ) ) { + class wpdb {} +} +$failing_store = new class() implements AgentsAPI\AI\WP_Agent_Atomic_Run_Control_Store { + public array $state = array( 'runs' => array(), 'queues' => array(), 'events' => array() ); + public function get_state( string $store_key ): array { unset( $store_key ); return $this->state; } + public function save_state( string $store_key, array $state ): void { unset( $store_key ); $this->state = $state; } + public function mutate_state( string $store_key, callable $mutation ): mixed { + unset( $store_key ); + $mutated = $mutation( $this->state ); + foreach ( $mutated['state']['runs'] ?? array() as $run ) { + if ( is_array( $run ) && 'running' !== ( $run['status'] ?? 'running' ) ) { + throw new AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception( 'terminal state write unavailable' ); + } + } + $this->state = $mutated['state']; + return $mutated['result']; + } +}; +AgentsAPI\AI\WP_Agent_Run_Control::set_store( $failing_store ); +$finalization_error = null; +try { + AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'finish' ) ), + $turn_runner, + array( 'run_id' => 'fail-closed-finalization', 'transcript_session_id' => 'fail-closed-finalization-session' ) + ); +} catch ( AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception $error ) { + $finalization_error = $error; +} +agents_api_smoke_assert_equals( true, $finalization_error instanceof AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception, 'terminal storage failure escapes as the canonical retryable exception', $failures, $passes ); +agents_api_smoke_assert_equals( 'running', $failing_store->state['runs']['fail-closed-finalization']['status'] ?? '', 'failed terminal write remains visibly retryable instead of pretending completion', $failures, $passes ); agents_api_smoke_finish( 'Agents API conversation loop fail-closed', $failures, $passes ); From e2e48c9d0fc85198e53eb20c156bbe3a0db272b3 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 22:14:07 +0000 Subject: [PATCH 3/7] fix: preserve conversation failure audit state --- .../class-wp-agent-conversation-loop.php | 230 +++++++++++++++--- tests/conversation-loop-fail-closed-smoke.php | 120 ++++++++- tests/provider-turn-adapter-smoke.php | 2 + 3 files changed, 316 insertions(+), 36 deletions(-) diff --git a/src/Runtime/class-wp-agent-conversation-loop.php b/src/Runtime/class-wp-agent-conversation-loop.php index f134450..af781af 100644 --- a/src/Runtime/class-wp-agent-conversation-loop.php +++ b/src/Runtime/class-wp-agent-conversation-loop.php @@ -318,11 +318,24 @@ public static function run( array $messages, ?callable $turn_runner = null, arra 'failure' => $failure, ) ); - if ( '' !== $run_id && '' !== $lock_session_id ) { - self::finish_run_or_throw( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); + $persistence_error = self::persist_transcript( $transcript_persister, $messages, $options, $failure_result ); + try { + if ( '' !== $run_id && '' !== $lock_session_id ) { + self::finish_run_or_throw( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); + } + } catch ( WP_Agent_Run_Control_Store_Exception $finalization_error ) { + if ( null !== $persistence_error ) { + throw new WP_Agent_Run_Control_Store_Exception( $finalization_error->getMessage(), 0, $persistence_error ); + } + throw $finalization_error; + } + if ( null !== $persistence_error ) { + throw new WP_Agent_Run_Control_Store_Exception( + 'Transcript persistence failed while finalizing the failed conversation run.', + 0, + $persistence_error + ); } - - self::persist_transcript( $transcript_persister, $messages, $options, $failure_result ); return $failure_result; } @@ -853,6 +866,11 @@ public static function mediate_tool_calls( $pre_tool_mediator, $mediation_context ); + $effect_result_index = null; + $effect_event_index = null; + $effect_audit_index = null; + $effect_message_index = null; + $effect_occurred = false; if ( 'reject' === $mediator_decision['action'] ) { $exec_result = $mediator_decision['result']; @@ -873,6 +891,60 @@ public static function mediate_tool_calls( $executor, $tool_context ); + $effect_occurred = true; + + // The executor returning is the audit commit point. Record its raw + // completed result before stores, hooks, truncators, or policies run. + $effect_result_index = count( $tool_execution_results ); + $tool_execution_results[] = self::tool_execution_result( + $tool_name, + $tool_call_id, + $exec_result, + $parameter_exposure, + $turn, + true + ); + $effect_event_index = count( $tool_events ); + $tool_events[] = self::tool_event( + 'tool_result', + $tool_name, + $tool_call_id, + $turn, + array( + 'status' => ! empty( $exec_result['success'] ) ? 'success' : 'error', + 'success' => (bool) ( $exec_result['success'] ?? false ), + 'effect_occurred' => true, + ) + ); + $effect_audit_index = count( $tool_audit_events ); + $tool_audit_events[] = array_merge( + self::tool_audit_event( $tool_name, $tool_call_id, $parameters_for_policy, $exec_result, $tool_definition, $turn_context, $turn ), + array( 'effect_occurred' => true ) + ); + $checkpoint = self::mediation_checkpoint( + $messages, + $tool_execution_results, + $tool_events, + $tool_audit_events, + $events + ); + $raw_result_content = ! empty( $exec_result['success'] ) + ? self::json_encode_safe( $exec_result['result'] ?? array() ) + : ( $exec_result['error'] ?? 'Tool execution failed.' ); + $effect_message_index = count( $messages ); + $messages[] = WP_Agent_Message::toolResult( + is_string( $raw_result_content ) ? $raw_result_content : '', + $tool_name, + $exec_result, + array( 'tool_call_id' => $tool_call_id ) + ); + $checkpoint = self::mediation_checkpoint( + $messages, + $tool_execution_results, + $tool_events, + $tool_audit_events, + $events + ); } $pending_request = self::runtime_tool_pending_request( $exec_result, $tool_name, $tool_call_id, $parameters_for_policy, $turn_context ); @@ -895,7 +967,7 @@ public static function mediate_tool_calls( } $pending_request_json = self::json_encode_safe( $pending_request ); $runtime_tool_pending = $pending_request; - $messages[] = WP_Agent_Message::toolResult( + $pending_message = WP_Agent_Message::toolResult( false !== $pending_request_json ? $pending_request_json : '', $tool_name, array( @@ -905,6 +977,11 @@ public static function mediate_tool_calls( ), array( 'tool_call_id' => $tool_call_id ) ); + if ( null !== $effect_message_index ) { + $messages[ $effect_message_index ] = $pending_message; + } else { + $messages[] = $pending_message; + } self::emit_event( $on_event, WP_Agent_Runtime_Tool_Request::STATUS_PENDING, array( 'turn' => $turn, @@ -912,7 +989,7 @@ public static function mediate_tool_calls( 'tool_call_id' => $tool_call_id, 'request_id' => $pending_request['request_id'], ) ); - $tool_events[] = self::tool_event( + $pending_event = self::tool_event( WP_Agent_Runtime_Tool_Request::STATUS_PENDING, $tool_name, $tool_call_id, @@ -922,6 +999,22 @@ public static function mediate_tool_calls( 'request_id' => $pending_request['request_id'], ) ); + if ( null !== $effect_event_index ) { + $tool_events[ $effect_event_index ] = $pending_event; + } else { + $tool_events[] = $pending_event; + } + if ( null !== $effect_result_index ) { + $exec_result['runtime_tool_request'] = $pending_request; + $tool_execution_results[ $effect_result_index ]['result'] = $exec_result; + } + if ( null !== $effect_audit_index ) { + $tool_audit_events[ $effect_audit_index ] = array_merge( + self::tool_audit_event( $tool_name, $tool_call_id, $parameters_for_policy, $exec_result, $tool_definition, $turn_context, $turn ), + array( 'effect_occurred' => true ) + ); + } + $checkpoint = self::mediation_checkpoint( $messages, $tool_execution_results, $tool_events, $tool_audit_events, $events ); $complete = true; break; } @@ -975,33 +1068,26 @@ public static function mediate_tool_calls( 'tool_call_id' => $tool_call_id, 'success' => (bool) ( $exec_result['success'] ?? false ), ) ); - $tool_events[] = self::tool_event( + $normalized_tool_event = self::tool_event( 'tool_result', $tool_name, $tool_call_id, $turn, array( - 'status' => ! empty( $exec_result['success'] ) ? 'success' : 'error', - 'success' => (bool) ( $exec_result['success'] ?? false ), - 'rejected' => 'reject' === $mediator_decision['action'], + 'status' => ! empty( $exec_result['success'] ) ? 'success' : 'error', + 'success' => (bool) ( $exec_result['success'] ?? false ), + 'rejected' => 'reject' === $mediator_decision['action'], + 'effect_occurred' => $effect_occurred, ) ); + if ( null !== $effect_event_index ) { + $tool_events[ $effect_event_index ] = $normalized_tool_event; + } else { + $tool_events[] = $normalized_tool_event; + } // Build the tool_execution_results entry. - $execution_result = array( - 'tool_name' => $tool_name, - 'tool_call_id' => $tool_call_id, - 'result' => $exec_result, - 'parameters' => $parameter_exposure['parameters'], - 'parameters_sha256' => $parameter_exposure['parameters_sha256'], - 'parameters_redacted' => true, - 'turn_count' => $turn, - ); - - $runtime = isset( $exec_result['runtime'] ) && is_array( $exec_result['runtime'] ) ? $exec_result['runtime'] : array(); - if ( ! empty( $runtime ) ) { - $execution_result['runtime'] = $runtime; - } + $execution_result = self::tool_execution_result( $tool_name, $tool_call_id, $exec_result, $parameter_exposure, $turn, $effect_occurred ); $diagnostics = self::post_tool_result_diagnostics( $post_tool_diagnostics, @@ -1033,9 +1119,13 @@ public static function mediate_tool_calls( self::emit_event( $on_event, 'tool_result_diagnostics', $diagnostics_metadata ); } - $tool_execution_results[] = $execution_result; + if ( null !== $effect_result_index ) { + $tool_execution_results[ $effect_result_index ] = $execution_result; + } else { + $tool_execution_results[] = $execution_result; + } - $tool_audit_events[] = self::tool_audit_event( + $audit_event = self::tool_audit_event( $tool_name, $tool_call_id, $parameters_for_policy, @@ -1044,18 +1134,31 @@ public static function mediate_tool_calls( $turn_context, $turn ); + if ( $effect_occurred ) { + $audit_event['effect_occurred'] = true; + } + if ( null !== $effect_audit_index ) { + $tool_audit_events[ $effect_audit_index ] = $audit_event; + } else { + $tool_audit_events[] = $audit_event; + } // Add tool-result message to transcript. $result_content = ( $exec_result['success'] ?? false ) ? self::json_encode_safe( $exec_result['result'] ?? array() ) : ( $exec_result['error'] ?? 'Tool execution failed.' ); - $messages[] = WP_Agent_Message::toolResult( + $tool_result_message = WP_Agent_Message::toolResult( is_string( $result_content ) ? $result_content : '', $tool_name, $exec_result, array( 'tool_call_id' => $tool_call_id ) ); + if ( null !== $effect_message_index ) { + $messages[ $effect_message_index ] = $tool_result_message; + } else { + $messages[] = $tool_result_message; + } $checkpoint = self::mediation_checkpoint( $messages, $tool_execution_results, @@ -1207,6 +1310,35 @@ private static function mediation_checkpoint( array $messages, array $tool_resul ); } + /** + * Build one canonical mediated tool execution result. + * + * @param array $result Tool execution result. + * @param array{parameters:array,parameters_sha256:string,parameters_redacted:bool} $parameter_exposure Safe parameter envelope. + * @return array + */ + private static function tool_execution_result( string $tool_name, string $tool_call_id, array $result, array $parameter_exposure, int $turn, bool $effect_occurred ): array { + $execution_result = array( + 'tool_name' => $tool_name, + 'tool_call_id' => $tool_call_id, + 'result' => $result, + 'parameters' => $parameter_exposure['parameters'], + 'parameters_sha256' => $parameter_exposure['parameters_sha256'], + 'parameters_redacted' => true, + 'turn_count' => $turn, + ); + if ( $effect_occurred ) { + $execution_result['effect_occurred'] = true; + } + + $runtime = isset( $result['runtime'] ) && is_array( $result['runtime'] ) ? $result['runtime'] : array(); + if ( ! empty( $runtime ) ) { + $execution_result['runtime'] = $runtime; + } + + return $execution_result; + } + /** * Fail closed: finalize a run that threw and return the structured failure result. * @@ -1269,10 +1401,25 @@ private static function finalize_loop_failure( $request_metadata ); - self::persist_transcript( $transcript_persister, $messages, $options, $failure_result ); + $persistence_error = self::persist_transcript( $transcript_persister, $messages, $options, $failure_result ); - if ( '' !== $run_id && '' !== $lock_session_id ) { - self::finish_run_or_throw( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); + try { + if ( '' !== $run_id && '' !== $lock_session_id ) { + self::finish_run_or_throw( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); + } + } catch ( WP_Agent_Run_Control_Store_Exception $finalization_error ) { + if ( null !== $persistence_error ) { + throw new WP_Agent_Run_Control_Store_Exception( $finalization_error->getMessage(), 0, $persistence_error ); + } + throw $finalization_error; + } + + if ( null !== $persistence_error ) { + throw new WP_Agent_Run_Control_Store_Exception( + 'Transcript persistence failed while finalizing the failed conversation run.', + 0, + $persistence_error + ); } return $failure_result; @@ -1286,6 +1433,20 @@ private static function finish_run_or_throw( string $run_id, string $status, ?\A if ( is_wp_error( $finished ) ) { throw new WP_Agent_Run_Control_Store_Exception( $finished->get_error_message() ); } + if ( ! is_array( $finished ) ) { + throw new WP_Agent_Run_Control_Store_Exception( 'Run-control finalization did not return a stored run.' ); + } + + try { + $finished = WP_Agent_Chat_Run_Control::normalize_run( $finished ); + } catch ( \Throwable $error ) { + throw new WP_Agent_Run_Control_Store_Exception( 'Run-control finalization returned a malformed stored run.', 0, $error ); + } + + $expected_status = WP_Agent_Chat_Run_Control::normalize_status( $status ); + if ( $run_id !== $finished['run_id'] || $expected_status !== $finished['status'] ) { + throw new WP_Agent_Run_Control_Store_Exception( 'Run-control finalization did not persist the requested terminal result.' ); + } } /** @@ -1910,24 +2071,25 @@ private static function resolve_tool_call_id( array $raw_call, int $turn, int $s * @param array> $messages Final messages. * @param array $options Loop options. * @param array $result Loop result. + * @return \Throwable|null Persistence failure for the finalizer to surface. */ private static function persist_transcript( ?WP_Agent_Transcript_Persister $persister, array $messages, array $options, array $result - ): void { + ): ?\Throwable { if ( null === $persister ) { - return; + return null; } $request = self::resolve_request( $messages, $options ); try { $persister->persist( $messages, $request, $result ); + return null; } catch ( \Throwable $error ) { - // Persister failures must not change loop results. - unset( $error ); + return $error; } } diff --git a/tests/conversation-loop-fail-closed-smoke.php b/tests/conversation-loop-fail-closed-smoke.php index 6a5cc67..c4094ea 100644 --- a/tests/conversation-loop-fail-closed-smoke.php +++ b/tests/conversation-loop-fail-closed-smoke.php @@ -217,8 +217,10 @@ public function timeout( string $request_id ): void { unset( $request_id ); } public function recent_pending( array $query = array() ): array { unset( $query ); return array(); } }; $pending_executor = new class() implements AgentsAPI\AI\Tools\WP_Agent_Tool_Executor { + public int $calls = 0; public function executeWP_Agent_Tool_Call( array $tool_call, array $tool_definition, array $context = array() ): array { unset( $tool_definition, $context ); + ++$this->calls; return array( 'success' => false, 'tool_name' => $tool_call['tool_name'], @@ -243,6 +245,8 @@ public function executeWP_Agent_Tool_Call( array $tool_call, array $tool_definit agents_api_smoke_assert_equals( 'failed', $runtime_store_result['status'] ?? '', 'runtime-tool storage throw returns a failed result', $failures, $passes ); agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-runtime-store' )['status'] ?? '', 'runtime-tool storage throw durably finalizes run control', $failures, $passes ); agents_api_smoke_assert_equals( 1, count( $persist_log ), 'runtime-tool storage throw persists the failed transcript', $failures, $passes ); +agents_api_smoke_assert_equals( 1, $pending_executor->calls, 'runtime-tool storage throw does not repeat the effect', $failures, $passes ); +agents_api_smoke_assert_equals( true, $runtime_store_result['tool_audit_events'][0]['effect_occurred'] ?? false, 'runtime-tool storage failure audit records the completed effect', $failures, $passes ); echo "\n[5] Completed tool effects remain in the failed audit when later policy throws:\n"; $policy_executor->calls = 0; @@ -271,7 +275,59 @@ public function recordToolResult( string $tool_name, ?array $tool_def, array $to agents_api_smoke_assert_equals( 1, count( $post_tool_result['tool_audit_events'] ?? array() ), 'failed result retains completed tool audit event', $failures, $passes ); agents_api_smoke_assert_equals( 1, count( $persist_log[0]['result']['tool_execution_results'] ?? array() ), 'persisted transcript retains completed tool effect', $failures, $passes ); -echo "\n[6] The turn-runner contract violation finalizes run control and still escapes:\n"; +echo "\n[6] Hook and truncator throws retain the immediate effect checkpoint:\n"; +$hook_store = new class() implements AgentsAPI\AI\WP_Agent_Runtime_Tool_Request_Store { + public function create( array $request ): void { unset( $request ); } + public function get( string $request_id ): ?array { unset( $request_id ); return null; } + public function complete( string $request_id, array $result ): void { unset( $request_id, $result ); } + public function timeout( string $request_id ): void { unset( $request_id ); } + public function recent_pending( array $query = array() ): array { unset( $query ); return array(); } +}; +add_action( + 'agents_api_runtime_tool_request_created', + static function (): void { + throw new RuntimeException( 'runtime lifecycle hook unavailable' ); + } +); +$pending_executor->calls = 0; +$hook_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'hook failure' ) ), + $tool_turn, + array( + 'run_id' => 'fail-closed-post-tool-hook', + 'transcript_session_id' => 'fail-closed-post-tool-hook-session', + 'tool_executor' => $pending_executor, + 'tool_declarations' => $policy_tools, + 'runtime_tool_request_store' => $hook_store, + ) +); +agents_api_smoke_assert_equals( 1, $pending_executor->calls, 'post-effect hook throw executes the tool exactly once', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $hook_result['status'] ?? '', 'post-effect hook throw fails the run', $failures, $passes ); +agents_api_smoke_assert_equals( true, $hook_result['tool_audit_events'][0]['effect_occurred'] ?? false, 'post-effect hook failure audit records the completed effect', $failures, $passes ); + +$throwing_truncator = new class() implements AgentsAPI\AI\WP_Agent_Tool_Result_Truncator { + public function truncate_result( array $result, string $tool_name, array $context = array() ): array { + unset( $result, $tool_name, $context ); + throw new RuntimeException( 'truncator unavailable' ); + } +}; +$policy_executor->calls = 0; +$truncator_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'truncator failure' ) ), + $tool_turn, + array( + 'run_id' => 'fail-closed-post-tool-truncator', + 'transcript_session_id' => 'fail-closed-post-tool-truncator-session', + 'tool_executor' => $policy_executor, + 'tool_declarations' => $policy_tools, + 'tool_result_truncator' => $throwing_truncator, + ) +); +agents_api_smoke_assert_equals( 1, $policy_executor->calls, 'truncator throw executes the tool exactly once', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $truncator_result['status'] ?? '', 'truncator throw fails the run', $failures, $passes ); +agents_api_smoke_assert_equals( true, $truncator_result['tool_audit_events'][0]['effect_occurred'] ?? false, 'truncator failure audit records the completed effect', $failures, $passes ); + +echo "\n[7] The turn-runner contract violation finalizes run control and still escapes:\n"; $threw = false; $persist_log = array(); try { @@ -293,7 +349,7 @@ static function (): string { agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-contract' )['status'] ?? '', 'contract violation does not strand running run control', $failures, $passes ); agents_api_smoke_assert_equals( 'failed', $persist_log[0]['status'] ?? '', 'contract violation persists a canonical failed result', $failures, $passes ); -echo "\n[7] Run-control finalization storage failures remain retryable and visible:\n"; +echo "\n[8] Run-control finalization storage failures remain retryable and visible:\n"; if ( ! class_exists( 'WP_Error' ) ) { class WP_Error { public function __construct( private string $code = '', private string $message = '' ) {} @@ -334,4 +390,64 @@ public function mutate_state( string $store_key, callable $mutation ): mixed { agents_api_smoke_assert_equals( true, $finalization_error instanceof AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception, 'terminal storage failure escapes as the canonical retryable exception', $failures, $passes ); agents_api_smoke_assert_equals( 'running', $failing_store->state['runs']['fail-closed-finalization']['status'] ?? '', 'failed terminal write remains visibly retryable instead of pretending completion', $failures, $passes ); +echo "\n[9] Missing finalization records are rejected as retryable failures:\n"; +$missing_store = new class() implements AgentsAPI\AI\WP_Agent_Atomic_Run_Control_Store { + public array $state = array( 'runs' => array(), 'queues' => array(), 'events' => array() ); + private int $mutations = 0; + public function get_state( string $store_key ): array { unset( $store_key ); return $this->state; } + public function save_state( string $store_key, array $state ): void { unset( $store_key ); $this->state = $state; } + public function mutate_state( string $store_key, callable $mutation ): mixed { + unset( $store_key ); + ++$this->mutations; + $mutated = $mutation( 1 < $this->mutations ? array( 'runs' => array(), 'queues' => array(), 'events' => array() ) : $this->state ); + if ( 1 === $this->mutations ) { + $this->state = $mutated['state']; + } + return $mutated['result']; + } +}; +AgentsAPI\AI\WP_Agent_Run_Control::set_store( $missing_store ); +$missing_error = null; +try { + AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'missing terminal record' ) ), + $turn_runner, + array( 'run_id' => 'fail-closed-missing-record', 'transcript_session_id' => 'fail-closed-missing-record-session' ) + ); +} catch ( AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception $error ) { + $missing_error = $error; +} +agents_api_smoke_assert_equals( true, $missing_error instanceof AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception, 'missing terminal record is a retryable finalization failure', $failures, $passes ); +agents_api_smoke_assert_equals( 'running', $missing_store->state['runs']['fail-closed-missing-record']['status'] ?? '', 'missing finalization does not claim a terminal status', $failures, $passes ); + +echo "\n[10] Failed transcript persistence is explicit after durable run finalization:\n"; +AgentsAPI\AI\WP_Agent_Run_Control::set_store( new Agents_API_Memory_Atomic_Run_Control_Store() ); +$throwing_persister = new class() implements AgentsAPI\AI\WP_Agent_Transcript_Persister { + public function persist( array $messages, AgentsAPI\AI\WP_Agent_Conversation_Request $request, array $result ): string { + unset( $messages, $request, $result ); + throw new RuntimeException( 'transcript store unavailable' ); + } +}; +$persistence_error = null; +$persistence_result = null; +try { + $persistence_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'persist failure audit' ) ), + $turn_runner, + array( + 'run_id' => 'fail-closed-transcript-store', + 'transcript_session_id' => 'fail-closed-transcript-store-session', + 'max_turns' => 2, + 'should_continue' => static function (): bool { throw new RuntimeException( 'continuation failed' ); }, + 'transcript_persister' => $throwing_persister, + ) + ); +} catch ( AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception $error ) { + $persistence_error = $error; +} +agents_api_smoke_assert_equals( null, $persistence_result, 'transcript storage failure never returns an ordinary failed result', $failures, $passes ); +agents_api_smoke_assert_equals( true, $persistence_error instanceof AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception, 'transcript storage failure is exposed as retryable finalization diagnostics', $failures, $passes ); +agents_api_smoke_assert_equals( 'transcript store unavailable', $persistence_error?->getPrevious()?->getMessage(), 'retryable diagnostics preserve the underlying persistence error', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-transcript-store' )['status'] ?? '', 'transcript storage failure still leaves an observable durable failed run', $failures, $passes ); + agents_api_smoke_finish( 'Agents API conversation loop fail-closed', $failures, $passes ); diff --git a/tests/provider-turn-adapter-smoke.php b/tests/provider-turn-adapter-smoke.php index c5ecf7e..3a16ef9 100644 --- a/tests/provider-turn-adapter-smoke.php +++ b/tests/provider-turn-adapter-smoke.php @@ -18,6 +18,8 @@ require_once __DIR__ . '/agents-api-smoke-helpers.php'; agents_api_smoke_require_module(); +require_once __DIR__ . '/class-agents-api-memory-atomic-run-control-store.php'; +AgentsAPI\AI\WP_Agent_Run_Control::set_store( new Agents_API_Memory_Atomic_Run_Control_Store() ); $tools = array( 'client/lookup' => array( From 9d02311fddfe450d4bc0a8d2b1c00600a467252e Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 22:28:11 +0000 Subject: [PATCH 4/7] fix: separate effect receipts from tool responses --- .../class-wp-agent-conversation-loop.php | 91 ++++--------------- tests/conversation-loop-fail-closed-smoke.php | 10 +- tests/pre-execute-approval-smoke.php | 6 ++ 3 files changed, 30 insertions(+), 77 deletions(-) diff --git a/src/Runtime/class-wp-agent-conversation-loop.php b/src/Runtime/class-wp-agent-conversation-loop.php index af781af..444df0f 100644 --- a/src/Runtime/class-wp-agent-conversation-loop.php +++ b/src/Runtime/class-wp-agent-conversation-loop.php @@ -866,11 +866,8 @@ public static function mediate_tool_calls( $pre_tool_mediator, $mediation_context ); - $effect_result_index = null; - $effect_event_index = null; - $effect_audit_index = null; - $effect_message_index = null; - $effect_occurred = false; + $effect_audit_index = null; + $effect_occurred = false; if ( 'reject' === $mediator_decision['action'] ) { $exec_result = $mediator_decision['result']; @@ -893,51 +890,18 @@ public static function mediate_tool_calls( ); $effect_occurred = true; - // The executor returning is the audit commit point. Record its raw - // completed result before stores, hooks, truncators, or policies run. - $effect_result_index = count( $tool_execution_results ); - $tool_execution_results[] = self::tool_execution_result( - $tool_name, - $tool_call_id, - $exec_result, - $parameter_exposure, - $turn, - true - ); - $effect_event_index = count( $tool_events ); - $tool_events[] = self::tool_event( - 'tool_result', - $tool_name, - $tool_call_id, - $turn, + // The executor returning is the audit commit point. Record a safe, + // non-provider-facing receipt before stores, hooks, truncators, or + // policies run. Mediation will append the single outward response. + $effect_audit_index = count( $tool_audit_events ); + $effect_receipt = array_merge( + self::tool_audit_event( $tool_name, $tool_call_id, $parameters_for_policy, $exec_result, $tool_definition, $turn_context, $turn ), array( - 'status' => ! empty( $exec_result['success'] ) ? 'success' : 'error', - 'success' => (bool) ( $exec_result['success'] ?? false ), + 'type' => 'tool_effect_completed', 'effect_occurred' => true, ) ); - $effect_audit_index = count( $tool_audit_events ); - $tool_audit_events[] = array_merge( - self::tool_audit_event( $tool_name, $tool_call_id, $parameters_for_policy, $exec_result, $tool_definition, $turn_context, $turn ), - array( 'effect_occurred' => true ) - ); - $checkpoint = self::mediation_checkpoint( - $messages, - $tool_execution_results, - $tool_events, - $tool_audit_events, - $events - ); - $raw_result_content = ! empty( $exec_result['success'] ) - ? self::json_encode_safe( $exec_result['result'] ?? array() ) - : ( $exec_result['error'] ?? 'Tool execution failed.' ); - $effect_message_index = count( $messages ); - $messages[] = WP_Agent_Message::toolResult( - is_string( $raw_result_content ) ? $raw_result_content : '', - $tool_name, - $exec_result, - array( 'tool_call_id' => $tool_call_id ) - ); + $tool_audit_events[] = $effect_receipt; $checkpoint = self::mediation_checkpoint( $messages, $tool_execution_results, @@ -977,11 +941,7 @@ public static function mediate_tool_calls( ), array( 'tool_call_id' => $tool_call_id ) ); - if ( null !== $effect_message_index ) { - $messages[ $effect_message_index ] = $pending_message; - } else { - $messages[] = $pending_message; - } + $messages[] = $pending_message; self::emit_event( $on_event, WP_Agent_Runtime_Tool_Request::STATUS_PENDING, array( 'turn' => $turn, @@ -999,16 +959,9 @@ public static function mediate_tool_calls( 'request_id' => $pending_request['request_id'], ) ); - if ( null !== $effect_event_index ) { - $tool_events[ $effect_event_index ] = $pending_event; - } else { - $tool_events[] = $pending_event; - } - if ( null !== $effect_result_index ) { - $exec_result['runtime_tool_request'] = $pending_request; - $tool_execution_results[ $effect_result_index ]['result'] = $exec_result; - } + $tool_events[] = $pending_event; if ( null !== $effect_audit_index ) { + $exec_result['runtime_tool_request'] = $pending_request; $tool_audit_events[ $effect_audit_index ] = array_merge( self::tool_audit_event( $tool_name, $tool_call_id, $parameters_for_policy, $exec_result, $tool_definition, $turn_context, $turn ), array( 'effect_occurred' => true ) @@ -1080,11 +1033,7 @@ public static function mediate_tool_calls( 'effect_occurred' => $effect_occurred, ) ); - if ( null !== $effect_event_index ) { - $tool_events[ $effect_event_index ] = $normalized_tool_event; - } else { - $tool_events[] = $normalized_tool_event; - } + $tool_events[] = $normalized_tool_event; // Build the tool_execution_results entry. $execution_result = self::tool_execution_result( $tool_name, $tool_call_id, $exec_result, $parameter_exposure, $turn, $effect_occurred ); @@ -1119,11 +1068,7 @@ public static function mediate_tool_calls( self::emit_event( $on_event, 'tool_result_diagnostics', $diagnostics_metadata ); } - if ( null !== $effect_result_index ) { - $tool_execution_results[ $effect_result_index ] = $execution_result; - } else { - $tool_execution_results[] = $execution_result; - } + $tool_execution_results[] = $execution_result; $audit_event = self::tool_audit_event( $tool_name, @@ -1154,11 +1099,7 @@ public static function mediate_tool_calls( $exec_result, array( 'tool_call_id' => $tool_call_id ) ); - if ( null !== $effect_message_index ) { - $messages[ $effect_message_index ] = $tool_result_message; - } else { - $messages[] = $tool_result_message; - } + $messages[] = $tool_result_message; $checkpoint = self::mediation_checkpoint( $messages, $tool_execution_results, diff --git a/tests/conversation-loop-fail-closed-smoke.php b/tests/conversation-loop-fail-closed-smoke.php index c4094ea..de71b76 100644 --- a/tests/conversation-loop-fail-closed-smoke.php +++ b/tests/conversation-loop-fail-closed-smoke.php @@ -247,6 +247,8 @@ public function executeWP_Agent_Tool_Call( array $tool_call, array $tool_definit agents_api_smoke_assert_equals( 1, count( $persist_log ), 'runtime-tool storage throw persists the failed transcript', $failures, $passes ); agents_api_smoke_assert_equals( 1, $pending_executor->calls, 'runtime-tool storage throw does not repeat the effect', $failures, $passes ); agents_api_smoke_assert_equals( true, $runtime_store_result['tool_audit_events'][0]['effect_occurred'] ?? false, 'runtime-tool storage failure audit records the completed effect', $failures, $passes ); +agents_api_smoke_assert_equals( 'tool_effect_completed', $runtime_store_result['tool_audit_events'][0]['type'] ?? '', 'runtime-tool storage failure retains the explicit effect receipt', $failures, $passes ); +agents_api_smoke_assert_equals( 0, count( $runtime_store_result['tool_execution_results'] ?? array() ), 'runtime-tool storage failure does not invent an outward response', $failures, $passes ); echo "\n[5] Completed tool effects remain in the failed audit when later policy throws:\n"; $policy_executor->calls = 0; @@ -271,9 +273,11 @@ public function recordToolResult( string $tool_name, ?array $tool_def, array $to ); agents_api_smoke_assert_equals( 1, $policy_executor->calls, 'side-effecting tool executes exactly once', $failures, $passes ); agents_api_smoke_assert_equals( 'failed', $post_tool_result['status'] ?? '', 'later completion-policy throw produces a terminal failure', $failures, $passes ); -agents_api_smoke_assert_equals( 1, count( $post_tool_result['tool_execution_results'] ?? array() ), 'failed result retains completed tool execution', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $post_tool_result['tool_execution_results'] ?? array() ), 'failed result retains the single normalized mediation response', $failures, $passes ); agents_api_smoke_assert_equals( 1, count( $post_tool_result['tool_audit_events'] ?? array() ), 'failed result retains completed tool audit event', $failures, $passes ); -agents_api_smoke_assert_equals( 1, count( $persist_log[0]['result']['tool_execution_results'] ?? array() ), 'persisted transcript retains completed tool effect', $failures, $passes ); +agents_api_smoke_assert_equals( 'tool_call', $post_tool_result['tool_audit_events'][0]['type'] ?? '', 'completed mediation refines the effect receipt into the canonical audit event', $failures, $passes ); +agents_api_smoke_assert_equals( true, str_starts_with( $post_tool_result['tool_audit_events'][0]['result_sha256'] ?? '', 'sha256:' ), 'effect receipt safely hashes the raw diagnostic', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( $persist_log[0]['result']['tool_audit_events'] ?? array() ), 'persisted transcript retains completed tool effect receipt', $failures, $passes ); echo "\n[6] Hook and truncator throws retain the immediate effect checkpoint:\n"; $hook_store = new class() implements AgentsAPI\AI\WP_Agent_Runtime_Tool_Request_Store { @@ -304,6 +308,7 @@ static function (): void { agents_api_smoke_assert_equals( 1, $pending_executor->calls, 'post-effect hook throw executes the tool exactly once', $failures, $passes ); agents_api_smoke_assert_equals( 'failed', $hook_result['status'] ?? '', 'post-effect hook throw fails the run', $failures, $passes ); agents_api_smoke_assert_equals( true, $hook_result['tool_audit_events'][0]['effect_occurred'] ?? false, 'post-effect hook failure audit records the completed effect', $failures, $passes ); +agents_api_smoke_assert_equals( 0, count( $hook_result['tool_execution_results'] ?? array() ), 'post-effect hook failure retains no unresolved outward response', $failures, $passes ); $throwing_truncator = new class() implements AgentsAPI\AI\WP_Agent_Tool_Result_Truncator { public function truncate_result( array $result, string $tool_name, array $context = array() ): array { @@ -326,6 +331,7 @@ public function truncate_result( array $result, string $tool_name, array $contex agents_api_smoke_assert_equals( 1, $policy_executor->calls, 'truncator throw executes the tool exactly once', $failures, $passes ); agents_api_smoke_assert_equals( 'failed', $truncator_result['status'] ?? '', 'truncator throw fails the run', $failures, $passes ); agents_api_smoke_assert_equals( true, $truncator_result['tool_audit_events'][0]['effect_occurred'] ?? false, 'truncator failure audit records the completed effect', $failures, $passes ); +agents_api_smoke_assert_equals( 0, count( $truncator_result['tool_execution_results'] ?? array() ), 'truncator failure retains no unresolved outward response', $failures, $passes ); echo "\n[7] The turn-runner contract violation finalizes run control and still escapes:\n"; $threw = false; diff --git a/tests/pre-execute-approval-smoke.php b/tests/pre-execute-approval-smoke.php index 2bd75fb..72973c0 100644 --- a/tests/pre-execute-approval-smoke.php +++ b/tests/pre-execute-approval-smoke.php @@ -274,5 +274,11 @@ static function ( array $messages ): array { agents_api_smoke_assert_equals( false, (bool) ( $loop_result['completed'] ?? true ), 'loop result is not marked completed', $failures, $passes ); agents_api_smoke_assert_equals( WP_Agent_Message::TYPE_APPROVAL_REQUIRED, $loop_result['approval_required']['type'] ?? '', 'loop carries the approval envelope through', $failures, $passes ); agents_api_smoke_assert_equals( 'pa-smoke-001', $loop_result['approval_required']['payload']['action_id'] ?? '', 'approval envelope action_id preserved', $failures, $passes ); +$approval_messages = array_values( array_filter( $loop_result['messages'] ?? array(), static fn( array $message ): bool => WP_Agent_Message::TYPE_APPROVAL_REQUIRED === ( $message['type'] ?? '' ) ) ); +$tool_result_messages = array_values( array_filter( $loop_result['messages'] ?? array(), static fn( array $message ): bool => WP_Agent_Message::TYPE_TOOL_RESULT === ( $message['type'] ?? '' ) ) ); +agents_api_smoke_assert_equals( 1, count( $approval_messages ), 'approval path persists exactly one approval response', $failures, $passes ); +agents_api_smoke_assert_equals( 0, count( $tool_result_messages ), 'approval path does not persist a provisional raw tool response', $failures, $passes ); +agents_api_smoke_assert_equals( 0, count( $loop_result['tool_execution_results'] ?? array() ), 'approval path exposes no misleading raw execution response', $failures, $passes ); +agents_api_smoke_assert_equals( 'tool_effect_completed', $loop_result['tool_audit_events'][0]['type'] ?? '', 'approval path retains the non-provider-facing effect receipt', $failures, $passes ); agents_api_smoke_finish( 'pre-execute approval smoke', $failures, $passes ); From 7b316e9b612d1445eb64cd122d52eab9cb4ae2db Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 22:39:24 +0000 Subject: [PATCH 5/7] fix: require durable conversation completion --- .../class-wp-agent-conversation-loop.php | 46 ++++++++++++----- tests/conversation-loop-fail-closed-smoke.php | 51 +++++++++++++++++++ ...sation-loop-transcript-persister-smoke.php | 43 +++++++++------- 3 files changed, 108 insertions(+), 32 deletions(-) diff --git a/src/Runtime/class-wp-agent-conversation-loop.php b/src/Runtime/class-wp-agent-conversation-loop.php index 444df0f..bdf8c79 100644 --- a/src/Runtime/class-wp-agent-conversation-loop.php +++ b/src/Runtime/class-wp-agent-conversation-loop.php @@ -143,18 +143,6 @@ public static function run( array $messages, ?callable $turn_runner = null, arra $mediation_enabled = null !== $tool_executor && ! empty( $tool_declarations ); self::emit_tool_declaration_diagnostics( $on_event, $rejected_declarations, $tool_declarations, $tool_executor ); $messages = self::normalize_messages( $messages ); - if ( '' !== $run_id && '' !== $lock_session_id ) { - $conversation_store = ( $context['conversation_store'] ?? null ) instanceof \AgentsAPI\Core\Database\Chat\WP_Agent_Conversation_Store ? $context['conversation_store'] : null; - $run_metadata = array( 'source' => 'conversation_loop', '_claim_token' => WP_Agent_Run_Control::string_value( $context['_agents_run_claim_token'] ?? '' ) ); - if ( $principal instanceof WP_Agent_Execution_Principal ) { - $run_metadata['principal'] = $principal->to_safe_metadata(); - } - $started = WP_Agent_Chat_Run_Control::start_run( $run_id, $lock_session_id, $run_metadata, $run_workspace, $run_owner, $conversation_store ); - if ( is_wp_error( $started ) ) { - self::emit_event( $on_event, 'failed', array( 'error' => $started->get_error_message() ) ); - return self::run_control_failure_result( $messages, $started ); - } - } $turn_runner = self::resolve_turn_runner( $turn_runner, $options, $tool_declarations, $run_id, $lock_session_id, $request, $budgets ); $events = array(); $tool_results = array(); @@ -208,6 +196,19 @@ public static function run( array $messages, ?callable $turn_runner = null, arra $loop_contract_error = null; try { + if ( '' !== $run_id && '' !== $lock_session_id ) { + $conversation_store = ( $context['conversation_store'] ?? null ) instanceof \AgentsAPI\Core\Database\Chat\WP_Agent_Conversation_Store ? $context['conversation_store'] : null; + $run_metadata = array( 'source' => 'conversation_loop', '_claim_token' => WP_Agent_Run_Control::string_value( $context['_agents_run_claim_token'] ?? '' ) ); + if ( $principal instanceof WP_Agent_Execution_Principal ) { + $run_metadata['principal'] = $principal->to_safe_metadata(); + } + $started = WP_Agent_Chat_Run_Control::start_run( $run_id, $lock_session_id, $run_metadata, $run_workspace, $run_owner, $conversation_store ); + if ( is_wp_error( $started ) ) { + self::emit_event( $on_event, 'failed', array( 'error' => $started->get_error_message() ) ); + return self::run_control_failure_result( $messages, $started ); + } + } + for ( $turn = 1; $turn <= $max_turns; ++$turn ) { $wall_clock_exceeded = self::check_wall_clock_budget( $budgets, $wall_clock_started_at, $wall_clock_initial, $on_event ); if ( null !== $wall_clock_exceeded ) { @@ -595,13 +596,30 @@ public static function run( array $messages, ?callable $turn_runner = null, arra } $final_result = self::normalize_conversation_result( $final_result_data ); + $persistence_error = self::persist_transcript( $transcript_persister, $messages, $options, $final_result ); + if ( null !== $persistence_error ) { + self::emit_event( $on_event, 'failed', array( + 'turn' => $turns_run, + 'error' => $persistence_error->getMessage(), + ) ); + try { + if ( '' !== $run_id && '' !== $lock_session_id ) { + self::finish_run_or_throw( $run_id, WP_Agent_Chat_Run_Control::STATUS_FAILED, $run_workspace ); + } + } catch ( WP_Agent_Run_Control_Store_Exception $finalization_error ) { + throw new WP_Agent_Run_Control_Store_Exception( $finalization_error->getMessage(), 0, $persistence_error ); + } + throw new WP_Agent_Run_Control_Store_Exception( + 'Transcript persistence failed while finalizing the conversation run.', + 0, + $persistence_error + ); + } if ( '' !== $run_id && '' !== $lock_session_id ) { self::finish_run_or_throw( $run_id, WP_Agent_Run_Outcome::run_control_status( $final_result ), $run_workspace ); } - self::persist_transcript( $transcript_persister, $messages, $options, $final_result ); - self::emit_event( $on_event, 'completed', array( 'turn' => $turns_run, 'message_count' => count( $messages ), diff --git a/tests/conversation-loop-fail-closed-smoke.php b/tests/conversation-loop-fail-closed-smoke.php index de71b76..6739991 100644 --- a/tests/conversation-loop-fail-closed-smoke.php +++ b/tests/conversation-loop-fail-closed-smoke.php @@ -456,4 +456,55 @@ public function persist( array $messages, AgentsAPI\AI\WP_Agent_Conversation_Req agents_api_smoke_assert_equals( 'transcript store unavailable', $persistence_error?->getPrevious()?->getMessage(), 'retryable diagnostics preserve the underlying persistence error', $failures, $passes ); agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-transcript-store' )['status'] ?? '', 'transcript storage failure still leaves an observable durable failed run', $failures, $passes ); +echo "\n[11] Successful execution cannot publish completion when transcript persistence fails:\n"; +AgentsAPI\AI\WP_Agent_Run_Control::set_store( new Agents_API_Memory_Atomic_Run_Control_Store() ); +$success_events = array(); +$success_turns = 0; +$success_result = null; +$success_persistence_error = null; +try { + $success_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'complete once' ) ), + static function ( array $messages ) use ( &$success_turns ): array { + ++$success_turns; + $messages[] = AgentsAPI\AI\WP_Agent_Message::text( 'assistant', 'completed once' ); + return array( 'messages' => $messages, 'tool_execution_results' => array() ); + }, + array( + 'run_id' => 'fail-closed-success-transcript-store', + 'transcript_session_id' => 'fail-closed-success-transcript-store-session', + 'transcript_persister' => $throwing_persister, + 'on_event' => static function ( string $event ) use ( &$success_events ): void { $success_events[] = $event; }, + ) + ); +} catch ( AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception $error ) { + $success_persistence_error = $error; +} +agents_api_smoke_assert_equals( null, $success_result, 'success transcript failure returns no false completed result', $failures, $passes ); +agents_api_smoke_assert_equals( 1, $success_turns, 'success transcript failure does not repeat provider execution', $failures, $passes ); +agents_api_smoke_assert_equals( 'transcript store unavailable', $success_persistence_error?->getPrevious()?->getMessage(), 'success transcript failure preserves retryable storage diagnostics', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-success-transcript-store' )['status'] ?? '', 'success transcript failure durably records failed run control', $failures, $passes ); +agents_api_smoke_assert_equals( 0, count( array_filter( $success_events, static fn( string $event ): bool => 'completed' === $event ) ), 'success transcript failure emits no completed event', $failures, $passes ); +agents_api_smoke_assert_equals( 1, count( array_filter( $success_events, static fn( string $event ): bool => 'failed' === $event ) ), 'success transcript failure emits one failed event', $failures, $passes ); + +echo "\n[12] Transcript lock contention creates no running run-control record:\n"; +AgentsAPI\AI\WP_Agent_Run_Control::set_store( new Agents_API_Memory_Atomic_Run_Control_Store() ); +$contended_lock = new class() implements AgentsAPI\Core\Database\Chat\WP_Agent_Conversation_Lock { + public function acquire_session_lock( string $session_id, int $ttl_seconds = 300 ): ?string { unset( $session_id, $ttl_seconds ); return null; } + public function release_session_lock( string $session_id, string $lock_token ): bool { unset( $session_id, $lock_token ); return false; } +}; +$contention_turns = 0; +$contention_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'contended' ) ), + static function () use ( &$contention_turns ): array { ++$contention_turns; return array(); }, + array( + 'run_id' => 'fail-closed-lock-contention', + 'transcript_session_id' => 'fail-closed-lock-contention-session', + 'transcript_lock' => $contended_lock, + ) +); +agents_api_smoke_assert_equals( 'transcript_lock_contention', $contention_result['status'] ?? '', 'lock contention returns the canonical busy result', $failures, $passes ); +agents_api_smoke_assert_equals( 0, $contention_turns, 'lock contention starts no provider execution', $failures, $passes ); +agents_api_smoke_assert_equals( null, AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-lock-contention' ), 'lock contention creates no running run-control zombie', $failures, $passes ); + agents_api_smoke_finish( 'Agents API conversation loop fail-closed', $failures, $passes ); diff --git a/tests/conversation-loop-transcript-persister-smoke.php b/tests/conversation-loop-transcript-persister-smoke.php index 7911635..7a7e4ef 100644 --- a/tests/conversation-loop-transcript-persister-smoke.php +++ b/tests/conversation-loop-transcript-persister-smoke.php @@ -117,31 +117,38 @@ static function ( array $messages ): array { agents_api_smoke_assert_equals( 0, count( $persister_log ), 'persister was not called when not provided', $failures, $passes ); -echo "\n[4] Persister failure does not change loop result:\n"; +echo "\n[4] Persister failure prevents false successful completion:\n"; $crashing_persister = new class() implements AgentsAPI\AI\WP_Agent_Transcript_Persister { public function persist( array $messages, AgentsAPI\AI\WP_Agent_Conversation_Request $request, array $result ): string { throw new \RuntimeException( 'database down' ); } }; -$result4 = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( - array( array( 'role' => 'user', 'content' => 'hello' ) ), - static function ( array $messages ): array { - $messages[] = AgentsAPI\AI\WP_Agent_Message::text( 'assistant', 'ok' ); - - return array( - 'messages' => $messages, - 'tool_execution_results' => array(), - 'events' => array(), - ); - }, - array( - 'max_turns' => 1, - 'transcript_persister' => $crashing_persister, - ) -); +$result4 = null; +$result4_error = null; +try { + $result4 = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'hello' ) ), + static function ( array $messages ): array { + $messages[] = AgentsAPI\AI\WP_Agent_Message::text( 'assistant', 'ok' ); + + return array( + 'messages' => $messages, + 'tool_execution_results' => array(), + 'events' => array(), + ); + }, + array( + 'max_turns' => 1, + 'transcript_persister' => $crashing_persister, + ) + ); +} catch ( AgentsAPI\AI\WP_Agent_Run_Control_Store_Exception $error ) { + $result4_error = $error; +} -agents_api_smoke_assert_equals( 2, count( $result4['messages'] ), 'loop result is unaffected by persister failure', $failures, $passes ); +agents_api_smoke_assert_equals( null, $result4, 'persister failure returns no false successful result', $failures, $passes ); +agents_api_smoke_assert_equals( 'database down', $result4_error?->getPrevious()?->getMessage(), 'persister failure preserves retryable storage diagnostics', $failures, $passes ); echo "\n[5] Persister receives the original request when provided:\n"; $persister_log = array(); From 836c2a46d787650fe73972a0301864893d4361a9 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Wed, 26 Aug 2026 22:50:06 +0000 Subject: [PATCH 6/7] fix: terminalize retryable conversation failures --- .../class-wp-agent-conversation-loop.php | 22 ++++++--- tests/agents-chat-ability-smoke.php | 29 ++++++++++++ tests/conversation-loop-fail-closed-smoke.php | 45 ++++++++++++++++++- ...sation-loop-transcript-persister-smoke.php | 4 +- 4 files changed, 92 insertions(+), 8 deletions(-) diff --git a/src/Runtime/class-wp-agent-conversation-loop.php b/src/Runtime/class-wp-agent-conversation-loop.php index bdf8c79..6cc22c2 100644 --- a/src/Runtime/class-wp-agent-conversation-loop.php +++ b/src/Runtime/class-wp-agent-conversation-loop.php @@ -184,7 +184,19 @@ public static function run( array $messages, ?callable $turn_runner = null, arra 'messages' => $messages, 'tool_execution_results' => array(), 'events' => array(), - 'status' => 'transcript_lock_contention', + 'status' => 'failed', + 'completed' => false, + 'failure' => array( + 'type' => 'transcript_lock_contention', + 'message' => 'Transcript session is busy with another active conversation run.', + 'code' => 'transcript_lock_contention', + 'turn_count' => 0, + 'retryable' => true, + 'diagnostics' => array( + 'reason' => 'transcript_lock_busy', + 'session_id' => $lock_session_id, + ), + ), ) ); } } @@ -1685,8 +1697,7 @@ private static function normalize_pre_tool_mediation_decision( $decision, array ) ) ); } catch ( \Throwable $error ) { - unset( $error ); - return $fallback; + throw new \InvalidArgumentException( 'invalid_agent_pre_tool_mediation_decision: pending decision contains an invalid runtime tool request', 0, $error ); } $metadata['status'] = WP_Agent_Runtime_Tool_Request::STATUS_PENDING; @@ -1702,14 +1713,13 @@ private static function normalize_pre_tool_mediation_decision( $decision, array } else { $raw_result = $decision['result'] ?? null; if ( ! is_array( $raw_result ) ) { - return $fallback; + throw new \InvalidArgumentException( 'invalid_agent_pre_tool_mediation_decision: replace_result decision must contain a result array' ); } $raw_result['tool_name'] = is_string( $raw_result['tool_name'] ?? null ) && '' !== $raw_result['tool_name'] ? $raw_result['tool_name'] : $tool_name; try { $result = WP_Agent_Tool_Result::normalize( $raw_result ); } catch ( \Throwable $error ) { - unset( $error ); - return $fallback; + throw new \InvalidArgumentException( 'invalid_agent_pre_tool_mediation_decision: replace_result decision contains an invalid tool result', 0, $error ); } } diff --git a/tests/agents-chat-ability-smoke.php b/tests/agents-chat-ability-smoke.php index 2870ee0..9e6a904 100644 --- a/tests/agents-chat-ability-smoke.php +++ b/tests/agents-chat-ability-smoke.php @@ -339,6 +339,35 @@ public function update_title( string $session_id, string $title ): bool { smoke_assert( true, $failed_claim instanceof WP_Error, 'run_claimed_runtime_error_propagates', $failures, $passes ); smoke_assert( AgentsAPI\AI\WP_Agent_Chat_Run_Control::STATUS_FAILED, $failed_run['status'] ?? null, 'run_claimed_runtime_error_terminalizes_pending_run', $failures, $passes ); +$claimed_lock = new class() implements AgentsAPI\Core\Database\Chat\WP_Agent_Conversation_Lock { + public function acquire_session_lock( string $session_id, int $ttl_seconds = 300 ): ?string { unset( $session_id, $ttl_seconds ); return null; } + public function release_session_lock( string $session_id, string $lock_token ): bool { unset( $session_id, $lock_token ); return false; } +}; +$claimed_lock_turns = 0; +$claimed_lock_result = agents_chat_run_claimed( + array( 'agent' => 'x', 'message' => 'locked', 'run_id' => 'claimed-lock-contention-1' ), + static function ( array $claimed_input ) use ( $claimed_lock, &$claimed_lock_turns ): array { + return AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'locked' ) ), + static function () use ( &$claimed_lock_turns ): array { ++$claimed_lock_turns; return array(); }, + array( + 'run_id' => $claimed_input['run_id'], + 'transcript_session_id' => 'claimed-lock-session-1', + 'transcript_lock' => $claimed_lock, + 'context' => array( '_agents_run_claim_token' => $claimed_input['_agents_run_claim_token'] ), + ) + ); + } +); +$claimed_lock_run = AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'claimed-lock-contention-1' ); +smoke_assert( false, $claimed_lock_result instanceof WP_Error, 'run_claimed_lock_contention_returns_typed_loop_result', $failures, $passes ); +smoke_assert( 'failed', $claimed_lock_result['status'] ?? null, 'run_claimed_lock_contention_result_is_failed', $failures, $passes ); +smoke_assert( 'transcript_lock_contention', $claimed_lock_result['failure']['type'] ?? null, 'run_claimed_lock_contention_preserves_busy_type', $failures, $passes ); +smoke_assert( true, $claimed_lock_result['failure']['retryable'] ?? false, 'run_claimed_lock_contention_is_retryable', $failures, $passes ); +smoke_assert( 0, $claimed_lock_turns, 'run_claimed_lock_contention_starts_no_provider_turn', $failures, $passes ); +smoke_assert( 1, count( $claimed_lock_result['messages'] ?? array() ), 'run_claimed_lock_contention_does_not_mutate_transcript', $failures, $passes ); +smoke_assert( AgentsAPI\AI\WP_Agent_Chat_Run_Control::STATUS_FAILED, $claimed_lock_run['status'] ?? null, 'run_claimed_lock_contention_terminalizes_preclaim', $failures, $passes ); + $canonical_id = agents_chat_run_claimed( array( 'agent' => 'runtime-local-agent', 'message' => 'hi', 'session_id' => 'runtime-s-1', 'run_id' => 'claimed-id-1', 'principal' => $runtime_principal ), static fn() => array( 'session_id' => 'runtime-s-1', 'reply' => 'ok', 'run_id' => 'runtime-id' ) diff --git a/tests/conversation-loop-fail-closed-smoke.php b/tests/conversation-loop-fail-closed-smoke.php index 6739991..6d2a6b1 100644 --- a/tests/conversation-loop-fail-closed-smoke.php +++ b/tests/conversation-loop-fail-closed-smoke.php @@ -208,6 +208,47 @@ static function ( array $messages ): array { agents_api_smoke_assert_equals( 'failed', $gate_result['status'] ?? '', 'throwing policy filter fails the run closed', $failures, $passes ); agents_api_smoke_assert_equals( 0, $policy_executor->calls, 'throwing policy filter never executes the tool', $failures, $passes ); +echo "\n[3b] Malformed pending and replacement policy payloads fail closed:\n"; +$policy_executor->calls = 0; +$persist_log = array(); +$malformed_pending = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'malformed pending' ) ), + $tool_turn, + array( + 'run_id' => 'fail-closed-malformed-pending', + 'transcript_session_id' => 'fail-closed-malformed-pending-session', + 'tool_executor' => $policy_executor, + 'tool_declarations' => $policy_tools, + 'pre_tool_mediator' => static fn(): array => array( + 'action' => 'pending', + 'runtime_tool_request' => array( 'tool_name' => array( 'invalid' ) ), + ), + 'transcript_persister' => $persister, + ) +); +agents_api_smoke_assert_equals( 'failed', $malformed_pending['status'] ?? '', 'malformed pending decision fails the loop', $failures, $passes ); +agents_api_smoke_assert_equals( 0, $policy_executor->calls, 'malformed pending decision executes no tool effect', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-malformed-pending' )['status'] ?? '', 'malformed pending decision durably fails run control', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $persist_log[0]['status'] ?? '', 'malformed pending decision persists failed audit state', $failures, $passes ); + +$persist_log = array(); +$malformed_replace = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'malformed replacement' ) ), + $tool_turn, + array( + 'run_id' => 'fail-closed-malformed-replace', + 'transcript_session_id' => 'fail-closed-malformed-replace-session', + 'tool_executor' => $policy_executor, + 'tool_declarations' => $policy_tools, + 'pre_tool_mediator' => static fn(): array => array( 'action' => 'replace_result', 'result' => 'invalid' ), + 'transcript_persister' => $persister, + ) +); +agents_api_smoke_assert_equals( 'failed', $malformed_replace['status'] ?? '', 'malformed replace_result decision fails the loop', $failures, $passes ); +agents_api_smoke_assert_equals( 0, $policy_executor->calls, 'malformed replace_result decision executes no tool effect', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-malformed-replace' )['status'] ?? '', 'malformed replace_result decision durably fails run control', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $persist_log[0]['status'] ?? '', 'malformed replace_result decision persists failed audit state', $failures, $passes ); + echo "\n[4] Runtime-tool store exceptions fail the run and persist the transcript:\n"; $runtime_store = new class() implements AgentsAPI\AI\WP_Agent_Runtime_Tool_Request_Store { public function create( array $request ): void { unset( $request ); throw new RuntimeException( 'runtime store unavailable' ); } @@ -503,7 +544,9 @@ static function () use ( &$contention_turns ): array { ++$contention_turns; retu 'transcript_lock' => $contended_lock, ) ); -agents_api_smoke_assert_equals( 'transcript_lock_contention', $contention_result['status'] ?? '', 'lock contention returns the canonical busy result', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $contention_result['status'] ?? '', 'lock contention returns a terminal failed result', $failures, $passes ); +agents_api_smoke_assert_equals( 'transcript_lock_contention', $contention_result['failure']['type'] ?? '', 'lock contention retains busy diagnostics', $failures, $passes ); +agents_api_smoke_assert_equals( true, $contention_result['failure']['retryable'] ?? false, 'lock contention remains retryable', $failures, $passes ); agents_api_smoke_assert_equals( 0, $contention_turns, 'lock contention starts no provider execution', $failures, $passes ); agents_api_smoke_assert_equals( null, AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-lock-contention' ), 'lock contention creates no running run-control zombie', $failures, $passes ); diff --git a/tests/conversation-loop-transcript-persister-smoke.php b/tests/conversation-loop-transcript-persister-smoke.php index 7a7e4ef..3980946 100644 --- a/tests/conversation-loop-transcript-persister-smoke.php +++ b/tests/conversation-loop-transcript-persister-smoke.php @@ -301,7 +301,9 @@ static function () use ( &$contention_runs ): array { ) ); -agents_api_smoke_assert_equals( 'transcript_lock_contention', $result7['status'] ?? '', 'contention result is explicit', $failures, $passes ); +agents_api_smoke_assert_equals( 'failed', $result7['status'] ?? '', 'contention result is terminally failed', $failures, $passes ); +agents_api_smoke_assert_equals( 'transcript_lock_contention', $result7['failure']['type'] ?? '', 'contention failure type is explicit', $failures, $passes ); +agents_api_smoke_assert_equals( true, $result7['failure']['retryable'] ?? false, 'contention failure is retryable', $failures, $passes ); agents_api_smoke_assert_equals( 0, $contention_runs, 'turn runner is skipped on lock contention', $failures, $passes ); agents_api_smoke_assert_equals( 0, count( $persister_log ), 'persister is skipped on lock contention', $failures, $passes ); From 3e97e695cf286df2d555a6b6abab58894fbdc877 Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Fri, 28 Aug 2026 03:23:10 +0000 Subject: [PATCH 7/7] fix: publish authoritative chat cancellation outcomes --- .../class-wp-agent-chat-run-control.php | 65 ++++++++++++++++-- .../class-wp-agent-conversation-loop.php | 68 ++++++++++++++++++- tests/conversation-loop-fail-closed-smoke.php | 45 ++++++++++++ 3 files changed, 168 insertions(+), 10 deletions(-) diff --git a/src/Runtime/class-wp-agent-chat-run-control.php b/src/Runtime/class-wp-agent-chat-run-control.php index 771141c..b3ec088 100644 --- a/src/Runtime/class-wp-agent-chat-run-control.php +++ b/src/Runtime/class-wp-agent-chat-run-control.php @@ -347,11 +347,22 @@ static function ( array $state ) use ( $run_id, $status ): array { if ( ! is_array( $run ) ) { return array( 'state' => $state, 'result' => null ); } - $run['status'] = self::normalize_status( $status ); - $run['updated_at'] = self::now(); - if ( self::STATUS_CANCELLED === $run['status'] ) { - $run['cancelled'] = true; + $run = self::normalize_cancellation_state( $run ); + $state['runs'][ $run_id ] = $run; + if ( self::is_terminal_status( $run['status'] ?? null ) ) { + return array( 'state' => $state, 'result' => self::normalize_run( $run ) ); } + + // A committed cancellation request wins over a success claim, but a + // caller that already terminalized the run as cancelled or interrupted + // keeps its own, more specific cancellation representation. + $requested = self::normalize_status( $status ); + $honors_cancellation = in_array( $requested, array( self::STATUS_CANCELLED, self::STATUS_INTERRUPTED ), true ); + $run['status'] = self::is_cancellation_requested( $run ) && ! $honors_cancellation + ? self::STATUS_CANCELLED + : $requested; + $run['updated_at'] = self::now(); + $run = self::normalize_cancellation_state( $run ); $state['runs'][ $run_id ] = $run; $state = self::record_event( $state, $run_id, 'run_finished', array( 'status' => $run['status'] ) ); return array( 'state' => $state, 'result' => self::normalize_run( $run ) ); @@ -400,9 +411,14 @@ static function ( array $state ) use ( $run_id, $owner ): array { if ( ! is_array( $run ) || ! self::owner_matches( $run['_owner'] ?? '', $owner ) ) { return array( 'state' => $state, 'result' => null ); } - $terminal = in_array( self::normalize_status( $run['status'] ?? '' ), array( self::STATUS_COMPLETED, self::STATUS_FAILED, self::STATUS_CANCELLED, self::STATUS_BUDGET_EXCEEDED, self::STATUS_STALLED, self::STATUS_INTERRUPTED ), true ); - $run['status'] = $terminal ? self::normalize_status( $run['status'] ?? '' ) : self::STATUS_CANCELLING; - $run['cancelled'] = ! $terminal; + $run = self::normalize_cancellation_state( $run ); + if ( self::is_terminal_status( $run['status'] ?? null ) ) { + $state['runs'][ $run_id ] = $run; + return array( 'state' => $state, 'result' => self::normalize_run( $run ) ); + } + + $run['status'] = self::STATUS_CANCELLING; + $run['cancelled'] = true; $run['updated_at'] = self::now(); $state['runs'][ $run_id ] = $run; $state = self::record_event( $state, $run_id, 'cancel_requested', array( 'status' => $run['status'] ) ); @@ -711,6 +727,41 @@ private static function atomic_unavailable( \RuntimeException $error ): \WP_Erro return new \WP_Error( 'agents_chat_run_atomic_unavailable', $error->getMessage() ); } + private static function is_terminal_status( mixed $status ): bool { + return in_array( + self::normalize_status( $status ), + array( + self::STATUS_COMPLETED, + self::STATUS_FAILED, + self::STATUS_CANCELLED, + self::STATUS_BUDGET_EXCEEDED, + self::STATUS_STALLED, + self::STATUS_INTERRUPTED, + ), + true + ); + } + + /** @param array $run */ + private static function is_cancellation_requested( array $run ): bool { + return self::STATUS_CANCELLING === self::normalize_status( $run['status'] ?? null ) || true === ( $run['cancelled'] ?? false ); + } + + /** + * @param array $run + * @return array + */ + private static function normalize_cancellation_state( array $run ): array { + $status = self::normalize_status( $run['status'] ?? null ); + if ( in_array( $status, array( self::STATUS_CANCELLING, self::STATUS_CANCELLED ), true ) ) { + $run['cancelled'] = true; + } elseif ( isset( $run['cancelled'] ) ) { + $run['cancelled'] = false; + } + + return $run; + } + /** * @param callable(array{runs:array>,queues:array>>,events:array>>}):array{state:array{runs:array>,queues:array>>,events:array>>},result:mixed} $mutation */ diff --git a/src/Runtime/class-wp-agent-conversation-loop.php b/src/Runtime/class-wp-agent-conversation-loop.php index 6cc22c2..a584a47 100644 --- a/src/Runtime/class-wp-agent-conversation-loop.php +++ b/src/Runtime/class-wp-agent-conversation-loop.php @@ -629,7 +629,30 @@ public static function run( array $messages, ?callable $turn_runner = null, arra } if ( '' !== $run_id && '' !== $lock_session_id ) { - self::finish_run_or_throw( $run_id, WP_Agent_Run_Outcome::run_control_status( $final_result ), $run_workspace ); + $requested_status = WP_Agent_Run_Outcome::run_control_status( $final_result ); + $finished_run = self::finish_run_or_throw( $run_id, $requested_status, $run_workspace ); + $authoritative = WP_Agent_Chat_Run_Control::normalize_status( $finished_run['status'] ?? null ); + + // A cancellation that commits after the candidate result was built wins the + // terminal publication. The candidate is only corrected when it would + // otherwise claim success against a stored cancellation. + if ( WP_Agent_Chat_Run_Control::STATUS_CANCELLED === $authoritative && $requested_status !== $authoritative ) { + $final_result = self::project_cancelled_terminal_result( $final_result, $finished_run ); + $correction_error = self::persist_transcript( $transcript_persister, $messages, $options, $final_result ); + if ( null !== $correction_error ) { + throw new WP_Agent_Run_Control_Store_Exception( + 'Transcript persistence failed while projecting the authoritative conversation outcome.', + 0, + $correction_error + ); + } + self::emit_event( $on_event, WP_Agent_Chat_Run_Control::STATUS_CANCELLED, array( + 'turn' => $turns_run, + 'status' => WP_Agent_Chat_Run_Control::STATUS_CANCELLED, + 'run_id' => $run_id, + ) ); + return $final_result; + } } self::emit_event( $on_event, 'completed', array( @@ -1398,8 +1421,10 @@ private static function finalize_loop_failure( /** * Finalize run control without hiding retryable storage failures. + * + * @return array Authoritative stored run. */ - private static function finish_run_or_throw( string $run_id, string $status, ?\AgentsAPI\Core\Workspace\WP_Agent_Workspace_Scope $workspace ): void { + private static function finish_run_or_throw( string $run_id, string $status, ?\AgentsAPI\Core\Workspace\WP_Agent_Workspace_Scope $workspace ): array { $finished = WP_Agent_Chat_Run_Control::finish_run( $run_id, $status, $workspace ); if ( is_wp_error( $finished ) ) { throw new WP_Agent_Run_Control_Store_Exception( $finished->get_error_message() ); @@ -1415,9 +1440,46 @@ private static function finish_run_or_throw( string $run_id, string $status, ?\A } $expected_status = WP_Agent_Chat_Run_Control::normalize_status( $status ); - if ( $run_id !== $finished['run_id'] || $expected_status !== $finished['status'] ) { + if ( $run_id !== $finished['run_id'] ) { throw new WP_Agent_Run_Control_Store_Exception( 'Run-control finalization did not persist the requested terminal result.' ); } + if ( $expected_status !== $finished['status'] && ! in_array( $finished['status'], self::terminal_run_control_statuses(), true ) ) { + throw new WP_Agent_Run_Control_Store_Exception( 'Run-control finalization did not return an authoritative terminal result.' ); + } + + return $finished; + } + + /** + * Project a stored cancellation winner back into the conversation result. + * + * @param array $result Normalized candidate conversation result. + * @param array $run Authoritative stored run. + * @return array + */ + private static function project_cancelled_terminal_result( array $result, array $run ): array { + unset( $result['run_outcome'], $result['failure'] ); + $result['status'] = WP_Agent_Chat_Run_Control::STATUS_CANCELLED; + $result['completed'] = false; + $result['interrupted'] = array( + 'action' => 'cancel', + 'reason' => 'run_control_cancelled', + 'run_id' => is_string( $run['run_id'] ?? null ) ? $run['run_id'] : '', + ); + + return self::normalize_conversation_result( $result ); + } + + /** @return array */ + private static function terminal_run_control_statuses(): array { + return array( + WP_Agent_Chat_Run_Control::STATUS_COMPLETED, + WP_Agent_Chat_Run_Control::STATUS_FAILED, + WP_Agent_Chat_Run_Control::STATUS_CANCELLED, + WP_Agent_Chat_Run_Control::STATUS_BUDGET_EXCEEDED, + WP_Agent_Chat_Run_Control::STATUS_STALLED, + WP_Agent_Chat_Run_Control::STATUS_INTERRUPTED, + ); } /** diff --git a/tests/conversation-loop-fail-closed-smoke.php b/tests/conversation-loop-fail-closed-smoke.php index 6d2a6b1..bee5e88 100644 --- a/tests/conversation-loop-fail-closed-smoke.php +++ b/tests/conversation-loop-fail-closed-smoke.php @@ -550,4 +550,49 @@ static function () use ( &$contention_turns ): array { ++$contention_turns; retu agents_api_smoke_assert_equals( 0, $contention_turns, 'lock contention starts no provider execution', $failures, $passes ); agents_api_smoke_assert_equals( null, AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-lock-contention' ), 'lock contention creates no running run-control zombie', $failures, $passes ); +echo "\n[13] Cancellation committed during transcript persistence wins terminal publication:\n"; +AgentsAPI\AI\WP_Agent_Run_Control::set_store( new Agents_API_Memory_Atomic_Run_Control_Store() ); +$late_cancel_results = array(); +$late_cancel_persister = new class( $late_cancel_results ) implements AgentsAPI\AI\WP_Agent_Transcript_Persister { + private array $results; + public function __construct( array &$results ) { $this->results = &$results; } + public function persist( array $messages, AgentsAPI\AI\WP_Agent_Conversation_Request $request, array $result ): string { + unset( $messages, $request ); + $this->results[] = $result; + if ( 1 === count( $this->results ) ) { + AgentsAPI\AI\WP_Agent_Chat_Run_Control::request_cancel( 'fail-closed-late-cancel' ); + } + return 'late-cancel-transcript'; + } +}; +$late_cancel_events = array(); +$late_cancel_turns = 0; +$late_cancel_result = AgentsAPI\AI\WP_Agent_Conversation_Loop::run( + array( array( 'role' => 'user', 'content' => 'cancel during persistence' ) ), + static function ( array $messages ) use ( &$late_cancel_turns ): array { + ++$late_cancel_turns; + $messages[] = AgentsAPI\AI\WP_Agent_Message::text( 'assistant', 'candidate completion' ); + return array( 'messages' => $messages, 'tool_execution_results' => array() ); + }, + array( + 'run_id' => 'fail-closed-late-cancel', + 'transcript_session_id' => 'fail-closed-late-cancel-session', + 'transcript_persister' => $late_cancel_persister, + 'on_event' => static function ( string $event, array $payload ) use ( &$late_cancel_events ): void { + $late_cancel_events[] = array( 'event' => $event, 'status' => $payload['status'] ?? '' ); + }, + ) +); +$late_cancel_run = AgentsAPI\AI\WP_Agent_Chat_Run_Control::get_run( 'fail-closed-late-cancel' ); +$late_cancel_terminal_events = array_values( array_filter( $late_cancel_events, static fn( array $event ): bool => in_array( $event['event'], array( 'completed', 'cancelled' ), true ) ) ); +agents_api_smoke_assert_equals( 1, $late_cancel_turns, 'late cancellation does not repeat provider execution', $failures, $passes ); +agents_api_smoke_assert_equals( 'cancelled', $late_cancel_run['status'] ?? '', 'stored run keeps the atomic cancelled winner', $failures, $passes ); +agents_api_smoke_assert_equals( true, $late_cancel_run['cancelled'] ?? false, 'stored cancellation fields remain consistent', $failures, $passes ); +agents_api_smoke_assert_equals( 'cancelled', $late_cancel_result['status'] ?? '', 'returned conversation result projects cancelled winner', $failures, $passes ); +agents_api_smoke_assert_equals( false, $late_cancel_result['completed'] ?? true, 'returned cancelled result is not successful completion', $failures, $passes ); +agents_api_smoke_assert_equals( 'cancelled', $late_cancel_result['run_outcome']['status'] ?? '', 'returned run outcome agrees with cancelled run control', $failures, $passes ); +agents_api_smoke_assert_equals( 2, count( $late_cancel_results ), 'transcript receives corrected authoritative terminal projection', $failures, $passes ); +agents_api_smoke_assert_equals( 'cancelled', $late_cancel_results[1]['status'] ?? '', 'final persisted transcript result is cancelled', $failures, $passes ); +agents_api_smoke_assert_equals( array( array( 'event' => 'cancelled', 'status' => 'cancelled' ) ), $late_cancel_terminal_events, 'terminal event reports cancelled with no completion success', $failures, $passes ); + agents_api_smoke_finish( 'Agents API conversation loop fail-closed', $failures, $passes );