From b8618d57ea6142c30af41067206c758cee67d873 Mon Sep 17 00:00:00 2001 From: Y1fe1Zh0u Date: Wed, 12 Aug 2026 14:26:28 +0800 Subject: [PATCH 1/9] Keep Gemini tool results paired with their originating turn Gemini reuses provider call identifiers across assistant turns, so a history-wide lookup allowed later calls to overwrite earlier tool names. Scope name resolution to the active assistant exchange while preserving provider wire identifiers. Constraint: Gemini provider call IDs are not globally unique across turns Rejected: Persist globally unique provider IDs | would alter provider wire identity and conflict with existing Runtime correlation Confidence: high Scope-risk: narrow Directive: Keep Tool Result name resolution exchange-local when modifying Gemini history lowering Tested: backend/tests/test_llm_single_step.py; LLM scoped tests; Ruff Not-tested: Live Gemini API --- backend/app/services/llm/client.py | 23 ++++------- backend/tests/test_llm_single_step.py | 58 +++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 15 deletions(-) diff --git a/backend/app/services/llm/client.py b/backend/app/services/llm/client.py index f866e5780..cc07cd329 100644 --- a/backend/app/services/llm/client.py +++ b/backend/app/services/llm/client.py @@ -1564,19 +1564,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 +1604,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 +1617,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,7 +1659,7 @@ 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: diff --git a/backend/tests/test_llm_single_step.py b/backend/tests/test_llm_single_step.py index 91a1d659b..88becf8b0 100644 --- a/backend/tests/test_llm_single_step.py +++ b/backend/tests/test_llm_single_step.py @@ -119,6 +119,64 @@ 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_provider_payloads_preserve_static_and_dynamic_system_context_once() -> None: messages = [ LLMMessage( From bed0797b3b23f2c9f1cee78a7540d3afd6db4488 Mon Sep 17 00:00:00 2001 From: Y1fe1Zh0u Date: Wed, 12 Aug 2026 15:58:28 +0800 Subject: [PATCH 2/9] Keep async Tool polls on their accepted contract Synthetic poll calls belong to the original accepted Tool call but use a new runtime call ID. Record the origin call ID on the synthetic Assistant message and derive the poll execution entry from the frozen StepToolContext without replacing checkpoint state. Constraint: Keep the fix local to the reproduced async poll context mismatch. Rejected: Add a separate async lifecycle state machine | unnecessary for preserving the accepted Tool contract. Confidence: high Scope-risk: narrow Directive: Do not resolve the ToolProvider again for a poll generated from an accepted Tool call. Tested: 130 scoped Runtime Tool, node executor, LLM, and runtime schema tests; scoped Ruff; git diff --check. Not-tested: Real MCP provider and process-restart end-to-end execution. --- .../agent_runtime/tool_step_service.py | 43 +++++- .../test_agent_runtime_tool_step_service.py | 135 ++++++++++++++++++ 2 files changed, 171 insertions(+), 7 deletions(-) diff --git a/backend/app/services/agent_runtime/tool_step_service.py b/backend/app/services/agent_runtime/tool_step_service.py index 1865e5940..20765c2fc 100644 --- a/backend/app/services/agent_runtime/tool_step_service.py +++ b/backend/app/services/agent_runtime/tool_step_service.py @@ -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=( @@ -1923,6 +1925,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 +1956,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 +2005,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 +2030,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 +2226,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, @@ -2749,6 +2777,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, diff --git a/backend/tests/test_agent_runtime_tool_step_service.py b/backend/tests/test_agent_runtime_tool_step_service.py index a0f7fb2a2..a8ed72f96 100644 --- a/backend/tests/test_agent_runtime_tool_step_service.py +++ b/backend/tests/test_agent_runtime_tool_step_service.py @@ -1430,6 +1430,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, From 0b9ecfcd45a1f6633e6f9569645c2ebf926cd200 Mon Sep 17 00:00:00 2001 From: Y1fe1Zh0u Date: Wed, 12 Aug 2026 16:24:03 +0800 Subject: [PATCH 3/9] Make Tool failures actionable to the model Runtime Tool results already retain sanitized failure summaries and optional remediation, but the model boundary discarded the failure signal and remediation text. Carry one provider-neutral error bit, render actionable failure text, and map it to native Anthropic and Gemini representations. Constraint: Keep Runtime control fields such as model_action and side_effect_state out of the model protocol. Rejected: Serialize the complete internal Tool outcome | couples provider prompts to Runtime control metadata. Confidence: high Scope-risk: narrow Directive: Expected Tool failures should provide sanitized result_summary and safe_remediation at their source. Tested: 151 scoped model-step, provider-boundary, and Tool-step tests; scoped Ruff; git diff --check. Not-tested: Live Anthropic, Gemini, and OpenAI provider calls. --- .../agent_runtime/model_step_service.py | 17 ++++- backend/app/services/llm/client.py | 16 +++-- .../test_agent_runtime_model_step_service.py | 54 ++++++++++++++ backend/tests/test_llm_single_step.py | 71 +++++++++++++++++++ 4 files changed, 150 insertions(+), 8 deletions(-) diff --git a/backend/app/services/agent_runtime/model_step_service.py b/backend/app/services/agent_runtime/model_step_service.py index 16e73d454..6935627db 100644 --- a/backend/app/services/agent_runtime/model_step_service.py +++ b/backend/app/services/agent_runtime/model_step_service.py @@ -740,7 +740,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 +852,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 ), diff --git a/backend/app/services/llm/client.py b/backend/app/services/llm/client.py index cc07cd329..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, } ] } @@ -1664,16 +1666,16 @@ def _build_payload( 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..0eaae82e5 100644 --- a/backend/tests/test_agent_runtime_model_step_service.py +++ b/backend/tests/test_agent_runtime_model_step_service.py @@ -351,6 +351,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_llm_single_step.py b/backend/tests/test_llm_single_step.py index 88becf8b0..6e5347e7f 100644 --- a/backend/tests/test_llm_single_step.py +++ b/backend/tests/test_llm_single_step.py @@ -177,6 +177,77 @@ def test_native_gemini_pairs_reused_tool_call_ids_with_their_assistant_turn() -> 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( From 923232a9e8fb6a827de38ab0d30be6534853c8ee Mon Sep 17 00:00:00 2001 From: Y1fe1Zh0u Date: Wed, 12 Aug 2026 16:53:25 +0800 Subject: [PATCH 4/9] Keep MCP execution on the route selected by the model step A model-visible MCP definition now carries a secret-free execution binding into the durable Tool context. The Tool step dispatches through that binding, resolves the exact assignment, and rejects route changes instead of re-selecting an endpoint by name. Readiness and binding creation share one database query, while provider payloads remain standard Tool schemas. Constraint: Live authorization, enablement, and credential values must still be checked at execution time. Rejected: Freeze decrypted credentials in the checkpoint | would persist secrets and prevent revocation or rotation. Rejected: Refactor the complete Tool registry | unnecessary for closing the execution-route correctness gap. Confidence: high Scope-risk: moderate Directive: Do not send _runtime_binding to model providers or replace exact assignment checks with name lookup. Tested: 969 Tool, Model Step, and agent_tools pytest cases; scoped Ruff; py_compile; git diff --check. Not-tested: Live MCP provider call and production database process restart. --- .../agent_runtime/model_step_service.py | 28 ++- .../services/agent_runtime/tool_registry.py | 3 + .../agent_runtime/tool_step_service.py | 7 +- backend/app/services/agent_tools.py | 172 ++++++++++++++++-- .../test_agent_runtime_model_step_service.py | 36 ++++ .../test_agent_runtime_tool_step_service.py | 76 ++++++++ backend/tests/test_agent_tools_agentbay_a0.py | 2 +- .../test_agent_tools_deploy_contracts.py | 2 +- .../tests/test_agent_tools_email_contracts.py | 2 +- ...ent_tools_legacy_contract_compatibility.py | 165 +++++++++++++++++ .../tests/test_agent_tools_okr_contracts.py | 6 +- .../test_agent_tools_typed_agentbay_reads.py | 2 +- .../test_agent_tools_typed_deploy_reads.py | 4 +- ..._agent_tools_typed_deploy_simple_writes.py | 2 +- .../test_agent_tools_typed_dynamic_mcp.py | 37 ++-- ...test_agent_tools_typed_feishu_remaining.py | 6 +- ...est_agent_tools_typed_image_outcomes_v2.py | 4 +- .../tests/test_agent_tools_typed_okr_jobs.py | 4 +- 18 files changed, 503 insertions(+), 55 deletions(-) diff --git a/backend/app/services/agent_runtime/model_step_service.py b/backend/app/services/agent_runtime/model_step_service.py index 6935627db..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" @@ -1471,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, @@ -1526,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, ) @@ -1676,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_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 20765c2fc..8b7d316dd 100644 --- a/backend/app/services/agent_runtime/tool_step_service.py +++ b/backend/app/services/agent_runtime/tool_step_service.py @@ -219,6 +219,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: ... @@ -1559,9 +1560,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, ( 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/tests/test_agent_runtime_model_step_service.py b/backend/tests/test_agent_runtime_model_step_service.py index 0eaae82e5..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 []) diff --git a/backend/tests/test_agent_runtime_tool_step_service.py b/backend/tests/test_agent_runtime_tool_step_service.py index a8ed72f96..3dd6a814a 100644 --- a/backend/tests/test_agent_runtime_tool_step_service.py +++ b/backend/tests/test_agent_runtime_tool_step_service.py @@ -1135,6 +1135,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() 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_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( From 795613c2f791e53c32f5f0d9de39b463d9fdb285 Mon Sep 17 00:00:00 2001 From: Y1fe1Zh0u Date: Wed, 12 Aug 2026 16:56:26 +0800 Subject: [PATCH 5/9] Reject Tool arguments that violate advertised schemas The Runtime validator now enforces the JSON Schema keywords already used by production Tool definitions, so arguments rejected by the advertised contract cannot reach a handler. Failures remain bounded, value-free, and use the existing repairable Tool result path. Constraint: No new dependency and no attempt to implement the entire JSON Schema specification. Rejected: Add a general JSON Schema package | broader dependency and behavior change than required by the current Tool catalog. Confidence: high Scope-risk: moderate Directive: Add validator support when introducing a new schema keyword; unsupported schema contracts must not silently pass. Tested: 118 Runtime Tool validation, Tool Step, and builtin contract tests; scoped Ruff; git diff --check. Not-tested: Property-based comparison against a complete JSON Schema implementation. --- .../services/agent_runtime/tool_validation.py | 217 +++++++++++++++++- .../test_agent_runtime_tool_step_service.py | 3 +- .../test_agent_runtime_tool_validation.py | 69 ++++++ 3 files changed, 276 insertions(+), 13 deletions(-) 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/tests/test_agent_runtime_tool_step_service.py b/backend/tests/test_agent_runtime_tool_step_service.py index 3dd6a814a..35dd76ec2 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 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", + } From 3afdb790964a3e45fff952bfd165fb57226dfe04 Mon Sep 17 00:00:00 2001 From: Y1fe1Zh0u Date: Wed, 12 Aug 2026 16:58:44 +0800 Subject: [PATCH 6/9] Keep Runtime deadlines outside provider budgets Network reads and image generation now carry deadline policies that cover their declared provider timeout. Custom image generation retains its existing 600 second configurable upper bound instead of being cancelled by the generic Runtime default. Constraint: Provider-specific internal timeouts may remain shorter, but the Runtime wrapper must not expire first. Rejected: Read live Tool configuration inside the Tool Step deadline resolver | would reintroduce mutable routing policy after call acceptance. Confidence: high Scope-risk: narrow Directive: Keep builtin timeout declarations and Runtime deadline policy tests aligned when changing provider budgets. Tested: 171 Tool contract, Model Step, Tool Step, and builtin contract tests; scoped Ruff; git diff --check. Not-tested: Wall-clock calls to web and image providers. --- .../services/agent_runtime/tool_contracts.py | 16 +++++++++++- .../app/services/builtin_tool_definitions.py | 2 +- .../test_agent_runtime_tool_contracts.py | 25 +++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) 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/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/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: From dd2c4457cb8d0b55d245bc4fc9bf951e996c1c35 Mon Sep 17 00:00:00 2001 From: Y1fe1Zh0u Date: Wed, 12 Aug 2026 17:02:24 +0800 Subject: [PATCH 7/9] Keep resolved Tool context across legacy waits Legacy checkpoints now return the reconstructed StepToolContext from every ordinary wait, unknown, A2A, group failure, and error exit. A resumed Tool node therefore reuses the accepted Workset instead of querying the current ToolProvider again. Constraint: Async poll continuation remains owned by the dedicated async-context repair. Rejected: Mutate graph state directly inside ToolStepService | would bypass the existing ToolStepResult checkpoint boundary. Confidence: high Scope-risk: narrow Directive: Any new ToolStep early return after legacy context resolution must propagate step_tool_context. Tested: 68 Tool Step tests plus targeted legacy unknown and A2A resume regressions; scoped Ruff; git diff --check. Not-tested: Production checkpoint database restart and legacy approval fixture outside current canonical schemas. --- .../agent_runtime/tool_step_service.py | 19 ++- .../test_agent_runtime_tool_step_service.py | 141 ++++++++++++++++++ 2 files changed, 159 insertions(+), 1 deletion(-) diff --git a/backend/app/services/agent_runtime/tool_step_service.py b/backend/app/services/agent_runtime/tool_step_service.py index 8b7d316dd..20f1ac605 100644 --- a/backend/app/services/agent_runtime/tool_step_service.py +++ b/backend/app/services/agent_runtime/tool_step_service.py @@ -1748,6 +1748,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( @@ -1777,6 +1778,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}, ) @@ -2276,6 +2278,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: @@ -2389,6 +2392,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( @@ -2432,6 +2436,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( @@ -2454,6 +2459,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), @@ -2464,6 +2470,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: @@ -2555,6 +2562,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), @@ -2565,6 +2573,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: @@ -2580,6 +2589,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( @@ -2594,6 +2604,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 @@ -2711,6 +2722,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), @@ -2721,6 +2733,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): @@ -2798,6 +2811,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), @@ -2808,6 +2822,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( @@ -2832,13 +2847,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/tests/test_agent_runtime_tool_step_service.py b/backend/tests/test_agent_runtime_tool_step_service.py index 35dd76ec2..d3766898b 100644 --- a/backend/tests/test_agent_runtime_tool_step_service.py +++ b/backend/tests/test_agent_runtime_tool_step_service.py @@ -1335,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, From 6b5b97ee4bf0166cd747a607d5abc43bda661746 Mon Sep 17 00:00:00 2001 From: Y1fe1Zh0u Date: Wed, 12 Aug 2026 17:05:31 +0800 Subject: [PATCH 8/9] Give unknown MCP calls an explicit recovery path Unknown dynamic MCP receipts now use the existing audited Direct Chat reconciliation flow. A user must confirm whether the external action happened before the old receipt is settled and the Run may resume; the Runtime never replays the original MCP call automatically. Constraint: Dynamic MCP Tools are conservatively classified external_write with retry policy never. Rejected: Automatically retry an unknown MCP call | could duplicate an external side effect. Rejected: Add a separate MCP reconciler service | unnecessary while explicit human confirmation provides a real recovery outlet. Confidence: high Scope-risk: narrow Directive: Do not make unknown MCP receipts retryable without an explicit idempotency contract. Tested: 47 Tool ledger and Direct Chat runtime-state tests; 17 targeted unknown/reconciliation tests; scoped Ruff; git diff --check. Not-tested: Browser UI interaction and a live MCP server with an ambiguous transport outcome. --- .../services/agent_runtime/tool_execution.py | 18 ++++++++++++++---- .../tests/test_chat_session_runtime_state.py | 17 ++++++++++++++--- backend/tests/test_tool_execution.py | 16 ++++++++++++---- 3 files changed, 40 insertions(+), 11 deletions(-) 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/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_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] From aa808a0ac9d35e622fbfdfb6e12287c336d9cc77 Mon Sep 17 00:00:00 2001 From: Y1fe1Zh0u Date: Wed, 12 Aug 2026 19:23:19 +0800 Subject: [PATCH 9/9] Keep v1.11.4 checks aligned with the Tool contract The integrated Tool fixes expose two stale assertions in the release test suite: network reads now use the declared 60-second Runtime budget, and the durable execution model already includes provider identity and contract version columns. Align those assertions and remove a duplicate import so the combined release branch can be verified as one unit. Constraint: Preserve the eight reviewed fix commits without folding or rewriting their history Rejected: Ignore the full-suite failures | they would leave the integration PR without a clean release-level regression result Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep deadline and schema assertions synchronized with the production Tool contract Tested: Full backend suite, 2357 passed; scoped Ruff excluding pre-existing agent_tools.py debt; git diff --check Not-tested: Live external MCP provider execution --- backend/app/services/agent_runtime/tool_step_service.py | 1 - backend/tests/test_agent_tools_deadlines.py | 2 +- backend/tests/test_runtime_schema.py | 2 ++ 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/backend/app/services/agent_runtime/tool_step_service.py b/backend/app/services/agent_runtime/tool_step_service.py index 20f1ac605..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, 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_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",