diff --git a/includes/class-openclawp-runner.php b/includes/class-openclawp-runner.php index 93397be..028d12c 100644 --- a/includes/class-openclawp-runner.php +++ b/includes/class-openclawp-runner.php @@ -152,12 +152,13 @@ public static function run_turn( string $agent_slug, string $message, ?string $s // the `client/` form the agents-api conversation loop validates and // matches tool calls against. for_agent()'s unprefixed output still backs // the MCP tool surface and the provider-facing function names. - $tools = OpenclaWP_Tools_Resolver::loop_tools( $agent ); + $tools = OpenclaWP_Tools_Resolver::loop_tools( $agent ); + $loop_context = self::tool_context_from_runtime_context( $runtime_context ); // Stamp identity onto the runtime context so the executor can // resolve user/session/agent without an extra round-trip back into // the runner. Used by the confirmation gate (#40) to look up // per-user "always allow" entries and create decision rows. - $executor_context = $runtime_context; + $executor_context = array_merge( $runtime_context, $loop_context ); $executor_context['user_id'] = $user_id; $executor_context['session_id'] = $session_id; $executor_context['agent_slug'] = $agent_slug; @@ -187,7 +188,6 @@ public static function run_turn( string $agent_slug, string $message, ?string $s 'transcript_session_id' => $session_id, 'transcript_lock_ttl' => 300, ); - $loop_context = self::tool_context_from_runtime_context( $runtime_context ); if ( ! empty( $loop_context['client_context'] ) || count( $loop_context ) > 1 ) { $loop_options['context'] = $loop_context; } @@ -656,11 +656,11 @@ private static function emit_chat_telemetry( array $telemetry ): void { /** * Extract tool calls from a GenerativeAiResult, in the shape - * `[ ['name' => '…', 'parameters' => […]], … ]` that + * `[ ['id' => '…', 'name' => '…', 'parameters' => […]], … ]` that * `WP_Agent_Conversation_Loop::mediate_tool_calls()` consumes. * * @param mixed $result - * @return array + * @return array */ private static function extract_tool_calls( $result ): array { if ( ! is_object( $result ) || ! method_exists( $result, 'toMessage' ) ) { @@ -697,6 +697,7 @@ private static function extract_tool_calls( $result ): array { continue; } + $id = method_exists( $fc, 'getId' ) ? trim( (string) $fc->getId() ) : ''; $name = method_exists( $fc, 'getName' ) ? (string) $fc->getName() : ''; $args = method_exists( $fc, 'getArgs' ) ? $fc->getArgs() : array(); @@ -708,10 +709,15 @@ private static function extract_tool_calls( $result ): array { // to the loop-facing `client/` form so the conversation loop's // mediation matches it against the tool declarations and the executor's // name map (both keyed on the loop name). {@see OpenclaWP_Tools_Resolver::loop_name()}. - $out[] = array( + $tool_call = array( 'name' => OpenclaWP_Tools_Resolver::loop_name( $name ), 'parameters' => is_array( $args ) ? $args : array(), ); + if ( '' !== $id ) { + $tool_call['id'] = $id; + } + + $out[] = $tool_call; } return $out; diff --git a/includes/class-openclawp-tool-executor.php b/includes/class-openclawp-tool-executor.php index 472c09f..2aeeadc 100644 --- a/includes/class-openclawp-tool-executor.php +++ b/includes/class-openclawp-tool-executor.php @@ -173,7 +173,7 @@ private function maybe_gate( string $ability_name, string $declared_name, array // follow-up turn after a user decision wants to let THIS particular // invocation through (matched by ability + decision_id). $override = $this->runtime_context['openclawp_decision_override'] ?? null; - if ( is_array( $override ) && ( $override['ability'] ?? '' ) === $ability_name ) { + if ( self::decision_override_allows( $override, $ability_name, $parameters ) ) { return null; } @@ -230,6 +230,58 @@ private function maybe_gate( string $ability_name, string $declared_name, array ); } + /** + * Whether a one-shot runtime override allows this exact ability call. + * + * Older callers supplied only `{ ability }`; keep that behavior. Newer + * callers can add `parameters` to bind the bypass to the exact tool + * arguments the user already confirmed. + * + * @param mixed $override Runtime override payload. + * @param string $ability_name Fully namespaced ability name. + * @param array $parameters Tool-call parameters. + * @return bool + */ + private static function decision_override_allows( $override, string $ability_name, array $parameters ): bool { + if ( ! is_array( $override ) || ( $override['ability'] ?? '' ) !== $ability_name ) { + return false; + } + + if ( ! array_key_exists( 'parameters', $override ) ) { + return true; + } + + if ( ! is_array( $override['parameters'] ) ) { + return false; + } + + return self::normalized_parameters( $override['parameters'] ) === self::normalized_parameters( $parameters ); + } + + /** + * Normalize associative-key order before comparing tool parameters. + * + * @param array $parameters Raw parameters. + * @return array + */ + private static function normalized_parameters( array $parameters ): array { + if ( array_is_list( $parameters ) ) { + return array_map( + static fn( $value ) => is_array( $value ) ? self::normalized_parameters( $value ) : $value, + $parameters + ); + } + + ksort( $parameters ); + foreach ( $parameters as $key => $value ) { + if ( is_array( $value ) ) { + $parameters[ $key ] = self::normalized_parameters( $value ); + } + } + + return $parameters; + } + /** * Dispatch a coordinator's tool call into the subagent's chat session. * diff --git a/tests/unit/RunnerToolCallsTest.php b/tests/unit/RunnerToolCallsTest.php new file mode 100644 index 0000000..0613a6c --- /dev/null +++ b/tests/unit/RunnerToolCallsTest.php @@ -0,0 +1,149 @@ +setAccessible( true ); + + $result = new RunnerToolCallsStubs\Result( + array( + new RunnerToolCallsStubs\Part( + new RunnerToolCallsStubs\FunctionCall( + 'toolu_01ABC', + 'carpeta__delete-potreros', + array( + 'ids' => array( 987654321 ), + 'confirm' => true, + ) + ) + ), + ) + ); + + $this->assertSame( + array( + array( + 'name' => 'client/carpeta__delete-potreros', + 'parameters' => array( + 'ids' => array( 987654321 ), + 'confirm' => true, + ), + 'id' => 'toolu_01ABC', + ), + ), + $method->invoke( null, $result ) + ); + } +} + +namespace OpenclaWP\Tests\Unit\RunnerToolCallsStubs; + +final class Result { + private Message $message; + + /** + * @param array $parts + */ + public function __construct( array $parts ) { + $this->message = new Message( $parts ); + } + + public function toMessage(): Message { + return $this->message; + } +} + +final class Message { + /** @var array */ + private array $parts; + + /** + * @param array $parts + */ + public function __construct( array $parts ) { + $this->parts = $parts; + } + + /** + * @return array + */ + public function getParts(): array { + return $this->parts; + } +} + +final class Part { + private FunctionCall $function_call; + + public function __construct( FunctionCall $function_call ) { + $this->function_call = $function_call; + } + + public function getType(): Type { + return new Type( 'function_call' ); + } + + public function getFunctionCall(): FunctionCall { + return $this->function_call; + } +} + +final class Type { + private string $value; + + public function __construct( string $value ) { + $this->value = $value; + } + + public function value(): string { + return $this->value; + } +} + +final class FunctionCall { + private string $id; + private string $name; + /** @var array */ + private array $args; + + /** + * @param array $args + */ + public function __construct( string $id, string $name, array $args ) { + $this->id = $id; + $this->name = $name; + $this->args = $args; + } + + public function getId(): string { + return $this->id; + } + + public function getName(): string { + return $this->name; + } + + /** + * @return array + */ + public function getArgs(): array { + return $this->args; + } +} diff --git a/tests/unit/ToolExecutorDecisionOverrideTest.php b/tests/unit/ToolExecutorDecisionOverrideTest.php new file mode 100644 index 0000000..56ae1e0 --- /dev/null +++ b/tests/unit/ToolExecutorDecisionOverrideTest.php @@ -0,0 +1,83 @@ +assertTrue( + self::override_allows( + array( 'ability' => 'carpeta/delete-potreros' ), + 'carpeta/delete-potreros', + array( 'ids' => array( 1 ), 'confirm' => true ) + ) + ); + } + + public function test_decision_override_with_matching_parameters_allows_call(): void { + $this->assertTrue( + self::override_allows( + array( + 'ability' => 'carpeta/delete-potreros', + 'parameters' => array( + 'confirm' => true, + 'ids' => array( 1312, 1313 ), + ), + ), + 'carpeta/delete-potreros', + array( + 'ids' => array( 1312, 1313 ), + 'confirm' => true, + ) + ) + ); + } + + public function test_decision_override_with_different_parameters_does_not_allow_call(): void { + $this->assertFalse( + self::override_allows( + array( + 'ability' => 'carpeta/delete-potreros', + 'parameters' => array( + 'ids' => array( 1312, 1313 ), + 'confirm' => true, + ), + ), + 'carpeta/delete-potreros', + array( + 'ids' => array( 1312, 9999 ), + 'confirm' => true, + ) + ) + ); + } + + private static function override_allows( array $override, string $ability, array $parameters ): bool { + $method = new ReflectionMethod( OpenclaWP_Tool_Executor::class, 'decision_override_allows' ); + $method->setAccessible( true ); + + return (bool) $method->invoke( null, $override, $ability, $parameters ); + } +}