diff --git a/docsgpt/api/user/agents/routes.py b/docsgpt/api/user/agents/routes.py index 9cfc6cc1..619362d0 100644 --- a/docsgpt/api/user/agents/routes.py +++ b/docsgpt/api/user/agents/routes.py @@ -29,6 +29,8 @@ from docsgpt.storage.db.base_repository import canonical_uuid, looks_like_uuid from docsgpt.api.user.resource_access import ( AccessDenied, agent_refs, + best_effort, + cached_resolves, delete_settings, parse_confirmations, payload_for, @@ -564,7 +566,7 @@ class GetAgent(Resource): sponsored: list = [] states: list = [] audience = None - with db_readonly() as conn: + with db_readonly() as conn, cached_resolves(): # Anyone who can see the agent reads it (a viewer needs it to # chat); what they get back is trimmed by their actions. ra = resolve(conn, "agent", agent_id, user) @@ -573,11 +575,18 @@ class GetAgent(Resource): # Edit-page detail: who vouches for resources the owner can't # use, and which attached resources stopped running and why, # with their names. Only for people who may edit the agent: - # the names can be an editor's private resources. + # the names can be an editor's private resources. Run state + # never fails the read. if agent and ra.can("edit"): sponsored = sponsor_details(conn, "agent", agent, viewer=user) - states = resource_states(conn, "agent", agent, agent_refs(agent), user) - audience = sponsor_audience(conn, "agent", agent, states, sponsored) + states = best_effort( + conn, "the agent's resource states", + lambda: resource_states(conn, "agent", agent, agent_refs(agent), user), [], + ) + audience = best_effort( + conn, "the agent's audience", + lambda: sponsor_audience(conn, "agent", agent, states, sponsored), None, + ) if not agent: return {"status": "Not found"}, 404 is_owner = ra.access == "owner" diff --git a/docsgpt/api/user/resource_access.py b/docsgpt/api/user/resource_access.py index 3a75bf04..2cebe72a 100644 --- a/docsgpt/api/user/resource_access.py +++ b/docsgpt/api/user/resource_access.py @@ -22,6 +22,8 @@ from __future__ import annotations import json import logging +from contextlib import contextmanager +from contextvars import ContextVar from dataclasses import dataclass, field from typing import Iterable, Optional @@ -296,10 +298,41 @@ def payload_for(resource_type: str, access: Optional[str], settings: Optional[di } +# Per-read memo of :func:`resolve`, on only inside :func:`cached_resolves`. +_RESOLVE_CACHE: ContextVar[Optional[dict]] = ContextVar("resource_access_resolve_cache", default=None) + + +@contextmanager +def cached_resolves(): + """Remember :func:`resolve` answers for the rest of one read. + + A page read asks about the same resources several times (sponsor details, + run state, names); inside this block each ``(type, id, user)`` is looked + up once. Only for reads: a write must see a grant change at once. + """ + token = _RESOLVE_CACHE.set({}) + try: + yield + finally: + _RESOLVE_CACHE.reset(token) + + def resolve( conn: Connection, resource_type: str, resource_id: str, user_id: str ) -> Optional[ResourceAccess]: """The caller's access to a resource, or None when they can't see it.""" + cache = _RESOLVE_CACHE.get() + if cache is None: + return _resolve_uncached(conn, resource_type, resource_id, user_id) + key = (resource_type, str(resource_id or "").lower(), user_id) + if key not in cache: + cache[key] = _resolve_uncached(conn, resource_type, resource_id, user_id) + return cache[key] + + +def _resolve_uncached( + conn: Connection, resource_type: str, resource_id: str, user_id: str +) -> Optional[ResourceAccess]: repo_cls = _REPO_FOR_TYPE.get(resource_type) if repo_cls is None or not resource_id or not user_id: return None @@ -1077,53 +1110,77 @@ def _user_labels(conn: Connection, user_ids: Iterable[Optional[str]]) -> dict[st ) -def _connection_state(conn: Connection, tool: dict, owner: Optional[str], policies_box: list) -> tuple: - """``(reason, connection, mode, writes_allowed)`` for a tool the holder runs as ``owner``. +# ``resource_states`` ``note`` for a running tool on each caller's own account. +NOTE_PER_USER_ACCOUNT = "per_user_account" - ``connection`` (``{id, connector_key, name}``) and ``mode`` (``owner`` or - ``member``, after any mode an admin forces) are set for a tool that has a - connection, and ``connection`` also for one that lost it. ``reason`` is - why the connection keeps the tool from running, else None. - ``writes_allowed`` is False when an admin turned off changes through the - tool's connector. +# ``resource_states`` ``contact_role``: whom to ask, when someone can fix it. +CONTACT_RESOURCE_OWNER = "resource_owner" + + +def _connection_state(conn: Connection, tool: dict, owner: Optional[str], policies_box: list) -> tuple: + """``(reason, connection, note, mode, writes_allowed)`` for a tool the holder runs as ``owner``. + + Resolved the way the run resolves it. Only an owner-mode tool's account + is judged (see ``connection_stop_reason``); a member-mode one runs on + each caller's own account and gets :data:`NOTE_PER_USER_ACCOUNT`. + ``connection`` is ``(id, connector_key, name)`` for a tool with a + connection or one that lost it, else None. ``mode`` (``owner`` or + ``member``, after any mode an admin forces) is set for a tool that has + a connection. ``writes_allowed`` is False when an admin turned off + changes through the tool's connector. """ from docsgpt.connectors import catalog, service - from docsgpt.connectors.resolve import connection_stop_reason, effective_credential_mode, resolve_connection + from docsgpt.connectors.resolve import ( + MODE_MEMBER, + REMOVED_CONNECTION_KEY, + connection_stop_reason, + resolve_connection, + ) - if not tool.get("connection_id") and not catalog.definition_for_tool(tool.get("name") or ""): - return None, None, None, True + if not tool.get("connection_id"): + reason = connection_stop_reason(tool, None) + if reason is None: + return None, None, None, None, True + key = (tool.get("config") or {}).get(REMOVED_CONNECTION_KEY) or None + definition = catalog.get_definition(key) if key else catalog.definition_for_tool(tool.get("name") or "") + connection = (None, definition.key if definition else key, definition.name if definition else None) + return reason, connection, None, None, True if not policies_box: policies_box.append(service.load_policies(conn)) resolved = resolve_connection(tool, owner, conn=conn, policies=policies_box[0]) reason = connection_stop_reason(tool, resolved) - if resolved is not None: - connection = { - "id": resolved.connection_id, - "connector_key": resolved.connector_key, - "name": resolved.connector_name, - } - mode = effective_credential_mode(tool, policies_box[0], resolved.connector_key) - return reason, connection, mode, resolved.writes_allowed - if reason is None: - return None, None, None, True - definition = catalog.definition_for_tool(tool.get("name") or "") - return reason, {"id": None, "connector_key": definition.key, "name": definition.name}, None, True + note = NOTE_PER_USER_ACCOUNT if reason is None and resolved.mode == MODE_MEMBER else None + connection = (resolved.connection_id, resolved.connector_key, resolved.connector_name) + return reason, connection, note, resolved.mode, resolved.writes_allowed + + +def _connection_payload(connection: Optional[tuple], reader_owns: bool, can_reconnect: bool) -> Optional[dict]: + """The ``connection`` a reader may see: the id only to reconnect it, its own name only to its owner.""" + if connection is None: + return None + from docsgpt.connectors import catalog + + connection_id, connector_key, name = connection + if not reader_owns: + # The owner's name for the account (a custom MCP server's label, say) + # is theirs; others see the service's own name. + definition = catalog.get_definition(connector_key) if connector_key else None + name = definition.name if definition else None + return {"id": connection_id if can_reconnect else None, "connector_key": connector_key, "name": name} def _tool_run_details(conn: Connection, tool: dict, owner: Optional[str], policies_box: list) -> tuple: - """``(reason, connection, details)`` for a tool the holder runs as ``owner``. + """``(reason, connection, note, details)`` for a tool the holder runs as ``owner``. ``details`` holds what the share dialog says about a running tool: ``credential_mode``, ``account`` (the user id whose saved credentials or ``owner``-mode connection it acts with: the tool's owner), the ``owner_credential_writes`` outside callers need allowlisted, and - ``writes_allowed``. A running tool's connection id is left out. + ``writes_allowed``. """ - reason, connection, mode, writes_allowed = _connection_state(conn, tool, owner, policies_box) - if reason is None and connection is not None: - connection = {**connection, "id": None} + reason, connection, note, mode, writes_allowed = _connection_state(conn, tool, owner, policies_box) holds = mode != "member" and holds_owner_credentials(tool) - return reason, connection, { + return reason, connection, note, { "credential_mode": mode, "account": tool.get("user_id") if holds else None, "owner_credential_writes": owner_credential_writes(tool) if writes_allowed else [], @@ -1168,10 +1225,13 @@ def resource_states( """Whether each resource a holder references runs, for its edit page. Uses the run's own checks (:func:`ref_access`, :func:`resolve_holder_tool` - and the tool's connection as the run resolves it for the owner), so a - resource shown as stopped is one the run leaves out. Presets and builtin - tools always run and are not listed. Only for readers who may edit the - holder: the caller checks that. + and an owner-mode tool's connection as the run resolves it for the + owner), so a source or prompt shown as stopped is one the run leaves out, + and a tool shown as stopped is left out or can't run until someone acts: + a tool whose owner-mode account needs attention is still offered to the + model, and pauses (or, with nobody to ask, is refused) when called. + Presets and builtin tools always run and are not listed. Only for readers + who may edit the holder: the caller checks that. A name is given only for a resource that runs, that the reader can see themselves, that someone sponsored on the holder, or that is attached to @@ -1186,16 +1246,22 @@ def resource_states( viewer: The user reading the page. Returns: - list: Per resource ``{key, type, id, name, state, reason, sponsor, - contact, connection, can_confirm, can_reconnect}``. ``state`` is + list: Per resource ``{key, type, id, name, state, reason, note, + sponsor, contact, contact_role, connection, runs_as, credential_mode, + account, owner_credential_writes, writes_allowed, can_confirm, + can_reconnect}``. ``state`` is ``active`` or ``stopped``; ``reason`` (None while active) is one of ``deleted``, ``owner_lost_access``, the sponsor reasons, ``connection_needs_reconnect``, ``connection_removed`` or - ``connector_disabled``. ``sponsor`` and ``contact`` are - ``{user_id, label}`` or None: the recorded sponsor, and someone other - than the reader who can fix it. ``connection`` (``{id, - connector_key, name}``) names the service of a tool with a - connection (its id only when a connection reason stopped it). + ``connector_disabled``. ``note`` is ``per_user_account`` for a tool + that runs on each caller's own account. ``sponsor`` is the recorded + sponsor as ``{user_id, label}``. ``contact_role`` is + ``resource_owner`` when the resource's owner (not the reader) can fix + it; ``contact`` names them only when the reader can see the resource + or they own the holder, else None. ``connection`` (``{id, + connector_key, name}``) names the service of a connected tool, and of + one whose connection reason stopped it; ``id`` only when the reader + may reconnect it, and the account's own name only for its owner. ``runs_as`` (``{user_id, label}``) is the live sponsor a running item runs as, None when it runs as the owner or doesn't run. For a running tool, ``credential_mode`` is ``owner`` or ``member`` @@ -1228,16 +1294,20 @@ def resource_states( tools_repo = UserToolsRepository(conn) policies_box: list = [] entries = [] + + def reader_sees(resource_type: str, rid: str) -> bool: + return bool(viewer) and resolve(conn, resource_type, rid, viewer) is not None + for resource_type, rid in pairs: key = sponsor_key(resource_type, rid) info = rows.get(key) or {} - connection = None + connection = note = None details = dict(_NO_RUN_DETAILS) if resource_type == "tool": tool_row, access = resolve_holder_tool(conn, holder_type, holder, rid, tools_repo=tools_repo) reason = access.reason if tool_row is not None and reason is None: - reason, connection, details = _tool_run_details(conn, tool_row, owner, policies_box) + reason, connection, note, details = _tool_run_details(conn, tool_row, owner, policies_box) else: access = ref_access(conn, holder_type, holder, resource_type, rid) reason = access.reason @@ -1252,19 +1322,24 @@ def resource_states( and viewer_edits and can_sponsor_ref(conn, resource_type, rid, viewer) ) - contact = None - if reason == REASON_OWNER_LOST_ACCESS: - # The owner asks whoever shared it; an editor asks the owner. - contact = resource_owner if viewer == owner else owner - elif reason in (REASON_CONNECTION_NEEDS_RECONNECT, REASON_CONNECTION_REMOVED): - contact = resource_owner - if contact == viewer: - contact = None - name_visible = bool( - reason is None - or sponsor - or holder_type == "agent" - or (viewer and resolve(conn, resource_type, rid, viewer) is not None) + contact = contact_role = None + if ( + reason in (REASON_OWNER_LOST_ACCESS, REASON_CONNECTION_NEEDS_RECONNECT, REASON_CONNECTION_REMOVED) + and resource_owner + and resource_owner != viewer + ): + # Whoever owns the resource can share it again or fix its + # account; name them only to someone who knows them already. + contact_role = CONTACT_RESOURCE_OWNER + if resource_owner == owner or reader_sees(resource_type, rid): + contact = resource_owner + name_visible = bool(reason is None or sponsor or holder_type == "agent" or reader_sees(resource_type, rid)) + can_reconnect = bool( + reason == REASON_CONNECTION_NEEDS_RECONNECT + and connection + and connection[0] + and viewer + and viewer == resource_owner ) entries.append({ "key": key, @@ -1273,19 +1348,15 @@ def resource_states( "name": info.get("name") if name_visible else None, "state": "active" if reason is None else "stopped", "reason": reason, + "note": note, "sponsor": sponsor, "contact": contact, - "connection": connection, + "contact_role": contact_role, + "connection": _connection_payload(connection, bool(viewer) and viewer == resource_owner, can_reconnect), "runs_as": runs_as, **details, "can_confirm": can_confirm, - "can_reconnect": bool( - reason == REASON_CONNECTION_NEEDS_RECONNECT - and connection - and connection.get("id") - and viewer - and viewer == resource_owner - ), + "can_reconnect": can_reconnect, }) labels = _user_labels( conn, [u for e in entries for u in (e["sponsor"], e["contact"], e["runs_as"], e["account"])] @@ -1308,38 +1379,39 @@ def resource_states( return entries -def visible_ref_ids( - conn: Connection, holder_type: str, holder: dict, refs: Iterable[tuple[str, str]], viewer: Optional[str] -) -> set[str]: - """Keys of references whose names ``viewer`` may read on the holder's edit page. +def best_effort(conn: Connection, label: str, compute, default): + """``compute()``, or ``default`` when it fails, without failing the read. - Those the holder runs (its owner or a live sponsor may use them), those - someone sponsored on it, and those the reader can see themselves. + Runs inside a savepoint, so a failed query doesn't poison the rest of + the read's transaction. Args: conn: Open database connection. - holder_type: ``agent`` or ``workflow``. - holder: The holder row. - refs: ``(type, id)`` pairs. - viewer: The reader. + label: What is computed, for the log line. + compute: A no-argument callable. + default: What to return when it raises. + """ + try: + with conn.begin_nested(): + return compute() + except Exception: + logger.exception("Could not work out %s; left out of the read", label) + return default + + +def named_ref_keys(states: list[dict]) -> set[str]: + """Keys of the references whose names :func:`resource_states` gave the reader. + + The same rule decides which node resources a workflow read names in its + ``ref_details``, so the two never disagree. + + Args: + states: :func:`resource_states` for the reader. Returns: set: ``":"`` keys, ids lowercased. """ - recorded = {k.lower() for k in (holder.get("resource_sponsors") or {})} - out: set[str] = set() - for resource_type, resource_id in refs: - rid = str(resource_id).lower() - key = sponsor_key(resource_type, rid) - if key in out or resource_type not in REF_USE_ACTION: - continue - if ( - key in recorded - or ref_access(conn, holder_type, holder, resource_type, rid).principal - or (viewer and resolve(conn, resource_type, rid, viewer) is not None) - ): - out.add(key) - return out + return {state["key"] for state in states if state.get("name") is not None} def sponsor_audience( diff --git a/docsgpt/api/user/workflows/routes.py b/docsgpt/api/user/workflows/routes.py index 34673961..4cb08bde 100644 --- a/docsgpt/api/user/workflows/routes.py +++ b/docsgpt/api/user/workflows/routes.py @@ -13,7 +13,10 @@ from docsgpt.agents.workflows.cel_evaluator import ( from docsgpt.api.user import resource_access from docsgpt.api.user.resource_access import ( AccessDenied, + best_effort, + cached_resolves, can_use_ref, + named_ref_keys, parse_confirmations, plan_sponsors, resolve, @@ -21,7 +24,6 @@ from docsgpt.api.user.resource_access import ( sponsor_audience, sponsor_details, sponsor_refusal, - visible_ref_ids, ) from docsgpt.storage.db.base_repository import looks_like_uuid from docsgpt.storage.db.repositories.workflow_edges import WorkflowEdgesRepository @@ -137,7 +139,7 @@ def _node_ref_details(nodes: List[Dict], visible: Optional[Set[str]] = None) -> Looked up by id whoever owns them, so an editor's node pickers can show (and remove) the owner's private tools and sources. Builtin tool ids always resolve; any other id only when its ``":"`` key is in - ``visible`` (see ``resource_access.visible_ref_ids``), so a node naming + ``visible`` (see ``resource_access.named_ref_keys``), so a node naming someone else's resource never reveals its name. Args: @@ -688,12 +690,21 @@ class WorkflowDetail(Resource): states: list = [] audience = None visible: Optional[Set[str]] = None - if resource_access.holder_editable_by(conn, "workflow", workflow, user_id): - refs = _node_refs(serialized_nodes) - sponsored = sponsor_details(conn, "workflow", workflow, viewer=user_id) - states = resource_states(conn, "workflow", workflow, refs, user_id) - audience = sponsor_audience(conn, "workflow", workflow, states, sponsored) - visible = visible_ref_ids(conn, "workflow", workflow, refs, user_id) + with cached_resolves(): + if resource_access.holder_editable_by(conn, "workflow", workflow, user_id): + refs = _node_refs(serialized_nodes) + sponsored = sponsor_details(conn, "workflow", workflow, viewer=user_id) + # Run state never fails the read; node names follow + # the same rule as the state's names. + states = best_effort( + conn, "the workflow's resource states", + lambda: resource_states(conn, "workflow", workflow, refs, user_id), [], + ) + audience = best_effort( + conn, "the workflow's audience", + lambda: sponsor_audience(conn, "workflow", workflow, states, sponsored), None, + ) + visible = named_ref_keys(states) ref_details = ( _node_ref_details(serialized_nodes, visible) if visible is not None diff --git a/docsgpt/connectors/resolve.py b/docsgpt/connectors/resolve.py index 21593e89..e8ffdfac 100644 --- a/docsgpt/connectors/resolve.py +++ b/docsgpt/connectors/resolve.py @@ -28,6 +28,10 @@ CONNECTION_NEEDS_RECONNECT = "connection_needs_reconnect" CONNECTION_REMOVED = "connection_removed" CONNECTOR_DISABLED = "connector_disabled" +# Tool config key ``remove_connection`` sets to the connector key when it +# keeps a connection's tools, so they can say what they lost. +REMOVED_CONNECTION_KEY = "removed_connection" + @dataclass(frozen=True) class ResolvedConnection: @@ -42,6 +46,8 @@ class ResolvedConnection: writes_allowed: Whether an admin lets agents make changes through a connector that offers them as an opt-in (GitHub); True elsewhere. enabled: Whether the connector is switched on (an admin can turn it off). + mode: Whose account the resource runs with, after any mode an admin + forces: :data:`MODE_OWNER` or :data:`MODE_MEMBER`. """ row: Optional[dict] @@ -51,6 +57,7 @@ class ResolvedConnection: delegated: bool = False writes_allowed: bool = True enabled: bool = True + mode: str = MODE_OWNER @property def connection_id(self) -> Optional[str]: @@ -149,34 +156,41 @@ def _resolve(conn, resource: dict, invoker_user_id: Optional[str], policies: Opt delegated=bool(row and invoker_user_id and row.get("user_id") != invoker_user_id), writes_allowed=_writes_allowed(policies, key), enabled=enabled, + mode=mode, ) def connection_stop_reason(tool: dict, resolved: Optional[ResolvedConnection]) -> Optional[str]: - """Why a tool's connection keeps it from running, or None when it can run. + """Why an owner-mode tool's connection keeps it from running, or None. + + Only the connection the owner's account runs on is judged. A member-mode + tool runs on each caller's own account, so the owner's account says + nothing about whether it runs; only an admin turning the service off + stops it for everyone. Args: tool: The ``user_tools`` row. - resolved: What :func:`resolve_connection` returned for it. + resolved: What :func:`resolve_connection` returned for it, resolved + for the tool's holder's owner. Returns: - :data:`CONNECTION_REMOVED` when the connection is gone (its row was - deleted, or a built-in service's tool lost its connection and has no - credentials of its own), :data:`CONNECTOR_DISABLED` when an admin - turned the service off, :data:`CONNECTION_NEEDS_RECONNECT` when the - account must sign in again; else None. + :data:`CONNECTION_REMOVED` when its connection was removed and the + tool kept (``remove_connection`` notes it on the tool), or points at + one it may not use; :data:`CONNECTOR_DISABLED` when an admin turned + the service off; :data:`CONNECTION_NEEDS_RECONNECT` when the account + must sign in again; else None. """ if resolved is None: - if tool.get("connection_id") or not catalog.definition_for_tool(tool.get("name") or ""): - return None - # Removing a connection but keeping its tools nulls their link; a - # built-in service's tool has no secrets of its own to fall back to. - config = tool.get("config") or {} - return None if config.get("encrypted_credentials") else CONNECTION_REMOVED - if resolved.row is None: - return CONNECTION_REMOVED + # Only a tool that had a connection lost it: a tool that never had + # one (a tokenless ntfy, a legacy tool) runs on its own config. + removed = (tool.get("config") or {}).get(REMOVED_CONNECTION_KEY) + return CONNECTION_REMOVED if removed and not tool.get("connection_id") else None if not resolved.enabled: return CONNECTOR_DISABLED + if resolved.mode == MODE_MEMBER: + return None + if resolved.row is None: + return CONNECTION_REMOVED if not resolved.available: return CONNECTION_NEEDS_RECONNECT return None diff --git a/docsgpt/connectors/service.py b/docsgpt/connectors/service.py index 1db6c585..6fa4521b 100644 --- a/docsgpt/connectors/service.py +++ b/docsgpt/connectors/service.py @@ -1454,9 +1454,15 @@ def remove_connection(conn, row: dict, *, sources: str = "keep", tools: str = "d {"id": connection_id, "user_id": row["user_id"]}, ) else: + # The link itself goes with the row (ON DELETE SET NULL); note which + # service the tools lost, so an agent can say why they stopped. conn.execute( - text("UPDATE user_tools SET status = false WHERE connection_id = CAST(:id AS uuid)"), - {"id": connection_id}, + text( + "UPDATE user_tools SET status = false, " + "config = COALESCE(config, '{}'::jsonb) || jsonb_build_object('removed_connection', " + "CAST(:key AS text)) WHERE connection_id = CAST(:id AS uuid)" + ), + {"id": connection_id, "key": catalog.connector_key_for_row(row) or ""}, ) conn.execute( text( diff --git a/tests/api/user/test_resource_states.py b/tests/api/user/test_resource_states.py index 4a7766ed..3f563322 100644 --- a/tests/api/user/test_resource_states.py +++ b/tests/api/user/test_resource_states.py @@ -127,8 +127,10 @@ class TestReasons: TeamResourceGrantsRepository(pg_conn).revoke(team_id, "source", source) state = _states(pg_conn, agent_id)[f"source:{source}"] assert (state["state"], state["reason"]) == ("stopped", REASON_OWNER_LOST_ACCESS) - # The owner is told whom to ask: the source's owner. - assert state["contact"] == {"user_id": OTHER, "label": OTHER} + # The owner is told to ask the source's owner, but not who that is: + # they can no longer see the source. + assert state["contact"] is None + assert state["contact_role"] == "resource_owner" assert state["name"] == "team-src" def test_deleted_tool(self, pg_conn): @@ -175,6 +177,7 @@ class TestReasons: assert (state["state"], state["reason"]) == ("stopped", REASON_CONNECTION_NEEDS_RECONNECT) assert state["can_reconnect"] is True assert state["connection"]["connector_key"] == "telegram" + assert state["connection"]["id"] assert state["contact"] is None def test_disconnected_account_needs_reconnect(self, pg_conn): @@ -194,7 +197,10 @@ class TestReasons: with _patch_db(pg_conn): state = _states(pg_conn, agent_id, viewer=EDITOR)[f"tool:{tool}"] assert state["can_reconnect"] is False + # The agent's owner is someone the editor knows. assert state["contact"] == {"user_id": OWNER, "label": OWNER} + # Only the account's owner gets its connection id and own name. + assert state["connection"] == {"id": None, "connector_key": "telegram", "name": "Telegram"} def test_connection_removed_but_tool_kept(self, pg_conn): from docsgpt.connectors import service @@ -734,3 +740,193 @@ class TestWorkflowStates: data = _body(self._get(app, pg_conn, wid, OTHER))["data"] assert _by_key(data["resource_states"])[f"tool:{tool}"]["can_confirm"] is True assert data["sponsor_audience"]["teams"] == ["T"] + + +# --------------------------------------------------------------------------- +# Connections as the run resolves them +# --------------------------------------------------------------------------- + + +class TestConnectionModes: + def test_member_mode_tool_runs_on_each_persons_account(self, pg_conn): + """The owner's own account doesn't decide a member-mode tool; each caller's does.""" + tool = str(UserToolsRepository(pg_conn).create( + OWNER, "telegram", connection_id=_connection(pg_conn, status="reconnect_needed"), + credential_mode="member", + )["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id)[f"tool:{tool}"] + assert (state["state"], state["reason"]) == ("active", None) + assert state["note"] == "per_user_account" + + def test_admin_forced_member_mode_runs_on_each_persons_account(self, pg_conn): + tool = str(UserToolsRepository(pg_conn).create( + OWNER, "telegram", connection_id=_connection(pg_conn, status="disconnected"), + )["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + ConnectorPoliciesRepository(pg_conn).upsert("telegram", credential_mode="member") + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id)[f"tool:{tool}"] + assert (state["state"], state["note"]) == ("active", "per_user_account") + + def test_member_mode_tool_still_stops_when_the_service_is_off(self, pg_conn): + tool = str(UserToolsRepository(pg_conn).create( + OWNER, "telegram", connection_id=_connection(pg_conn), credential_mode="member", + )["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + ConnectorPoliciesRepository(pg_conn).upsert("telegram", enabled=False) + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id)[f"tool:{tool}"] + assert state["reason"] == REASON_CONNECTOR_DISABLED + + def test_owner_mode_tool_has_no_note(self, pg_conn): + tool = str(UserToolsRepository(pg_conn).create( + OWNER, "telegram", connection_id=_connection(pg_conn), + )["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id)[f"tool:{tool}"] + assert (state["state"], state["note"]) == ("active", None) + + @pytest.mark.parametrize("name", ["ntfy", "brave"]) + def test_service_tool_that_never_had_a_connection_runs(self, pg_conn, name): + """A tokenless ntfy (or a legacy tool) has no connection and needs none.""" + tool = str(UserToolsRepository(pg_conn).create(OWNER, name, config={"server_url": "https://ntfy.sh"})["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id)[f"tool:{tool}"] + assert (state["state"], state["reason"]) == ("active", None) + + def test_removed_connection_is_remembered_on_the_kept_tool(self, pg_conn): + from docsgpt.connectors import service + + cid = _connection(pg_conn) + tool = str(UserToolsRepository(pg_conn).create(OWNER, "telegram", connection_id=cid)["id"]) + row = pg_conn.execute(text("SELECT * FROM connector_sessions WHERE id = CAST(:id AS uuid)"), + {"id": cid}).mappings().one() + service.remove_connection(pg_conn, dict(row), tools="keep") + kept = UserToolsRepository(pg_conn).get_any(tool, OWNER) + assert kept["connection_id"] is None + assert kept["config"]["removed_connection"] == "telegram" + + +# --------------------------------------------------------------------------- +# Whom to ask, without naming strangers +# --------------------------------------------------------------------------- + + +class TestContact: + def _owners_grant_only(self, conn, team_id, resource_type, resource_id): + """Share OTHER's resource with OWNER alone, so the agent's editors can't see it.""" + for member in (OWNER, OTHER): + if not TeamMembersRepository(conn).is_member(member, team_id): + TeamMembersRepository(conn).add_member(team_id, member) + TeamResourceGrantsRepository(conn).grant( + team_id, resource_type, resource_id, OTHER, OTHER, target_user_id=OWNER + ) + + def test_editor_who_can_see_the_item_is_told_its_owner(self, pg_conn): + agent_id, team_id = _agent(pg_conn) + source = _team_source(pg_conn, team_id) + AgentsRepository(pg_conn).update_by_id(agent_id, {"extra_source_ids": [source]}) + TeamResourceGrantsRepository(pg_conn).revoke(team_id, "source", source) + TeamResourceGrantsRepository(pg_conn).grant( + team_id, "source", source, OTHER, OTHER, target_user_id=EDITOR + ) + state = _states(pg_conn, agent_id, viewer=EDITOR)[f"source:{source}"] + assert state["contact"] == {"user_id": OTHER, "label": OTHER} + assert state["contact_role"] == "resource_owner" + + def test_editor_who_cannot_see_the_item_gets_no_identity(self, pg_conn): + agent_id, team_id = _agent(pg_conn) + tool = str(UserToolsRepository(pg_conn).create( + OTHER, "telegram", connection_id=_connection(pg_conn, user=OTHER, status="reconnect_needed"), + )["id"]) + self._owners_grant_only(pg_conn, team_id, "tool", tool) + AgentsRepository(pg_conn).update_by_id(agent_id, {"tools": [tool]}) + with _patch_db(pg_conn): + state = _states(pg_conn, agent_id, viewer=EDITOR)[f"tool:{tool}"] + assert state["reason"] == REASON_CONNECTION_NEEDS_RECONNECT + assert state["contact"] is None + assert state["contact_role"] == "resource_owner" + assert state["connection"]["id"] is None + + def test_old_graph_naming_a_strangers_tool_reveals_nobody(self, app, pg_conn): + from docsgpt.api.user.workflows.routes import WorkflowDetail + + wf = WorkflowsRepository(pg_conn).create(OWNER, "wf") + wid = str(wf["id"]) + _agent(pg_conn, agent_type="workflow", workflow_id=wid) + stranger_tool = str(UserToolsRepository(pg_conn).create("sp-stranger", "api_tool")["id"]) + own = str(UserToolsRepository(pg_conn).create(OWNER, "api_tool")["id"]) + assert _status(_call(app, pg_conn, WorkflowDetail, "put", f"/api/workflows/{wid}", OWNER, + json=_wf_body(own), args=(wid,))) == 200 + pg_conn.execute( + text("UPDATE workflow_nodes SET config = jsonb_set(config, '{config,tools}', CAST(:t AS jsonb)) " + "WHERE workflow_id = CAST(:w AS uuid) AND node_type = 'agent'"), + {"t": f'["{stranger_tool}"]', "w": wid}, + ) + data = _body(_call(app, pg_conn, WorkflowDetail, "get", f"/api/workflows/{wid}", OWNER, args=(wid,))) + state = _by_key(data["data"]["resource_states"])[f"tool:{stranger_tool}"] + assert state["reason"] == REASON_OWNER_LOST_ACCESS + assert state["contact"] is None + assert state["name"] is None + + +# --------------------------------------------------------------------------- +# Reads never fail on run state +# --------------------------------------------------------------------------- + + +class TestBestEffort: + def test_agent_read_survives_a_state_error(self, app, pg_conn, monkeypatch): + from docsgpt.api.user.agents import routes + + tool = str(UserToolsRepository(pg_conn).create(OWNER, "api_tool")["id"]) + agent_id, _ = _agent(pg_conn, tools=[tool]) + + def _boom(*_a, **_k): + raise RuntimeError("state failed") + + monkeypatch.setattr(routes, "resource_states", _boom) + data = _get_agent(app, pg_conn, agent_id, OWNER) + assert data["resource_states"] == [] + assert data["name"] == "Shared" + + def test_workflow_read_survives_a_state_error(self, app, pg_conn, monkeypatch): + from docsgpt.api.user.workflows import routes + from docsgpt.api.user.workflows.routes import WorkflowDetail + + wf = WorkflowsRepository(pg_conn).create(OWNER, "wf") + wid = str(wf["id"]) + _agent(pg_conn, agent_type="workflow", workflow_id=wid) + + def _boom(*_a, **_k): + raise RuntimeError("state failed") + + monkeypatch.setattr(routes, "resource_states", _boom) + resp = _call(app, pg_conn, WorkflowDetail, "get", f"/api/workflows/{wid}", OWNER, args=(wid,)) + assert _status(resp) == 200 + assert _body(resp)["data"]["resource_states"] == [] + + def test_resolves_are_cached_within_a_read(self, pg_conn, monkeypatch): + from docsgpt.api.user import resource_access + + tool = str(UserToolsRepository(pg_conn).create(OWNER, "api_tool")["id"]) + calls = [] + real = resource_access._resolve_uncached + + def _spy(*args): + calls.append(args[1:]) + return real(*args) + + monkeypatch.setattr(resource_access, "_resolve_uncached", _spy) + with resource_access.cached_resolves(): + first = resource_access.resolve(pg_conn, "tool", tool, OWNER) + second = resource_access.resolve(pg_conn, "tool", tool.upper(), OWNER) + assert first == second + assert len(calls) == 1 + # Outside a read nothing is cached: a revoked grant denies at once. + resource_access.resolve(pg_conn, "tool", tool, OWNER) + assert len(calls) == 2