diff --git a/backend/app/services/agent_runtime/model_step_service.py b/backend/app/services/agent_runtime/model_step_service.py index 16e73d454..af6f335cf 100644 --- a/backend/app/services/agent_runtime/model_step_service.py +++ b/backend/app/services/agent_runtime/model_step_service.py @@ -76,6 +76,7 @@ ToolResultStoreError, ) from app.services.agent_runtime.tool_registry import ( + RUNTIME_TOOL_BINDING_KEY, resolve_registered_tool, ) from app.services.agent_tools import get_runtime_agent_tools_for_llm @@ -426,6 +427,16 @@ def _application_tools_for_model( ] +def _provider_tools(tools: Sequence[Mapping[str, object]]) -> list[dict]: + """Remove Runtime-only routing facts before sending Tool schemas to a model.""" + result: list[dict] = [] + for tool in tools: + model_tool = deepcopy(dict(tool)) + model_tool.pop(RUNTIME_TOOL_BINDING_KEY, None) + result.append(model_tool) + return result + + def _runtime_workset_entry(tool: Mapping[str, object]) -> ToolWorksetEntry: """Join one model definition to a stable, secret-free execution route.""" name = _tool_name(tool) @@ -449,7 +460,16 @@ def _runtime_workset_entry(tool: Mapping[str, object]) -> ToolWorksetEntry: dynamic_mcp_names=dynamic_mcp_names, ) if registered is not None: - return registered.to_workset_entry() + entry = registered.to_workset_entry() + raw_binding = tool.get(RUNTIME_TOOL_BINDING_KEY) + if raw_binding is None: + return entry + binding = ToolExecutionBinding.from_json(raw_binding) + if binding.kind != "mcp" or binding.handler_key != name: + raise ToolContractError( + "Runtime Tool binding does not match its model definition" + ) + return replace(entry, binding=binding) if name in GROUP_READ_TOOL_NAMES: effect, retry_policy = "read", "safe" binding_kind = "group" @@ -740,7 +760,18 @@ def _model_message_content(raw: Mapping[str, object], build: RuntimeContextBuild resumed_content = payload.get("content") if isinstance(resumed_content, (str, list)): return parse_multimodal_content(resumed_content) - return _message_content(content) + model_content = _message_content(content) + status = raw.get("execution_status") + if raw.get("role") != "tool" or status not in {"failed", "unknown"}: + return model_content + if not isinstance(model_content, str): + return model_content + label = "Tool failed" if status == "failed" else "Tool outcome is unknown" + result = f"{label}: {model_content}" + remediation = raw.get("safe_remediation") + if isinstance(remediation, str) and remediation.strip(): + result += f"\n\nSuggested correction: {remediation.strip()}" + return result def _prompt_messages( @@ -841,6 +872,10 @@ def append_history(raw: Mapping[str, object]) -> None: content=_model_message_content(raw, build), tool_calls=provider_tool_calls, tool_call_id=provider_tool_call_id, + is_error=( + role == "tool" + and raw.get("execution_status") in {"failed", "unknown"} + ), reasoning_content=( cast(str, raw.get("reasoning_content")) if isinstance(raw.get("reasoning_content"), str) else None ), @@ -1456,7 +1491,7 @@ async def compact_inputs( model, requested_max_output_tokens=requested_output, static_prompt_tokens=fixed_prompt_tokens, - tool_schema_tokens=_estimate_tokens(tools), + tool_schema_tokens=_estimate_tokens(_provider_tools(tools)), reserved_runtime_tokens=256, safety_margin_tokens=256, compact_threshold_ratio=0.80, @@ -1511,7 +1546,7 @@ async def _prepare_messages( model, requested_max_output_tokens=requested_output, static_prompt_tokens=fixed_prompt_tokens, - tool_schema_tokens=_estimate_tokens(tools), + tool_schema_tokens=_estimate_tokens(_provider_tools(tools)), reserved_runtime_tokens=256, safety_margin_tokens=256, ) @@ -1661,7 +1696,7 @@ async def _call_prepared( return await self._completion( model, messages, - tools=tools, + tools=_provider_tools(tools), agent_id=agent.id, supports_vision=bool(model.supports_vision), ) diff --git a/backend/app/services/agent_runtime/tool_contracts.py b/backend/app/services/agent_runtime/tool_contracts.py index 0096a83b2..3223395ec 100644 --- a/backend/app/services/agent_runtime/tool_contracts.py +++ b/backend/app/services/agent_runtime/tool_contracts.py @@ -66,7 +66,13 @@ def __post_init__(self) -> None: "runtime_default", 60.0, 300.0, "stop_waiting_only" ), "network_read": ToolDeadlinePolicy( - "network_read", 30.0, 60.0, "stop_waiting_only" + "network_read", 60.0, 60.0, "stop_waiting_only" + ), + "image_generation": ToolDeadlinePolicy( + "image_generation", 120.0, 120.0, "stop_waiting_only" + ), + "custom_image_generation": ToolDeadlinePolicy( + "custom_image_generation", 600.0, 600.0, "stop_waiting_only" ), "local_code": ToolDeadlinePolicy( "local_code", 30.0, 3600.0, "cooperative" @@ -93,6 +99,14 @@ def deadline_policy_for_tool(tool_name: str) -> ToolDeadlinePolicy: return _DEADLINE_POLICIES["agentbay_read"] if tool_name in {"read_emails", "read_webpage", "jina_read"}: return _DEADLINE_POLICIES["network_read"] + if tool_name == "generate_image_custom": + return _DEADLINE_POLICIES["custom_image_generation"] + if tool_name in { + "generate_image_siliconflow", + "generate_image_openai", + "generate_image_google", + }: + return _DEADLINE_POLICIES["image_generation"] return _DEADLINE_POLICIES["runtime_default"] diff --git a/backend/app/services/agent_runtime/tool_execution.py b/backend/app/services/agent_runtime/tool_execution.py index 612340c0c..292559191 100644 --- a/backend/app/services/agent_runtime/tool_execution.py +++ b/backend/app/services/agent_runtime/tool_execution.py @@ -25,6 +25,7 @@ from app.models.agent_run import AgentRun from app.models.agent_tool_execution import AgentToolExecution +from app.services.builtin_tool_definitions import BUILTIN_TOOL_NAMES ToolExecutionStatus = Literal[ "not_started", @@ -2015,7 +2016,7 @@ async def reconcile_unknown_tool_execution( if not is_user_reconcilable_unknown_execution(execution): raise ToolExecutionError( "tool_execution_reconciliation_not_supported", - "manual reconciliation is only supported for conditional write_file or image-generation receipts", + "manual reconciliation is not supported for this Tool receipt", ) prior_metadata = ( @@ -2082,12 +2083,21 @@ def is_user_reconcilable_unknown_execution(execution: AgentToolExecution) -> boo new tool call, so the original provider request is never replayed. """ effect, retry_policy = _execution_metadata(execution) + contract_version = getattr(execution, "contract_version", None) + tool_name = str(getattr(execution, "tool_name", "") or "") + is_registered_dynamic_mcp = ( + tool_name not in BUILTIN_TOOL_NAMES + and isinstance(contract_version, str) + and contract_version.startswith(f"registered:{tool_name}:") + and effect == "external_write" + and retry_policy == "never" + ) return ( - execution.tool_name == "write_file" + tool_name == "write_file" and effect == "write" and retry_policy == "conditional" ) or ( - execution.tool_name in _IMAGE_GENERATION_TOOL_NAMES + tool_name in _IMAGE_GENERATION_TOOL_NAMES and effect == "external_write" and retry_policy == "never" - ) + ) or is_registered_dynamic_mcp diff --git a/backend/app/services/agent_runtime/tool_registry.py b/backend/app/services/agent_runtime/tool_registry.py index 979adf55a..b23a85ee9 100644 --- a/backend/app/services/agent_runtime/tool_registry.py +++ b/backend/app/services/agent_runtime/tool_registry.py @@ -30,6 +30,8 @@ is_reserved_custom_tool_name, ) +RUNTIME_TOOL_BINDING_KEY = "_runtime_binding" + def _function_contract(model_definition: Mapping[str, object]) -> tuple[str, JsonObject]: function = model_definition.get("function") @@ -188,6 +190,7 @@ def resolve_registered_tool( __all__ = [ + "RUNTIME_TOOL_BINDING_KEY", "STATIC_REGISTERED_TOOL_NAMES", "RegisteredTool", "registered_dynamic_mcp", diff --git a/backend/app/services/agent_runtime/tool_step_service.py b/backend/app/services/agent_runtime/tool_step_service.py index 1865e5940..286856892 100644 --- a/backend/app/services/agent_runtime/tool_step_service.py +++ b/backend/app/services/agent_runtime/tool_step_service.py @@ -114,7 +114,6 @@ get_runtime_agent_tools_for_llm, validate_feishu_approval_create_arguments, ) -from app.services.autonomy_service import autonomy_service from app.services.builtin_tool_definitions import ( BUILTIN_TOOL_NAMES, builtin_cross_space_action, @@ -219,6 +218,7 @@ async def __call__( runtime_execution_id: str | None = None, runtime_lease_owner: str | None = None, runtime_tenant_id: str | None = None, + execution_binding: Mapping[str, object] | None = None, ) -> ToolExecutionOutcome | str: ... @@ -577,6 +577,7 @@ def _async_pending_step_result( run_id: uuid.UUID, execution_id: uuid.UUID, call_id: str, + origin_call_id: str, tool_name: str, outcome: ToolExecutionOutcome, prior_messages: Sequence[JsonObject], @@ -633,6 +634,7 @@ def _async_pending_step_result( "tool_calls": [poll_call], "runtime_intent": "async_poll", "runtime_run_id": str(run_id), + "runtime_origin_tool_call_id": origin_call_id, } return ToolStepResult( messages=( @@ -1557,9 +1559,13 @@ async def _execute_application_with_controls( "runtime_tenant_id": context.tenant_id, } try: + if accepted.entry.binding.kind == "mcp": + executor_arguments["execution_binding"] = ( + accepted.entry.binding.to_json() + ) operation_task = asyncio.create_task( self._tool_executor( - accepted.entry.tool_name, + accepted.entry.binding.handler_key, arguments, agent.id, ( @@ -1741,6 +1747,7 @@ def _group_unknown_failure( outcome: ToolExecutionOutcome, messages: Sequence[JsonObject], pending_tool_calls: Sequence[JsonObject], + step_tool_context: JsonObject | None = None, ) -> ToolStepResult: """End an unresumable Group Run without creating a user interrupt.""" normalized, _ = normalize_tool_outcome( @@ -1770,6 +1777,7 @@ def _group_unknown_failure( ), ), pending_tool_calls=tuple(pending_tool_calls), + step_tool_context=step_tool_context, error={"code": error_code, "message": error_message}, ) @@ -1923,6 +1931,7 @@ async def execute_pending( tool_calls: tuple[JsonObject, ...], ) -> ToolStepResult: step_context_update: JsonObject | None = None + async_origin_call_id: str | None = None try: tenant_id = uuid.UUID(context.tenant_id) run_id = uuid.UUID(context.run_id) @@ -1953,7 +1962,18 @@ async def execute_pending( ) except ToolContractError as exc: raise ToolExecutionError("tool_context_corrupt", str(exc)) from exc - if ( + is_async_poll = assistant_message.get("runtime_intent") == "async_poll" + if is_async_poll: + raw_origin_call_id = assistant_message.get( + "runtime_origin_tool_call_id" + ) + if not isinstance(raw_origin_call_id, str) or not raw_origin_call_id: + raise ToolExecutionError( + "tool_context_corrupt", + "async poll is missing its origin Tool Call ID", + ) + async_origin_call_id = raw_origin_call_id + elif ( step_context is not None and step_context.assistant_message_id != assistant_message_id ): @@ -1991,6 +2011,7 @@ async def execute_pending( tools=legacy_tools, ) step_context_update = step_context.to_json() + async_origin_call_id = None logger.warning( "[RuntimeToolCompatibility] event=legacy_tool_context_resolved " "run_id={} assistant_message_id={} accepted_call_count={} " @@ -2015,15 +2036,27 @@ async def execute_pending( step_tool_context=step_context_update, ) call_id, tool_name, arguments = _call_fields(call) - accepted = ( - _accepted_call( + if async_origin_call_id is not None: + origin_call = _accepted_call( step_context, - call_id=call_id, + call_id=async_origin_call_id, tool_name=tool_name, ) - if step_context is not None - else None - ) + accepted = AcceptedToolCall( + call_instance_id=call_id, + provider_call_id=None, + entry=origin_call.entry, + ) + else: + accepted = ( + _accepted_call( + step_context, + call_id=call_id, + tool_name=tool_name, + ) + if step_context is not None + else None + ) if accepted is None: # pragma: no cover - new contexts are mandatory here raise ToolExecutionError( "tool_context_corrupt", @@ -2199,6 +2232,7 @@ async def execute_pending( run_id=run_id, execution_id=reservation.execution.id, call_id=call_id, + origin_call_id=async_origin_call_id or call_id, tool_name=tool_name, outcome=reservation.reusable_result, prior_messages=messages, @@ -2243,6 +2277,7 @@ async def execute_pending( messages=tuple(messages), waiting_request=waiting_request, pending_tool_calls=tool_calls[index + 1 :], + step_tool_context=step_context_update, ) continue if reservation.blocked: @@ -2356,6 +2391,7 @@ async def execute_pending( outcome=outcome, messages=messages, pending_tool_calls=tool_calls[index + 1 :], + step_tool_context=step_context_update, ) messages.append( _result_message( @@ -2399,6 +2435,7 @@ async def execute_pending( outcome=outcome, messages=messages, pending_tool_calls=tool_calls[index + 1 :], + step_tool_context=step_context_update, ) messages.append( _result_message( @@ -2421,6 +2458,7 @@ async def execute_pending( outcome=execution_outcome(reservation.execution), messages=messages, pending_tool_calls=tool_calls[index + 1 :], + step_tool_context=step_context_update, ) return ToolStepResult( messages=tuple(messages), @@ -2431,6 +2469,7 @@ async def execute_pending( error_code=reservation.error_code, ), pending_tool_calls=tool_calls[index:], + step_tool_context=step_context_update, ) if autonomy_outcome is not None: @@ -2522,6 +2561,7 @@ async def execute_pending( outcome=outcome, messages=messages, pending_tool_calls=tool_calls[index + 1 :], + step_tool_context=step_context_update, ) return ToolStepResult( messages=tuple(messages), @@ -2532,6 +2572,7 @@ async def execute_pending( error_code="tool_outcome_unknown", ), pending_tool_calls=tool_calls[index:], + step_tool_context=step_context_update, ) else: if a2a_result is not None: @@ -2547,6 +2588,7 @@ async def execute_pending( outcome=a2a_result.outcome, messages=messages, pending_tool_calls=tool_calls[index + 1 :], + step_tool_context=step_context_update, ) messages.append( _result_message( @@ -2561,6 +2603,7 @@ async def execute_pending( messages=tuple(messages), waiting_request=a2a_result.waiting_request, pending_tool_calls=tool_calls[index + 1 :], + step_tool_context=step_context_update, ) continue @@ -2678,6 +2721,7 @@ async def execute_pending( outcome=outcome, messages=messages, pending_tool_calls=tool_calls[index + 1 :], + step_tool_context=step_context_update, ) return ToolStepResult( messages=tuple(messages), @@ -2688,6 +2732,7 @@ async def execute_pending( error_code="tool_outcome_unknown", ), pending_tool_calls=tool_calls[index:], + step_tool_context=step_context_update, ) else: if isinstance(raw_result, ToolExecutionOutcome): @@ -2749,6 +2794,7 @@ async def execute_pending( run_id=run_id, execution_id=reservation.execution.id, call_id=call_id, + origin_call_id=async_origin_call_id or call_id, tool_name=tool_name, outcome=outcome, prior_messages=messages, @@ -2764,6 +2810,7 @@ async def execute_pending( outcome=outcome, messages=messages, pending_tool_calls=tool_calls[index + 1 :], + step_tool_context=step_context_update, ) return ToolStepResult( messages=tuple(messages), @@ -2774,6 +2821,7 @@ async def execute_pending( error_code=outcome.error_code or "tool_outcome_unknown", ), pending_tool_calls=tool_calls[index:], + step_tool_context=step_context_update, ) messages.append( _result_message( @@ -2798,13 +2846,15 @@ async def execute_pending( except ToolExecutionError as exc: return ToolStepResult( error={"code": exc.code, "message": str(exc)}, + step_tool_context=step_context_update, ) except Exception as exc: return ToolStepResult( error={ "code": "tool_execution_failed", "message": f"Runtime tool step failed: {type(exc).__name__}", - } + }, + step_tool_context=step_context_update, ) diff --git a/backend/app/services/agent_runtime/tool_validation.py b/backend/app/services/agent_runtime/tool_validation.py index 07ffdfe3b..d07a57a7d 100644 --- a/backend/app/services/agent_runtime/tool_validation.py +++ b/backend/app/services/agent_runtime/tool_validation.py @@ -3,8 +3,11 @@ from __future__ import annotations import math +import re +import uuid from collections.abc import Mapping from dataclasses import dataclass +from urllib.parse import urlparse from app.services.agent_runtime.state import JsonObject @@ -64,6 +67,39 @@ def _schema_object(value: object, *, field_name: str) -> Mapping[str, object]: return value +def _positive_integer(value: object, *, field_name: str) -> int: + if isinstance(value, bool) or not isinstance(value, int) or value < 0: + raise ToolValidationContractError(f"{field_name} must be a non-negative integer") + return value + + +def _number(value: object, *, field_name: str) -> int | float: + if ( + isinstance(value, bool) + or not isinstance(value, (int, float)) + or not math.isfinite(value) + ): + raise ToolValidationContractError(f"{field_name} must be a finite number") + return value + + +def _matching_subschema( + value: object, + schema: object, + *, + field_name: str, + path: str, +) -> tuple[bool, list[ToolValidationIssue]]: + candidate_issues: list[ToolValidationIssue] = [] + _validate( + value, + _schema_object(schema, field_name=field_name), + path=path, + issues=candidate_issues, + ) + return not candidate_issues, candidate_issues + + def _validate( value: object, schema: Mapping[str, object], @@ -102,6 +138,76 @@ def _validate( if value not in enum: issues.append(_issue("enum", path, f"{path} must use one allowed value.")) + if "const" in schema and value != schema["const"]: + issues.append(_issue("const", path, f"{path} must use the required value.")) + + if isinstance(value, str): + if "minLength" in schema: + minimum_length = _positive_integer( + schema["minLength"], field_name="schema minLength" + ) + if len(value) < minimum_length: + issues.append( + _issue( + "min_length", + path, + f"{path} must contain at least {minimum_length} characters.", + ) + ) + if "maxLength" in schema: + maximum_length = _positive_integer( + schema["maxLength"], field_name="schema maxLength" + ) + if len(value) > maximum_length: + issues.append( + _issue( + "max_length", + path, + f"{path} must contain at most {maximum_length} characters.", + ) + ) + pattern = schema.get("pattern") + if pattern is not None: + if not isinstance(pattern, str): + raise ToolValidationContractError("schema pattern must be text") + try: + matches = re.search(pattern, value) is not None + except re.error as exc: + raise ToolValidationContractError("schema pattern is invalid") from exc + if not matches: + issues.append( + _issue("pattern", path, f"{path} does not match the required format.") + ) + format_name = schema.get("format") + if format_name is not None: + if format_name == "uuid": + try: + uuid.UUID(value) + except ValueError: + issues.append(_issue("format", path, f"{path} must be a UUID.")) + elif format_name == "uri": + parsed = urlparse(value) + if not parsed.scheme or not parsed.netloc: + issues.append(_issue("format", path, f"{path} must be a URI.")) + else: + raise ToolValidationContractError( + f"unsupported schema format {format_name!r}" + ) + + if isinstance(value, (int, float)) and not isinstance(value, bool): + if "minimum" in schema: + minimum = _number(schema["minimum"], field_name="schema minimum") + if value < minimum: + issues.append( + _issue("minimum", path, f"{path} must be at least {minimum}.") + ) + if "maximum" in schema: + maximum = _number(schema["maximum"], field_name="schema maximum") + if value > maximum: + issues.append( + _issue("maximum", path, f"{path} must be at most {maximum}.") + ) + if isinstance(value, Mapping): raw_properties = schema.get("properties", {}) properties = _schema_object(raw_properties, field_name="schema properties") @@ -122,6 +228,30 @@ def _validate( ) if len(issues) >= MAX_VALIDATION_ISSUES: return + dependent_required = schema.get("dependentRequired", {}) + dependent_required = _schema_object( + dependent_required, + field_name="schema dependentRequired", + ) + for trigger, dependencies in dependent_required.items(): + if not isinstance(dependencies, list) or any( + not isinstance(item, str) for item in dependencies + ): + raise ToolValidationContractError( + "schema dependentRequired entries must be arrays of text" + ) + if trigger not in value: + continue + for dependency in dependencies: + if dependency not in value: + dependency_path = _path(path, dependency) + issues.append( + _issue( + "dependent_required", + dependency_path, + f"{dependency_path} is required when {_path(path, trigger)} is provided.", + ) + ) for property_name, property_schema in properties.items(): if property_name not in value: continue @@ -165,12 +295,25 @@ def _validate( if len(issues) >= MAX_VALIDATION_ISSUES: return - if isinstance(value, list) and "items" in schema: - item_schema = _schema_object(schema["items"], field_name="schema items") - for index, item in enumerate(value): - _validate(item, item_schema, path=f"{path}[{index}]", issues=issues) - if len(issues) >= MAX_VALIDATION_ISSUES: - return + if isinstance(value, list): + if "minItems" in schema: + minimum_items = _positive_integer( + schema["minItems"], field_name="schema minItems" + ) + if len(value) < minimum_items: + issues.append( + _issue( + "min_items", + path, + f"{path} must contain at least {minimum_items} items.", + ) + ) + if "items" in schema: + item_schema = _schema_object(schema["items"], field_name="schema items") + for index, item in enumerate(value): + _validate(item, item_schema, path=f"{path}[{index}]", issues=issues) + if len(issues) >= MAX_VALIDATION_ISSUES: + return alternatives = schema.get("anyOf") if alternatives is not None: @@ -178,15 +321,13 @@ def _validate( raise ToolValidationContractError("schema anyOf must be a non-empty array") matched = False for alternative in alternatives: - candidate_issues: list[ToolValidationIssue] = [] - _validate( + matched, _ = _matching_subschema( value, - _schema_object(alternative, field_name="schema anyOf entry"), + alternative, + field_name="schema anyOf entry", path=path, - issues=candidate_issues, ) - if not candidate_issues: - matched = True + if matched: break if not matched: issues.append( @@ -197,6 +338,58 @@ def _validate( ) ) + alternatives = schema.get("oneOf") + if alternatives is not None: + if not isinstance(alternatives, list) or not alternatives: + raise ToolValidationContractError("schema oneOf must be a non-empty array") + match_count = sum( + _matching_subschema( + value, + alternative, + field_name="schema oneOf entry", + path=path, + )[0] + for alternative in alternatives + ) + if match_count != 1: + issues.append( + _issue( + "one_of", + path, + f"{path} must satisfy exactly one accepted argument shape.", + ) + ) + + combined = schema.get("allOf") + if combined is not None: + if not isinstance(combined, list) or not combined: + raise ToolValidationContractError("schema allOf must be a non-empty array") + for entry in combined: + _, entry_issues = _matching_subschema( + value, + entry, + field_name="schema allOf entry", + path=path, + ) + issues.extend(entry_issues[: MAX_VALIDATION_ISSUES - len(issues)]) + + condition = schema.get("if") + if condition is not None: + condition_matches, _ = _matching_subschema( + value, + condition, + field_name="schema if", + path=path, + ) + branch_name = "then" if condition_matches else "else" + if branch_name in schema: + _validate( + value, + _schema_object(schema[branch_name], field_name=f"schema {branch_name}"), + path=path, + issues=issues, + ) + def validate_tool_arguments( arguments: JsonObject, diff --git a/backend/app/services/agent_tools.py b/backend/app/services/agent_tools.py index c8088af0e..c8ba3f060 100644 --- a/backend/app/services/agent_tools.py +++ b/backend/app/services/agent_tools.py @@ -97,6 +97,8 @@ sanitize_tool_arguments, ) from app.services.agent_runtime.tool_contracts import ( + ToolContractError, + ToolExecutionBinding, resolve_tool_deadline_seconds, ) from app.services.agent_runtime.feishu_approval_authorization import ( @@ -105,6 +107,7 @@ verify_feishu_approval_create_authorization, ) from app.services.agent_runtime.tool_registry import ( + RUNTIME_TOOL_BINDING_KEY, STATIC_REGISTERED_TOOL_NAMES, resolve_registered_tool, ) @@ -1126,10 +1129,31 @@ async def _agent_is_designated_okr_agent(agent_id: uuid.UUID) -> bool: return False -async def _get_runtime_dynamic_mcp_tool_names( +def _mcp_route_digest( + *, + server_url: str, + server_name: str, + raw_name: str, + async_completion: object, +) -> str: + encoded = json.dumps( + { + "server_url": server_url, + "server_name": server_name, + "raw_name": raw_name, + "async_completion": async_completion, + }, + ensure_ascii=False, + sort_keys=True, + separators=(",", ":"), + ).encode("utf-8") + return hashlib.sha256(encoded).hexdigest() + + +async def _get_runtime_dynamic_mcp_bindings( agent_id: uuid.UUID, -) -> set[str]: - """Resolve locally ready dynamic MCP names without provider I/O.""" +) -> dict[str, dict]: + """Resolve ready MCP tools and freeze their secret-free route identity.""" from urllib.parse import urlparse from app.models.tool import AgentTool, Tool @@ -1137,7 +1161,7 @@ async def _get_runtime_dynamic_mcp_tool_names( try: async with async_session() as db: result = await db.execute( - select(Tool) + select(Tool, AgentTool) .join(AgentTool, AgentTool.tool_id == Tool.id) .where( AgentTool.agent_id == agent_id, @@ -1146,24 +1170,25 @@ async def _get_runtime_dynamic_mcp_tool_names( Tool.type == "mcp", ) ) - tools = result.scalars().all() + rows = result.all() except Exception as exc: logger.warning( - "[Tools] Dynamic MCP readiness lookup failed: {}", + "[Tools] Dynamic MCP binding lookup failed: {}", type(exc).__name__, ) - return set() + return {} - ready: set[str] = set() - for tool in tools: - name = str(tool.name or "") + bindings: dict[str, dict] = {} + for tool, assignment in rows: + name = str(tool.name or "").strip() server_url = str(tool.mcp_server_url or "").strip() + raw_name = str(tool.mcp_tool_name or "").strip() parsed = urlparse(server_url) if ( not name or name in BUILTIN_TOOL_NAMES or is_reserved_custom_tool_name(name) - or not str(tool.mcp_tool_name or "").strip() + or not raw_name or parsed.scheme not in {"http", "https"} or not parsed.netloc ): @@ -1172,14 +1197,29 @@ async def _get_runtime_dynamic_mcp_tool_names( name or "", ) continue - ready.add(name) - return _project_active_tool_descriptions(ready) + binding = ToolExecutionBinding( + kind="mcp", + handler_key=name, + target={ + "tool_id": str(tool.id), + "route_digest": _mcp_route_digest( + server_url=server_url, + server_name=str(tool.mcp_server_name or ""), + raw_name=raw_name, + async_completion=(tool.config or {}).get("async_completion"), + ), + }, + credential_ref=str(assignment.id), + ) + bindings[name] = binding.to_json() + return bindings async def get_runtime_agent_tools_for_llm(agent_id: uuid.UUID) -> list[dict]: """Resolve the current Durable Runtime workset with typed-outcome gating.""" tools = await get_agent_tools_for_llm(agent_id) - dynamic_mcp_names = await _get_runtime_dynamic_mcp_tool_names(agent_id) + dynamic_mcp_bindings = await _get_runtime_dynamic_mcp_bindings(agent_id) + dynamic_mcp_names = set(dynamic_mcp_bindings) resolved = _runtime_typed_tools( tools, dynamic_mcp_names=dynamic_mcp_names, @@ -1188,6 +1228,9 @@ async def get_runtime_agent_tools_for_llm(agent_id: uuid.UUID) -> list[dict]: is_designated_okr_agent: bool | None = None for tool in resolved: name = str(tool.get("function", {}).get("name") or "") + if name in dynamic_mcp_bindings: + tool = deepcopy(tool) + tool[RUNTIME_TOOL_BINDING_KEY] = dynamic_mcp_bindings[name] if name in _OKR_AGENT_ONLY_TOOL_NAMES: if is_designated_okr_agent is None: is_designated_okr_agent = ( @@ -2673,6 +2716,7 @@ async def execute_builtin_tool_outcome( runtime_execution_id: str | None = None, runtime_lease_owner: str | None = None, runtime_tenant_id: str | None = None, + execution_binding: Mapping[str, object] | None = None, ) -> ToolExecutionOutcome | str: """Execute only explicitly migrated builtin branches as typed outcomes. @@ -3058,7 +3102,13 @@ async def execute_builtin_tool_outcome( and tool_name not in BUILTIN_TOOL_NAMES and not is_reserved_custom_tool_name(tool_name) ): - mcp_target = await _resolve_mcp_execution_target(tool_name, agent_id) + if execution_binding is not None: + mcp_target = await _resolve_frozen_mcp_execution_target( + execution_binding, + agent_id, + ) + else: + mcp_target = await _resolve_mcp_execution_target(tool_name, agent_id) if mcp_target is not None: return await _execute_resolved_mcp_target_outcome( mcp_target, @@ -6043,6 +6093,83 @@ def _mcp_call_response_outcome( return _typed_success(summary, metadata=metadata) +async def _resolve_frozen_mcp_execution_target( + raw_binding: Mapping[str, object], + agent_id: uuid.UUID, +) -> dict: + """Resolve credentials for one frozen route and reject live route drift.""" + from app.models.tool import AgentTool, Tool + + try: + binding = ToolExecutionBinding.from_json(raw_binding) + if binding.kind != "mcp" or binding.credential_ref is None: + raise ToolContractError("MCP execution binding is incomplete") + tool_id = uuid.UUID(str(binding.target.get("tool_id") or "")) + assignment_id = uuid.UUID(binding.credential_ref) + except (ToolContractError, ValueError, TypeError): + return { + "full_name": str(raw_binding.get("handler_key") or "mcp"), + "unavailable_error_code": "mcp_binding_invalid", + } + + async with async_session() as db: + tool_result = await db.execute( + select(Tool).where(Tool.id == tool_id, Tool.type == "mcp") + ) + tool = tool_result.scalar_one_or_none() + assignment_result = await db.execute( + select(AgentTool).where( + AgentTool.id == assignment_id, + AgentTool.agent_id == agent_id, + AgentTool.tool_id == tool_id, + ) + ) + assignment = assignment_result.scalar_one_or_none() + + if tool is None or assignment is None or not tool.enabled or not assignment.enabled: + return { + "full_name": binding.handler_key, + "unavailable_error_code": "mcp_tool_not_available", + } + + server_url = str(tool.mcp_server_url or "").strip() + server_name = str(tool.mcp_server_name or "") + raw_name = str(tool.mcp_tool_name or "").strip() + current_route_digest = _mcp_route_digest( + server_url=server_url, + server_name=server_name, + raw_name=raw_name, + async_completion=(tool.config or {}).get("async_completion"), + ) + if ( + str(tool.name or "") != binding.handler_key + or binding.target.get("route_digest") != current_route_digest + ): + return { + "full_name": binding.handler_key, + "unavailable_error_code": "mcp_binding_changed", + } + + merged_config = { + **(tool.config or {}), + **(assignment.config or {}), + } + merged_config = _decrypt_sensitive_fields( + merged_config, + tool.config_schema, + ) + return { + "full_name": binding.handler_key, + "raw_name": raw_name, + "server_url": server_url, + "server_name": server_name, + "config": merged_config, + "async_completion": deepcopy( + (tool.config or {}).get("async_completion") + ), + } + + async def _resolve_mcp_execution_target( tool_name: str, agent_id, @@ -6155,8 +6282,21 @@ async def _execute_resolved_mcp_target_outcome( ) -> ToolExecutionOutcome: unavailable_error = target.get("unavailable_error_code") if unavailable_error: - return _typed_failure( + summary = { + "mcp_binding_changed": ( + "MCP tool configuration changed after this call was selected. " + "Refresh the available tools before retrying." + ), + "mcp_binding_invalid": ( + "The saved MCP execution route is invalid. Refresh the " + "available tools before retrying." + ), + }.get( + str(unavailable_error), "MCP tool is not enabled, assigned, or locally configured.", + ) + return _typed_failure( + summary, str(unavailable_error), ) diff --git a/backend/app/services/builtin_tool_definitions.py b/backend/app/services/builtin_tool_definitions.py index 761a88680..fcbcbfb0e 100644 --- a/backend/app/services/builtin_tool_definitions.py +++ b/backend/app/services/builtin_tool_definitions.py @@ -3936,7 +3936,7 @@ "generate_image_siliconflow": 120, "generate_image_openai": 120, "generate_image_google": 120, - "generate_image_custom": 120, + "generate_image_custom": 600, } diff --git a/backend/app/services/llm/client.py b/backend/app/services/llm/client.py index f866e5780..48e8c35bc 100644 --- a/backend/app/services/llm/client.py +++ b/backend/app/services/llm/client.py @@ -239,6 +239,7 @@ class LLMMessage: content: str | list | None = None tool_calls: list[dict] | None = None tool_call_id: str | None = None + is_error: bool = False reasoning_content: str | None = None reasoning_signature: str | None = None dynamic_content: str | None = None @@ -303,6 +304,7 @@ def to_anthropic_format(self) -> dict | None: "type": "tool_result", "tool_use_id": self.tool_call_id, "content": result_content, + "is_error": self.is_error, } ] } @@ -1564,19 +1566,6 @@ def _content_to_gemini_parts(self, content: Any) -> list[dict[str, Any]]: return [{"text": str(content)}] - def _extract_tool_name_map(self, messages: list[LLMMessage]) -> dict[str, str]: - """Build tool_call_id -> function_name map from assistant messages.""" - out: dict[str, str] = {} - for msg in messages: - if msg.role != "assistant" or not msg.tool_calls: - continue - for tc in msg.tool_calls: - tc_id = tc.get("id") - tc_name = tc.get("function", {}).get("name") - if tc_id and tc_name: - out[tc_id] = tc_name - return out - def _convert_tools(self, tools: list[dict] | None) -> tuple[list[dict[str, Any]] | None, dict[str, Any] | None]: """Convert OpenAI-style tools to Gemini function declarations.""" if not tools: @@ -1617,7 +1606,7 @@ def _build_payload( messages = normalize_provider_messages(messages) system_blocks: list[str] = [] contents: list[dict[str, Any]] = [] - tool_name_map = self._extract_tool_name_map(messages) + pending_tool_names: dict[str, str] = {} for msg in messages: if msg.role == "system": @@ -1630,16 +1619,22 @@ def _build_payload( continue if msg.role == "user": + pending_tool_names = {} parts = self._content_to_gemini_parts(msg.content) if parts: contents.append({"role": "user", "parts": parts}) continue if msg.role == "assistant": + pending_tool_names = {} parts = self._content_to_gemini_parts(msg.content) if msg.tool_calls: for tc in msg.tool_calls: fn = tc.get("function", {}) + tc_id = tc.get("id") + tc_name = fn.get("name") + if tc_id and tc_name: + pending_tool_names[tc_id] = tc_name args = fn.get("arguments", "{}") if isinstance(args, str): try: @@ -1666,21 +1661,21 @@ def _build_payload( continue if msg.role == "tool": - name = tool_name_map.get(msg.tool_call_id or "", msg.tool_call_id or "tool_result") + name = pending_tool_names.get(msg.tool_call_id or "", msg.tool_call_id or "tool_result") response_content = msg.content or "" if isinstance(response_content, str): try: parsed = json.loads(response_content) - if isinstance(parsed, dict): - response_obj: dict[str, Any] = parsed - else: - response_obj = {"result": parsed} + response_value: Any = parsed except json.JSONDecodeError: - response_obj = {"result": response_content} + response_value = response_content elif isinstance(response_content, dict): - response_obj = response_content + response_value = response_content else: - response_obj = {"result": str(response_content)} + response_value = str(response_content) + response_obj = { + "error" if msg.is_error else "output": response_value, + } contents.append({ "role": "user", diff --git a/backend/tests/test_agent_runtime_model_step_service.py b/backend/tests/test_agent_runtime_model_step_service.py index b1d6ccdc3..c5900fae2 100644 --- a/backend/tests/test_agent_runtime_model_step_service.py +++ b/backend/tests/test_agent_runtime_model_step_service.py @@ -20,7 +20,9 @@ RuntimeModelStepService, _group_mention_mismatches, _message_token_counter, + _provider_tools, _prompt_messages, + _runtime_workset_entry, _tool_repair_reset_reason, _visible_mention_names, ) @@ -32,6 +34,7 @@ runtime_message_to_json, ) from app.services.agent_runtime.tool_contracts import parse_step_tool_context +from app.services.agent_runtime.tool_registry import RUNTIME_TOOL_BINDING_KEY from app.services.llm.finish import FINISH_PROTOCOL_REMINDER from app.services.llm.single_step import LLMCompletionStep from app.services.token_tracker import TokenUsage @@ -43,6 +46,39 @@ _TINY_PNG_DATA_URL = f"data:image/png;base64,{_TINY_PNG_BASE64}" +def test_runtime_binding_is_checkpointed_but_not_sent_to_provider() -> None: + tool_id = uuid.uuid4() + assignment_id = uuid.uuid4() + tool = { + "type": "function", + "function": { + "name": "tenant_search", + "description": "Search the tenant source", + "parameters": {"type": "object", "properties": {}}, + }, + RUNTIME_TOOL_BINDING_KEY: { + "kind": "mcp", + "handler_key": "tenant_search", + "target": { + "tool_id": str(tool_id), + "route_digest": "digest", + }, + "credential_ref": str(assignment_id), + }, + } + + entry = _runtime_workset_entry(tool) + + assert entry.binding.target["tool_id"] == str(tool_id) + assert entry.binding.credential_ref == str(assignment_id) + assert _provider_tools((tool,)) == [ + { + "type": "function", + "function": tool["function"], + } + ] + + class _Result: def __init__(self, values=None) -> None: self.values = list(values or []) @@ -351,6 +387,60 @@ def test_prompt_messages_restore_provider_tool_call_pairing() -> None: assert tool.tool_call_id == "provider-call-1" +@pytest.mark.parametrize( + ("status", "label"), + (("failed", "Tool failed"), ("unknown", "Tool outcome is unknown")), +) +def test_prompt_messages_make_tool_failure_actionable_for_the_model( + status: str, + label: str, +) -> None: + build = _build( + current_run={"run_id": str(uuid.uuid4()), "goal": "Write"}, + recent_session_messages_snapshot=(), + recent_thread_messages=( + { + "id": "assistant-1", + "role": "assistant", + "content": "", + "tool_calls": [ + { + "id": "call-instance-1", + "type": "function", + "function": { + "name": "write_file", + "arguments": "{}", + }, + } + ], + }, + { + "id": "tool-result-1", + "role": "tool", + "tool_call_id": "call-instance-1", + "content": "$.path is required", + "execution_status": status, + "safe_remediation": "Provide a non-empty path.", + }, + ), + initial_input={"input_content": "Continue"}, + ) + + messages = _prompt_messages( + static_prompt="Static", + dynamic_prompt="Dynamic", + build=build, + ) + + tool = next(message for message in messages if message.role == "tool") + assert tool.tool_call_id == "call-instance-1" + assert tool.is_error is True + assert tool.content == ( + f"{label}: $.path is required\n\n" + "Suggested correction: Provide a non-empty path." + ) + + def test_message_budget_does_not_treat_large_base64_as_text_tokens() -> None: padded_png = base64.b64encode( base64.b64decode(_TINY_PNG_BASE64) + b"x" * (1024 * 1024) diff --git a/backend/tests/test_agent_runtime_tool_contracts.py b/backend/tests/test_agent_runtime_tool_contracts.py index 548fbe093..b0ea8f1e0 100644 --- a/backend/tests/test_agent_runtime_tool_contracts.py +++ b/backend/tests/test_agent_runtime_tool_contracts.py @@ -8,9 +8,34 @@ ToolContractError, ToolExecutionBinding, ToolWorksetEntry, + deadline_policy_for_tool, + resolve_tool_deadline_seconds, parse_step_tool_context, workset_version, ) +from app.services.builtin_tool_definitions import BUILTIN_TOOL_DEFINITIONS + + +def test_runtime_deadlines_cover_declared_network_and_image_provider_budgets() -> None: + expected = { + "read_webpage": 60.0, + "jina_read": 60.0, + "generate_image_siliconflow": 120.0, + "generate_image_openai": 120.0, + "generate_image_google": 120.0, + "generate_image_custom": 600.0, + } + + assert { + name: resolve_tool_deadline_seconds(deadline_policy_for_tool(name).name) + for name in expected + } == expected + declared = { + item["name"]: float(item["timeout_seconds"]) + for item in BUILTIN_TOOL_DEFINITIONS + if item["name"] in expected + } + assert declared == expected def _entry() -> ToolWorksetEntry: diff --git a/backend/tests/test_agent_runtime_tool_step_service.py b/backend/tests/test_agent_runtime_tool_step_service.py index a0f7fb2a2..d3766898b 100644 --- a/backend/tests/test_agent_runtime_tool_step_service.py +++ b/backend/tests/test_agent_runtime_tool_step_service.py @@ -527,7 +527,8 @@ async def test_invalid_group_at_arguments_return_failed_tool_result_for_repair() assert result.error is None assert result.pending_group_at_changed is False assert result.messages[0]["execution_status"] == "failed" - assert result.messages[0]["error_code"] == "group_at_arguments_invalid" + assert result.messages[0]["error_code"] == "tool_arguments_invalid" + assert "UUID" in result.messages[0]["content"] @pytest.mark.asyncio @@ -1135,6 +1136,82 @@ async def mark(db, **kwargs): assert result.messages[0]["execution_status"] == "succeeded" +@pytest.mark.asyncio +async def test_mcp_checkpoint_dispatches_the_frozen_execution_binding( + monkeypatch, +) -> None: + tenant_id = uuid.uuid4() + agent = _agent(tenant_id) + call = _call("call-frozen-mcp", "mcp.demo.lookup") + state = _state(tenant_id, agent, (call,)) + entry = ToolWorksetEntry( + tool_name="mcp.demo.lookup", + contract_version="registered:mcp.demo.lookup:v1", + parameters_schema={"type": "object", "properties": {}}, + binding=ToolExecutionBinding( + kind="mcp", + handler_key="mcp.demo.lookup", + target={ + "tool_id": str(uuid.uuid4()), + "route_digest": "digest", + }, + credential_ref=str(uuid.uuid4()), + ), + effect="external_write", + retry_policy="never", + ) + state["lifecycle"]["step_tool_context"] = StepToolContext( + assistant_message_id="assistant-message-1", + model_step=1, + workset_version=workset_version((entry,)), + accepted_calls=( + AcceptedToolCall( + call_instance_id="call-frozen-mcp", + provider_call_id="provider-frozen-mcp", + entry=entry, + ), + ), + ).to_json() + context = _context(state) + execution = _execution( + tenant_id, + uuid.UUID(context.run_id), + "call-frozen-mcp", + "mcp.demo.lookup", + ) + dispatched: list[tuple[tuple, dict]] = [] + + async def reserve(db, **kwargs): + del db, kwargs + return _reservation(execution) + + async def execute(*args, **kwargs): + dispatched.append((args, kwargs)) + return ToolExecutionOutcome( + status="succeeded", + result_summary="frozen result", + result_ref=None, + ) + + async def mark(db, **kwargs): + del db, kwargs + execution.status = "succeeded" + execution.result_summary = "frozen result" + return execution + + monkeypatch.setattr(tool_step_service, "reserve_tool_execution", reserve) + monkeypatch.setattr(tool_step_service, "mark_tool_execution_succeeded", mark) + result = await _service(agent, _CancelSource(None), execute).execute_pending( + state, + context, + (call,), + ) + + assert result.error is None + assert dispatched[0][0][0] == "mcp.demo.lookup" + assert dispatched[0][1]["execution_binding"] == entry.binding.to_json() + + @pytest.mark.asyncio async def test_new_checkpoint_context_mismatch_fails_before_provider_or_receipt() -> None: tenant_id = uuid.uuid4() @@ -1258,6 +1335,147 @@ async def mark(db, **kwargs): assert provider_calls == 1 +@pytest.mark.asyncio +async def test_legacy_unknown_wait_keeps_resolved_context_on_resume( + monkeypatch, +) -> None: + tenant_id = uuid.uuid4() + agent = _agent(tenant_id) + call = _call("legacy-unknown", "write_file") + state = _state(tenant_id, agent, (call,)) + context = _context(state) + execution = _execution( + tenant_id, + uuid.UUID(context.run_id), + "legacy-unknown", + "write_file", + ) + provider_calls = 0 + + async def tools_once(agent_id): + nonlocal provider_calls + del agent_id + provider_calls += 1 + if provider_calls > 1: + raise AssertionError("legacy wait rebuilt its Workset") + return await _tools(agent.id) + + async def reserve(db, **kwargs): + del db, kwargs + return _reservation( + execution, + blocked=True, + requires_confirmation=True, + error_code="tool_outcome_unknown", + ) + + monkeypatch.setattr(tool_step_service, "reserve_tool_execution", reserve) + service = tool_step_service.RuntimeToolStepService( + session_factory=_session_factory(agent), + cancel_source=_CancelSource(None, None), + tool_provider=tools_once, + tool_executor=_unexpected_executor, + ) + + first = await service.execute_pending(state, context, (call,)) + assert first.step_tool_context is not None + state["lifecycle"]["step_tool_context"] = first.step_tool_context + second = await service.execute_pending(state, context, (call,)) + + assert first.waiting_request is not None + assert second.waiting_request is not None + assert provider_calls == 1 + + +@pytest.mark.asyncio +async def test_legacy_a2a_wait_keeps_context_for_tail_call(monkeypatch) -> None: + tenant_id = uuid.uuid4() + agent = _agent(tenant_id) + delegate = _a2a_call("legacy-delegate", mode="task_delegate") + tail = _call("legacy-tail", "read_file") + state = _state(tenant_id, agent, (delegate, tail)) + context = _context(state) + executions = { + "legacy-delegate": _execution( + tenant_id, + uuid.UUID(context.run_id), + "legacy-delegate", + "send_message_to_agent", + ), + "legacy-tail": _execution( + tenant_id, + uuid.UUID(context.run_id), + "legacy-tail", + "read_file", + ), + } + provider_calls = 0 + + async def tools_once(agent_id): + nonlocal provider_calls + del agent_id + provider_calls += 1 + if provider_calls > 1: + raise AssertionError("legacy A2A wait rebuilt its Workset") + return await _tools(agent.id) + + async def reserve(db, **kwargs): + del db + return _reservation(executions[kwargs["tool_call_id"]]) + + async def execute(name, *args, **kwargs): + del args, kwargs + return ToolExecutionOutcome( + status="succeeded", + result_summary=f"{name} done", + result_ref=None, + ) + + async def mark(db, **kwargs): + del db + execution = next( + item for item in executions.values() if item.id == kwargs["execution_id"] + ) + execution.status = "succeeded" + execution.result_summary = kwargs["result_summary"] + return execution + + monkeypatch.setattr(tool_step_service, "reserve_tool_execution", reserve) + monkeypatch.setattr(tool_step_service, "mark_tool_execution_succeeded", mark) + a2a = _A2AService( + A2ARuntimeToolResult( + outcome=ToolExecutionOutcome( + status="succeeded", + result_summary="accepted", + result_ref="agent-run:target", + ), + target_run_id=uuid.uuid4(), + waiting_request={ + "waiting_type": "agent", + "correlation_id": "a2a:legacy", + "reason": "waiting_for_task_delegate", + }, + ) + ) + service = tool_step_service.RuntimeToolStepService( + session_factory=_session_factory(agent), + cancel_source=_CancelSource(None, None), + tool_provider=tools_once, + tool_executor=execute, + a2a_service=a2a, + ) + + first = await service.execute_pending(state, context, (delegate, tail)) + assert first.step_tool_context is not None + state["lifecycle"]["step_tool_context"] = first.step_tool_context + state["lifecycle"]["pending_tool_calls"] = [tail] + second = await service.execute_pending(state, context, (tail,)) + + assert first.waiting_request is not None + assert second.error is None + assert provider_calls == 1 + + @pytest.mark.asyncio async def test_legacy_batch_records_compatibility_usage_and_explicit_delete_gate( monkeypatch, @@ -1430,6 +1648,141 @@ async def terminal_forbidden(*args, **kwargs): assert result.messages[1]["tool_calls"] == [poll_call] +@pytest.mark.asyncio +async def test_async_poll_reuses_the_origin_frozen_tool_context(monkeypatch) -> None: + tenant_id = uuid.uuid4() + agent = _agent(tenant_id) + launch_call = _call("call-async-resume", "read_file") + state = _state(tenant_id, agent, (launch_call,)) + _with_step_tool_context(state, launch_call) + context = _context(state) + executions = deque( + [ + _execution( + tenant_id, + uuid.UUID(context.run_id), + "call-async-resume", + "read_file", + ), + _execution( + tenant_id, + uuid.UUID(context.run_id), + "poll-call", + "read_file", + ), + _execution( + tenant_id, + uuid.UUID(context.run_id), + "poll-call-2", + "read_file", + ), + ] + ) + def async_outcome(status: str) -> ToolExecutionOutcome: + pending = status == "pending" + operation = { + "version": 1, + "operation_key": "operation-key", + "operation_id": "op-1", + "state": "running" if pending else "success", + } + if pending: + operation["poll"] = { + "tool": "read_file", + "arguments": {"operation_id": "op-1"}, + "interval_ms": 0, + } + return ToolExecutionOutcome( + status=status, # type: ignore[arg-type] + result_summary="still running" if pending else "done", + result_ref=None, + metadata={ + "runtime_async_pending": pending, + "async_operation": operation, + }, + ) + + outcomes = deque( + [async_outcome("pending"), async_outcome("pending"), async_outcome("succeeded")] + ) + dispatched_arguments: list[dict] = [] + + async def reserve(db, **kwargs): + del db, kwargs + return _reservation(executions.popleft()) + + async def execute(tool_name, arguments, *args, **kwargs): + del tool_name, args, kwargs + dispatched_arguments.append(arguments) + return outcomes.popleft() + + async def mark_pending(db, **kwargs): + del db + execution = _execution( + tenant_id, + uuid.UUID(context.run_id), + "call-async-resume", + "read_file", + ) + execution.id = uuid.UUID(str(kwargs["execution_id"])) + execution.result_metadata = kwargs["metadata"] + return execution + + async def settle_async(db, **kwargs): + del db + execution = _execution( + tenant_id, + uuid.UUID(context.run_id), + "poll-call", + "read_file", + ) + execution.status = kwargs["status"] + execution.result_metadata = kwargs["metadata"] + return execution + + monkeypatch.setattr(tool_step_service, "reserve_tool_execution", reserve) + monkeypatch.setattr( + tool_step_service, + "mark_tool_execution_async_pending", + mark_pending, + ) + monkeypatch.setattr( + tool_step_service, + "settle_async_operation_executions", + settle_async, + ) + service = _service(agent, _CancelSource(None, None, None), execute) + + launch = await service.execute_pending(state, context, (launch_call,)) + poll_call = launch.pending_tool_calls[0] + state["lifecycle"]["run_messages"] = [ + *state["lifecycle"]["run_messages"], + *launch.messages, + ] + state["lifecycle"]["pending_tool_calls"] = [poll_call] + + first_poll = await service.execute_pending(state, context, (poll_call,)) + next_poll_call = first_poll.pending_tool_calls[0] + state["lifecycle"]["run_messages"] = [ + *state["lifecycle"]["run_messages"], + *first_poll.messages, + ] + state["lifecycle"]["pending_tool_calls"] = [next_poll_call] + + poll = await service.execute_pending(state, context, (next_poll_call,)) + + assert poll.error is None + assert poll.messages[-1]["execution_status"] == "succeeded" + assert dispatched_arguments == [ + {}, + {"operation_id": "op-1"}, + {"operation_id": "op-1"}, + ] + assert state["lifecycle"]["step_tool_context"]["assistant_message_id"] == ( + "assistant-message-1" + ) + + @pytest.mark.asyncio async def test_terminal_async_poll_settles_same_run_operation( monkeypatch, diff --git a/backend/tests/test_agent_runtime_tool_validation.py b/backend/tests/test_agent_runtime_tool_validation.py index d8fe5f058..ab8b8ba06 100644 --- a/backend/tests/test_agent_runtime_tool_validation.py +++ b/backend/tests/test_agent_runtime_tool_validation.py @@ -1,6 +1,14 @@ """Accepted Tool schema validation contract tests.""" +import pytest + from app.services.agent_runtime.tool_validation import validate_tool_arguments +from app.services.builtin_tool_definitions import BUILTIN_TOOL_DEFINITIONS + + +_BUILTIN_SCHEMAS = { + item["name"]: item["parameters_schema"] for item in BUILTIN_TOOL_DEFINITIONS +} def _schema() -> dict: @@ -90,3 +98,64 @@ def test_any_of_required_alternatives_accept_one_complete_branch() -> None: assert validate_tool_arguments({"document_id": "doc-1"}, schema) == () issues = validate_tool_arguments({}, schema) assert [(issue.code, issue.path) for issue in issues] == [("any_of", "$")] + + +@pytest.mark.parametrize( + ("tool_name", "arguments", "expected_code"), + [ + ("upload_image", {}, "one_of"), + ("send_email", {"to": "", "subject": "", "body": ""}, "min_length"), + ("write_file", {"path": "x", "content": "x" * 6001}, "max_length"), + ("query_directory", {"limit": 0}, "minimum"), + ("query_directory", {"limit": 51}, "maximum"), + ( + "vercel_deploy", + {"project_name": "demo", "deploy_method": "upload"}, + "required", + ), + ], +) +def test_builtin_schema_constraints_are_enforced_before_execution( + tool_name: str, + arguments: dict, + expected_code: str, +) -> None: + issues = validate_tool_arguments(arguments, _BUILTIN_SCHEMAS[tool_name]) + + assert expected_code in {issue.code for issue in issues} + + +def test_const_pattern_format_dependent_required_and_min_items() -> None: + schema = { + "type": "object", + "properties": { + "mode": {"const": "safe"}, + "path": {"type": "string", "pattern": "^[a-z]+$"}, + "request_id": {"type": "string", "format": "uuid"}, + "url": {"type": "string", "format": "uri"}, + "token": {"type": "string"}, + "secret": {"type": "string"}, + "targets": {"type": "array", "minItems": 1}, + }, + "dependentRequired": {"token": ["secret"]}, + } + + issues = validate_tool_arguments( + { + "mode": "unsafe", + "path": "../bad", + "request_id": "not-a-uuid", + "url": "not-a-uri", + "token": "present", + "targets": [], + }, + schema, + ) + + assert {issue.code for issue in issues} == { + "const", + "pattern", + "format", + "dependent_required", + "min_items", + } diff --git a/backend/tests/test_agent_tools_agentbay_a0.py b/backend/tests/test_agent_tools_agentbay_a0.py index 880e65f0f..472bd72b8 100644 --- a/backend/tests/test_agent_tools_agentbay_a0.py +++ b/backend/tests/test_agent_tools_agentbay_a0.py @@ -119,7 +119,7 @@ def __init__(self, *args, **kwargs): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned_tools) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic_mcp, ) monkeypatch.setattr(agent_tools, "_get_tool_config", local_tool_config) diff --git a/backend/tests/test_agent_tools_deadlines.py b/backend/tests/test_agent_tools_deadlines.py index b05516bbc..db7dd3632 100644 --- a/backend/tests/test_agent_tools_deadlines.py +++ b/backend/tests/test_agent_tools_deadlines.py @@ -18,7 +18,7 @@ def test_deadline_precedence_is_explicit_then_default_capped_by_policy() -> None: - assert resolve_tool_deadline_seconds("network_read") == 30 + assert resolve_tool_deadline_seconds("network_read") == 60 assert resolve_tool_deadline_seconds("network_read", 12) == 12 assert resolve_tool_deadline_seconds("network_read", 120) == 60 assert deadline_policy_for_tool("read_emails").name == "network_read" diff --git a/backend/tests/test_agent_tools_deploy_contracts.py b/backend/tests/test_agent_tools_deploy_contracts.py index 4f210cfbc..a105e43ba 100644 --- a/backend/tests/test_agent_tools_deploy_contracts.py +++ b/backend/tests/test_agent_tools_deploy_contracts.py @@ -219,7 +219,7 @@ async def no_dynamic_mcp(_agent_id): monkeypatch.setattr(agent_tools, "_get_tool_config", config) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic_mcp, ) monkeypatch.setattr( diff --git a/backend/tests/test_agent_tools_email_contracts.py b/backend/tests/test_agent_tools_email_contracts.py index f57e9fac5..a718a8fb7 100644 --- a/backend/tests/test_agent_tools_email_contracts.py +++ b/backend/tests/test_agent_tools_email_contracts.py @@ -86,7 +86,7 @@ async def no_dynamic_mcp(_agent_id): monkeypatch.setattr(agent_tools, "_get_email_config", email_config) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic_mcp, ) if include_untyped_email_writes: diff --git a/backend/tests/test_agent_tools_legacy_contract_compatibility.py b/backend/tests/test_agent_tools_legacy_contract_compatibility.py index 1cecf14a5..62c4b9891 100644 --- a/backend/tests/test_agent_tools_legacy_contract_compatibility.py +++ b/backend/tests/test_agent_tools_legacy_contract_compatibility.py @@ -1,6 +1,8 @@ from __future__ import annotations +from contextlib import asynccontextmanager from pathlib import Path +from types import SimpleNamespace import uuid import pytest @@ -11,6 +13,30 @@ from app.services.builtin_tool_definitions import builtin_model_definition +class _ScalarResult: + def __init__(self, value) -> None: + self.value = value + + def scalar_one_or_none(self): + return self.value + + +def _mcp_binding_session(tool, assignment): + @asynccontextmanager + async def factory(): + class Session: + def __init__(self) -> None: + self.results = iter((tool, assignment)) + + async def execute(self, statement): + del statement + return _ScalarResult(next(self.results)) + + yield Session() + + return factory + + def _definition(name: str) -> dict: definition = builtin_model_definition(name) assert definition is not None @@ -237,3 +263,142 @@ async def execute(resolved_target, arguments, *, agent_id): assert isinstance(outcome, ToolExecutionOutcome) assert outcome.status == "succeeded" assert calls == [(target, {"query": "contract"}, agent_id)] + + +@pytest.mark.asyncio +async def test_registered_dynamic_mcp_uses_frozen_binding_without_name_lookup( + monkeypatch: pytest.MonkeyPatch, +) -> None: + agent_id = uuid.uuid4() + binding = { + "kind": "mcp", + "handler_key": "tenant_search", + "target": { + "tool_id": str(uuid.uuid4()), + "route_digest": "digest", + }, + "credential_ref": str(uuid.uuid4()), + } + target = { + "full_name": "tenant_search", + "raw_name": "search", + "server_url": "https://frozen.example/mcp", + "config": {}, + } + calls: list[tuple[dict, dict, uuid.UUID]] = [] + + async def live_name_lookup_forbidden(*args, **kwargs): + raise AssertionError(f"frozen binding used live name lookup: {args}, {kwargs}") + + async def resolve_frozen(raw_binding, resolved_agent_id): + assert raw_binding == binding + assert resolved_agent_id == agent_id + return target + + async def execute(resolved_target, arguments, *, agent_id): + calls.append((resolved_target, arguments, agent_id)) + return ToolExecutionOutcome( + status="succeeded", + result_summary="MCP typed receipt", + result_ref=None, + ) + + monkeypatch.setattr( + agent_tools, + "_resolve_mcp_execution_target", + live_name_lookup_forbidden, + ) + monkeypatch.setattr( + agent_tools, + "_resolve_frozen_mcp_execution_target", + resolve_frozen, + raising=False, + ) + monkeypatch.setattr( + agent_tools, + "_execute_resolved_mcp_target_outcome", + execute, + ) + + outcome = await agent_tools.execute_builtin_tool_outcome( + "tenant_search", + {"query": "contract"}, + agent_id, + uuid.uuid4(), + execution_binding=binding, + ) + + assert isinstance(outcome, ToolExecutionOutcome) + assert outcome.status == "succeeded" + assert calls == [(target, {"query": "contract"}, agent_id)] + + +@pytest.mark.asyncio +async def test_frozen_mcp_binding_resolves_assignment_and_rejects_route_drift( + monkeypatch: pytest.MonkeyPatch, +) -> None: + agent_id = uuid.uuid4() + tool_id = uuid.uuid4() + assignment_id = uuid.uuid4() + tool = SimpleNamespace( + id=tool_id, + name="tenant_search", + enabled=True, + mcp_server_url="https://frozen.example/mcp", + mcp_server_name="search", + mcp_tool_name="lookup", + config={}, + config_schema={}, + ) + assignment = SimpleNamespace( + id=assignment_id, + agent_id=agent_id, + tool_id=tool_id, + enabled=True, + config={}, + ) + monkeypatch.setattr( + agent_tools, + "async_session", + _mcp_binding_session(tool, assignment), + ) + + binding = { + "kind": "mcp", + "handler_key": "tenant_search", + "target": { + "tool_id": str(tool_id), + "route_digest": agent_tools._mcp_route_digest( + server_url="https://frozen.example/mcp", + server_name="search", + raw_name="lookup", + async_completion=None, + ), + }, + "credential_ref": str(assignment_id), + } + + target = await agent_tools._resolve_frozen_mcp_execution_target( + binding, + agent_id, + ) + + assert target == { + "full_name": "tenant_search", + "raw_name": "lookup", + "server_url": "https://frozen.example/mcp", + "server_name": "search", + "config": {}, + "async_completion": None, + } + + tool.mcp_server_url = "https://changed.example/mcp" + target = await agent_tools._resolve_frozen_mcp_execution_target( + binding, + agent_id, + ) + + assert target == { + "full_name": "tenant_search", + "unavailable_error_code": "mcp_binding_changed", + } diff --git a/backend/tests/test_agent_tools_okr_contracts.py b/backend/tests/test_agent_tools_okr_contracts.py index 7d28bf60c..ccf7db2b4 100644 --- a/backend/tests/test_agent_tools_okr_contracts.py +++ b/backend/tests/test_agent_tools_okr_contracts.py @@ -387,7 +387,7 @@ async def not_designated(_agent_id): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic, ) monkeypatch.setattr( @@ -433,7 +433,7 @@ async def designated(_agent_id): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic, ) monkeypatch.setattr( @@ -476,7 +476,7 @@ async def designated(_agent_id): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic, ) monkeypatch.setattr( diff --git a/backend/tests/test_agent_tools_typed_agentbay_reads.py b/backend/tests/test_agent_tools_typed_agentbay_reads.py index c80d61e3d..813321b08 100644 --- a/backend/tests/test_agent_tools_typed_agentbay_reads.py +++ b/backend/tests/test_agent_tools_typed_agentbay_reads.py @@ -435,7 +435,7 @@ def __init__(self, *_args, **_kwargs) -> None: monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned_tools) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic_mcp, ) monkeypatch.setattr(agent_tools, "_get_tool_config", local_config) diff --git a/backend/tests/test_agent_tools_typed_deploy_reads.py b/backend/tests/test_agent_tools_typed_deploy_reads.py index 9ef070602..bd26771e1 100644 --- a/backend/tests/test_agent_tools_typed_deploy_reads.py +++ b/backend/tests/test_agent_tools_typed_deploy_reads.py @@ -192,7 +192,7 @@ async def config(_agent_id, requested_name): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic_mcp, ) monkeypatch.setattr(agent_tools, "_get_tool_config", config) @@ -224,7 +224,7 @@ async def no_config(_agent_id, _requested_name): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic_mcp, ) monkeypatch.setattr(agent_tools, "_get_tool_config", no_config) diff --git a/backend/tests/test_agent_tools_typed_deploy_simple_writes.py b/backend/tests/test_agent_tools_typed_deploy_simple_writes.py index 3be8baf33..7bdc5db48 100644 --- a/backend/tests/test_agent_tools_typed_deploy_simple_writes.py +++ b/backend/tests/test_agent_tools_typed_deploy_simple_writes.py @@ -295,7 +295,7 @@ async def no_dynamic(_agent_id): monkeypatch.setattr(agent_tools, "_get_tool_config", config) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic, ) monkeypatch.setattr(httpx, "AsyncClient", NetworkMustNotBeUsed) diff --git a/backend/tests/test_agent_tools_typed_dynamic_mcp.py b/backend/tests/test_agent_tools_typed_dynamic_mcp.py index 468119449..5058d489b 100644 --- a/backend/tests/test_agent_tools_typed_dynamic_mcp.py +++ b/backend/tests/test_agent_tools_typed_dynamic_mcp.py @@ -24,6 +24,18 @@ def _tool(name: str) -> dict: } +def _binding(name: str) -> dict: + return { + "kind": "mcp", + "handler_key": name, + "target": { + "tool_id": str(uuid.uuid4()), + "route_digest": "digest", + }, + "credential_ref": str(uuid.uuid4()), + } + + def _async_completion_contract() -> dict: return { "version": 1, @@ -65,23 +77,14 @@ async def test_runtime_resolver_exposes_only_enabled_assigned_non_reserved_mcp( async def assigned(_agent_id): return tools - async def dynamic_names(_agent_id): - # The DB resolver returns only locally ready rows whose Tool and - # AgentTool records are both enabled. - return { - "mcp_visible_lookup", - "at", - "finish", - "wait", - "group_private_lookup", - "generate_image_openai", - } + async def dynamic_bindings(_agent_id): + return {"mcp_visible_lookup": _binding("mcp_visible_lookup")} monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", - dynamic_names, + "_get_runtime_dynamic_mcp_bindings", + dynamic_bindings, ) resolved = await agent_tools.get_runtime_agent_tools_for_llm(uuid.uuid4()) @@ -98,8 +101,8 @@ async def test_runtime_mcp_readiness_is_local_and_never_pings_provider( async def assigned(_agent_id): return [_tool("mcp_visible_lookup")] - async def dynamic_names(_agent_id): - return {"mcp_visible_lookup"} + async def dynamic_bindings(_agent_id): + return {"mcp_visible_lookup": _binding("mcp_visible_lookup")} async def network_forbidden(*_args, **_kwargs): raise AssertionError("model-step readiness must not ping MCP providers") @@ -107,8 +110,8 @@ async def network_forbidden(*_args, **_kwargs): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", - dynamic_names, + "_get_runtime_dynamic_mcp_bindings", + dynamic_bindings, ) monkeypatch.setattr(MCPClient, "list_tools", network_forbidden) diff --git a/backend/tests/test_agent_tools_typed_feishu_remaining.py b/backend/tests/test_agent_tools_typed_feishu_remaining.py index 52d44d67e..290f23b9c 100644 --- a/backend/tests/test_agent_tools_typed_feishu_remaining.py +++ b/backend/tests/test_agent_tools_typed_feishu_remaining.py @@ -341,7 +341,7 @@ async def no_dynamic(_agent_id): monkeypatch.setattr(agent_tools, "_agent_has_feishu", not_ready) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic, ) monkeypatch.setattr( @@ -378,7 +378,7 @@ async def no_dynamic(_agent_id): monkeypatch.setattr(agent_tools, "_agent_has_feishu", ready) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic, ) monkeypatch.setattr( @@ -416,7 +416,7 @@ async def no_dynamic(_agent_id): monkeypatch.setattr(agent_tools, "_agent_has_feishu", ready) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic, ) diff --git a/backend/tests/test_agent_tools_typed_image_outcomes_v2.py b/backend/tests/test_agent_tools_typed_image_outcomes_v2.py index 1e36fd354..f0d5032c5 100644 --- a/backend/tests/test_agent_tools_typed_image_outcomes_v2.py +++ b/backend/tests/test_agent_tools_typed_image_outcomes_v2.py @@ -409,7 +409,7 @@ def __init__(self, *args, **kwargs): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic_mcp, ) monkeypatch.setattr(agent_tools, "_get_tool_config", configured) @@ -439,7 +439,7 @@ async def missing_config(_agent_id, _requested_name): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic_mcp, ) monkeypatch.setattr(agent_tools, "_get_tool_config", missing_config) diff --git a/backend/tests/test_agent_tools_typed_okr_jobs.py b/backend/tests/test_agent_tools_typed_okr_jobs.py index eb45a26bc..11221c291 100644 --- a/backend/tests/test_agent_tools_typed_okr_jobs.py +++ b/backend/tests/test_agent_tools_typed_okr_jobs.py @@ -236,7 +236,7 @@ async def designated(_agent_id): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic, ) monkeypatch.setattr( @@ -268,7 +268,7 @@ async def not_designated(_agent_id): monkeypatch.setattr(agent_tools, "get_agent_tools_for_llm", assigned) monkeypatch.setattr( agent_tools, - "_get_runtime_dynamic_mcp_tool_names", + "_get_runtime_dynamic_mcp_bindings", no_dynamic, ) monkeypatch.setattr( diff --git a/backend/tests/test_chat_session_runtime_state.py b/backend/tests/test_chat_session_runtime_state.py index c0bfb67c7..b4aa6e643 100644 --- a/backend/tests/test_chat_session_runtime_state.py +++ b/backend/tests/test_chat_session_runtime_state.py @@ -239,7 +239,17 @@ async def test_runtime_state_exposes_unknown_write_and_blocks_plain_resume() -> @pytest.mark.asyncio -async def test_runtime_state_exposes_unknown_image_generation_for_user_confirmation() -> None: +@pytest.mark.parametrize( + ("tool_name", "contract_version"), + [ + ("generate_image_openai", None), + ("tenant_search", "registered:tenant_search:0123456789abcdef"), + ], +) +async def test_runtime_state_exposes_reconcilable_unknown_tool_for_user_confirmation( + tool_name: str, + contract_version: str | None, +) -> None: agent, user, session, run = _records() reader = SimpleNamespace(get_run_state=AsyncMock(return_value=_view(run))) execution = AgentToolExecution( @@ -247,7 +257,8 @@ async def test_runtime_state_exposes_unknown_image_generation_for_user_confirmat tenant_id=run.tenant_id, run_id=run.id, tool_call_id="call-image-1", - tool_name="generate_image_openai", + tool_name=tool_name, + contract_version=contract_version, assistant_message_id="assistant-1", arguments_hash="hash", sanitized_arguments={}, @@ -287,7 +298,7 @@ async def test_runtime_state_exposes_unknown_image_generation_for_user_confirmat assert response.active_run is not None assert response.active_run.can_resume is False - assert response.active_run.pending_tool_reconciliations[0].tool_name == "generate_image_openai" + assert response.active_run.pending_tool_reconciliations[0].tool_name == tool_name assert response.active_run.pending_tool_reconciliations[0].can_reconcile is True diff --git a/backend/tests/test_llm_single_step.py b/backend/tests/test_llm_single_step.py index 91a1d659b..6e5347e7f 100644 --- a/backend/tests/test_llm_single_step.py +++ b/backend/tests/test_llm_single_step.py @@ -119,6 +119,135 @@ def test_native_gemini_preserves_dynamic_system_context_once() -> None: ] +def test_native_gemini_pairs_reused_tool_call_ids_with_their_assistant_turn() -> None: + client = GeminiClient(api_key="test", model="gemini-test") + + payload = client._build_payload( + [ + LLMMessage(role="user", content="Inspect and then update the record"), + LLMMessage( + role="assistant", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": {"name": "lookup_record", "arguments": "{}"}, + "_gemini_extra": {"id": "provider-call-1"}, + }, + { + "id": "call_2", + "type": "function", + "function": {"name": "read_policy", "arguments": "{}"}, + "_gemini_extra": {"id": "provider-call-2"}, + }, + ], + ), + LLMMessage(role="tool", tool_call_id="call_1", content='{"record_id":"r1"}'), + LLMMessage(role="tool", tool_call_id="call_2", content='{"allowed":true}'), + LLMMessage( + role="assistant", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": {"name": "update_record", "arguments": '{"id":"r1"}'}, + "_gemini_extra": {"id": "provider-call-1"}, + } + ], + ), + LLMMessage(role="tool", tool_call_id="call_1", content='{"updated":true}'), + ], + tools=None, + temperature=0.2, + max_tokens=1024, + ) + + function_response_names = [ + content["parts"][0]["functionResponse"]["name"] + for content in payload["contents"] + if "functionResponse" in content["parts"][0] + ] + assert function_response_names == ["lookup_record", "read_policy", "update_record"] + function_call_ids = [ + part["functionCall"]["id"] + for content in payload["contents"] + for part in content["parts"] + if "functionCall" in part + ] + assert function_call_ids == ["provider-call-1", "provider-call-2", "provider-call-1"] + + +def test_tool_failure_uses_provider_native_error_signals() -> None: + tool_result = LLMMessage( + role="tool", + tool_call_id="call_1", + content="Tool failed: path is required", + is_error=True, + ) + + anthropic = tool_result.to_anthropic_format() + assert anthropic is not None + assert anthropic["content"][0]["is_error"] is True + + gemini = GeminiClient(api_key="test", model="gemini-test")._build_payload( + [ + LLMMessage( + role="assistant", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": {"name": "write_file", "arguments": "{}"}, + } + ], + ), + tool_result, + ], + tools=None, + temperature=0.2, + max_tokens=1024, + ) + response = gemini["contents"][-1]["parts"][0]["functionResponse"]["response"] + assert response == {"error": "Tool failed: path is required"} + + gemini_success = GeminiClient( + api_key="test", + model="gemini-test", + )._build_payload( + [ + LLMMessage( + role="assistant", + tool_calls=[ + { + "id": "call_1", + "type": "function", + "function": {"name": "read_file", "arguments": "{}"}, + } + ], + ), + LLMMessage( + role="tool", + tool_call_id="call_1", + content='{"path":"README.md"}', + ), + ], + tools=None, + temperature=0.2, + max_tokens=1024, + ) + success_response = gemini_success["contents"][-1]["parts"][0][ + "functionResponse" + ]["response"] + assert success_response == {"output": {"path": "README.md"}} + + openai = tool_result.to_openai_format() + assert openai == { + "role": "tool", + "content": "Tool failed: path is required", + "tool_call_id": "call_1", + } + + def test_provider_payloads_preserve_static_and_dynamic_system_context_once() -> None: messages = [ LLMMessage( diff --git a/backend/tests/test_runtime_schema.py b/backend/tests/test_runtime_schema.py index 5311803cc..41199b7ea 100644 --- a/backend/tests/test_runtime_schema.py +++ b/backend/tests/test_runtime_schema.py @@ -279,8 +279,10 @@ def test_agent_tool_execution_model_captures_idempotency_and_lease_contract(): "tenant_id", "run_id", "tool_call_id", + "provider_call_id", "tool_name", "assistant_message_id", + "contract_version", "arguments_hash", "sanitized_arguments", "request_ref", diff --git a/backend/tests/test_tool_execution.py b/backend/tests/test_tool_execution.py index 37157be5f..cb9fe2887 100644 --- a/backend/tests/test_tool_execution.py +++ b/backend/tests/test_tool_execution.py @@ -187,10 +187,16 @@ def _sql(statement) -> str: ], ) @pytest.mark.parametrize( - ("tool_name", "effect", "retry_policy"), + ("tool_name", "effect", "retry_policy", "contract_version"), [ - ("write_file", "write", "conditional"), - ("generate_image_openai", "external_write", "never"), + ("write_file", "write", "conditional", None), + ("generate_image_openai", "external_write", "never", None), + ( + "tenant_search", + "external_write", + "never", + "registered:tenant_search:0123456789abcdef", + ), ], ) async def test_user_reconcilable_unknown_receipt_can_be_settled( @@ -199,6 +205,7 @@ async def test_user_reconcilable_unknown_receipt_can_be_settled( tool_name: str, effect: str, retry_policy: str, + contract_version: str | None, ) -> None: tenant_id = uuid.uuid4() run_id = uuid.uuid4() @@ -211,6 +218,7 @@ async def test_user_reconcilable_unknown_receipt_can_be_settled( retry_policy=retry_policy, ) execution.tool_name = tool_name + execution.contract_version = contract_version execution.completed_at = _NOW db = _FakeSession(execution) @@ -249,7 +257,7 @@ async def test_unknown_reconciliation_rejects_unsupported_tool() -> None: with pytest.raises( tool_execution.ToolExecutionError, - match="only supported for conditional write_file or image-generation", + match="not supported for this Tool receipt", ): await tool_execution.reconcile_unknown_tool_execution( db, # type: ignore[arg-type]