Skip to content

Fix tool-mediation loop: agents never executed tools #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lezama merged 1 commit into from
Jun 3, 2026
Merged

Fix tool-mediation loop: agents never executed tools #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 76 additions & 12 deletions includes/class-openclawp-message-adapter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ final class OpenclaWP_Message_Adapter {
* Convert a transcript array into a list of WP AI Client `Message` DTOs.
*
* Roles map: `user` → `MessageRoleEnum::user()`, `assistant`/`model` →
* `MessageRoleEnum::model()`. System and tool messages are dropped — the
* loop's tool-mediation path isn't wired through this adapter yet.
* `MessageRoleEnum::model()`. Tool-mediation turns round-trip as native
* function-call parts so the model sees its own calls and their results
* (without this, a tool-using turn re-issues the same call every turn
* because the result never reaches the model):
* - `tool_call` → model message carrying a `FunctionCall` part.
* - `tool_result` → user message carrying a `FunctionResponse` part.
* Returns the original transcript unchanged if WP AI Client is missing.
*
* @param array<int,array<string,mixed>> $messages Transcript messages.
Expand All @@ -33,32 +37,92 @@ public static function to_ai_client_messages( array $messages ): array {
return $messages;
}

$user_role = \WordPress\AiClient\Messages\Enums\MessageRoleEnum::user();
$model_role = \WordPress\AiClient\Messages\Enums\MessageRoleEnum::model();

$out = array();
foreach ( $messages as $message ) {
$type = (string) ( $message['type'] ?? 'text' );

if ( 'tool_call' === $type ) {
$part = self::tool_call_part( $message );
if ( null !== $part ) {
$out[] = new \WordPress\AiClient\Messages\DTO\Message( $model_role, array( $part ) );
}
continue;
}

if ( 'tool_result' === $type ) {
$part = self::tool_result_part( $message );
if ( null !== $part ) {
$out[] = new \WordPress\AiClient\Messages\DTO\Message( $user_role, array( $part ) );
}
continue;
}

$role = (string) ( $message['role'] ?? '' );
$text = self::extract_text( $message['content'] ?? '' );

if ( '' === $text ) {
continue;
}

if ( 'user' === $role ) {
$role_enum = \WordPress\AiClient\Messages\Enums\MessageRoleEnum::user();
$out[] = new \WordPress\AiClient\Messages\DTO\Message( $user_role, array( new \WordPress\AiClient\Messages\DTO\MessagePart( $text ) ) );
} elseif ( 'assistant' === $role || 'model' === $role ) {
$role_enum = \WordPress\AiClient\Messages\Enums\MessageRoleEnum::model();
} else {
continue;
$out[] = new \WordPress\AiClient\Messages\DTO\Message( $model_role, array( new \WordPress\AiClient\Messages\DTO\MessagePart( $text ) ) );
}

$out[] = new \WordPress\AiClient\Messages\DTO\Message(
$role_enum,
array( new \WordPress\AiClient\Messages\DTO\MessagePart( $text ) )
);
}

return $out;
}

/**
* Build a `MessagePart` wrapping the `FunctionCall` for a stored `tool_call`
* transcript message. The function name is mapped back to the provider-safe
* form the model originally used (the stored name carries the `client/`
* loop prefix). Returns null when the DTO is unavailable or the name is empty.
*
* @param array<string,mixed> $message Stored tool_call message.
* @return object|null
*/
private static function tool_call_part( array $message ) {
if ( ! class_exists( '\\WordPress\\AiClient\\Tools\\DTO\\FunctionCall' ) ) {
return null;
}
$payload = is_array( $message['payload'] ?? null ) ? $message['payload'] : array();
$meta = is_array( $message['metadata'] ?? null ) ? $message['metadata'] : array();
$name = OpenclaWP_Tools_Resolver::provider_name( (string) ( $payload['tool_name'] ?? '' ) );
if ( '' === $name ) {
return null;
}
$id = (string) ( $meta['tool_call_id'] ?? '' );
$args = is_array( $payload['parameters'] ?? null ) ? $payload['parameters'] : array();

$call = new \WordPress\AiClient\Tools\DTO\FunctionCall( '' !== $id ? $id : null, $name, $args );
return new \WordPress\AiClient\Messages\DTO\MessagePart( $call );
}

/**
* Build a `MessagePart` wrapping the `FunctionResponse` for a stored
* `tool_result` transcript message, matched to its call by `tool_call_id`.
*
* @param array<string,mixed> $message Stored tool_result message.
* @return object|null
*/
private static function tool_result_part( array $message ) {
if ( ! class_exists( '\\WordPress\\AiClient\\Tools\\DTO\\FunctionResponse' ) ) {
return null;
}
$payload = is_array( $message['payload'] ?? null ) ? $message['payload'] : array();
$meta = is_array( $message['metadata'] ?? null ) ? $message['metadata'] : array();
$name = OpenclaWP_Tools_Resolver::provider_name( (string) ( $payload['tool_name'] ?? '' ) );
$id = (string) ( $meta['tool_call_id'] ?? '' );
$response = array_key_exists( 'result', $payload ) ? $payload['result'] : ( $message['content'] ?? '' );

$resp = new \WordPress\AiClient\Tools\DTO\FunctionResponse( '' !== $id ? $id : null, '' !== $name ? $name : null, $response );
return new \WordPress\AiClient\Messages\DTO\MessagePart( $resp );
}

/**
* Walk the transcript backwards and return the most recent assistant text.
*
Expand Down
12 changes: 10 additions & 2 deletions includes/class-openclawp-runner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ public static function run_turn( string $agent_slug, string $message, ?string $s
'content' => $message_for_model,
);

$tools = OpenclaWP_Tools_Resolver::for_agent( $agent );
// loop_tools() (not for_agent()) re-keys declarations + executor maps under
// the `client/<name>` 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 );
// 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
Expand Down Expand Up @@ -577,8 +581,12 @@ private static function extract_tool_calls( $result ): array {
continue;
}

// The model returns the provider-safe function name (no slash). Map it
// to the loop-facing `client/<name>` 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(
'name' => $name,
'name' => OpenclaWP_Tools_Resolver::loop_name( $name ),
'parameters' => is_array( $args ) ? $args : array(),
);
}
Expand Down
94 changes: 91 additions & 3 deletions includes/class-openclawp-tools-resolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,44 @@

final class OpenclaWP_Tools_Resolver {

/**
* Source segment for client tool declarations. The agents-api conversation
* loop validates client tool names against `^client/[a-z][a-z0-9_-]*$` and
* derives the declaration's source from the segment before the slash, which
* MUST equal `client` (see WP_Agent_Tool_Declaration::validate()). Provider
* APIs reject `/` in function names, so the name the model sees stays the
* sanitized `__` form; the loop-facing declaration + executor key it as
* `client/<sanitized>`. {@see self::loop_name()}.
*/
public const TOOL_SOURCE = 'client';

/**
* Map a provider-safe declaration name (what the model sees, e.g.
* `openclawp__get-recent-posts`) to the loop-facing name the agents-api
* conversation loop validates and matches tool calls against
* (`client/openclawp__get-recent-posts`). Idempotent.
*/
public static function loop_name( string $declared_name ): string {
$prefix = self::TOOL_SOURCE . '/';
if ( 0 === strpos( $declared_name, $prefix ) ) {
return $declared_name;
}
return $prefix . $declared_name;
}

/**
* Inverse of {@see self::loop_name()}: strip the `client/` prefix off a
* loop-facing tool name to recover the provider-safe name the model used
* (and expects back in FunctionCall / FunctionResponse parts). Idempotent.
*/
public static function provider_name( string $loop_name ): string {
$prefix = self::TOOL_SOURCE . '/';
if ( 0 === strpos( $loop_name, $prefix ) ) {
return substr( $loop_name, strlen( $prefix ) );
}
return $loop_name;
}

/**
* @return array{
* declarations: array<string, array{name:string,source:string,description:string,parameters:array,executor:string,scope:string}>,
Expand Down Expand Up @@ -165,13 +203,63 @@ public static function for_agent( WP_Agent $agent ): array {
);
}

/**
* Build the loop-facing variant of {@see self::for_agent()} for the
* agents-api conversation loop.
*
* The loop validates client tool declarations as `client/<name>` (deriving
* source `client` from the segment before the slash) and matches the model's
* tool calls against those same names. So this re-keys the declarations and
* the executor maps under {@see self::loop_name()} and stamps the
* source/executor/scope the loop requires. The provider-facing declarations
* and the raw {@see self::for_agent()} output are intentionally left
* unprefixed — the MCP tool surface and the names the model sees must NOT
* carry the `client/` prefix. The runner maps the model's returned tool-call
* names back through {@see self::loop_name()}.
*
* @return array{declarations:array<string,array<string,mixed>>,declarations_for_provider:array,name_to_ability:array<string,string>,delegate_targets:array<string,string>}
*/
public static function loop_tools( WP_Agent $agent ): array {
$resolved = self::for_agent( $agent );

$declarations = array();
foreach ( $resolved['declarations'] as $name => $decl ) {
$loop = self::loop_name( (string) $name );
$decl['name'] = $loop;
$decl['source'] = self::TOOL_SOURCE;
$decl['executor'] = 'client';
$decl['scope'] = 'run';

$declarations[ $loop ] = $decl;
}

$name_to_ability = array();
foreach ( $resolved['name_to_ability'] as $name => $ability ) {
$name_to_ability[ self::loop_name( (string) $name ) ] = $ability;
}

$delegate_targets = array();
foreach ( ( $resolved['delegate_targets'] ?? array() ) as $name => $slug ) {
$delegate_targets[ self::loop_name( (string) $name ) ] = $slug;
}

return array(
'declarations' => $declarations,
'declarations_for_provider' => $resolved['declarations_for_provider'],
'name_to_ability' => $name_to_ability,
'delegate_targets' => $delegate_targets,
);
}

/**
* Convert a `namespace/slug` ability name into a provider-safe function
* name. `/` becomes `__`; other unsupported characters get stripped.
* name. `/` becomes `__`; other unsupported characters get stripped; the
* result is lowercased so the `client/<name>` loop form satisfies the
* agents-api client-tool name pattern (`^[a-z][a-z0-9_-]*$` per segment).
*/
public static function sanitize_name( string $ability_name ): string {
$sanitized = str_replace( '/', '__', $ability_name );
$sanitized = preg_replace( '/[^A-Za-z0-9_-]/', '', $sanitized );
$sanitized = str_replace( '/', '__', strtolower( $ability_name ) );
$sanitized = preg_replace( '/[^a-z0-9_-]/', '', $sanitized );
return is_string( $sanitized ) ? $sanitized : '';
}
}
50 changes: 50 additions & 0 deletions tests/smoke.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1282,6 +1282,56 @@ function_exists( 'wp_has_ability' ) && wp_has_ability( 'a2a/smoke-peer' )
);
}

// ---------------------------------------------------------------------------
// Tool-mediation round-trip: a tool_call / tool_result pair must convert into
// native FunctionCall / FunctionResponse message parts (provider name, no
// `client/` loop prefix). Regression guard for the loop that re-issued the same
// tool call every turn because the result never reached the model.
// ---------------------------------------------------------------------------
if ( class_exists( '\\WordPress\\AiClient\\Messages\\DTO\\Message' ) ) {
$transcript = array(
array( 'type' => 'text', 'role' => 'user', 'content' => 'latest post?' ),
array(
'type' => 'tool_call',
'role' => 'assistant',
'content' => '',
'payload' => array( 'tool_name' => 'client/openclawp__get-recent-posts', 'parameters' => array( 'limit' => 1 ) ),
'metadata' => array( 'tool_call_id' => 'tc-1' ),
),
array(
'type' => 'tool_result',
'role' => 'user',
'content' => '{"posts":[]}',
'payload' => array( 'result' => array( 'posts' => array() ), 'tool_name' => 'client/openclawp__get-recent-posts' ),
'metadata' => array( 'tool_call_id' => 'tc-1' ),
),
);

$converted = OpenclaWP_Message_Adapter::to_ai_client_messages( $transcript );
$part_type = static function ( $part ): string {
$type = $part->getType();
return is_object( $type ) && method_exists( $type, 'value' ) ? (string) $type->value() : (string) $type;
};
$call_name = '';
$has_response = false;
if ( count( $converted ) >= 3 ) {
foreach ( $converted[1]->getParts() as $part ) {
if ( 'function_call' === $part_type( $part ) && method_exists( $part, 'getFunctionCall' ) ) {
$call_name = (string) $part->getFunctionCall()->getName();
}
}
foreach ( $converted[2]->getParts() as $part ) {
if ( 'function_response' === $part_type( $part ) ) {
$has_response = true;
}
}
}

OpenclaWP_Smoke::check( 'adapter converts tool round-trip into 3 messages', 3 === count( $converted ) );
OpenclaWP_Smoke::check( 'tool_call becomes a FunctionCall with the provider name (client/ prefix stripped)', 'openclawp__get-recent-posts' === $call_name );
OpenclaWP_Smoke::check( 'tool_result becomes a FunctionResponse part', true === $has_response );
}

$failed = OpenclaWP_Smoke::summarize();
if ( $failed > 0 ) {
exit( 1 );
Expand Down
Loading
Loading