From 04db04328858b6fed9b7d202bb83dbd13fbb1101 Mon Sep 17 00:00:00 2001 From: Alexliu Date: Fri, 4 Sep 2026 22:06:40 +0800 Subject: [PATCH 1/5] feat(web): add connector runtime requirements read endpoints Two owner-scoped read endpoints report which runtime inputs an agent's connectors declare and which of them a task already has: one keyed by agent for a pre-flight check, one keyed by task for the in-chat prompt. Both return declared key names, their normalized type and whether a value is already stored - never a stored value, and never a connector's URL, headers, environment or authentication configuration. Both live on the chat router, next to the task-keyed runtime-extensions endpoint they are shaped after, and both reuse the predicate task creation already applies to an agent id rather than introducing a second authorization path for the same resource. The team scope both endpoints resolve is pinned to the one tool loading uses at run time, so a team-shared connector that the run-time gate demands a value for is always one the caller can also see here. Neither endpoint asserts that required values are present: reporting what is missing is the whole point, and raising on a missing value belongs to the per-turn gate that runs later. --- src/xagent/web/api/chat.py | 83 +++ src/xagent/web/schemas/connector_runtime.py | 72 ++ src/xagent/web/services/connector_runtime.py | 180 +++++ .../test_connector_runtime_entrypoints_e2e.py | 620 +++++++++++++++++- 4 files changed, 953 insertions(+), 2 deletions(-) create mode 100644 src/xagent/web/schemas/connector_runtime.py diff --git a/src/xagent/web/api/chat.py b/src/xagent/web/api/chat.py index 4364884371..1754cbf4c7 100644 --- a/src/xagent/web/api/chat.py +++ b/src/xagent/web/api/chat.py @@ -75,6 +75,7 @@ parse_user_sandbox_key, ) from ..schemas.chat import TaskCreateRequest, TaskCreateResponse +from ..schemas.connector_runtime import ConnectorRuntimeRequirementsModel from ..services.agent_access import list_accessible_published_agents from ..services.agent_team_scope import ( get_agent_team_scope, @@ -89,7 +90,9 @@ ) from ..services.connector_runtime import ( bind_connector_runtime_selection_snapshot, + build_task_runtime_requirements, prepare_connector_runtime_selection_snapshot, + resolve_agent_runtime_requirements, ) from ..services.db_runtime import ( drain_async_task_cancellation_safe, @@ -5288,6 +5291,86 @@ async def get_task_runtime_extensions( } +@chat_router.get( + "/agent/{agent_id}/connector-runtime-requirements", + response_model=ConnectorRuntimeRequirementsModel, +) +async def get_agent_connector_runtime_requirements( + agent_id: int, + db: Session = Depends(get_db), + user: User = Depends(get_current_user), +) -> ConnectorRuntimeRequirementsModel: + """Report the runtime inputs a prospective task would need, before one + exists. + + Reuses the same predicate ``POST /task/create`` applies to an agent id + (``_load_agent_for_task_create``) rather than a second authorization + path for the same resource, so a caller who could create a task with + this agent sees exactly the same "not found" boundary here that they + would hit on that call. Lives on the chat router, next to the + task-keyed sibling below and the existing task-keyed + ``/task/{task_id}/runtime-extensions``, rather than under + ``/api/agents`` -- this endpoint has no consumer outside chat. + + There is no task yet, so every reported input is unsatisfied and the + connector team scope is whatever ``resolve_agent_selected_connectors`` + derives from the agent's own team, never a value this endpoint passes + in itself. + """ + + agent = _load_agent_for_task_create(db, user, agent_id) + if agent is None: + raise HTTPException(status_code=404, detail="Agent not found or access denied") + try: + _refs, requirements = resolve_agent_runtime_requirements( + db=db, agent=agent, connector_user_id=int(user.id) + ) + except ConnectorRuntimeError as exc: + raise HTTPException( + status_code=exc.status_code, detail=exc.safe_message + ) from exc + return requirements + + +@chat_router.get( + "/task/{task_id}/connector-runtime-requirements", + response_model=ConnectorRuntimeRequirementsModel, +) +async def get_task_connector_runtime_requirements( + task_id: int, + db: Session = Depends(get_db), + user: User = Depends(get_current_user), +) -> ConnectorRuntimeRequirementsModel: + """Report which of a task's declared connector runtime inputs already + have a value. + + Access is plain task ownership -- ``Task.user_id == current_user.id`` in + the same query that loads the task, unlike + ``/task/{task_id}/runtime-extensions`` above, which additionally lets an + admin read any task; this endpoint does not extend that exception. + A task that does not exist or is not the caller's own is a uniform 404. + + Pure read: never writes, and never asserts that a required value is + present -- that assertion belongs to the per-turn gate that runs later, + not to this report. + """ + + task = db.query(Task).filter(Task.id == task_id, Task.user_id == user.id).first() + if task is None: + raise HTTPException(status_code=404, detail="Task not found") + agent = ( + db.query(Agent).filter(Agent.id == task.agent_id).first() + if task.agent_id is not None + else None + ) + try: + return build_task_runtime_requirements(db=db, task=task, agent=agent) + except ConnectorRuntimeError as exc: + raise HTTPException( + status_code=exc.status_code, detail=exc.safe_message + ) from exc + + @chat_router.delete("/task/{task_id}") async def delete_task( task_id: int, diff --git a/src/xagent/web/schemas/connector_runtime.py b/src/xagent/web/schemas/connector_runtime.py new file mode 100644 index 0000000000..38ceceebf8 --- /dev/null +++ b/src/xagent/web/schemas/connector_runtime.py @@ -0,0 +1,72 @@ +"""Connector runtime requirements: the shared response shape both read +endpoints return. + +The agent-keyed and task-keyed read endpoints both return this same +requirements report -- which runtime inputs a task's (or a prospective +task's) connectors declare, and whether each one already has a value -- +never a stored value itself, and never a connector's transport or +authentication configuration. + +Placed in its own module rather than ``schemas/chat.py`` because a values- +submission endpoint lands on top of it shortly and will share this same +response shape as its own 200 body; putting it here now avoids a later +move that would touch every existing importer. +""" + +from __future__ import annotations + +from pydantic import BaseModel + + +class ConnectorRuntimeRefModel(BaseModel): + """Wire identity of a connector, as returned in a requirements report.""" + + connector_type: str + connector_id: int + + +class ConnectorRuntimeInputModel(BaseModel): + """One declared runtime input and whether it is currently satisfied. + + ``key`` is the raw key name a connector owner wrote when declaring the + input -- there is no human-readable label anywhere in the declaration. + ``type`` is already normalized server-side to ``"string"`` or + ``"object"``; a client must not normalize it again or expect any other + value. ``satisfied`` and ``expired`` are constants in this phase for the + ``secrets``/``auth_selector`` sections: no secret store exists yet, so + both are always ``False``. + """ + + section: str + key: str + type: str + required: bool + satisfied: bool + expired: bool = False + + +class ConnectorRuntimeConnectorModel(BaseModel): + """One connector's declared runtime inputs. + + ``name`` is the only piece of connector identity beyond the ref that is + ever included -- never the connector's URL, headers, environment, or + authentication configuration. + """ + + connector_ref: ConnectorRuntimeRefModel + name: str + inputs: list[ConnectorRuntimeInputModel] + + +class ConnectorRuntimeRequirementsModel(BaseModel): + """A requirements report. Every field always appears. + + ``connectors`` is empty, never omitted, when nothing is selected or + declares a runtime input. ``secrets_expires_at`` is a constant ``null`` + in this phase; a later phase gives it a real value without changing + its meaning or making it optional. + """ + + satisfied: bool + secrets_expires_at: str | None + connectors: list[ConnectorRuntimeConnectorModel] diff --git a/src/xagent/web/services/connector_runtime.py b/src/xagent/web/services/connector_runtime.py index 1165ea3565..27e7bcb924 100644 --- a/src/xagent/web/services/connector_runtime.py +++ b/src/xagent/web/services/connector_runtime.py @@ -45,6 +45,12 @@ from ..models.custom_api import CustomApi, UserCustomApi from ..models.mcp import MCPServer, UserMCPServer from ..models.task import Task, TaskConnectorRuntimeContext +from ..schemas.connector_runtime import ( + ConnectorRuntimeConnectorModel, + ConnectorRuntimeInputModel, + ConnectorRuntimeRefModel, + ConnectorRuntimeRequirementsModel, +) logger = logging.getLogger(__name__) @@ -418,6 +424,74 @@ def prepare_connector_runtime_selection_snapshot( return _runtime_declared_refs(selected) +def resolve_agent_runtime_requirements( + *, db: Session, agent: Agent | None, connector_user_id: int | None +) -> tuple[tuple[ConnectorRef, ...], ConnectorRuntimeRequirementsModel]: + """Resolve an agent's declared runtime inputs before any task exists. + + Calls ``resolve_agent_selected_connectors`` exactly once. The returned + refs are ``_runtime_declared_refs`` applied to that same call's result + -- same filter, same order -- because a caller creating a task persists + them verbatim into ``Task.connector_runtime_selected_refs``, and every + later reader of that column (the per-turn gate, the values endpoint's + selection check, ``load_connector_runtime_view``) depends on it holding + exactly that set in exactly that order. Do not derive the refs any + other way, even one that looks equivalent. + + The report has no task to consult, so every input's ``satisfied`` is + ``False`` and the top-level ``satisfied`` answers "would a task created + from this agent right now need nothing else" -- i.e. it has no required + input at all. Never queries any task's stored values: doing so would + make the answer depend on which task happened to be looked up, and this + endpoint has none in scope. + + Never asserts that a required value is present -- reporting what is + missing is the whole point, and raising on it belongs to the per-turn + gate that runs later, not to this report. + """ + + if agent is None or connector_user_id is None: + return (), ConnectorRuntimeRequirementsModel( + satisfied=True, secrets_expires_at=None, connectors=[] + ) + selected = resolve_agent_selected_connectors( + db=db, agent=agent, connector_user_id=int(connector_user_id) + ) + refs = _runtime_declared_refs(selected) + connectors = [ + _build_connector_report(ref, selected[ref], stored_context=None) for ref in refs + ] + return refs, ConnectorRuntimeRequirementsModel( + satisfied=_all_required_inputs_satisfied(connectors), + secrets_expires_at=None, + connectors=connectors, + ) + + +def build_task_runtime_requirements( + *, db: Session, task: Task, agent: Agent | None +) -> ConnectorRuntimeRequirementsModel: + """Report a task's stored connector-runtime state. + + Pure read: issues no write of any kind. Never asserts that a required + value is present -- that assertion belongs to the per-turn gate, not to + this report; a caller who needs "can this task run right now" reads the + ``satisfied`` fields this returns rather than calling a helper that + raises. + """ + + connector_user_id = int(task.user_id) + agent_team_id = ( + int(agent.team_id) if agent is not None and agent.team_id is not None else None + ) + visible = _load_visible_runtime_connectors( + db, user_id=connector_user_id, agent_team_id=agent_team_id + ) + selected_refs = _load_task_selected_refs(task) + stored_context = _load_task_context_rows(db, task_id=int(task.id)) + return _build_task_requirements_model(selected_refs, visible, stored_context) + + def bind_connector_runtime_selection_snapshot( *, task: Task, selected_refs: Iterable[ConnectorRef] ) -> None: @@ -791,6 +865,112 @@ def _load_task_context_rows( return result +def _normalize_runtime_input_type(declaration: Any) -> str: + """Normalize a declared input's ``type`` the same way the connector- + owner form does (``frontend/src/components/mcp/runtime-inputs-form.tsx``): + ``"object"`` stays ``"object"``, everything else -- including a missing + or unrecognized value -- becomes ``"string"``. Never pass the raw + declared value through unnormalized. + """ + raw = declaration.get("type") if isinstance(declaration, dict) else None + return "object" if raw == "object" else "string" + + +def _build_connector_report( + ref: ConnectorRef, connector: Any, *, stored_context: dict[str, Any] | None +) -> ConnectorRuntimeConnectorModel: + """Project one connector's runtime declaration into a requirements + report entry. Reads only the fields ``ConnectorRuntimeInputModel`` + exposes -- never the connector's URL, headers, environment, + authentication configuration, or ``runtime_bindings``, and never a + stored value itself. + + ``stored_context`` is ``None`` for the agent-keyed report, which has no + task and therefore no value table to consult: every ``context`` key is + then unsatisfied, matching ``secrets``/``auth_selector``, which are + always unsatisfied at this phase regardless of ``stored_context`` + because no secret store exists yet. + """ + schema = _runtime_input_schema(connector) + context_stored = stored_context or {} + inputs: list[ConnectorRuntimeInputModel] = [] + for section_name in ( + RUNTIME_INPUT_CONTEXT, + RUNTIME_INPUT_SECRETS, + RUNTIME_INPUT_AUTH_SELECTOR, + ): + if ( + section_name == RUNTIME_INPUT_AUTH_SELECTOR + and ref.connector_type != CONNECTOR_TYPE_MCP + ): + continue + declarations = _schema_section(schema, section_name) + for key, declaration in declarations.items(): + satisfied = ( + key in context_stored + if section_name == RUNTIME_INPUT_CONTEXT + else False + ) + inputs.append( + ConnectorRuntimeInputModel( + section=section_name, + key=key, + type=_normalize_runtime_input_type(declaration), + required=_is_required(declaration), + satisfied=satisfied, + expired=False, + ) + ) + return ConnectorRuntimeConnectorModel( + connector_ref=ConnectorRuntimeRefModel( + connector_type=ref.connector_type, connector_id=ref.connector_id + ), + name=str(getattr(connector, "name", "")), + inputs=inputs, + ) + + +def _all_required_inputs_satisfied( + connectors: list[ConnectorRuntimeConnectorModel], +) -> bool: + """Top-level ``satisfied``: every required input, across every + reported connector and every section including ``secrets``, is + satisfied. ``all()`` over an empty sequence is ``True``, so no + connectors (or no required inputs) reports satisfied. + """ + return all( + input_item.satisfied + for connector in connectors + for input_item in connector.inputs + if input_item.required + ) + + +def _build_task_requirements_model( + selected_refs: tuple[ConnectorRef, ...], + visible: dict[ConnectorRef, Any], + stored_context: dict[ConnectorRef, dict[str, Any]], +) -> ConnectorRuntimeRequirementsModel: + connectors: list[ConnectorRuntimeConnectorModel] = [] + for ref in selected_refs: + connector = visible.get(ref) + if connector is None: + # Same rule as load_connector_runtime_view: a selected ref the + # caller can no longer see has no runtime tool either, so it is + # skipped rather than reported as missing something. + continue + connectors.append( + _build_connector_report( + ref, connector, stored_context=stored_context.get(ref) + ) + ) + return ConnectorRuntimeRequirementsModel( + satisfied=_all_required_inputs_satisfied(connectors), + secrets_expires_at=None, + connectors=connectors, + ) + + def _validate_values_against_schema( ref: ConnectorRef, connector: Any, diff --git a/tests/web/test_connector_runtime_entrypoints_e2e.py b/tests/web/test_connector_runtime_entrypoints_e2e.py index 453fbbbbb5..25e1cd3809 100644 --- a/tests/web/test_connector_runtime_entrypoints_e2e.py +++ b/tests/web/test_connector_runtime_entrypoints_e2e.py @@ -14,15 +14,16 @@ from fastapi.testclient import TestClient from sqlalchemy.orm import Session -from xagent.web.api.auth import auth_router +from xagent.web.api.auth import auth_router, create_access_token from xagent.web.api.chat import AgentServiceManager, chat_router +from xagent.web.api.public_chat_access import create_public_chat_access_token from xagent.web.api.share import share_router from xagent.web.api.websocket import handle_chat_message from xagent.web.api.widget import widget_router from xagent.web.channels.feishu.bot import FeishuBotInstance from xagent.web.channels.telegram import bot as telegram_bot_module from xagent.web.channels.telegram.bot import TelegramBotInstance -from xagent.web.models.agent import Agent, AgentStatus +from xagent.web.models.agent import Agent, AgentOrigin, AgentStatus from xagent.web.models.chat_message import TaskChatMessage from xagent.web.models.database import ( Base, @@ -34,6 +35,11 @@ from xagent.web.models.uploaded_file import UploadedFile from xagent.web.models.user import User from xagent.web.models.user_channel import UserChannel +from xagent.web.services import connector_team_scope +from xagent.web.services.agent_team_scope import ( + AgentTeamScope, + set_agent_team_scope_hook, +) def _override_get_db() -> Iterator[Session]: @@ -171,6 +177,63 @@ def _create_mcp_server( return server +def _create_user(db: Session, username: str) -> User: + user = User(username=username, password_hash="hash", is_admin=False) + db.add(user) + db.flush() + return user + + +def _auth_headers_for_user(user: User) -> dict[str, str]: + """Mint an access token for an already-created user, bypassing the + HTTP login round trip. Drives the same ``get_current_user`` dependency + every endpoint under test uses -- only the token minting is shortcut. + """ + token = create_access_token( + data={"sub": str(user.username), "user_id": int(user.id)} + ) + return {"Authorization": f"Bearer {token}"} + + +def _mcp_server_with_context_schema( + db: Session, + user: User, + *, + name: str, + context_schema: dict[str, Any], + url: str = "https://example.com/mcp", +) -> MCPServer: + server = MCPServer( + name=name, + description=f"{name} description", + managed="external", + transport="streamable_http", + url=url, + runtime_input_schema={"context": context_schema}, + runtime_bindings=[ + { + "source": {"input_type": "context", "key": key}, + "target": {"target_type": "mcp_meta", "key": key}, + } + for key in context_schema + ], + ) + db.add(server) + db.flush() + db.add( + UserMCPServer( + user_id=user.id, + mcpserver_id=server.id, + is_owner=True, + can_edit=True, + can_delete=True, + is_active=True, + ) + ) + db.flush() + return server + + def _task(task_id: int) -> Task: db = _db_session() try: @@ -1227,3 +1290,556 @@ async def answer(self, _text: str, **_kwargs: Any) -> _LoadingMessage: assert user_message.attachments == expected_attachments finally: db.close() + + +# --------------------------------------------------------------------------- +# Connector-runtime-requirements read endpoints +# (GET /agent/{agent_id}/connector-runtime-requirements, +# GET /task/{task_id}/connector-runtime-requirements). +# --------------------------------------------------------------------------- + + +def test_agent_requirements_hides_connection_config_and_normalizes_type( + e2e_db: None, +) -> None: + """The agent-keyed report never leaks a connector's transport or + authentication configuration, a declared ``type`` other than the raw + string ``"object"`` normalizes to ``"string"``, and the report is + untouched by any task's stored values -- not even a task created + against the same agent and connector. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + server = MCPServer( + name="leaky-server", + description="leaky-server description", + managed="external", + transport="streamable_http", + url="https://leak.example/probe", + headers={"Authorization": "Bearer leak-header-secret"}, + env={"SECRET": "leak-env-secret"}, + auth={"type": "oauth", "client_secret": "leak-auth-secret"}, + runtime_input_schema={ + "context": { + "auth_token": {"type": "string", "required": True}, + "profile": {"type": {"$ref": "leak"}, "required": False}, + } + }, + runtime_bindings=[ + { + "source": {"input_type": "context", "key": "auth_token"}, + "target": {"target_type": "mcp_meta", "key": "auth_token"}, + }, + { + "source": {"input_type": "context", "key": "profile"}, + "target": {"target_type": "mcp_meta", "key": "profile"}, + }, + ], + ) + db.add(server) + db.flush() + db.add( + UserMCPServer( + user_id=user.id, + mcpserver_id=server.id, + is_owner=True, + can_edit=True, + can_delete=True, + is_active=True, + ) + ) + agent = _create_agent( + db, user, name="Leaky Requirements Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + db.refresh(server) + agent_id = int(agent.id) + server_id = int(server.id) + finally: + db.close() + + response = client.get( + f"/api/chat/agent/{agent_id}/connector-runtime-requirements", + headers=headers, + ) + assert response.status_code == 200, response.text + for leaked in ( + "leak.example", + "leak-header-secret", + "leak-env-secret", + "leak-auth-secret", + ): + assert leaked not in response.text + + payload = response.json() + assert payload["secrets_expires_at"] is None + connectors = payload["connectors"] + assert len(connectors) == 1 + inputs_by_key = {item["key"]: item for item in connectors[0]["inputs"]} + assert inputs_by_key["auth_token"]["type"] == "string" + # Declared as {"$ref": "leak"}, not the literal string "object" -- must + # normalize to "string", not pass through unnormalized. + assert inputs_by_key["profile"]["type"] == "string" + assert inputs_by_key["auth_token"]["satisfied"] is False + + # A task created from this same agent, with this same connector's + # required key filled directly in storage, must not move this report's + # numbers: it has no task in scope. + create_response = client.post( + "/api/chat/task/create", + headers=headers, + json={ + "title": "leak isolation task", + "description": "d", + "agent_id": agent_id, + }, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + db = _db_session() + try: + db.add( + TaskConnectorRuntimeContext( + task_id=task_id, + connector_type="mcp", + connector_id=server_id, + context={"auth_token": "filled"}, + ) + ) + db.commit() + finally: + db.close() + + second_response = client.get( + f"/api/chat/agent/{agent_id}/connector-runtime-requirements", + headers=headers, + ) + assert second_response.status_code == 200, second_response.text + second_payload = second_response.json() + assert second_payload["secrets_expires_at"] is None + second_inputs = { + item["key"]: item for item in second_payload["connectors"][0]["inputs"] + } + assert second_inputs["auth_token"]["satisfied"] is False + + +@pytest.mark.parametrize( + "scenario", + [ + "different_user", + "admins_only_team", + "unpublished_other_user", + "workforce_manager_owned", + ], +) +def test_agent_requirements_hides_non_visible_agents( + e2e_db: None, scenario: str +) -> None: + """Four identities that must all see a uniform 404, with the agent's + name absent from the response body. + """ + team_hook_installed = False + db = _db_session() + try: + owner = _create_user(db, "agent-owner") + caller = _create_user(db, "agent-caller") + db.flush() + caller_id = int(caller.id) + + if scenario == "different_user": + agent = Agent( + user_id=owner.id, + name="Secret Agent", + instructions="i", + execution_mode="balanced", + status=AgentStatus.DRAFT, + tool_categories=[], + ) + elif scenario == "admins_only_team": + agent = Agent( + user_id=owner.id, + name="Secret Agent", + instructions="i", + execution_mode="balanced", + status=AgentStatus.PUBLISHED, + tool_categories=[], + team_id=101, + visibility="admins", + ) + set_agent_team_scope_hook( + lambda db, user_id: ( + AgentTeamScope(team_id=101, is_team_admin=False) + if user_id == caller_id + else None + ) + ) + team_hook_installed = True + elif scenario == "unpublished_other_user": + agent = Agent( + user_id=owner.id, + name="Secret Agent", + instructions="i", + execution_mode="balanced", + status=AgentStatus.DRAFT, + tool_categories=[], + ) + elif scenario == "workforce_manager_owned": + # Owned by the caller: proves the workforce-manager check runs + # before -- not as part of -- the ownership check. + agent = Agent( + user_id=caller.id, + name="Secret Agent", + instructions="i", + execution_mode="balanced", + status=AgentStatus.PUBLISHED, + tool_categories=[], + origin=AgentOrigin.WORKFORCE_GENERATED_MANAGER.value, + ) + else: + raise AssertionError(scenario) + db.add(agent) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + caller_headers = _auth_headers_for_user(caller) + finally: + db.close() + + try: + response = client.get( + f"/api/chat/agent/{agent_id}/connector-runtime-requirements", + headers=caller_headers, + ) + assert response.status_code == 404, response.text + assert "Secret Agent" not in response.text + finally: + if team_hook_installed: + set_agent_team_scope_hook(None) + + +def test_team_shared_connector_visible_across_read_endpoints(e2e_db: None) -> None: + """A connector shared only through the agent's team is listed by both + read endpoints for a non-owning team member, and the task-keyed read + endpoint returns 200 + (not 400) while the connector's one required key is still unfilled. + Reversing the connector-team hook to withhold sharing removes it from + both endpoints for the same caller and agent. + """ + db = _db_session() + try: + owner = _create_user(db, "team-connector-owner") + member = _create_user(db, "team-connector-member") + db.flush() + member_id = int(member.id) + + server = MCPServer( + name="team-shared-server", + description="team-shared-server description", + managed="external", + transport="streamable_http", + url="https://example.com/mcp", + runtime_input_schema={ + "context": {"auth_token": {"type": "string", "required": True}} + }, + runtime_bindings=[ + { + "source": {"input_type": "context", "key": "auth_token"}, + "target": {"target_type": "mcp_meta", "key": "auth_token"}, + } + ], + ) + db.add(server) + db.flush() + # No UserMCPServer link for `member` at all -- reachable only + # through the team hook below. + server_id = int(server.id) + + agent = Agent( + user_id=owner.id, + name="Team Shared Agent", + instructions="i", + execution_mode="balanced", + status=AgentStatus.PUBLISHED, + tool_categories=["mcp"], + team_id=101, + visibility="team", + ) + db.add(agent) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + member_headers = _auth_headers_for_user(member) + finally: + db.close() + + def _connector_ids_for_team(shared: bool): + def _hook(db: Session, *, team_id: int) -> dict[str, set[int]]: + if shared and team_id == 101: + return {"mcp": {server_id}, "custom_api": set()} + return {"mcp": set(), "custom_api": set()} + + return _hook + + set_agent_team_scope_hook( + lambda db, user_id: ( + AgentTeamScope(team_id=101, is_team_admin=False) + if user_id == member_id + else None + ) + ) + connector_team_scope.set_connector_team_hooks( + team_visibility=_connector_ids_for_team(shared=True) + ) + try: + agent_response = client.get( + f"/api/chat/agent/{agent_id}/connector-runtime-requirements", + headers=member_headers, + ) + assert agent_response.status_code == 200, agent_response.text + agent_refs = [ + item["connector_ref"] for item in agent_response.json()["connectors"] + ] + assert {"connector_type": "mcp", "connector_id": server_id} in agent_refs + + create_response = client.post( + "/api/chat/task/create", + headers=member_headers, + json={ + "title": "team shared task", + "description": "d", + "agent_id": agent_id, + }, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + + task_response = client.get( + f"/api/chat/task/{task_id}/connector-runtime-requirements", + headers=member_headers, + ) + assert task_response.status_code == 200, task_response.text + task_payload = task_response.json() + assert task_payload["satisfied"] is False + task_refs = [item["connector_ref"] for item in task_payload["connectors"]] + assert {"connector_type": "mcp", "connector_id": server_id} in task_refs + finally: + set_agent_team_scope_hook(None) + connector_team_scope.set_connector_team_hooks() + + # Reverse: withhold team sharing for the same agent/caller pair. Uses a + # fresh task (the earlier one already persisted its selected refs) so + # this is purely a visibility check on the read endpoints. + set_agent_team_scope_hook( + lambda db, user_id: ( + AgentTeamScope(team_id=101, is_team_admin=False) + if user_id == member_id + else None + ) + ) + connector_team_scope.set_connector_team_hooks( + team_visibility=_connector_ids_for_team(shared=False) + ) + try: + agent_response = client.get( + f"/api/chat/agent/{agent_id}/connector-runtime-requirements", + headers=member_headers, + ) + assert agent_response.status_code == 200, agent_response.text + assert agent_response.json()["connectors"] == [] + + create_response = client.post( + "/api/chat/task/create", + headers=member_headers, + json={ + "title": "team unshared task", + "description": "d", + "agent_id": agent_id, + }, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + task_response = client.get( + f"/api/chat/task/{task_id}/connector-runtime-requirements", + headers=member_headers, + ) + assert task_response.status_code == 200, task_response.text + assert task_response.json()["connectors"] == [] + finally: + set_agent_team_scope_hook(None) + connector_team_scope.set_connector_team_hooks() + + +def test_agent_requirements_endpoint_bypassing_team_resolver_hides_shared_connector( + e2e_db: None, +) -> None: + """If the agent-keyed endpoint ever bypasses + ``resolve_agent_selected_connectors`` and loads visible connectors with + ``agent_team_id=None`` instead of the value that resolver derives from + the agent, a team-shared-only connector silently disappears from the + report. This test pins the *correct* behavior (the connector is + listed); the mutation itself has no test-visible seam to monkeypatch + without changing which production code path runs, so it is applied and + reverted directly against ``resolve_agent_runtime_requirements`` in the + execution report's mutation table rather than parametrized here. + """ + db = _db_session() + try: + owner = _create_user(db, "team-connector-owner-2") + member = _create_user(db, "team-connector-member-2") + db.flush() + member_id = int(member.id) + server = MCPServer( + name="team-shared-server-2", + description="team-shared-server-2 description", + managed="external", + transport="streamable_http", + url="https://example.com/mcp", + runtime_input_schema={ + "context": {"auth_token": {"type": "string", "required": True}} + }, + runtime_bindings=[ + { + "source": {"input_type": "context", "key": "auth_token"}, + "target": {"target_type": "mcp_meta", "key": "auth_token"}, + } + ], + ) + db.add(server) + db.flush() + server_id = int(server.id) + agent = Agent( + user_id=owner.id, + name="Team Shared Agent 2", + instructions="i", + execution_mode="balanced", + status=AgentStatus.PUBLISHED, + tool_categories=["mcp"], + team_id=202, + visibility="team", + ) + db.add(agent) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + member_headers = _auth_headers_for_user(member) + finally: + db.close() + + set_agent_team_scope_hook( + lambda db, user_id: ( + AgentTeamScope(team_id=202, is_team_admin=False) + if user_id == member_id + else None + ) + ) + connector_team_scope.set_connector_team_hooks( + team_visibility=lambda db, *, team_id: ( + {"mcp": {server_id}, "custom_api": set()} + if team_id == 202 + else {"mcp": set(), "custom_api": set()} + ) + ) + try: + response = client.get( + f"/api/chat/agent/{agent_id}/connector-runtime-requirements", + headers=member_headers, + ) + assert response.status_code == 200, response.text + refs = [item["connector_ref"] for item in response.json()["connectors"]] + assert {"connector_type": "mcp", "connector_id": server_id} in refs + finally: + set_agent_team_scope_hook(None) + connector_team_scope.set_connector_team_hooks() + + +def test_task_requirements_endpoint_requires_task_ownership_by_caller( + e2e_db: None, +) -> None: + """A task belonging to another logged-in user is a uniform 404 on the + task-keyed read endpoint, matching the values endpoint's ownership + predicate. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + owner = _admin_user(db) + other = _create_user(db, "task-requirements-other") + agent = _create_agent( + db, owner, name="Task Owner Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + other_headers = _auth_headers_for_user(other) + finally: + db.close() + + create_response = client.post( + "/api/chat/task/create", + headers=headers, + json={"title": "owner only task", "description": "d", "agent_id": agent_id}, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + + response = client.get( + f"/api/chat/task/{task_id}/connector-runtime-requirements", + headers=other_headers, + ) + assert response.status_code == 404, response.text + + +@pytest.mark.parametrize("endpoint_kind", ["agent", "task"]) +def test_read_endpoints_reject_anonymous_and_widget_credentials( + e2e_db: None, endpoint_kind: str +) -> None: + """No ``Authorization`` header is a bare 403 + (``HTTPBearer`` itself, ``auto_error=True``), and a well-formed widget + guest token is a 401 ``"Invalid token type"`` -- the same two doors + every other authenticated-only endpoint in this module is gated by + (``get_current_user``'s ``type: "access"`` check, ``auth_dependencies.py``). + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + agent = _create_agent( + db, user, name="Anon Guard Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + finally: + db.close() + + if endpoint_kind == "agent": + url = f"/api/chat/agent/{agent_id}/connector-runtime-requirements" + else: + create_response = client.post( + "/api/chat/task/create", + headers=headers, + json={"title": "anon guard task", "description": "d", "agent_id": agent_id}, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + url = f"/api/chat/task/{task_id}/connector-runtime-requirements" + + no_auth_response = client.get(url) + assert no_auth_response.status_code == 403, no_auth_response.text + + widget_token = create_public_chat_access_token( + {"guest_id": "anon-guard-guest", "widget_agent_id": agent_id} + ) + widget_response = client.get( + url, headers={"Authorization": f"Bearer {widget_token}"} + ) + assert widget_response.status_code == 401, widget_response.text + assert widget_response.json()["detail"] == "Invalid token type" + + +# --------------------------------------------------------------------------- +# A3: connector_runtime_requirements on the task-create response. +# --------------------------------------------------------------------------- From 10d04f41443f036c809307a4fb747f254e33d472 Mon Sep 17 00:00:00 2001 From: Alexliu Date: Fri, 4 Sep 2026 22:09:57 +0800 Subject: [PATCH 2/5] feat(web): report missing connector runtime inputs on task create Task creation now returns the same requirements report as the read endpoints, so a client can prompt for the missing values before sending the first message instead of after a failed turn. The request body is unchanged and still ignores any runtime values a caller smuggles in. The report reuses the connector resolution the creation path already performs, so it costs no extra query. The field is null rather than a report on the public widget and share-link create paths: those callers are anonymous guests who never see a connector's declared key names, so evaluating requirements for them would hand that information to a new audience this change does not intend to reach. The field still always appears in the response body either way. --- src/xagent/web/api/chat.py | 14 +- src/xagent/web/api/public_chat_access.py | 16 + src/xagent/web/schemas/chat.py | 15 + tests/web/api/test_websocket_preview.py | 10 + .../test_connector_runtime_entrypoints_e2e.py | 413 +++++++++++++++++- 5 files changed, 461 insertions(+), 7 deletions(-) diff --git a/src/xagent/web/api/chat.py b/src/xagent/web/api/chat.py index 1754cbf4c7..2f086ef9e5 100644 --- a/src/xagent/web/api/chat.py +++ b/src/xagent/web/api/chat.py @@ -91,7 +91,6 @@ from ..services.connector_runtime import ( bind_connector_runtime_selection_snapshot, build_task_runtime_requirements, - prepare_connector_runtime_selection_snapshot, resolve_agent_runtime_requirements, ) from ..services.db_runtime import ( @@ -4491,10 +4490,12 @@ def _get_default_internal_model_ids() -> Dict[str, Optional[str]]: agent_id=request.agent_id, # Set agent_id if provided is_visible=False if request.is_preview else request.is_visible, ) - selected_refs = prepare_connector_runtime_selection_snapshot( - db=db, - agent=selected_agent, - connector_user_id=int(user.id), + selected_refs, connector_runtime_requirements = ( + resolve_agent_runtime_requirements( + db=db, + agent=selected_agent, + connector_user_id=int(user.id), + ) ) bind_connector_runtime_selection_snapshot( task=task, selected_refs=selected_refs @@ -4671,6 +4672,7 @@ def _get_default_internal_model_ids() -> Dict[str, Optional[str]]: runtime_extensions=runtime_extensions, runtime_extensions_status=runtime_extensions_status, runtime_extensions_omitted=runtime_extensions_omitted, + connector_runtime_requirements=connector_runtime_requirements, ) except HTTPException: @@ -4697,7 +4699,7 @@ def _get_default_internal_model_ids() -> Dict[str, Optional[str]]: # # The asymmetry with the ConnectorRuntimeError arm above is real # and deliberate: that arm has a live producer inside this endpoint - # (``prepare_connector_runtime_selection_snapshot``), this one has + # (``resolve_agent_runtime_requirements``), this one has # none. It is kept because what the two arms share is the # failure-path contract, not the producer: both errors carry their # own status and a caller-safe message, and the blanket handler diff --git a/src/xagent/web/api/public_chat_access.py b/src/xagent/web/api/public_chat_access.py index e7f73996b9..fa7b8e1bf0 100644 --- a/src/xagent/web/api/public_chat_access.py +++ b/src/xagent/web/api/public_chat_access.py @@ -1036,6 +1036,10 @@ async def _create_workforce_widget_chat_task( else None, channel_id=task.channel_id, channel_name=task.channel_name, + # This path never resolves connector-runtime requirements for + # its caller: the widget/share guest never sees connector key + # names. + connector_runtime_requirements=None, ) @@ -1140,6 +1144,10 @@ async def create_public_chat_task( else None, channel_id=task.channel_id, channel_name=task.channel_name, + # This path never resolves connector-runtime requirements for + # its caller: the widget/share guest never sees connector key + # names. + connector_runtime_requirements=None, ) @@ -1209,6 +1217,10 @@ async def _create_workforce_share_chat_task( else None, channel_id=task.channel_id, channel_name=task.channel_name, + # This path never resolves connector-runtime requirements for + # its caller: the widget/share guest never sees connector key + # names. + connector_runtime_requirements=None, ) @@ -1297,6 +1309,10 @@ async def create_share_chat_task( else None, channel_id=task.channel_id, channel_name=task.channel_name, + # This path never resolves connector-runtime requirements for + # its caller: the widget/share guest never sees connector key + # names. + connector_runtime_requirements=None, ) diff --git a/src/xagent/web/schemas/chat.py b/src/xagent/web/schemas/chat.py index 424f36164b..059aac192d 100644 --- a/src/xagent/web/schemas/chat.py +++ b/src/xagent/web/schemas/chat.py @@ -6,6 +6,7 @@ from pydantic import BaseModel, Field, model_validator from ...core.task_runtime import MAX_TASK_RUNTIME_EXTENSIONS +from .connector_runtime import ConnectorRuntimeRequirementsModel # Only ever read here (TaskCreateRequest.seed_interactions is passed straight # through to create_task_with_message as-is - see api/chat.py), unlike @@ -194,6 +195,20 @@ class TaskCreateResponse(BaseModel): "the status is `truncated`. Empty otherwise." ), ) + connector_runtime_requirements: ConnectorRuntimeRequirementsModel | None = Field( + ..., + description=( + "Which runtime inputs this task's connectors still need, and " + "which of them already have a value. Always present in the " + "response body -- a client must not treat its absence as " + "meaning anything. Never includes a stored value itself, only " + "whether one exists. Present with a report on the web chat " + "create path (`POST /api/chat/task/create`); `null` on the " + "public chat and share-link create paths, meaning the " + "requirements were not evaluated there -- those visitors never " + "receive connector key names." + ), + ) class ExecutionStatus(BaseModel): diff --git a/tests/web/api/test_websocket_preview.py b/tests/web/api/test_websocket_preview.py index fc58169c7b..7262acb1cb 100644 --- a/tests/web/api/test_websocket_preview.py +++ b/tests/web/api/test_websocket_preview.py @@ -25,6 +25,7 @@ from xagent.web.models.uploaded_file import UploadedFile from xagent.web.models.user import User from xagent.web.schemas.chat import TaskCreateResponse +from xagent.web.schemas.connector_runtime import ConnectorRuntimeRequirementsModel class _BlockingPreviewWebSocket: @@ -103,6 +104,9 @@ async def test_handle_build_preview_execution_uses_normal_task_flow(): title="test message", status="pending", created_at="2026-05-20T00:00:00Z", + connector_runtime_requirements=ConnectorRuntimeRequirementsModel( + satisfied=True, secrets_expires_at=None, connectors=[] + ), ) with ( patch("xagent.web.models.database.get_db", return_value=iter([mock_db])), @@ -158,6 +162,9 @@ async def test_handle_build_preview_execution_does_not_use_preview_sessions(): title="test message", status="pending", created_at="2026-05-20T00:00:00Z", + connector_runtime_requirements=ConnectorRuntimeRequirementsModel( + satisfied=True, secrets_expires_at=None, connectors=[] + ), ) with ( patch("xagent.web.models.database.get_db", return_value=iter([mock_db])), @@ -243,6 +250,9 @@ async def test_handle_build_preview_execution_creates_task_when_no_preview_task_ title="second turn", status="pending", created_at="2026-05-20T00:00:00Z", + connector_runtime_requirements=ConnectorRuntimeRequirementsModel( + satisfied=True, secrets_expires_at=None, connectors=[] + ), ) with ( patch("xagent.web.models.database.get_db", return_value=iter([mock_db])), diff --git a/tests/web/test_connector_runtime_entrypoints_e2e.py b/tests/web/test_connector_runtime_entrypoints_e2e.py index 25e1cd3809..795fc761ed 100644 --- a/tests/web/test_connector_runtime_entrypoints_e2e.py +++ b/tests/web/test_connector_runtime_entrypoints_e2e.py @@ -30,11 +30,13 @@ get_db, get_engine, ) +from xagent.web.models.deployment import Deployment, DeploymentOwnerType from xagent.web.models.mcp import MCPServer, UserMCPServer from xagent.web.models.task import Task, TaskConnectorRuntimeContext, TaskStatus from xagent.web.models.uploaded_file import UploadedFile from xagent.web.models.user import User from xagent.web.models.user_channel import UserChannel +from xagent.web.models.workforce import Workforce from xagent.web.services import connector_team_scope from xagent.web.services.agent_team_scope import ( AgentTeamScope, @@ -1841,5 +1843,414 @@ def test_read_endpoints_reject_anonymous_and_widget_credentials( # --------------------------------------------------------------------------- -# A3: connector_runtime_requirements on the task-create response. +# The connector_runtime_requirements field on the task-create response. +# --------------------------------------------------------------------------- + + +def test_create_task_reports_missing_context_requirement(e2e_db: None) -> None: + """An agent with an unmet required ``context`` key reports it on the + create response without writing anything, and the task starts PENDING. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + server = _mcp_server_with_context_schema( + db, + user, + name="a3-context-server", + context_schema={"auth_token": {"type": "string", "required": True}}, + ) + agent = _create_agent( + db, user, name="Context Requirement Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + db.refresh(server) + agent_id = int(agent.id) + finally: + db.close() + + response = client.post( + "/api/chat/task/create", + headers=headers, + json={"title": "a3 context task", "description": "d", "agent_id": agent_id}, + ) + assert response.status_code == 200, response.text + payload = response.json() + requirements = payload["connector_runtime_requirements"] + assert requirements["satisfied"] is False + keys = { + item["key"] + for connector in requirements["connectors"] + for item in connector["inputs"] + } + assert "auth_token" in keys + task_id = int(payload["task_id"]) + assert _context_row_count(task_id) == 0 + assert _task(task_id).status == TaskStatus.PENDING + + +def test_create_task_reports_missing_secret_requirement_without_reading_any_column( + e2e_db: None, +) -> None: + """A required ``secrets`` key makes the top-level ``satisfied`` false + purely from the phase-2 constant, with no secret store or column read + anywhere in this phase. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + server = MCPServer( + name="a3-secret-server", + description="a3-secret-server description", + managed="external", + transport="streamable_http", + url="https://example.com/mcp", + runtime_input_schema={ + "secrets": {"authorization": {"type": "string", "required": True}} + }, + runtime_bindings=[ + { + "source": {"input_type": "secrets", "key": "authorization"}, + "target": { + "target_type": "transport_headers", + "key": "Authorization", + }, + } + ], + ) + db.add(server) + db.flush() + db.add( + UserMCPServer( + user_id=user.id, + mcpserver_id=server.id, + is_owner=True, + can_edit=True, + can_delete=True, + is_active=True, + ) + ) + agent = _create_agent( + db, user, name="Secret Requirement Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + finally: + db.close() + + response = client.post( + "/api/chat/task/create", + headers=headers, + json={"title": "a3 secret task", "description": "d", "agent_id": agent_id}, + ) + assert response.status_code == 200, response.text + requirements = response.json()["connector_runtime_requirements"] + assert requirements["satisfied"] is False + secret_input = next( + item + for connector in requirements["connectors"] + for item in connector["inputs"] + if item["section"] == "secrets" + ) + assert secret_input["satisfied"] is False + assert requirements["secrets_expires_at"] is None + + +def test_create_task_reports_empty_requirements_when_nothing_is_declared( + e2e_db: None, +) -> None: + """On the logged-in web chat create path, the field always appears, and + with no declared connectors it is the empty, always-satisfied report -- + never absent, never ``null`` (``null`` is reserved for the public/share + paths, which never evaluate this at all; see the public-path tests + below). + """ + headers = _setup_admin_headers() + response = client.post( + "/api/chat/task/create", + headers=headers, + json={"title": "a3 empty task", "description": "d"}, + ) + assert response.status_code == 200, response.text + payload = response.json() + assert "connector_runtime_requirements" in payload + assert payload["connector_runtime_requirements"] == { + "satisfied": True, + "secrets_expires_at": None, + "connectors": [], + } + + +def _create_workforce_with_deployment( + db: Session, + user: User, + *, + name: str, + widget_enabled: bool = False, + share_enabled: bool = False, +) -> tuple[Workforce, Deployment]: + """A minimal published workforce with a deployment row, for the two + workforce-backed public create paths (widget and share). The workforce's + own manager agent is a bystander here -- only its FK needs to resolve -- + so it is created with no runtime declarations of its own.""" + manager = _create_agent(db, user, name=f"{name} Manager", tool_categories=[]) + workforce = Workforce( + owner_user_id=user.id, + scope_type="user", + scope_id=str(user.id), + name=name, + manager_agent_id=manager.id, + status="active", + ) + db.add(workforce) + db.flush() + deployment = Deployment( + owner_type=DeploymentOwnerType.WORKFORCE.value, + owner_id=workforce.id, + widget_enabled=widget_enabled, + widget_key=f"wfwk-{secrets.token_urlsafe(24)}" if widget_enabled else None, + share_enabled=share_enabled, + share_token=f"wfst-{secrets.token_urlsafe(24)}" if share_enabled else None, + ) + db.add(deployment) + db.flush() + return workforce, deployment + + +@pytest.mark.parametrize( + "producer", + ["widget_agent", "workforce_widget", "share_agent", "workforce_share"], +) +def test_public_create_paths_all_report_null_requirements( + e2e_db: None, monkeypatch: pytest.MonkeyPatch, producer: str +) -> None: + """Every ``TaskCreateResponse`` producer in ``public_chat_access.py`` + that serves an anonymous widget or share guest sets + ``connector_runtime_requirements`` to ``None``, never a real report: the + widget-agent, workforce-widget, share-agent and workforce-share paths + are four separate call sites with four separate explicit ``None`` + literals, all guarding the same decision that a guest never sees a + connector's declared key names. + + The two workforce producers are reached through the real auth and route + layers; only ``create_workforce_run`` -- the heavy collaborator that + snapshots agent config and starts the first turn -- is stubbed to return + an already-created task, which is all a response-shape assertion needs. + """ + is_widget = producer in ("widget_agent", "workforce_widget") + is_workforce = producer in ("workforce_widget", "workforce_share") + + _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + if not is_workforce: + owner_agent = _create_agent( + db, + user, + name=f"Null {producer} Agent", + tool_categories=["mcp"], + widget_enabled=is_widget, + share_enabled=not is_widget, + share_token=( + None if is_widget else f"null-share-{secrets.token_urlsafe(16)}" + ), + ) + db.commit() + db.refresh(owner_agent) + credential = ( + owner_agent.widget_key if is_widget else owner_agent.share_token + ) + else: + _workforce, deployment = _create_workforce_with_deployment( + db, + user, + name=f"Null {producer} Workforce", + widget_enabled=is_widget, + share_enabled=not is_widget, + ) + db.commit() + db.refresh(deployment) + credential = deployment.widget_key if is_widget else deployment.share_token + + stub_task = Task( + user_id=user.id, + title="stub workforce task", + status=TaskStatus.PENDING, + source="widget" if is_widget else "shared_link", + ) + db.add(stub_task) + db.commit() + db.refresh(stub_task) + + from xagent.web.api import public_chat_access as public_chat_access_module + + async def _fake_create_workforce_run(*_args: Any, **_kwargs: Any) -> Any: + return SimpleNamespace(task=stub_task) + + monkeypatch.setattr( + public_chat_access_module, + "create_workforce_run", + _fake_create_workforce_run, + ) + finally: + db.close() + + if is_widget: + auth_response = client.post( + "/api/widget/auth", + json={"guest_id": f"null-{producer}-guest", "widget_key": credential}, + ) + assert auth_response.status_code == 200, auth_response.text + guest_token = auth_response.json()["access_token"] + create_response = client.post( + "/api/widget/chat/task/create", + headers={"Authorization": f"Bearer {guest_token}"}, + json={"title": f"null {producer} task", "description": "d"}, + ) + else: + auth_response = client.post("/api/share/auth", json={"share_token": credential}) + assert auth_response.status_code == 200, auth_response.text + guest_token = auth_response.json()["access_token"] + create_response = client.post( + "/api/share/chat/task/create", + headers={"Authorization": f"Bearer {guest_token}"}, + json={"title": f"null {producer} task", "description": "d"}, + ) + + assert create_response.status_code == 200, create_response.text + payload = create_response.json() + assert "connector_runtime_requirements" in payload + assert payload["connector_runtime_requirements"] is None + + +def test_create_task_calls_connector_resolution_exactly_once( + e2e_db: None, monkeypatch: pytest.MonkeyPatch +) -> None: + """The create response's requirements report costs no extra query -- + ``resolve_agent_selected_connectors`` is called exactly once per task + creation, not once for the snapshot and again for the report. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + server = _mcp_server_with_context_schema( + db, + user, + name="a3-call-count-server", + context_schema={"auth_token": {"type": "string", "required": False}}, + ) + agent = _create_agent( + db, user, name="Call Count Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + db.refresh(server) + agent_id = int(agent.id) + finally: + db.close() + + from xagent.web.services import connector_runtime as connector_runtime_service + + original = connector_runtime_service.resolve_agent_selected_connectors + calls: list[int] = [] + + def _counting_resolver(*args: Any, **kwargs: Any) -> Any: + calls.append(1) + return original(*args, **kwargs) + + monkeypatch.setattr( + connector_runtime_service, + "resolve_agent_selected_connectors", + _counting_resolver, + ) + + response = client.post( + "/api/chat/task/create", + headers=headers, + json={ + "title": "a3 call count task", + "description": "d", + "agent_id": agent_id, + }, + ) + assert response.status_code == 200, response.text + assert len(calls) == 1 + + +def test_create_task_persists_same_selected_refs_as_legacy_snapshot( + e2e_db: None, +) -> None: + """The persisted ``Task.connector_runtime_selected_refs`` column -- which + the per-turn gate, the values endpoint's selection check, and + ``load_connector_runtime_view`` all read -- is unchanged in content and + order by task creation's switch to ``resolve_agent_runtime_requirements``. + Compared against the legacy ``prepare_connector_runtime_selection_snapshot`` + (the column's pre-existing source of truth) on the same agent, with + list equality (not set equality) so a reordering would fail this too. + """ + from xagent.web.services.connector_runtime import ( + prepare_connector_runtime_selection_snapshot, + ) + + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + declared_one = _mcp_server_with_context_schema( + db, + user, + name="a3-refs-declared-1", + context_schema={"account_id": {"type": "string", "required": False}}, + ) + declared_two = _mcp_server_with_context_schema( + db, + user, + name="a3-refs-declared-2", + context_schema={"account_id": {"type": "string", "required": False}}, + ) + undeclared = _create_mcp_server( + db, user, name="a3-refs-undeclared", with_runtime_declaration=False + ) + agent = _create_agent( + db, user, name="Refs Order Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + db.refresh(declared_one) + db.refresh(declared_two) + db.refresh(undeclared) + agent_id = int(agent.id) + agent_row = db.query(Agent).filter(Agent.id == agent_id).one() + expected_refs = list( + prepare_connector_runtime_selection_snapshot( + db=db, agent=agent_row, connector_user_id=int(user.id) + ) + ) + finally: + db.close() + + response = client.post( + "/api/chat/task/create", + headers=headers, + json={ + "title": "a3 refs order task", + "description": "d", + "agent_id": agent_id, + }, + ) + assert response.status_code == 200, response.text + task_id = int(response.json()["task_id"]) + persisted_refs = _task(task_id).connector_runtime_selected_refs + expected_wire = [ref.to_wire() for ref in expected_refs] + assert persisted_refs == expected_wire + + +# --------------------------------------------------------------------------- +# A4: POST /task/{task_id}/connector-runtime-values. # --------------------------------------------------------------------------- From 8756d4732815e4bfbef63ca255ebac1e145dabbe Mon Sep 17 00:00:00 2001 From: Alexliu Date: Mon, 7 Sep 2026 02:21:44 +0800 Subject: [PATCH 3/5] fix(web): align the connector runtime report with the per-turn path The task-keyed requirements endpoint resolved the task's agent with a raw query, so it derived the connector team scope from the raw row instead of the scope a turn actually runs under. It now resolves the agent with the same two calls the per-turn tool build makes for the task, in the same order, so the report can neither over- nor under-report team-shared connectors. The report also treated a declared key whose syntax the per-turn gate rejects as an ordinary key, so a stored value under that name could make the report read satisfied for a task no turn can run. Such a key is now always reported unsatisfied. The report still never raises, and the per-turn gate still fails the turn on the same key. Adds the missing coverage for the report's positive branch: a stored context value reported as satisfied, a task whose agent the runtime resolves to none, an unfillable declared key, a custom_api connector, the auth_selector skip, and the expired field. Drops a section header left behind for tests that belong to a later change. Hoists the four identical "no requirements on this path" comments in the public create paths into one named constant. --- src/xagent/web/api/chat.py | 16 +- src/xagent/web/api/public_chat_access.py | 27 +- src/xagent/web/services/connector_runtime.py | 23 +- .../test_connector_runtime_entrypoints_e2e.py | 360 +++++++++++++++++- 4 files changed, 380 insertions(+), 46 deletions(-) diff --git a/src/xagent/web/api/chat.py b/src/xagent/web/api/chat.py index 2f086ef9e5..8d0abc4dec 100644 --- a/src/xagent/web/api/chat.py +++ b/src/xagent/web/api/chat.py @@ -5360,11 +5360,17 @@ async def get_task_connector_runtime_requirements( task = db.query(Task).filter(Task.id == task_id, Task.user_id == user.id).first() if task is None: raise HTTPException(status_code=404, detail="Task not found") - agent = ( - db.query(Agent).filter(Agent.id == task.agent_id).first() - if task.agent_id is not None - else None - ) + # The connector scope this report is built from must be the scope a + # turn would actually run under, so the agent is resolved by the same + # two calls the per-turn tool build makes for this task, in the same + # order. Reading the agent row directly would key team-shared + # connectors on the raw row's team even where the runtime resolves the + # agent to None, and resolving with no workforce runtime would drop + # the team of a workforce manager agent whose run the runtime does + # find. A report that says what a turn will need can afford neither + # the over-report nor the under-report. + workforce_runtime = resolve_workforce_task_runtime(db, task) + agent = _load_agent_for_task_runtime(db, task, workforce_runtime) try: return build_task_runtime_requirements(db=db, task=task, agent=agent) except ConnectorRuntimeError as exc: diff --git a/src/xagent/web/api/public_chat_access.py b/src/xagent/web/api/public_chat_access.py index fa7b8e1bf0..f0c977cbcd 100644 --- a/src/xagent/web/api/public_chat_access.py +++ b/src/xagent/web/api/public_chat_access.py @@ -34,6 +34,7 @@ from ..models.user_channel import UserChannel from ..models.workforce import Workforce, WorkforceRun from ..schemas.chat import TaskCreateRequest, TaskCreateResponse +from ..schemas.connector_runtime import ConnectorRuntimeRequirementsModel from ..services.client_error_messages import ClientErrorCode, client_error_message from ..services.connector_runtime import ( bind_connector_runtime_selection_snapshot, @@ -76,6 +77,12 @@ # the worst single-request abuse. Broader quota + orphan GC tracked in #973. MAX_TASKLESS_SHARE_UPLOAD_FILES = 10 +# Every public create path in this module answers this field the same way: +# it never resolves connector-runtime requirements for its caller, because +# the widget/share guest must not see connector key names. Kept as one +# named constant so the four call sites cannot drift apart. +_NO_CONNECTOR_RUNTIME_REQUIREMENTS: ConnectorRuntimeRequirementsModel | None = None + class PublicChatAuthResponse(BaseModel): access_token: str @@ -1036,10 +1043,7 @@ async def _create_workforce_widget_chat_task( else None, channel_id=task.channel_id, channel_name=task.channel_name, - # This path never resolves connector-runtime requirements for - # its caller: the widget/share guest never sees connector key - # names. - connector_runtime_requirements=None, + connector_runtime_requirements=_NO_CONNECTOR_RUNTIME_REQUIREMENTS, ) @@ -1144,10 +1148,7 @@ async def create_public_chat_task( else None, channel_id=task.channel_id, channel_name=task.channel_name, - # This path never resolves connector-runtime requirements for - # its caller: the widget/share guest never sees connector key - # names. - connector_runtime_requirements=None, + connector_runtime_requirements=_NO_CONNECTOR_RUNTIME_REQUIREMENTS, ) @@ -1217,10 +1218,7 @@ async def _create_workforce_share_chat_task( else None, channel_id=task.channel_id, channel_name=task.channel_name, - # This path never resolves connector-runtime requirements for - # its caller: the widget/share guest never sees connector key - # names. - connector_runtime_requirements=None, + connector_runtime_requirements=_NO_CONNECTOR_RUNTIME_REQUIREMENTS, ) @@ -1309,10 +1307,7 @@ async def create_share_chat_task( else None, channel_id=task.channel_id, channel_name=task.channel_name, - # This path never resolves connector-runtime requirements for - # its caller: the widget/share guest never sees connector key - # names. - connector_runtime_requirements=None, + connector_runtime_requirements=_NO_CONNECTOR_RUNTIME_REQUIREMENTS, ) diff --git a/src/xagent/web/services/connector_runtime.py b/src/xagent/web/services/connector_runtime.py index 27e7bcb924..3e7ec1efee 100644 --- a/src/xagent/web/services/connector_runtime.py +++ b/src/xagent/web/services/connector_runtime.py @@ -906,11 +906,24 @@ def _build_connector_report( continue declarations = _schema_section(schema, section_name) for key, declaration in declarations.items(): - satisfied = ( - key in context_stored - if section_name == RUNTIME_INPUT_CONTEXT - else False - ) + try: + validate_runtime_source_key(key) + except ValueError: + # No value can ever be stored under this key's syntax -- + # the per-turn gate rejects it with a 400, required or + # not. It is still listed, unconditionally unsatisfied + # even with a stored value under that name, so a required + # one holds the top-level ``satisfied`` at false; dropping + # the key would let that flag read true instead. This + # report still never raises: the real fix is validating + # key syntax at connector create/update time, not here. + satisfied = False + else: + satisfied = ( + key in context_stored + if section_name == RUNTIME_INPUT_CONTEXT + else False + ) inputs.append( ConnectorRuntimeInputModel( section=section_name, diff --git a/tests/web/test_connector_runtime_entrypoints_e2e.py b/tests/web/test_connector_runtime_entrypoints_e2e.py index 795fc761ed..edb5ca672d 100644 --- a/tests/web/test_connector_runtime_entrypoints_e2e.py +++ b/tests/web/test_connector_runtime_entrypoints_e2e.py @@ -25,6 +25,7 @@ from xagent.web.channels.telegram.bot import TelegramBotInstance from xagent.web.models.agent import Agent, AgentOrigin, AgentStatus from xagent.web.models.chat_message import TaskChatMessage +from xagent.web.models.custom_api import CustomApi, UserCustomApi from xagent.web.models.database import ( Base, get_db, @@ -1386,6 +1387,7 @@ def test_agent_requirements_hides_connection_config_and_normalizes_type( # normalize to "string", not pass through unnormalized. assert inputs_by_key["profile"]["type"] == "string" assert inputs_by_key["auth_token"]["satisfied"] is False + assert inputs_by_key["auth_token"]["expired"] is False # A task created from this same agent, with this same connector's # required key filled directly in storage, must not move this report's @@ -1522,6 +1524,80 @@ def test_agent_requirements_hides_non_visible_agents( set_agent_team_scope_hook(None) +def test_custom_api_requirements_report_lists_context_only(e2e_db: None) -> None: + """A custom_api connector's declared ``context`` key is listed, and its + declared ``auth_selector`` key is not: the ``auth_selector`` section is + only ever emitted for an MCP connector, so a non-MCP connector's + declaration of it is never surfaced in a report. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + api = CustomApi( + name="custom-runtime-api", + description="custom-runtime-api description", + url="https://example.com/custom", + method="GET", + runtime_input_schema={ + "context": {"tenant_id": {"type": "string", "required": True}}, + "auth_selector": {"profile": {"type": "string", "required": False}}, + }, + runtime_bindings=[ + { + "source": {"input_type": "context", "key": "tenant_id"}, + "target": {"target_type": "headers", "key": "X-Tenant-Id"}, + } + ], + ) + db.add(api) + db.flush() + api_id = int(api.id) + db.add( + UserCustomApi( + user_id=user.id, + custom_api_id=api_id, + is_owner=True, + can_edit=True, + can_delete=True, + is_active=True, + ) + ) + agent = _create_agent( + db, + user, + name="Custom API Agent", + tool_categories=["mcp:custom-runtime-api"], + ) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + finally: + db.close() + + response = client.get( + f"/api/chat/agent/{agent_id}/connector-runtime-requirements", + headers=headers, + ) + assert response.status_code == 200, response.text + assert "https://example.com/custom" not in response.text + payload = response.json() + assert len(payload["connectors"]) == 1 + connector = payload["connectors"][0] + assert connector["connector_ref"] == { + "connector_type": "custom_api", + "connector_id": api_id, + } + section_keys = {(item["section"], item["key"]) for item in connector["inputs"]} + assert section_keys == {("context", "tenant_id")} + tenant_id_input = next( + item for item in connector["inputs"] if item["key"] == "tenant_id" + ) + assert tenant_id_input["satisfied"] is False + assert tenant_id_input["expired"] is False + assert tenant_id_input["required"] is True + + def test_team_shared_connector_visible_across_read_endpoints(e2e_db: None) -> None: """A connector shared only through the agent's team is listed by both read endpoints for a non-owning team member, and the task-keyed read @@ -1674,18 +1750,14 @@ def _hook(db: Session, *, team_id: int) -> dict[str, set[int]]: connector_team_scope.set_connector_team_hooks() -def test_agent_requirements_endpoint_bypassing_team_resolver_hides_shared_connector( +def test_agent_requirements_lists_connector_shared_only_through_agent_team( e2e_db: None, ) -> None: - """If the agent-keyed endpoint ever bypasses - ``resolve_agent_selected_connectors`` and loads visible connectors with - ``agent_team_id=None`` instead of the value that resolver derives from - the agent, a team-shared-only connector silently disappears from the - report. This test pins the *correct* behavior (the connector is - listed); the mutation itself has no test-visible seam to monkeypatch - without changing which production code path runs, so it is applied and - reverted directly against ``resolve_agent_runtime_requirements`` in the - execution report's mutation table rather than parametrized here. + """A connector the caller can reach only through the agent's team -- + no personal link of their own -- is listed by the agent-keyed report. + The team id the report resolves connectors under comes from the agent, + not from the caller, so this is the case that would silently vanish if + that scope were ever taken from the caller instead. """ db = _db_session() try: @@ -1757,12 +1829,265 @@ def test_agent_requirements_endpoint_bypassing_team_resolver_hides_shared_connec connector_team_scope.set_connector_team_hooks() +def test_task_requirements_reports_stored_context_key_as_satisfied( + e2e_db: None, +) -> None: + """A ``context`` key that already has a stored value is reported + ``satisfied`` on the task-keyed read endpoint, and the top-level + ``satisfied`` follows it when it is the connector's only required key. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + server = _mcp_server_with_context_schema( + db, + user, + name="satisfied-server", + context_schema={"auth_token": {"type": "string", "required": True}}, + ) + agent = _create_agent( + db, user, name="Satisfied Context Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + db.refresh(server) + agent_id = int(agent.id) + server_id = int(server.id) + finally: + db.close() + + create_response = client.post( + "/api/chat/task/create", + headers=headers, + json={"title": "satisfied task", "description": "d", "agent_id": agent_id}, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + + db = _db_session() + try: + db.add( + TaskConnectorRuntimeContext( + task_id=task_id, + connector_type="mcp", + connector_id=server_id, + context={"auth_token": "stored"}, + ) + ) + db.commit() + finally: + db.close() + + response = client.get( + f"/api/chat/task/{task_id}/connector-runtime-requirements", + headers=headers, + ) + assert response.status_code == 200, response.text + payload = response.json() + inputs_by_key = { + item["key"]: item + for connector in payload["connectors"] + for item in connector["inputs"] + } + assert inputs_by_key["auth_token"]["satisfied"] is True + assert inputs_by_key["auth_token"]["expired"] is False + assert payload["satisfied"] is True + + +def test_task_requirements_uses_runtime_agent_resolution_for_team_scope( + e2e_db: None, +) -> None: + """A task whose agent is a workforce-generated manager agent, but for + which no matching ``WorkforceRun`` exists, gets its connector scope + from ``_load_agent_for_task_runtime`` returning ``None`` -- the same + outcome a turn would get for this task -- so a connector reachable + only through the agent's team is omitted while a personally linked + connector is still reported. + + The task is built directly rather than through ``POST /task/create``: + that path 404s for a workforce-generated manager agent + (``_load_agent_for_task_create`` returns ``None`` for one), so a task + with this agent can only exist by way of a row the workforce side + creates directly, and a direct row is the only reachable construction + for exercising the read endpoint against one. + """ + db = _db_session() + try: + caller = _create_user(db, "wf-task-owner") + db.flush() + personal = _mcp_server_with_context_schema( + db, + caller, + name="wf-personal-server", + context_schema={"auth_token": {"type": "string", "required": True}}, + url="https://example.com/personal/mcp", + ) + team_shared = MCPServer( + name="wf-team-shared-server", + description="wf-team-shared-server description", + managed="external", + transport="streamable_http", + url="https://example.com/team/mcp", + runtime_input_schema={ + "context": {"auth_token": {"type": "string", "required": True}} + }, + runtime_bindings=[ + { + "source": {"input_type": "context", "key": "auth_token"}, + "target": {"target_type": "mcp_meta", "key": "auth_token"}, + } + ], + ) + db.add(team_shared) + db.flush() + # No UserMCPServer link for `caller` at all -- reachable only + # through the team hook below. + personal_id = int(personal.id) + team_shared_id = int(team_shared.id) + agent = Agent( + user_id=caller.id, + name="WF Manager Agent", + instructions="i", + execution_mode="balanced", + status=AgentStatus.PUBLISHED, + tool_categories=["mcp"], + team_id=303, + origin=AgentOrigin.WORKFORCE_GENERATED_MANAGER.value, + ) + db.add(agent) + db.flush() + agent_id = int(agent.id) + ordered_ids = sorted([personal_id, team_shared_id]) + task = Task( + user_id=caller.id, + agent_id=agent_id, + title="workforce manager task", + description="d", + status=TaskStatus.PENDING, + connector_runtime_selected_refs=[ + {"connector_type": "mcp", "connector_id": ref_id} + for ref_id in ordered_ids + ], + ) + db.add(task) + db.commit() + db.refresh(task) + task_id = int(task.id) + caller_headers = _auth_headers_for_user(caller) + finally: + db.close() + + def _hook(db: Session, *, team_id: int) -> dict[str, set[int]]: + if team_id == 303: + return {"mcp": {team_shared_id}, "custom_api": set()} + return {"mcp": set(), "custom_api": set()} + + connector_team_scope.set_connector_team_hooks(team_visibility=_hook) + try: + response = client.get( + f"/api/chat/task/{task_id}/connector-runtime-requirements", + headers=caller_headers, + ) + finally: + connector_team_scope.set_connector_team_hooks() + + assert response.status_code == 200, response.text + payload = response.json() + refs_seen = [item["connector_ref"] for item in payload["connectors"]] + assert refs_seen == [{"connector_type": "mcp", "connector_id": personal_id}] + auth_token_input = next( + item + for item in payload["connectors"][0]["inputs"] + if item["key"] == "auth_token" + ) + assert auth_token_input["satisfied"] is False + + +def test_task_requirements_reports_unfillable_declared_key_as_unsatisfied( + e2e_db: None, +) -> None: + """A declared ``context`` key whose name the per-turn gate rejects as + malformed is still listed in the report, but is reported unsatisfied + unconditionally -- even with a stored value under that name -- so the + top-level ``satisfied`` stays false while a required key of that kind + is declared. The report itself never raises on this key; only the + per-turn gate does. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + server = _mcp_server_with_context_schema( + db, + user, + name="malformed-key-server", + context_schema={ + "auth_token": {"type": "string", "required": True}, + "bad.key": {"type": "string", "required": True}, + }, + url="https://example.com/malformed/mcp", + ) + agent = _create_agent( + db, user, name="Malformed Key Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + db.refresh(server) + agent_id = int(agent.id) + server_id = int(server.id) + finally: + db.close() + + create_response = client.post( + "/api/chat/task/create", + headers=headers, + json={ + "title": "malformed key task", + "description": "d", + "agent_id": agent_id, + }, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + + db = _db_session() + try: + db.add( + TaskConnectorRuntimeContext( + task_id=task_id, + connector_type="mcp", + connector_id=server_id, + context={"auth_token": "stored", "bad.key": "stored"}, + ) + ) + db.commit() + finally: + db.close() + + response = client.get( + f"/api/chat/task/{task_id}/connector-runtime-requirements", + headers=headers, + ) + assert response.status_code == 200, response.text + payload = response.json() + inputs_by_key = { + item["key"]: item + for connector in payload["connectors"] + for item in connector["inputs"] + } + assert set(inputs_by_key) == {"auth_token", "bad.key"} + assert inputs_by_key["bad.key"]["satisfied"] is False + assert inputs_by_key["auth_token"]["satisfied"] is True + assert payload["satisfied"] is False + + def test_task_requirements_endpoint_requires_task_ownership_by_caller( e2e_db: None, ) -> None: """A task belonging to another logged-in user is a uniform 404 on the - task-keyed read endpoint, matching the values endpoint's ownership - predicate. + task-keyed read endpoint, per plain task ownership with no admin + exception. """ headers = _setup_admin_headers() db = _db_session() @@ -2187,9 +2512,9 @@ def test_create_task_persists_same_selected_refs_as_legacy_snapshot( e2e_db: None, ) -> None: """The persisted ``Task.connector_runtime_selected_refs`` column -- which - the per-turn gate, the values endpoint's selection check, and - ``load_connector_runtime_view`` all read -- is unchanged in content and - order by task creation's switch to ``resolve_agent_runtime_requirements``. + the per-turn gate and ``load_connector_runtime_view`` both read -- is + unchanged in content and order by task creation's switch to + ``resolve_agent_runtime_requirements``. Compared against the legacy ``prepare_connector_runtime_selection_snapshot`` (the column's pre-existing source of truth) on the same agent, with list equality (not set equality) so a reordering would fail this too. @@ -2249,8 +2574,3 @@ def test_create_task_persists_same_selected_refs_as_legacy_snapshot( persisted_refs = _task(task_id).connector_runtime_selected_refs expected_wire = [ref.to_wire() for ref in expected_refs] assert persisted_refs == expected_wire - - -# --------------------------------------------------------------------------- -# A4: POST /task/{task_id}/connector-runtime-values. -# --------------------------------------------------------------------------- From d3e8f27a26b0ef8a3cb86955c8550d6c1a8071ad Mon Sep 17 00:00:00 2001 From: Alexliu Date: Mon, 7 Sep 2026 02:23:21 +0800 Subject: [PATCH 4/5] docs(web): state what the runtime requirements report actually promises The response schema now says which question `satisfied` answers on each endpoint that returns it, that `section` is what separates "not supplied yet" from "not supplyable at this phase", and that `expired` is a constant in every section rather than only in the secret-bearing ones. The agent-keyed resolver's docstring no longer claims the persisted refs keep the caller's order: both the write and the read of that column sort canonically. --- src/xagent/web/schemas/connector_runtime.py | 29 +++++++++++++++++--- src/xagent/web/services/connector_runtime.py | 15 ++++++---- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/src/xagent/web/schemas/connector_runtime.py b/src/xagent/web/schemas/connector_runtime.py index 38ceceebf8..75ef75dc30 100644 --- a/src/xagent/web/schemas/connector_runtime.py +++ b/src/xagent/web/schemas/connector_runtime.py @@ -5,7 +5,9 @@ requirements report -- which runtime inputs a task's (or a prospective task's) connectors declare, and whether each one already has a value -- never a stored value itself, and never a connector's transport or -authentication configuration. +authentication configuration -- the same shape from both, with one field, +``satisfied``, answering a different question on each; see +``ConnectorRuntimeRequirementsModel``. Placed in its own module rather than ``schemas/chat.py`` because a values- submission endpoint lands on top of it shortly and will share this same @@ -32,9 +34,18 @@ class ConnectorRuntimeInputModel(BaseModel): input -- there is no human-readable label anywhere in the declaration. ``type`` is already normalized server-side to ``"string"`` or ``"object"``; a client must not normalize it again or expect any other - value. ``satisfied`` and ``expired`` are constants in this phase for the - ``secrets``/``auth_selector`` sections: no secret store exists yet, so - both are always ``False``. + value. ``expired`` is a constant ``False`` in every section at this + phase; a later phase that adds a real secret store gives it a real + value without changing its meaning. ``satisfied`` is likewise a + constant ``False`` for the ``secrets`` and ``auth_selector`` sections, + because no secret store exists yet to hold such a value, and it is the + ``section`` field that tells the two kinds of ``False`` apart: in the + ``context`` section ``False`` means the value has not been supplied yet + and can be, while in ``secrets`` and ``auth_selector`` it means no + value can be supplied at this phase at all. One ``context`` key is + also always ``False``: a key whose name the per-turn gate rejects as + malformed, which is reported so a required key of that kind cannot + let the report read as met. """ section: str @@ -65,6 +76,16 @@ class ConnectorRuntimeRequirementsModel(BaseModel): declares a runtime input. ``secrets_expires_at`` is a constant ``null`` in this phase; a later phase gives it a real value without changing its meaning or making it optional. + + ``satisfied`` answers a different question on each endpoint that + returns this model, and a client must read it against the endpoint it + called. The agent-keyed report has no task, so no value can be stored + against it: there ``satisfied`` answers "a task created from this agent + right now would need no further input", i.e. nothing required is + declared at all. The task-keyed report and the task-create response + answer "every required input of this task already has a value". The + per-input ``satisfied`` follows the same split: always ``False`` on the + agent-keyed report, and a real per-key answer on the other two. """ satisfied: bool diff --git a/src/xagent/web/services/connector_runtime.py b/src/xagent/web/services/connector_runtime.py index 3e7ec1efee..09e4b4245d 100644 --- a/src/xagent/web/services/connector_runtime.py +++ b/src/xagent/web/services/connector_runtime.py @@ -431,12 +431,15 @@ def resolve_agent_runtime_requirements( Calls ``resolve_agent_selected_connectors`` exactly once. The returned refs are ``_runtime_declared_refs`` applied to that same call's result - -- same filter, same order -- because a caller creating a task persists - them verbatim into ``Task.connector_runtime_selected_refs``, and every - later reader of that column (the per-turn gate, the values endpoint's - selection check, ``load_connector_runtime_view``) depends on it holding - exactly that set in exactly that order. Do not derive the refs any - other way, even one that looks equivalent. + -- same filter, same canonical order -- because a caller creating a + task persists them into ``Task.connector_runtime_selected_refs``, and + every later reader of that column (the per-turn gate, + ``load_connector_runtime_view``) depends on it holding exactly that + set. Both the write and the read of that column sort by + ``(connector_type, connector_id)``, so the invariant the column + carries is the set under that canonical order, not the order a caller + happened to hand over. Do not derive the refs any other way, even one + that looks equivalent. The report has no task to consult, so every input's ``satisfied`` is ``False`` and the top-level ``satisfied`` answers "would a task created From af8672712489c648dcb78fa87af0ed7d5d54dcc5 Mon Sep 17 00:00:00 2001 From: Alexliu Date: Mon, 7 Sep 2026 13:06:04 +0800 Subject: [PATCH 5/5] fix(web): fail the runtime requirements report on any malformed declared key The top-level `satisfied` flag counted only required inputs, so a connector declaring an optional key whose name the per-turn gate rejects reported a task as ready to run while every turn on it answers 400: both `_require_context_values` and `_require_ephemeral_values` validate the syntax of every declared key and raise before they look at `required`. `_all_required_inputs_satisfied` now re-checks every listed key through the same validator, so a malformed key holds the flag at false whether or not it is required. The key stays listed, its own `satisfied` stays false, and the report still never raises. Type `section` and `type` as literals so the closed sets both fields already documented in prose reach the OpenAPI schema. Correct what the task-create response claims: it is computed from the agent before the task is persisted, so it answers what a task created from that agent needs, not what the new task still misses. Only the task-keyed read consults stored values. Tests: a malformed optional key holding the flag false; an MCP connector declaring an `auth_selector` key; a non-vacuous satisfied report whose declared inputs are all optional; a task with no agent; an unknown id on both read endpoints; and an admin denied another user's task. --- src/xagent/web/schemas/chat.py | 23 +- src/xagent/web/schemas/connector_runtime.py | 32 +- src/xagent/web/services/connector_runtime.py | 49 ++- .../test_connector_runtime_entrypoints_e2e.py | 290 ++++++++++++++++++ 4 files changed, 366 insertions(+), 28 deletions(-) diff --git a/src/xagent/web/schemas/chat.py b/src/xagent/web/schemas/chat.py index 059aac192d..d8ecef8f4c 100644 --- a/src/xagent/web/schemas/chat.py +++ b/src/xagent/web/schemas/chat.py @@ -198,15 +198,20 @@ class TaskCreateResponse(BaseModel): connector_runtime_requirements: ConnectorRuntimeRequirementsModel | None = Field( ..., description=( - "Which runtime inputs this task's connectors still need, and " - "which of them already have a value. Always present in the " - "response body -- a client must not treat its absence as " - "meaning anything. Never includes a stored value itself, only " - "whether one exists. Present with a report on the web chat " - "create path (`POST /api/chat/task/create`); `null` on the " - "public chat and share-link create paths, meaning the " - "requirements were not evaluated there -- those visitors never " - "receive connector key names." + "Which runtime inputs the new task's connectors declare. " + "Computed from the agent before the task is persisted, so no " + "value can have been stored against the task yet and every " + "input reads unsatisfied: it answers what a task created from " + "this agent needs, not what this task still misses. Read " + "`GET /api/chat/task/{task_id}/connector-runtime-requirements` " + "for the second question. Always present in the response body " + "-- a client must not treat its absence as meaning anything. " + "Never includes a stored value itself, only whether one " + "exists. Present with a report on the web chat create path " + "(`POST /api/chat/task/create`); `null` on the public chat and " + "share-link create paths, meaning the requirements were not " + "evaluated there -- those visitors never receive connector key " + "names." ), ) diff --git a/src/xagent/web/schemas/connector_runtime.py b/src/xagent/web/schemas/connector_runtime.py index 75ef75dc30..20f5d6c6f7 100644 --- a/src/xagent/web/schemas/connector_runtime.py +++ b/src/xagent/web/schemas/connector_runtime.py @@ -17,6 +17,8 @@ from __future__ import annotations +from typing import Literal + from pydantic import BaseModel @@ -44,13 +46,17 @@ class ConnectorRuntimeInputModel(BaseModel): and can be, while in ``secrets`` and ``auth_selector`` it means no value can be supplied at this phase at all. One ``context`` key is also always ``False``: a key whose name the per-turn gate rejects as - malformed, which is reported so a required key of that kind cannot - let the report read as met. + malformed, which is reported so that no key of that kind, required or + not, can let the report read as met. + + ``section`` and ``type`` are closed sets, and a client may switch on + them exhaustively: the server emits no other value in either field, + and adding one would be a wire change. """ - section: str + section: Literal["context", "secrets", "auth_selector"] key: str - type: str + type: Literal["string", "object"] required: bool satisfied: bool expired: bool = False @@ -82,10 +88,20 @@ class ConnectorRuntimeRequirementsModel(BaseModel): called. The agent-keyed report has no task, so no value can be stored against it: there ``satisfied`` answers "a task created from this agent right now would need no further input", i.e. nothing required is - declared at all. The task-keyed report and the task-create response - answer "every required input of this task already has a value". The - per-input ``satisfied`` follows the same split: always ``False`` on the - agent-keyed report, and a real per-key answer on the other two. + declared at all. The task-create response answers that same question: + it is computed from the agent, before the new task is persisted and so + before any value could have been stored against it. Only the + task-keyed report consults stored values, and only there does + ``satisfied`` answer "every required input of this task already has a + value". The per-input ``satisfied`` follows the same split: always + ``False`` on the agent-keyed report and on the task-create response, + and a real per-key answer on the task-keyed report. + + ``satisfied`` is also ``False``, on every endpoint and whatever the + per-input flags say, while any listed key's name is one the per-turn + gate rejects as malformed -- required or not, because that gate + refuses the whole turn over such a key rather than only over a + required one. """ satisfied: bool diff --git a/src/xagent/web/services/connector_runtime.py b/src/xagent/web/services/connector_runtime.py index 09e4b4245d..2ce93f781b 100644 --- a/src/xagent/web/services/connector_runtime.py +++ b/src/xagent/web/services/connector_runtime.py @@ -443,8 +443,9 @@ def resolve_agent_runtime_requirements( The report has no task to consult, so every input's ``satisfied`` is ``False`` and the top-level ``satisfied`` answers "would a task created - from this agent right now need nothing else" -- i.e. it has no required - input at all. Never queries any task's stored values: doing so would + from this agent right now need nothing else" -- i.e. no required input + is declared, and no declared key carries a name the per-turn gate would + reject. Never queries any task's stored values: doing so would make the answer depend on which task happened to be looked up, and this endpoint has none in scope. @@ -915,11 +916,15 @@ def _build_connector_report( # No value can ever be stored under this key's syntax -- # the per-turn gate rejects it with a 400, required or # not. It is still listed, unconditionally unsatisfied - # even with a stored value under that name, so a required - # one holds the top-level ``satisfied`` at false; dropping - # the key would let that flag read true instead. This - # report still never raises: the real fix is validating - # key syntax at connector create/update time, not here. + # even with a stored value under that name; dropping the + # key would hide from a caller the one thing that will + # fail the turn. The top-level flag is held at false by + # ``_all_required_inputs_satisfied``, which re-checks the + # syntax of every listed key rather than reading these + # per-key flags alone, because a key of this kind fails + # the turn whether or not it is required. This report + # still never raises: the real fix is validating key + # syntax at connector create/update time, not here. satisfied = False else: satisfied = ( @@ -951,17 +956,39 @@ def _all_required_inputs_satisfied( ) -> bool: """Top-level ``satisfied``: every required input, across every reported connector and every section including ``secrets``, is - satisfied. ``all()`` over an empty sequence is ``True``, so no - connectors (or no required inputs) reports satisfied. + satisfied, and no reported connector declares a key whose name the + per-turn gate rejects. An empty report is therefore satisfied: no + connectors, or none with a required input and none with a malformed + key, reports satisfied. + + The malformed-key rule ignores ``required`` on purpose. The per-turn + gate (``_require_context_values``, ``_require_ephemeral_values``) + validates the syntax of every declared key and raises before it looks + at ``required`` at all, so a connector declaring ``{"bad.key": + {"required": false}}`` fails every turn while nothing about it is + required. Reading only the per-key ``satisfied`` flags of required + inputs would report such a task as ready to run. """ return all( - input_item.satisfied + _key_syntax_is_accepted(input_item.key) + and (input_item.satisfied or not input_item.required) for connector in connectors for input_item in connector.inputs - if input_item.required ) +def _key_syntax_is_accepted(key: str) -> bool: + """Whether the per-turn gate would accept this declared key's name, + asked without raising. Same predicate the gate applies, via the same + validator, so the two cannot drift apart. + """ + try: + validate_runtime_source_key(key) + except ValueError: + return False + return True + + def _build_task_requirements_model( selected_refs: tuple[ConnectorRef, ...], visible: dict[ConnectorRef, Any], diff --git a/tests/web/test_connector_runtime_entrypoints_e2e.py b/tests/web/test_connector_runtime_entrypoints_e2e.py index edb5ca672d..dda240a0e1 100644 --- a/tests/web/test_connector_runtime_entrypoints_e2e.py +++ b/tests/web/test_connector_runtime_entrypoints_e2e.py @@ -1598,6 +1598,121 @@ def test_custom_api_requirements_report_lists_context_only(e2e_db: None) -> None assert tenant_id_input["required"] is True +def test_mcp_auth_selector_key_is_listed_as_its_own_section(e2e_db: None) -> None: + """The mirror of the custom_api case above: on an MCP connector the + declared ``auth_selector`` key is listed, under its own section, and is + unsatisfied because no store holds such a value at this phase -- so a + required one keeps the whole report unsatisfied. The key carries no + runtime binding: binding an ``auth_selector`` key to a connector target + is rejected at declaration time, unlike a ``context`` key. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + server = MCPServer( + name="auth-selector-server", + description="auth-selector-server description", + managed="external", + transport="streamable_http", + url="https://example.com/auth-selector/mcp", + runtime_input_schema={ + "context": {"account_id": {"type": "string", "required": False}}, + "auth_selector": { + "resource_owner_key": {"type": "string", "required": True} + }, + }, + runtime_bindings=[ + { + "source": {"input_type": "context", "key": "account_id"}, + "target": {"target_type": "mcp_meta", "key": "account_id"}, + } + ], + ) + db.add(server) + db.flush() + db.add( + UserMCPServer( + user_id=user.id, + mcpserver_id=server.id, + is_owner=True, + can_edit=True, + can_delete=True, + is_active=True, + ) + ) + agent = _create_agent( + db, user, name="Auth Selector Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + finally: + db.close() + + response = client.get( + f"/api/chat/agent/{agent_id}/connector-runtime-requirements", + headers=headers, + ) + assert response.status_code == 200, response.text + payload = response.json() + assert len(payload["connectors"]) == 1 + inputs = payload["connectors"][0]["inputs"] + assert {(item["section"], item["key"]) for item in inputs} == { + ("context", "account_id"), + ("auth_selector", "resource_owner_key"), + } + auth_selector_input = next( + item for item in inputs if item["section"] == "auth_selector" + ) + assert auth_selector_input["key"] == "resource_owner_key" + assert auth_selector_input["type"] == "string" + assert auth_selector_input["required"] is True + assert auth_selector_input["satisfied"] is False + assert auth_selector_input["expired"] is False + assert payload["satisfied"] is False + + +def test_agent_requirements_are_satisfied_when_every_declared_key_is_optional( + e2e_db: None, +) -> None: + """A non-vacuous top-level ``satisfied: true`` on the agent-keyed + report: the agent does select a connector that declares a runtime + input, the input is listed and unsatisfied -- that report can store no + value -- and the flag still reads true, because it answers "would a + task created from this agent right now need anything else" and nothing + declared is required. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + _create_mcp_server( + db, user, name="optional-only-server", with_runtime_declaration=True + ) + agent = _create_agent( + db, user, name="Optional Only Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + finally: + db.close() + + response = client.get( + f"/api/chat/agent/{agent_id}/connector-runtime-requirements", + headers=headers, + ) + assert response.status_code == 200, response.text + payload = response.json() + assert len(payload["connectors"]) == 1 + inputs = payload["connectors"][0]["inputs"] + assert [item["key"] for item in inputs] == ["account_id"] + assert inputs[0]["required"] is False + assert inputs[0]["satisfied"] is False + assert payload["satisfied"] is True + + def test_team_shared_connector_visible_across_read_endpoints(e2e_db: None) -> None: """A connector shared only through the agent's team is listed by both read endpoints for a non-owning team member, and the task-keyed read @@ -2082,6 +2197,181 @@ def test_task_requirements_reports_unfillable_declared_key_as_unsatisfied( assert payload["satisfied"] is False +def test_optional_unfillable_declared_key_holds_report_unsatisfied( + e2e_db: None, +) -> None: + """A malformed declared key holds the top-level ``satisfied`` at false + even when nothing the connector declares is required. The per-turn gate + validates the syntax of every declared key before it looks at + ``required``, so this task cannot run; a report that read only the + ``required`` keys would call it ready. The key stays listed, its own + ``satisfied`` is false, and the read still answers 200 -- reporting the + problem is this endpoint's job, raising on it is the gate's. + """ + headers = _setup_admin_headers() + db = _db_session() + try: + user = _admin_user(db) + server = _mcp_server_with_context_schema( + db, + user, + name="optional-malformed-key-server", + context_schema={ + "account_id": {"type": "string", "required": False}, + "bad.key": {"type": "string", "required": False}, + }, + url="https://example.com/optional-malformed/mcp", + ) + agent = _create_agent( + db, user, name="Optional Malformed Key Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + db.refresh(server) + agent_id = int(agent.id) + server_id = int(server.id) + finally: + db.close() + + create_response = client.post( + "/api/chat/task/create", + headers=headers, + json={ + "title": "optional malformed key task", + "description": "d", + "agent_id": agent_id, + }, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + + db = _db_session() + try: + db.add( + TaskConnectorRuntimeContext( + task_id=task_id, + connector_type="mcp", + connector_id=server_id, + context={"account_id": "stored"}, + ) + ) + db.commit() + finally: + db.close() + + response = client.get( + f"/api/chat/task/{task_id}/connector-runtime-requirements", + headers=headers, + ) + assert response.status_code == 200, response.text + payload = response.json() + inputs_by_key = { + item["key"]: item + for connector in payload["connectors"] + for item in connector["inputs"] + } + assert set(inputs_by_key) == {"account_id", "bad.key"} + assert inputs_by_key["bad.key"]["required"] is False + assert inputs_by_key["bad.key"]["satisfied"] is False + assert inputs_by_key["account_id"]["required"] is False + assert inputs_by_key["account_id"]["satisfied"] is True + assert payload["satisfied"] is False + + +def test_task_requirements_reports_empty_for_a_task_with_no_agent( + e2e_db: None, +) -> None: + """A task created with no agent has no connector selection to report: + the agent-keyed producer that fills the task's selection snapshot was + called with no agent, so the snapshot is empty, and the task-keyed read + resolves the same absent agent and answers 200 with an empty, + satisfied report rather than failing on the missing row. + """ + headers = _setup_admin_headers() + create_response = client.post( + "/api/chat/task/create", + headers=headers, + json={"title": "agentless task", "description": "d"}, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + assert _task(task_id).agent_id is None + + response = client.get( + f"/api/chat/task/{task_id}/connector-runtime-requirements", + headers=headers, + ) + assert response.status_code == 200, response.text + assert response.json() == { + "satisfied": True, + "secrets_expires_at": None, + "connectors": [], + } + + +@pytest.mark.parametrize( + ("endpoint_kind", "expected_detail"), + [ + ("agent", "Agent not found or access denied"), + ("task", "Task not found"), + ], +) +def test_read_endpoints_report_an_unknown_id_as_not_found( + e2e_db: None, endpoint_kind: str, expected_detail: str +) -> None: + """An id that exists on neither endpoint is a 404 on both -- the same + answer each gives for an id the caller may not read, so neither + endpoint tells an unauthorized caller that the id exists. + """ + headers = _setup_admin_headers() + response = client.get( + f"/api/chat/{endpoint_kind}/987654321/connector-runtime-requirements", + headers=headers, + ) + assert response.status_code == 404, response.text + assert response.json()["detail"] == expected_detail + + +def test_task_requirements_endpoint_gives_an_admin_no_read_of_another_task( + e2e_db: None, +) -> None: + """Being an admin buys no read of another user's task here: the + task-keyed read filters on the caller's own user id with no admin + exception, unlike ``/task/{task_id}/runtime-extensions``. The owner's + own read of the same task answers 200, so the admin's 404 is about the + caller, not about the id. + """ + admin_headers = _setup_admin_headers() + db = _db_session() + try: + owner = _create_user(db, "task-requirements-owner") + agent = _create_agent( + db, owner, name="Owner Only Agent", tool_categories=["mcp"] + ) + db.commit() + db.refresh(agent) + agent_id = int(agent.id) + owner_headers = _auth_headers_for_user(owner) + finally: + db.close() + + create_response = client.post( + "/api/chat/task/create", + headers=owner_headers, + json={"title": "owner task", "description": "d", "agent_id": agent_id}, + ) + assert create_response.status_code == 200, create_response.text + task_id = int(create_response.json()["task_id"]) + + url = f"/api/chat/task/{task_id}/connector-runtime-requirements" + owner_response = client.get(url, headers=owner_headers) + assert owner_response.status_code == 200, owner_response.text + + admin_response = client.get(url, headers=admin_headers) + assert admin_response.status_code == 404, admin_response.text + assert admin_response.json()["detail"] == "Task not found" + + def test_task_requirements_endpoint_requires_task_ownership_by_caller( e2e_db: None, ) -> None: