Skip to content

Harden tool approval contracts #2421

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

Open
chubes4 wants to merge 1 commit into
base: main
Choose a base branch
from
+99 −39
Open

Harden tool approval contracts #2421

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
53 changes: 16 additions & 37 deletions inc/Engine/AI/Tools/ToolExecutor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ public static function executeTool(
): array {
$core = new WP_Agent_Tool_Execution_Core();
$execution = new ToolExecutionCore();
$available_tools = self::prepareToolDeclarations( $available_tools, $payload );
$prepared = $core->prepareWP_Agent_Tool_Call( $tool_name, $tool_parameters, $available_tools, $payload );
if ( empty( $prepared['ready'] ) ) {
unset( $prepared['ready'] );
Expand Down Expand Up @@ -104,21 +103,30 @@ public static function executeTool(
}

if ( WP_Agent_Action_Policy::stagesApproval( $policy ) ) {
// Tool must declare how to build an action kind and summary.
// If it hasn't opted into the preview pipeline properly, fall
// back to 'direct' and log a warning — preview is a contract,
// not something we can synthesize for tools that don't cooperate.
// Tool must declare the pending-action kind. Preview is an approval
// contract, not metadata we can synthesize safely at execution time.
if ( empty( $tool_def['action_kind'] ) ) {
do_action(
'datamachine_log',
'warning',
'WP_Agent_Action_Policy: tool resolved to preview but is missing action_kind metadata; falling back to direct.',
'error',
'WP_Agent_Action_Policy: tool resolved to preview but is missing action_kind metadata; refusing execution.',
array(
'tool_name' => $tool_name,
'mode' => $mode,
'agent_id' => $agent_id,
)
);

return array(
'success' => false,
'error' => sprintf( 'Tool "%s" resolved to staged approval but is missing required pending-action metadata: action_kind.', $tool_name ),
'tool_name' => $tool_name,
'action_policy' => $policy,
'metadata' => array(
'error_type' => 'missing_pending_action_metadata',
'missing_metadata' => array( 'action_kind' ),
),
);
} else {
$staged = PendingActionHelper::stage(
array(
Expand Down Expand Up @@ -151,7 +159,7 @@ public static function executeTool(
}
}

// Policy is 'direct' (or 'preview' fell back) — execute the tool normally.
// Policy is 'direct' — execute the tool normally.
$tool_result = $core->executePreparedTool( $tool_call, $tool_def, $execution, $payload );

// Automatic post origin tracking — applies to every tool whose result
Expand All @@ -173,35 +181,6 @@ public static function executeTool(
return $tool_result;
}

/**
* Annotate Data Machine declarations with explicit runtime-context bindings.
*
* @param array $available_tools Tool declarations keyed by name.
* @param array $payload Step payload / invocation context.
* @return array Annotated tool declarations.
*/
private static function prepareToolDeclarations( array $available_tools, array $payload ): array {
$runtime_keys = array( 'job_id', 'flow_step_id', 'data', 'flow_step_config', 'agent_id', 'agent_slug', 'user_id' );

foreach ( $available_tools as $tool_name => $tool_def ) {
if ( ! is_array( $tool_def ) ) {
continue;
}

$bindings = is_array( $tool_def['client_context_bindings'] ?? null ) ? $tool_def['client_context_bindings'] : array();
foreach ( $runtime_keys as $runtime_key ) {
if ( array_key_exists( $runtime_key, $payload ) && ! array_key_exists( $runtime_key, $bindings ) && ! in_array( $runtime_key, $bindings, true ) ) {
$bindings[ $runtime_key ] = $runtime_key;
}
}

$tool_def['client_context_bindings'] = $bindings;
$available_tools[ $tool_name ] = $tool_def;
}

return $available_tools;
}

/**
* Build a one-line human summary for a staged invocation.
*
Expand Down
24 changes: 24 additions & 0 deletions inc/Engine/AI/Tools/ToolManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ public function resolveHandlerTools(
$tool_def['access_level'] = $access_level;
}

$tool_def = self::withHandlerContextBindings( $tool_def );

$resolved[ $tool_name ] = $tool_def;
}
}
Expand All @@ -350,6 +352,28 @@ public function resolveHandlerTools(
return $resolved;
}

/**
* Add the explicit runtime context bindings owned by handler tools.
*
* Handler tools execute inside a pipeline job and may write audit or origin
* metadata against that job. Keep that binding narrow and visible on the
* resolved tool declaration instead of letting every payload key satisfy
* tool parameters implicitly.
*
* @param array $tool_def Resolved handler tool definition.
* @return array Tool definition with handler-owned context bindings.
*/
private static function withHandlerContextBindings( array $tool_def ): array {
$bindings = is_array( $tool_def['client_context_bindings'] ?? null ) ? $tool_def['client_context_bindings'] : array();
if ( ! array_key_exists( 'job_id', $bindings ) && ! in_array( 'job_id', $bindings, true ) ) {
$bindings['job_id'] = 'job_id';
}

$tool_def['client_context_bindings'] = $bindings;

return $tool_def;
}

/**
* Build a cache key for adjacent handler tool resolution.
*
Expand Down
61 changes: 59 additions & 2 deletions tests/tool-executor-ability-native-smoke.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,39 @@ function execute_tool( string $tool_name, array $tool_parameters, array $tool_de
assert_smoke( 'ability-only result succeeds', true === ( $result['success'] ?? false ) );
assert_smoke( 'ability execute callback ran exactly once', 1 === $ability->execute_count );
assert_smoke( 'AI parameter reached ability input', 'hello' === ( $result['result']['received']['message'] ?? null ) );
assert_smoke( 'payload parameter reached ability input', 42 === ( $result['result']['received']['job_id'] ?? null ) );
assert_smoke( 'ambient payload does not implicitly satisfy ability input', ! array_key_exists( 'job_id', $result['result']['received'] ?? array() ) );
assert_smoke( 'successful ability result still participates in post tracking', 1 === post_tracking_count() );

echo "\n[ability:1b] Explicit context bindings satisfy runtime-owned parameters\n";
$bound_ability = new \Ability_Native_Smoke_Ability(
fn( $input ) => isset( $input['message'], $input['job_id'] ),
fn( $input ) => array(
'success' => true,
'received' => $input,
)
);
$registry->register_for_smoke( 'datamachine/bound-ability', $bound_ability );
$result = execute_tool(
'bound_ability_tool',
array( 'message' => 'hello' ),
array(
'ability' => 'datamachine/bound-ability',
'client_context_bindings' => array( 'job_id' ),
'parameters' => array(
'message' => array(
'type' => 'string',
'required' => true,
),
'job_id' => array(
'type' => 'integer',
'required' => true,
),
),
)
);
assert_smoke( 'explicit binding executes successfully', true === ( $result['success'] ?? false ) );
assert_smoke( 'explicit binding passed job_id from runtime context', 42 === ( $result['result']['received']['job_id'] ?? null ) );

echo "\n[core:1] Generic execution core runs without Data Machine decorators\n";
// @phpstan-ignore-next-line smoke-test stub property shadows production class.
\DataMachine\Core\WordPress\PostTracking::$stored = array();
Expand Down Expand Up @@ -318,10 +348,37 @@ function execute_tool( string $tool_name, array $tool_parameters, array $tool_de
)
);
assert_smoke( 'preview policy returns staged action result', true === ( $result['staged'] ?? false ) && 'pending_1' === ( $result['action_id'] ?? null ) );
assert_smoke( 'pending action helper received complete merged parameters', 42 === first_pending_apply_job_id() );
assert_smoke( 'pending action helper does not receive undeclared ambient job_id', null === first_pending_apply_job_id() );
assert_smoke( 'preview policy does not execute ability directly', 0 === ability_execute_count( $preview_ability ) );
assert_smoke( 'preview policy does not post-track unexecuted action', 0 === post_tracking_count() );

echo "\n[decorator:2] Staged policy fails closed without pending-action metadata\n";
// @phpstan-ignore-next-line smoke-test stub property shadows production class.
PendingActionHelper::$staged = array();
$missing_metadata_ability = new \Ability_Native_Smoke_Ability(
fn( $input ) => true,
fn( $input ) => array( 'success' => true )
);
$registry->register_for_smoke( 'datamachine/missing-metadata-ability', $missing_metadata_ability );
$result = execute_tool(
'preview_missing_metadata_tool',
array( 'message' => 'needs approval' ),
array(
'ability' => 'datamachine/missing-metadata-ability',
'action_policy' => ActionPolicyResolver::POLICY_PREVIEW,
'parameters' => array(
'message' => array(
'type' => 'string',
'required' => true,
),
),
)
);
assert_smoke( 'preview without action_kind fails closed', false === ( $result['success'] ?? true ) );
assert_smoke( 'missing metadata error is machine-readable', 'missing_pending_action_metadata' === ( $result['metadata']['error_type'] ?? null ) );
assert_smoke( 'preview without action_kind does not stage ambiguous action', 0 === count( PendingActionHelper::$staged ) );
assert_smoke( 'preview without action_kind does not execute ability directly', 0 === ability_execute_count( $missing_metadata_ability ) );

echo "\n[ability:2] Linked ability takes precedence over class/method metadata\n";
LegacyTool::$calls = 0;
$result = execute_tool(
Expand Down
Loading