From 7b386aff0a85ecdb0320868a70a5bffb5f44628e Mon Sep 17 00:00:00 2001 From: Maxim Date: Tue, 15 Sep 2026 17:00:34 +0200 Subject: [PATCH 01/15] feat(agent): select one connected-app provider from the keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A deployment reaches other people's apps through Composio or through Arcade, never both, and which one is decided from API-key presence alone. No selector variable is introduced, so there is nothing that can disagree with the keys. Two keys fail the boot. Every silent answer to "both are set" is worse than refusing: preferring the older key ignores what the operator just did, and preferring the newer one switches providers — which strands every personal account connected to the old one without anybody being asked. Selection is a pure function that imports no provider package, so the conflict is reported before either SDK is constructed and before a provider-specific setting is parsed. A test asserts the module's imports against its parsed AST rather than its text, because a substring search over source counts the word in a comment and passes for a module that imports an SDK inside a function. Absent, empty and whitespace-only keys are all unset. `COMPOSIO_API_KEY=` is routine in .env templates and container passthrough, and a deployment carrying an empty key for the provider it does not use has to keep starting. Arcade selection says out loud that its provider is not implemented yet. Registering nothing in silence would be indistinguishable from an agent with no connected apps configured at all, which is a different and wrong answer to give someone who just set a key. The Composio connect route now asks which provider is selected before it builds anything. The case that guards is a Connect button minted under Composio and clicked after the deployment switched to Arcade. Two fixture corrections came with it. `install_runtime` now sets the key it was implying, since a fixture that injects a runtime while leaving the key unset describes a deployment that cannot exist; and the unconfigured-deployment test sets it too, so it reaches the branch it asserts instead of being turned away earlier and passing for a reason it does not test. ARCADE_* joins the conftest scrub list. Without it, a developer with an Arcade key exported would fail every Composio test with a both-keys conflict the suite never asked for. Mutation-checked: removing the conflict check, ignoring the selected provider, and dropping the not-implemented warning each turn tests red. --- agent/agent.py | 31 ++- agent/connected_app_provider.py | 74 ++++++ agent/main.py | 12 + agent/pyproject.toml | 1 + agent/tests/conftest.py | 6 + agent/tests/test_composio_connect.py | 34 +++ agent/tests/test_connected_app_selection.py | 239 ++++++++++++++++++++ 7 files changed, 393 insertions(+), 4 deletions(-) create mode 100644 agent/connected_app_provider.py create mode 100644 agent/tests/test_connected_app_selection.py diff --git a/agent/agent.py b/agent/agent.py index 39e72f6a..44772516 100644 --- a/agent/agent.py +++ b/agent/agent.py @@ -29,6 +29,11 @@ from ag_ui_langgraph import CustomEventNames from langchain_core.callbacks.manager import adispatch_custom_event from langchain_core.runnables.config import ensure_config +from connected_app_provider import ( + PROVIDER_ARCADE, + PROVIDER_COMPOSIO, + selected_provider, +) from composio_tools.config import DEFAULT_WORKSPACE_USER_ID from composio_tools.runtime import composio_runtime from composio_tools.state import ComposioAgentState @@ -206,11 +211,29 @@ def build_agent(): # caches on this id, and `main.py` passes the constant for the connect # route: two spellings would build two caches, so the account an operator # connected through the route is not the one a turn runs in. - composio = composio_runtime( - default_user_id=os.environ.get( - "INTELLIGENCE_CHANNEL_NAME", DEFAULT_WORKSPACE_USER_ID - ), + # + # Which provider runs is decided from the keys before any of this is built, + # so two keys fail the boot rather than resolving to whichever SDK happened + # to construct first. + provider = selected_provider() + composio = ( + composio_runtime( + default_user_id=os.environ.get( + "INTELLIGENCE_CHANNEL_NAME", DEFAULT_WORKSPACE_USER_ID + ), + ) + if provider == PROVIDER_COMPOSIO + else None ) + if provider == PROVIDER_ARCADE: + # Said out loud rather than registering nothing in silence. An Arcade + # key is a deployer asking for connected apps, and answering that with + # the same behaviour as an unconfigured agent would read as "Arcade is + # set up and has nothing", which is a different and wrong statement. + logger.warning( + "[arcade] ARCADE_API_KEY selects Arcade, but the Arcade provider is " + "not implemented yet, so no connected-app tools are registered." + ) composio_tools: list = ( [] if composio is None diff --git a/agent/connected_app_provider.py b/agent/connected_app_provider.py new file mode 100644 index 00000000..564c0f5f --- /dev/null +++ b/agent/connected_app_provider.py @@ -0,0 +1,74 @@ +"""Which connected-app provider this deployment runs. + +A deployment reaches other people's apps through Composio or through Arcade, +never both. The presence of an API key is the whole selection: there is no +separate selector variable to keep in agreement with the keys, and therefore no +way for the two to disagree. + +Two keys is a startup failure rather than a preference order. An operator who +adds a second key has not told us which provider they meant, and every silent +answer to that is worse than refusing: picking the older one ignores what they +just did, picking the newer one switches providers — and switching providers +means every personal account connected to the old one stops being reachable, +without anybody being asked. + +Deliberately pure. It reads strings and returns a name, so the conflict is +reported before either SDK is constructed, before a network call, and before a +provider-specific setting is parsed. Nothing here imports a provider package; +a test asserts that, because the ordering is the reason this module exists. +""" + +from __future__ import annotations + +import os +from collections.abc import Mapping + +#: The names this module returns. Callers branch on these rather than on key +#: presence, so the selection rule lives in exactly one place. +PROVIDER_COMPOSIO = "composio" +PROVIDER_ARCADE = "arcade" + +COMPOSIO_API_KEY = "COMPOSIO_API_KEY" +ARCADE_API_KEY = "ARCADE_API_KEY" + + +class ProviderSelectionError(ValueError): + """The keys name no single provider, so the agent must not start.""" + + +def _configured(source: Mapping[str, str], name: str) -> bool: + """ + Absent, empty and whitespace-only are the same thing: unset. + + `COMPOSIO_API_KEY=` with nothing after it is routine — `.env` templates ship + that way and container passthrough produces it for an unset variable. A + deployment carrying an empty key for the provider it does not use must keep + starting, so an empty string can never count towards the conflict. + """ + return bool((source.get(name) or "").strip()) + + +def selected_provider(env: Mapping[str, str] | None = None) -> str | None: + """ + The provider this deployment uses, or `None` when it has no connected apps. + + Raises `ProviderSelectionError` when both keys are set. That is checked + first, so it is reported even when neither provider would have resolved to + a usable configuration — a key naming no apps is still a key, and still + leaves the operator's intent unknown. + """ + source = os.environ if env is None else env + composio = _configured(source, COMPOSIO_API_KEY) + arcade = _configured(source, ARCADE_API_KEY) + + if composio and arcade: + # Names the variables and never their values: this message is the one an + # operator pastes into a ticket. + raise ProviderSelectionError( + f"Configure only one of {COMPOSIO_API_KEY} or {ARCADE_API_KEY}." + ) + if composio: + return PROVIDER_COMPOSIO + if arcade: + return PROVIDER_ARCADE + return None diff --git a/agent/main.py b/agent/main.py index 7dd39cd7..18bac36b 100644 --- a/agent/main.py +++ b/agent/main.py @@ -14,6 +14,7 @@ from agent import build_agent from agent_auth import authorizes_capability, configured_secret, is_authorized from agui import AGENT_DESCRIPTION, AGENT_NAME, build_agui_agent +from connected_app_provider import PROVIDER_COMPOSIO, selected_provider from composio_tools.config import DEFAULT_WORKSPACE_USER_ID from composio_tools.connect import ConnectRefused, connect_link from composio_tools.runtime import composio_runtime @@ -133,6 +134,17 @@ def composio_connect(body: ConnectRequest, request: Request): if not authorizes_capability(request.headers.get("authorization")): return JSONResponse({"error": "unauthorized"}, status_code=401) + # This route only ever serves Composio. Asked explicitly rather than left to + # fall out of an absent key, because the case it guards is a card that + # outlived a provider change: a button minted under Composio, clicked after + # the deployment switched to Arcade, must be refused rather than answered by + # whichever runtime still happens to build. + if selected_provider() != PROVIDER_COMPOSIO: + return JSONResponse( + {"error": "Composio is not configured on this deployment."}, + status_code=503, + ) + runtime = composio_runtime( # The default spelled once, in the module that resolves it. A present # but empty `INTELLIGENCE_CHANNEL_NAME` reaches here as the empty diff --git a/agent/pyproject.toml b/agent/pyproject.toml index 3eda9b20..8b381df4 100644 --- a/agent/pyproject.toml +++ b/agent/pyproject.toml @@ -37,6 +37,7 @@ py-modules = [ "agent", "agent_auth", "agui", + "connected_app_provider", "internal_sources", "main", "tools", diff --git a/agent/tests/conftest.py b/agent/tests/conftest.py index 7a0cbdd6..6380a530 100644 --- a/agent/tests/conftest.py +++ b/agent/tests/conftest.py @@ -59,6 +59,12 @@ "COMPOSIO_APPROVALS", "COMPOSIO_WORKSPACE_USER_ID", "COMPOSIO_AUTH_CONFIGS", + "ARCADE_API_KEY", + "ARCADE_TOOLKITS", + "ARCADE_USER_TOOLKITS", + "ARCADE_APPROVALS", + "ARCADE_WORKSPACE_USER_ID", + "ARCADE_IDENTITY_NAMESPACE", "GITHUB_PERSONAL_ACCESS_TOKEN", "GITHUB_APP_ID", "GITHUB_APP_INSTALLATION_ID", diff --git a/agent/tests/test_composio_connect.py b/agent/tests/test_composio_connect.py index 46596226..1bc802d2 100644 --- a/agent/tests/test_composio_connect.py +++ b/agent/tests/test_composio_connect.py @@ -156,6 +156,11 @@ def client(monkeypatch): def install_runtime(monkeypatch, sessions_by_user, **overrides): + # The key, not just the patched builder. The route asks which provider this + # deployment selected before it builds anything, and that question is + # answered from the environment — so a fixture that injects a runtime while + # leaving the key unset is describing a deployment that cannot exist. + monkeypatch.setenv("COMPOSIO_API_KEY", "ak_test") runtime, client = runtime_for(sessions_by_user, **overrides) monkeypatch.setattr(runtime_mod, "build_composio_runtime", lambda *a, **k: runtime) reset_composio_runtime() @@ -213,6 +218,11 @@ def test_the_route_returns_a_link_for_the_named_person(client, monkeypatch): def test_the_route_reports_an_unconfigured_deployment(client, monkeypatch): monkeypatch.setenv("AGENT_AUTH_HEADER", "Bearer s3cret") + # Composio is the selected provider here, so this test reaches the branch it + # is about — a selected provider that still built no runtime — rather than + # being turned away earlier by the provider check and passing for a reason + # it does not assert. + monkeypatch.setenv("COMPOSIO_API_KEY", "ak_test") monkeypatch.setattr(runtime_mod, "build_composio_runtime", lambda *a, **k: None) reset_composio_runtime() @@ -225,6 +235,30 @@ def test_the_route_reports_an_unconfigured_deployment(client, monkeypatch): assert response.status_code == 503 +def test_the_composio_route_refuses_when_arcade_is_the_selected_provider( + client, monkeypatch +): + # The case this guards is a card that outlived a provider change: a Connect + # button minted under Composio, clicked after the deployment switched to + # Arcade. A working Composio runtime is installed here on purpose, so the + # refusal can only come from the provider check — without it, the route + # would happily mint against the provider nobody selected. + monkeypatch.setenv("AGENT_AUTH_HEADER", "Bearer s3cret") + _runtime, composio = install_runtime(monkeypatch, {}) + monkeypatch.delenv("COMPOSIO_API_KEY", raising=False) + monkeypatch.setenv("ARCADE_API_KEY", "arc_test") + + response = client.post( + "/composio/connect", + json={"actor_id": "U1", "kind": "human", "platform": "slack", "toolkit": "gmail"}, + headers={"Authorization": "Bearer s3cret"}, + ) + + assert response.status_code == 503 + # Nothing was minted, so no account was bound to anybody. + assert composio.created == [] + + def test_health_stays_reachable_without_the_secret(client, monkeypatch): monkeypatch.setenv("AGENT_AUTH_HEADER", "Bearer s3cret") assert client.get("/health").status_code == 200 diff --git a/agent/tests/test_connected_app_selection.py b/agent/tests/test_connected_app_selection.py new file mode 100644 index 00000000..aa6f7731 --- /dev/null +++ b/agent/tests/test_connected_app_selection.py @@ -0,0 +1,239 @@ +"""Which connected-app provider a deployment runs, decided from its keys alone. + +Selection happens before any provider client is constructed, because the answer +to "both keys are set" has to be a startup failure rather than whichever SDK +happened to initialize first. That ordering is the point of a separate pure +function, so these tests assert on it directly. +""" + +from __future__ import annotations + +import logging + +import pytest + +from connected_app_provider import ( + PROVIDER_ARCADE, + PROVIDER_COMPOSIO, + ProviderSelectionError, + selected_provider, +) + + +def test_neither_key_selects_no_provider(): + assert selected_provider({}) is None + + +def test_composio_key_alone_selects_composio(): + assert selected_provider({"COMPOSIO_API_KEY": "ak_test"}) == PROVIDER_COMPOSIO + + +def test_arcade_key_alone_selects_arcade(): + assert selected_provider({"ARCADE_API_KEY": "arc_test"}) == PROVIDER_ARCADE + + +def test_both_keys_is_a_startup_failure(): + with pytest.raises(ProviderSelectionError): + selected_provider({"COMPOSIO_API_KEY": "ak_test", "ARCADE_API_KEY": "arc_test"}) + + +def test_both_keys_fails_even_when_neither_names_an_app(): + # A key with no toolkits is a half-finished setup that Composio treats as + # unconfigured. That must not quietly resolve the conflict: the deployer + # still has two keys set, and which provider they meant is still unknown. + with pytest.raises(ProviderSelectionError): + selected_provider( + { + "COMPOSIO_API_KEY": "ak_test", + "COMPOSIO_TOOLKITS": "", + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "", + } + ) + + +def test_the_conflict_message_names_both_variables(): + with pytest.raises(ProviderSelectionError) as raised: + selected_provider({"COMPOSIO_API_KEY": "ak_test", "ARCADE_API_KEY": "arc_test"}) + assert str(raised.value) == ( + "Configure only one of COMPOSIO_API_KEY or ARCADE_API_KEY." + ) + + +def test_the_conflict_message_never_quotes_a_key(): + # An operator pastes this error into a ticket or a chat. A message that + # echoes the value it rejected turns a configuration mistake into a leak. + composio_key = "ak_live_secret_value" + arcade_key = "arc_live_secret_value" + with pytest.raises(ProviderSelectionError) as raised: + selected_provider( + {"COMPOSIO_API_KEY": composio_key, "ARCADE_API_KEY": arcade_key} + ) + message = str(raised.value) + assert composio_key not in message + assert arcade_key not in message + + +@pytest.mark.parametrize("blank", ["", " ", "\t", "\n "]) +def test_a_blank_composio_key_is_unset_not_configured(blank): + # `COMPOSIO_API_KEY=` is routine in `.env` files and in compose passthrough. + # Treating it as present would fail startup against a deployment that has + # only ever configured one provider. + assert ( + selected_provider({"COMPOSIO_API_KEY": blank, "ARCADE_API_KEY": "arc_test"}) + == PROVIDER_ARCADE + ) + + +@pytest.mark.parametrize("blank", ["", " ", "\t", "\n "]) +def test_a_blank_arcade_key_is_unset_not_configured(blank): + assert ( + selected_provider({"COMPOSIO_API_KEY": "ak_test", "ARCADE_API_KEY": blank}) + == PROVIDER_COMPOSIO + ) + + +@pytest.mark.parametrize("blank", ["", " ", "\t"]) +def test_two_blank_keys_select_no_provider(blank): + assert selected_provider({"COMPOSIO_API_KEY": blank, "ARCADE_API_KEY": blank}) is None + + +def test_inactive_provider_settings_do_not_affect_selection(): + # Only the key selects. An Arcade deployment that still carries a stale + # `COMPOSIO_TOOLKITS` from a previous provider must not be dragged back, and + # an unparseable setting belonging to the provider that is not selected is + # not this function's business. + assert ( + selected_provider( + { + "ARCADE_API_KEY": "arc_test", + "COMPOSIO_TOOLKITS": "linear,notion", + "COMPOSIO_APPROVALS": "nonsense-value", + "COMPOSIO_WORKSPACE_USER_ID": "someone", + } + ) + == PROVIDER_ARCADE + ) + + +def test_selection_reads_the_process_environment_by_default(monkeypatch): + monkeypatch.setenv("ARCADE_API_KEY", "arc_test") + monkeypatch.delenv("COMPOSIO_API_KEY", raising=False) + assert selected_provider() == PROVIDER_ARCADE + + +def _build_agent_recording_runtime_calls(monkeypatch): + """Build the agent, recording whether a Composio runtime was constructed.""" + import agent as agent_mod + + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + monkeypatch.setattr(agent_mod, "internal_source_toolsets", lambda _provider: {}) + monkeypatch.setattr(agent_mod, "ChatOpenAI", lambda **kwargs: object()) + + captured: dict = {"runtime_calls": 0} + + def recording_runtime(**kwargs): + captured["runtime_calls"] += 1 + return None + + def fake_create_deep_agent(**kwargs): + captured["agent"] = kwargs + + class _Graph: + def with_config(self, config): + return self + + return _Graph() + + monkeypatch.setattr(agent_mod, "composio_runtime", recording_runtime) + monkeypatch.setattr(agent_mod, "create_deep_agent", fake_create_deep_agent) + agent_mod.build_agent() + return captured + + +def test_both_keys_fail_the_boot_before_a_provider_client_exists(monkeypatch): + # The whole reason selection is a separate pure function. If the conflict + # were noticed after construction, the deployment would have already built a + # client — and paid a network round trip — for a provider it must not use. + monkeypatch.setenv("COMPOSIO_API_KEY", "ak_test") + monkeypatch.setenv("ARCADE_API_KEY", "arc_test") + + with pytest.raises(ProviderSelectionError): + _build_agent_recording_runtime_calls(monkeypatch) + + +def test_an_arcade_deployment_builds_no_composio_runtime(monkeypatch): + monkeypatch.delenv("COMPOSIO_API_KEY", raising=False) + monkeypatch.setenv("ARCADE_API_KEY", "arc_test") + + captured = _build_agent_recording_runtime_calls(monkeypatch) + + assert captured["runtime_calls"] == 0 + + +def test_an_arcade_deployment_says_its_provider_is_not_implemented_yet( + monkeypatch, caplog +): + # The interim state between selecting Arcade and implementing it. Registering + # nothing in silence would be indistinguishable from an agent that has no + # connected apps configured at all, which is a different and wrong answer to + # give a deployer who just set a key. + monkeypatch.delenv("COMPOSIO_API_KEY", raising=False) + monkeypatch.setenv("ARCADE_API_KEY", "arc_test") + + with caplog.at_level(logging.WARNING): + _build_agent_recording_runtime_calls(monkeypatch) + + assert "not implemented yet" in caplog.text + + +def test_a_composio_deployment_still_builds_its_runtime(monkeypatch): + # The regression guard for every existing deployment: adding selection must + # not have quietly stopped Composio from being constructed. + monkeypatch.setenv("COMPOSIO_API_KEY", "ak_test") + monkeypatch.delenv("ARCADE_API_KEY", raising=False) + + captured = _build_agent_recording_runtime_calls(monkeypatch) + + assert captured["runtime_calls"] == 1 + + +def test_no_key_at_all_builds_no_runtime_and_says_nothing_about_arcade( + monkeypatch, caplog +): + monkeypatch.delenv("COMPOSIO_API_KEY", raising=False) + monkeypatch.delenv("ARCADE_API_KEY", raising=False) + + with caplog.at_level(logging.WARNING): + captured = _build_agent_recording_runtime_calls(monkeypatch) + + assert captured["runtime_calls"] == 0 + assert "arcade" not in caplog.text.lower() + + +def test_selection_touches_no_provider_sdk(): + # The conflict has to be reported before a client exists, so this module is + # not allowed to import one. Importing an SDK here would also make an + # unconfigured deployment pay for a dependency it never calls. + # + # Asserted against the parsed module rather than its text: a substring + # search over source counts the word in this very comment, and passes for a + # module that imports the SDK inside a function. + import ast + import pathlib + + import connected_app_provider + + module_file = connected_app_provider.__file__ + assert module_file is not None + source = pathlib.Path(module_file).read_text(encoding="utf-8") + imported: set[str] = set() + for node in ast.walk(ast.parse(source)): + if isinstance(node, ast.Import): + imported.update(alias.name.split(".")[0] for alias in node.names) + elif isinstance(node, ast.ImportFrom) and node.module: + imported.add(node.module.split(".")[0]) + assert "composio" not in imported + assert "composio_tools" not in imported + assert "arcadepy" not in imported + assert "arcade_tools" not in imported From dafa78615ed6ad1a2277990c31f629d2346913df Mon Sep 17 00:00:00 2001 From: Maxim Date: Tue, 15 Sep 2026 17:00:43 +0200 Subject: [PATCH 02/15] docs(arcade): record the plan and what stage 1 established MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The implementation plan, plus the findings from reading arcadepy 1.10.0. The finding that matters contradicts an assumption the plan depends on: the client SDK's tool models carry no effect metadata. Arcade's tool authoring framework supports read-only and destructive behaviour flags, but `ToolDefinition` and `ToolGetResponse` declare no field that would return them. Under the plan's own fallback rule — unresolvable metadata is treated as destructive — a catalogue publishing no metadata gates every call behind an approval card, including reads. That teaches approvers to click through cards without reading them, which is worse than the Composio path rather than better. One door is still open and needs a key to check: the SDK's base model allows undeclared fields, so behaviour metadata may arrive on the wire and simply be missing from the generated types. One live `tools.list` settles it. Confirmed as the plan assumed: authorization is per action rather than per app, and the response names the scopes it covers, so "a prior read does not establish send access" is expressible directly. --- ...-09-14-exclusive-connected-app-provider.md | 253 ++++++++++++++++++ .../2026-09-15-arcade-stage-1-findings.md | 131 +++++++++ 2 files changed, 384 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-14-exclusive-connected-app-provider.md create mode 100644 docs/superpowers/plans/2026-09-15-arcade-stage-1-findings.md diff --git a/docs/superpowers/plans/2026-09-14-exclusive-connected-app-provider.md b/docs/superpowers/plans/2026-09-14-exclusive-connected-app-provider.md new file mode 100644 index 00000000..3b251090 --- /dev/null +++ b/docs/superpowers/plans/2026-09-14-exclusive-connected-app-provider.md @@ -0,0 +1,253 @@ +# Exclusive Connected-App Provider Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let a deployer choose Composio or Arcade for connected-app tools by configuring exactly one provider API key, while preserving existing Composio behavior. + +**Architecture:** The Python agent selects one provider and registers that provider's tools. Arcade is added alongside the existing Composio implementation, reusing the existing trusted-identity helpers and approval gate in place; the Channel continues to own click identity and private delivery. Extract common code only when both implementations demonstrate a concrete need. + +**Tech Stack:** Existing Python Deep Agents/LangGraph agent, FastAPI, TypeScript CopilotKit Channels, and the Arcade Python SDK (`arcadepy`). + +--- + +## Status and scope + +Feature specification and staged implementation plan, authored 2026-09-14 and revised 2026-09-15. This records the agreed behavior and implementation boundaries, rather than supplying unverified SDK code. Validate SDK contracts before implementing dependent calls. Investigate production browser verification early, but do not make its completion a prerequisite for independent selection, adapter, or regression-test work. + +The deliverable is a working Arcade integration, including personal linking for ordinary colleagues, while preserving Composio. An interface with no working Arcade implementation is an intermediate milestone, not completion. + +Agreed: both providers are supported by the codebase, but only one is active in a deployment. API-key presence selects the provider. No automatic provider fallback or combined catalog. + +Proposed implementation details below include configuration names, module boundaries, and a manual retry after account connection. They are design choices for this feature, not claims about existing behavior. + +Baseline inspected: local HEAD `be27c426131a86e8ce198d1d00c8e72d06a60991`, branch `feat/composio-in-agent`, and merged PR #68, whose final head is `27c5b2ecd22127a86e421da700e70f7a8949aee7`. These are different revisions. Recheck the target branch before implementation; do not treat this checkout as current main. + +## Product contract + +Treat absent, empty, and whitespace-only API keys as unset. Select before constructing clients or parsing provider-specific settings. + +| Composio key | Arcade key | Result | +| --- | --- | --- | +| Set | Unset | Select Composio | +| Unset | Set | Select Arcade | +| Unset | Unset | No connected-app provider | +| Set | Set | Fail startup: `Configure only one of COMPOSIO_API_KEY or ARCADE_API_KEY.` | + +Both keys is an error even if one provider has no configured apps. Adding an Arcade key to a Composio deployment therefore fails startup rather than silently switching providers. No separate provider-selector setting is introduced. Never choose by initialization order or connectivity. Never include credential values in errors. + +No provider leaves existing direct integrations, web search, coder, and other agent capabilities governed by their current configuration. Exclusivity applies only to Composio versus Arcade. + +Preserve Composio's environment variables, personal/shared routing, connection identities, approval modes and legacy aliases, and operator connection command. Preserve its current key-with-no-apps behavior: warning plus no registered connected-app tools. This qualifies the earlier conversational shorthand that every incomplete setup would fail startup; compatibility takes precedence for this existing case. + +For the new Arcade configuration, require at least one allowed app when a key is supplied. Invalid selected-provider configuration fails startup with a useful message. Ignore inactive-provider settings other than the key-selection conflict. Provider outages produce an unavailable result, never selection of another provider. + +The agent continues to expose `search_my_tools` and `run_my_tool`. Users ask for actions without choosing a provider in conversation. Show provider names where useful in setup, connection consent, and diagnostics. + +## Configuration + +Keep all `COMPOSIO_*` settings unchanged. Proposed Arcade settings: + +| Setting | Meaning | +| --- | --- | +| `ARCADE_API_KEY` | Selects Arcade; held only by the agent | +| `ARCADE_TOOLKITS` | Allowed apps acting through an operator-connected shared identity | +| `ARCADE_USER_TOOLKITS` | Allowed apps acting through each speaker's own identity | +| `ARCADE_WORKSPACE_USER_ID` | Shared identity; defaults to the Channel name, then `open-tag` | +| `ARCADE_IDENTITY_NAMESPACE` | Required for personal apps; stable deployment namespace within an Arcade project | +| `ARCADE_APPROVALS` | `on` by default, or `off`; no legacy aliases needed | + +An app listed in both scopes is personal only. Missing identity must never fall back to the shared account. Warn when personal-data apps are configured as shared, following the existing Composio convention. + +Arcade's catalog identifiers must be resolved against its actual definitions. Do not reuse Composio's uppercase-underscore slug parsing or assume app names coincide. Operator input may be normalized for lookup, but execution uses the canonical identifier returned by Arcade. + +Personal Arcade IDs are deterministic encodings of `(namespace, platform, actor_id)`, with unambiguous escaping or length encoding. Preserve the existing Composio identity format. Changing the namespace means a new set of Arcade connections and must be documented as such. + +Provider keys stay on the agent service. `AGENT_AUTH_HEADER` stays on both services and remains mandatory for capability-minting routes. Additional browser-verifier configuration is specified only after Stage 1 establishes the supported route and identity proof; do not invent a callback URL that the deployed topology cannot serve. + +## Minimal integration boundary + +Start with one selector function and explicit registration branches in the existing composition points. Both implementations satisfy these behavioral contracts; a common base class, shared tool implementation, or multi-module neutral package is not required: + +| Operation | Required contract | +| --- | --- | +| Describe capabilities | Configured app names and personal/shared scope; never claim authorization from configuration alone | +| Discover | Allowed canonical actions, descriptions, input schemas, scope, and connection requirements | +| Inspect action | Resolve canonical definition and effect; reject an action outside the configured allowlist | +| Execute | Explicit server-resolved identity, canonical action and validated arguments; normalized success/failure | +| Start connection | Verified clicker and validated connection target; private URL, already-authorized result, or safe error | + +Keep SDK response parsing, Composio sessions, Arcade authorization scopes, and provider errors inside their provider packages. Arcade may expose its own tools with the same `search_my_tools` and `run_my_tool` names because only one pair is registered. Leave Composio's existing tool builder intact. Avoid a generic plugin registry or a new provider framework. + +Only the selected provider's tools and prompt additions are registered. In particular, replace the Composio-specific absence statement in `agent/prompts/tools.py`: an Arcade deployment must not be told it has no connected apps. + +Reuse trusted actor handling directly from `agent/composio_tools/state.py`. Do not move or rename it as part of adding Arcade; its package name is not a reason to disturb the identity boundary. Preserve the persisted `channel_actor` key, anonymous-turn clearing, rejection of caller-supplied identity, and delayed approval behavior through the real AG-UI adapter. Reuse `require_write_confirmation` and existing effect constants in place as well. + +After both implementations work, extract a helper only if a concrete shared behavior otherwise needs two maintained implementations. Keep such changes independently reviewable and covered by existing behavior tests. No identity-module relocation is planned for this feature. + +## Discovery, execution, and approval + +Discovery searches only configured apps and respects pagination. A bounded local index of allowed Arcade definitions is acceptable for the first version; semantic search parity with Composio is not required. Return exact argument schemas for discovered actions. + +Execution independently enforces configured ownership and schema validity. A model-supplied action identifier is not authorization. Neither tool exposes a user ID, provider selector, or account selector as a model-controlled argument. + +Arcade authorization is action/scope-specific. Check authorization for the requested action; a prior successful read does not establish access to send or delete. Do not initiate account-binding flows during ordinary discovery or expose their URLs in tool results. + +Map explicit Arcade read-only metadata to `read`, explicit destructive metadata to `destructive`, and a non-read explicitly declared non-destructive to `write`. Missing, malformed, contradictory, or unresolvable metadata defaults to `destructive`; destructive wins over a contradictory read-only claim. Do not infer safety from an action name or idempotency. + +When approvals are on, gate every non-read through the existing `require_write_confirmation`. Preserve personal approver checks on both confirm and decline. Account consent does not count as write approval. Recheck required account access at execution without executing as a side effect of that check. + +Normalize authorization-needed, declined, expired/revoked authorization, provider unavailable, definitive execution failure, and unknown execution outcome separately. Never automatically retry a write after an ambiguous transport failure. Redact provider exceptions before returning or logging them; never expose authorization URLs or tokens. + +## Connection experience + +1. Search or execution reports that a validated action needs account access. It returns a safe connection target, never a URL. +2. `connect_app` posts a public button naming the app. The target may contain a canonical action so Arcade can request the right scopes; the agent validates it again on click. +3. On click, the Channel supplies the actual clicker's identity to an authenticated agent route. It never trusts an actor ID embedded in card props. +4. The agent initiates Arcade authorization for that clicker and action. The Channel delivers the URL through the existing private delivery path. If private delivery fails, it discards the URL and reports the failure without exposing it. +5. Browser verification binds the flow to the same person. Investigate the approach in Stage 1 and demonstrate the production binding in Stage 4. +6. The user returns and asks OpenTag to continue. The next call checks real authorization status and, for a write, asks for write approval. No automatic background execution after OAuth in the first version. + +Support link expired, access denied, already authorized, additional scopes required, and missing provider configuration with distinct useful messages. A successful click does not mean the account is connected. + +Shared accounts use an operator CLI analogous to the existing Composio command. A personal Connect button must not bind a shared account. First-version personal linking is Slack-only, matching the existing supported experience. + +### Switching providers and old interactions + +Changing providers requires restart and new account connections. No credential migration or automatic revocation of old-provider accounts is attempted. + +Bind new connection cards and pending tool approvals to the provider that created them. Reject stale interactions if it differs from the selected provider. Legacy Composio cards without a marker remain usable only in Composio mode; in Arcade mode they instruct the user to request a fresh action. + +Provider binding must reach the graph's execution boundary, not merely the displayed card. An old checkpoint must never resume against the newly selected provider. Cover checkpoint replay and resume through the real adapter. Do not add a provider argument that lets the model choose an inactive provider. + +## Production authorization prerequisite + +Arcade documents its default verifier as requiring an Arcade project-member login. Its production guidance calls for custom OAuth app credentials and a custom verifier so end users need not join the Arcade project. + +The runtime-to-agent shared secret authenticates a service call; it is not a browser login. OpenTag's forwarded Slack actor alone is not browser proof. Investigate a supported verifier using authenticated browser identity linked to the Slack actor, or an Arcade-supported equivalent, in Stage 1; demonstrate its binding in Stage 4. A browser query parameter or possession of the public Connect card is insufficient. + +Until demonstrated, classify personal linking as development-only. This does not prevent implementing the adapter, exercising developer accounts, or validating shared-account behavior. Do not claim general personal-account production readiness based on a maintainer successfully logging into Arcade. Completing this prerequisite is required for the full feature, not an optional follow-up. If external setup prevents it, report that specific incomplete capability while continuing independent work. + +## Proposed file map + +Paths are repository-relative. Add focused files as implementation requires them; the map does not mandate an abstraction layer or a file for each operation. + +| Files | Responsibility | +| --- | --- | +| New `agent/connected_app_provider.py` | Pure API-key selection; no SDK construction or identity logic | +| Existing `agent/composio_tools/` | Keep existing runtime, session and tool builders; reuse identity helpers in place; only targeted provider-binding changes | +| New `agent/arcade_tools/` | Arcade configuration, SDK client, search/run tool builder, action authorization and operator setup; split catalog/effect helpers only when useful | +| `agent/agent.py`, `agent/agui.py`, `agent/main.py` | Register selected tools, preserve actor/resume semantics, expose authenticated connection dispatch | +| `agent/write_confirmation.py`, `agent/prompts/tools.py`, `agent/prompts/__init__.py` | Reuse approval helper; add provider binding and selected-provider capability text without moving effect definitions | +| `app/tools/connect-app.tsx`, `app/tools/connect-click.tsx`, `app/human-in-the-loop/connect-account.tsx` | Provider-bound connection targets, click identity, private delivery and stale-card response | +| New `app/tools/connected-app-connect.ts`; existing `app/tools/composio-connect.ts` | Neutral client contract; retain compatibility where old callers require it | +| `app/channel.tsx` and approval renderer | Preserve personal approver checks; reject mismatched provider-bound approval | +| `agent/agent_auth.py` | Preserve secret protection; narrowly integrate verifier route authentication only if Stage 1 requires a public browser route | +| New verifier module and route selected in Stage 1 | Browser identity verification; placement depends on the demonstrated deployment path | +| `agent/pyproject.toml`, `agent/uv.lock`, deployment/build files | SDK dependency, package inclusion, and configuration wiring | +| `.env.example`, `README.md`, `setup.md`, `AGENTS.md` | Selection rules, setup, supported surfaces, switching and Arcade code locations | + +Retain `/composio/connect` and add `/arcade/connect`; each route refuses requests when its provider is inactive. Keep the current Composio client and add only the connection dispatch needed for provider-bound cards. A new generic HTTP endpoint is not required. Agent and runtime must agree on the response contract for connection targets and selected-provider mismatch. Provider information sent to the Channel is non-secret and cannot override agent-side selection. + +## Staged implementation + +### Stage 1: Validate SDK contracts and investigate browser verification + +- [ ] Read/install current repository-required skills before implementation: `npx copilotkit@latest skills install --skill copilotkit-channels` for Channel changes; install `runtime` if modifying SDK runtime internals. Do not commit installed skills. +- [ ] Recheck Git state and the implementation base; preserve unrelated work. +- [ ] Verify a supported `arcadepy` release against the project's Python version and dependency resolver. Record the tested SDK/API response shapes, including errors and effect metadata. +- [ ] Using an isolated developer project and test account, prove allowed-tool listing, schema retrieval, non-mutating auth checks, connection initiation, authorization completion, and a read. Keep credentials out of fixtures. +- [ ] Investigate the production verifier path: identify required OAuth credentials, a supported way to authenticate browser identity, and a publicly reachable route in the deployment topology. Record what is proven and what still needs external setup. +- [ ] Write the adapter recipe from the SDK evidence. Write the verifier recipe when its identity-binding approach is established; implement and prove it in Stage 4. External verifier setup must not block independent work in Stages 2 and 3. + +Exit: SDK contracts are grounded enough to implement the adapter, and the production-verification dependency is explicit. A successful maintainer-only OAuth flow is not evidence that colleague onboarding works. + +### Stage 2: Select one provider and preserve Composio + +- [ ] Add `agent/tests/test_connected_app_selection.py` covering all four key combinations, whitespace, both-key conflict before SDK construction, and inactive-provider settings. +- [ ] Run those tests before implementation and confirm the expected failing assertions. +- [ ] Add the selector in `agent/connected_app_provider.py` and branch at existing agent/HTTP composition points. Preserve Composio's no-apps warning behavior and verify no inactive SDK client is instantiated. +- [ ] Reuse actor/effect helpers at their existing import paths. Leave identity normalization, graph state keys, session construction and connection IDs unchanged. +- [ ] Run existing Composio, AG-UI identity, approval-resume, configuration and health suites. Confirm direct tools still register with neither provider selected. + +Exit: provider selection is tested and Composio behavior is preserved. Until Stage 3 is wired, an Arcade selection must report that implementation is unavailable rather than silently behaving as no provider. Do not ship this intermediate state as the completed feature. + +### Stage 3: Implement the Arcade adapter + +- [ ] Add `agent/tests/test_arcade_config.py`, `test_arcade_catalog.py`, `test_arcade_tools.py`, and `test_arcade_effects.py` with sanitized SDK-shaped fixtures captured in Stage 1. +- [ ] Cover pagination, unknown action, invalid arguments, disabled app, missing actor, overlapping scope lists, metadata fallback, revoked access, rate limiting and unknown write outcome. Verify failures before adding the corresponding behavior. +- [ ] Implement the Stage 1 adapter recipe and Arcade's own search/run tool builder. Enforce allowlists at execution and register this pair only when Arcade is selected. Do not rewrite Composio's tool builder to fit Arcade. +- [ ] Route all gated calls through the existing approval helper. Verify decline makes zero execute calls; approval executes once as the original actor. +- [ ] Generate prompt content for Composio, Arcade and neither. Assert Arcade-only never gets the Composio absence statement. + +Exit: Arcade discovery and execution work with configured test accounts, under existing identity and approval rules. Production personal linking can still be incomplete and must be described that way. + +### Stage 4: Connect and resume safely + +- [ ] Add `agent/tests/test_arcade_connect.py`, `test_arcade_approval_resume.py`, and `test_connected_app_provider_switch.py` plus the corresponding TypeScript connection-client and click tests. +- [ ] Cover missing/mismatched shared secret, clicker different from requester, non-human actor, unknown connection target, additional scopes and failed private delivery. +- [ ] Add the Arcade connection route and minimal dispatch for provider-bound cards. Preserve the existing Composio route and old Composio cards. Both routes reject an inactive provider. +- [ ] Implement and test the supported browser flow investigated in Stage 1. Demonstrate two distinct Slack identities, mismatch rejection and expired-flow handling. Confirm raw query-string identity cannot satisfy it; exclude URLs and sensitive state from logs and model-visible messages. +- [ ] Exercise two speakers and delayed approval through `OpenTagAGUIAgent`, including anonymous follow-up, regenerate, and provider switch with a pending interaction. UI-only unit tests are insufficient. +- [ ] Implement the Arcade operator connect command and verify it loads the root environment using existing conventions. + +Exit: ordinary colleagues can connect their own accounts without joining the Arcade project, and stale provider interactions cannot execute. This is the production personal-linking gate. + +### Stage 5: Package, document and verify + +- [ ] Review actual duplication after both paths work. Extract only a small helper whose shared behavior is demonstrated; skip extraction if it offers no concrete benefit. Leave trusted-identity code in place. +- [ ] Include new packages in the built Python distribution; verify imports from the built artifact, not just the source directory. +- [ ] Wire Arcade settings into the agent's Railway environment. Document Stage 1's verifier route deployment and its authentication separately from the internal connect route. +- [ ] Update setup examples for Composio-only, Arcade-only and neither; document the both-key error, new account connections after switching, and operator versus personal setup. +- [ ] Run the repository checks below and record actual results. Fix regressions introduced by the feature; identify unrelated failures explicitly. +- [ ] Restart changed processes, then prove Channel `online` through `controls.status()` on an isolated Channel/project. Health HTTP 200 and `ready()` alone are insufficient. +- [ ] Complete the live acceptance scenarios below. Do not merge, publish, deploy to production, or send test messages to other people as an implied part of this document-writing request. + +## Acceptance matrix + +| Scenario | Required result | +| --- | --- | +| Existing Composio deployment | Same accounts, tools, approvals and operator setup; no new required variables | +| Arcade-only | Only Arcade connected-app tools registered; correct prompt and configured apps | +| Both keys | Clear startup error before either client is constructed | +| Neither key | Other agent capabilities work; no connected-app tools or misleading claims | +| Selected provider unavailable | Actionable failure; no cross-provider fallback | +| Person A and person B in one thread | Each reads/acts as themselves; anonymous turn inherits neither identity | +| B clicks A's public Connect button | Any newly minted connection is B's; A's account remains unchanged | +| Browser verifier mismatch | Flow rejected without binding an account | +| Read authorized, send not authorized | Additional account authorization required; no send occurs | +| Write approved/declined | Correct person decides; declined executes zero times, approved executes once | +| Late approval | Original action, provider and account preserved through actual graph resume | +| Provider changed with old card/checkpoint | Explicit stale-interaction response; zero execution against replacement provider | +| Private delivery unavailable | No URL in thread, model output, or logs | +| Shared app requested personally | No private click can change the shared account | +| Ambiguous write timeout | Outcome reported as unknown; no automatic replay | + +Live acceptance uses a private test conversation and dedicated accounts. Sending or modifying data requires explicit test authorization. Automated tests must not load real provider credentials or contact production services. + +## Verification commands + +From the repository root: + +```bash +pnpm check-types +pnpm test +(cd agent && uv run pytest) +node node_modules/railway/dist/iac/bin.js +``` + +Packaging check from `agent/`: `uv build`, followed by installation/import checks of the resulting artifact in an isolated environment. The implementer must report exact commands and outcomes. These checks have not been run for this specification-only change. + +## Explicit exclusions + +Simultaneously active providers; combined catalogs; automatic fallback; account/token migration; a new approval system; a new agent framework; automatic execution upon OAuth completion; Teams personal-linking expansion; a marketplace or generic integration-management UI. + +## Sources + +- [OpenTag PR #68](https://github.com/CopilotKit/OpenTag/pull/68): agent-owned capabilities, identity and connection/approval boundaries. +- [Arcade authorization](https://docs.arcade.dev/en/build/tool-calling/custom-apps/auth-tool-calling): per-user authorize/execute contract. +- [Arcade production verification](https://docs.arcade.dev/en/build/user-facing-agents/secure-auth-production): project-member default verifier, custom verifier and OAuth app requirements. +- [Arcade tool definitions](https://docs.arcade.dev/en/build/tool-calling/custom-apps/get-tool-definitions): tool retrieval and pagination. +- [Arcade tool metadata](https://docs.arcade.dev/en/build/create-tools/tool-basics/add-tool-metadata): read-only and destructive flags. +- [Arcade LangChain integration](https://docs.arcade.dev/en/get-started/agent-frameworks/langchain/use-arcade-with-langchain-py): Python SDK and interrupt integration. + +Documentation was reviewed during the preceding assessment in this conversation. Revalidate SDK details during Stage 1; no live Arcade behavior has been verified yet. diff --git a/docs/superpowers/plans/2026-09-15-arcade-stage-1-findings.md b/docs/superpowers/plans/2026-09-15-arcade-stage-1-findings.md new file mode 100644 index 00000000..878e6601 --- /dev/null +++ b/docs/superpowers/plans/2026-09-15-arcade-stage-1-findings.md @@ -0,0 +1,131 @@ +# Stage 1 findings: the Arcade SDK boundary + +Recorded 2026-09-15 against `arcadepy` 1.10.0, the latest release the index +offers. Established by reading the installed package and by read-only calls +against a live Arcade project — 3000 tool definitions listed, no write call +made. The one part still unproven is browser verification, at the end. + +## The dependency itself + +`arcadepy` 1.10.0 declares `requires-python >=3.8`, and this agent pins +`>=3.12` and runs 3.13.2, so the floor is satisfied. Its dependencies are +`anyio`, `distro`, `httpx`, `pydantic`, `sniffio` and `typing-extensions` — +every one already resolved in this tree, and none with a ceiling that conflicts +with an existing floor. Adding it costs one new transitive package, `distro`. + +## What the client exposes + +`tools.list`, `tools.get`, `tools.authorize`, `tools.execute`, plus +`auth.authorize`, `auth.status` and `auth.wait_for_completion`. That is the +whole surface the plan's five operations need. + +`tools.authorize` takes `tool_name`, `user_id` and an optional `next_uri`, and +returns an `AuthorizationResponse` carrying `id`, `url`, `scopes`, `user_id` and +a `status` of `not_started` / `pending` / `completed` / `failed`. + +Two things follow, both of which the plan assumed and both of which hold: + +* **Authorization is per action, not per app.** `tool_name` is the unit, and the + response names the `scopes` that authorization covers. The plan's requirement + that a prior read must not establish send access is expressible directly. +* **The connect target can stay a target.** `authorize` mints the URL, so the + agent can hold an action identifier and mint on click, exactly as the Composio + path already does. + +`auth.status` accepts a `wait` of at most 59 seconds, so polling for completion +is bounded per call rather than open-ended. + +## Effect metadata: present, and richer than Composio's + +Established against a live project by listing 3000 tool definitions and reading +the raw JSON. + +The generated SDK types are a red herring. `ToolDefinition` declares no +`metadata` field, and reading the package alone suggests effect information is +unavailable. It is not: the API returns `metadata.behavior` and `arcadepy`'s +base model sets `extra="allow"`, so the field survives parsing and is reachable +even though no type declares it. A first pass that stopped at the type +definitions concluded the opposite, and was wrong. + +What comes back, on tools that publish it: + +```json +"metadata": { + "classification": {"service_domains": ["email"]}, + "behavior": { + "operations": ["update"], + "read_only": false, + "destructive": true, + "idempotent": true, + "open_world": true + } +} +``` + +**The plan's three-band mapping works as written, and the middle band is real.** +Across 346 classified tools: no tool claims `read_only` and `destructive` +together, so the contradiction case the plan defends against did not occur once. +More usefully, non-reads that explicitly declare themselves non-destructive are +common — Github alone publishes 26 reads, 16 writes and 1 destructive. That is +the `write` band the Composio path could not express at all, because MCP +behaviour tags say only "read" or "destructive". On Arcade, "this changes +something but will not destroy anything" is a statement a tool can actually +make, so the approval card can stop painting ordinary writes the same as +deletions. + +### Coverage splits on one clean line + +Overall coverage is 11.5%, and that number is misleading. The split is entirely +between two families of toolkit: + +| Family | Example | Metadata | +| --- | --- | --- | +| Curated toolkits | `Github`, `Asana`, `Attio`, `Figma`, `Clickup`, `Confluence`, `Daytona` | **100%**, every tool | +| Generated API wrappers | `GithubApi`, `AsanaApi`, `DatadogApi`, `AirtableApi` | **0%**, every tool | + +Every curated toolkit sampled classified all of its tools. Every `*Api` toolkit +classified none of them. The low headline percentage is arithmetic: the wrapper +families are enormous (`GithubApi` 780 tools, `DatadogApi` 588, +`FreshserviceApi` 214) and drown out the curated ones. + +### What that means for the gate + +The plan's fallback — unresolvable metadata is treated as destructive — is +correct and needs no change. Its consequence is now predictable rather than +surprising: a deployment that names `Github` gets a working three-band gate, and +a deployment that names `GithubApi` gets an approval card on every single call, +including reads. + +So this is a configuration concern, not a classification one. **Arcade config +validation should warn at startup when a configured toolkit publishes no +behaviour metadata**, naming the curated toolkit to use instead where one exists +under the same name minus the `Api` suffix. That warning belongs with the other +startup warnings in Stage 3, and it is cheap: the answer is already in the +listing the adapter has to fetch anyway. + +## Production browser verification + +Unchanged from what the plan already records, and not yet investigated against a +live project: Arcade's default verifier expects the person completing the OAuth +flow to be a member of the Arcade project, which ordinary colleagues in a Slack +workspace are not. Production use needs custom OAuth credentials and a custom +verifier on a publicly reachable route. This remains the Stage 4 gate. + +## What this changes about the plan + +Nothing structural. The effect-mapping paragraph stands as written and turns out +to be better supported on Arcade than on Composio, because the `write` band is +expressible here. + +Two additions for Stage 3, both small: + +1. **Warn on a toolkit that publishes no behaviour metadata**, at configuration + time, naming the curated equivalent where one exists. Without it, a deployer + who names a `*Api` toolkit gets an approval card on every read and no + explanation. +2. **Read `metadata.behavior` off the undeclared field rather than a typed + attribute**, and pin that with a contract test against a recorded payload. It + is reachable because the SDK's base model allows extra fields, which is a + property of the SDK's configuration rather than a documented guarantee — if a + future release tightens it, the classification silently falls back to + destructive-for-everything and nothing else goes red. From 62b8ac8eab84fa9ecb31c73a79e9a3dce2b6fea2 Mon Sep 17 00:00:00 2001 From: Maxim Date: Tue, 15 Sep 2026 21:51:29 +0200 Subject: [PATCH 03/15] feat(arcade): classify effects and normalize execute outcomes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first two Arcade-shaped pieces, both settled against the live API rather than against documentation. Effects come from `metadata.behavior`, read straight off the payload because no generated type declares the field — it survives parsing only because the SDK keeps fields it does not know about. A contract test pins that: if a release tightens the model, every tool silently becomes unclassified and asks for approval on every read, and nothing else would go red. All three bands are reachable here, unlike the Composio path. A tool can declare read_only false and destructive false, which is it saying it changes something and will not destroy anything, so an ordinary write need not be painted like a deletion. The fixtures carry one real example of each band plus one tool that publishes nothing. Unclassified still gates, and now says why, naming the tool. An operator meeting an approval card on an ordinary read otherwise blames the approval mode. Outcomes read Arcade's typed error kind rather than matching on message text. The distinction the module exists for is failed versus unknown: a transport error, a missing error body, and an error kind this build has never seen are all unknown, because none of them establish that the call did not happen. The provider's own `can_retry` is honoured only where the outcome is already determinate — on an unknown outcome it is how one approved write gets sent twice. Authorization URLs cannot survive into an outcome. They arrive on the execute response when an account is not connected, and would otherwise reach the model as a tool result; the field is typed as always-None and link-shaped words are stripped from provider messages too. Config differs from Composio's reader in three places, each because Arcade differs: a key naming no apps is an error rather than a shrug since there is no deployment history to preserve, toolkit names keep their case since Arcade's are not lowercase slugs, and personal apps require a namespace because Arcade identities are global to a project — without one, two deployments sharing a project would share each other's connected accounts. Packaging caught two omissions on its own: the new package was missing from both the wheel and the container image. --- agent/arcade_tools/__init__.py | 9 + agent/arcade_tools/config.py | 170 ++++++++ agent/arcade_tools/effects.py | 122 ++++++ agent/arcade_tools/outcomes.py | 192 +++++++++ agent/pyproject.toml | 8 +- agent/tests/fixtures/arcade_catalogue.json | 392 ++++++++++++++++++ agent/tests/test_arcade_config.py | 242 +++++++++++ agent/tests/test_arcade_effects.py | 146 +++++++ agent/tests/test_arcade_outcomes.py | 183 ++++++++ agent/tests/test_arcade_sdk_contract.py | 92 ++++ agent/uv.lock | 19 + deployment/docker/agent.Dockerfile | 1 + .../2026-09-15-arcade-stage-1-findings.md | 23 +- 13 files changed, 1591 insertions(+), 8 deletions(-) create mode 100644 agent/arcade_tools/__init__.py create mode 100644 agent/arcade_tools/config.py create mode 100644 agent/arcade_tools/effects.py create mode 100644 agent/arcade_tools/outcomes.py create mode 100644 agent/tests/fixtures/arcade_catalogue.json create mode 100644 agent/tests/test_arcade_config.py create mode 100644 agent/tests/test_arcade_effects.py create mode 100644 agent/tests/test_arcade_outcomes.py create mode 100644 agent/tests/test_arcade_sdk_contract.py diff --git a/agent/arcade_tools/__init__.py b/agent/arcade_tools/__init__.py new file mode 100644 index 00000000..fd506f24 --- /dev/null +++ b/agent/arcade_tools/__init__.py @@ -0,0 +1,9 @@ +"""The Arcade connected-app provider. + +Selected by `ARCADE_API_KEY`, and never constructed alongside Composio — see +`connected_app_provider`. Everything Arcade-shaped lives behind this package: +SDK response parsing, authorization scopes, and provider errors. + +Identity, the approval card and the effect vocabulary are not Arcade's and are +not re-implemented here. They are imported from where they already live. +""" diff --git a/agent/arcade_tools/config.py b/agent/arcade_tools/config.py new file mode 100644 index 00000000..73559354 --- /dev/null +++ b/agent/arcade_tools/config.py @@ -0,0 +1,170 @@ +"""Environment contract for the optional Arcade integration. + +Absent `ARCADE_API_KEY` returns `None` and nothing downstream is constructed. +Which provider a deployment runs is decided before this is called — see +`connected_app_provider` — so this reader never has to consider Composio. + +Three places this deliberately differs from the Composio reader, each because +Arcade differs rather than for variety's sake: + +* **A key naming no apps is an error, not a shrug.** Composio treats that as + unconfigured because deployments already ship it that way and failing their + boot over it would be a regression. Arcade has no such history, so the + half-finished setup is said out loud at the only moment anybody is looking. +* **Toolkit names keep their case.** Composio slugs are lowercase; Arcade's are + not, and `Github` is not interchangeable with `github` inside a qualified tool + name. Lowercasing here would produce identifiers that resolve to nothing. +* **Personal apps require a namespace.** Arcade user ids are global within a + project, so two deployments sharing one project and both calling somebody + `slack:U1` would silently share that person's connected accounts. +""" + +from __future__ import annotations + +import logging +import os +from collections.abc import Mapping +from dataclasses import dataclass + +logger = logging.getLogger(__name__) + +#: No legacy aliases. `destructive` and `writes` exist on the Composio side only +#: because refusing them would fail a running deployment; here they would be a +#: promise about MCP tag behaviour this gate does not implement. +APPROVAL_MODES = ("off", "on") + +#: The shared identity when nothing names one. A present but empty channel name +#: is not a name: an empty user id is a real Arcade identity that nothing else +#: resolves to, so the shared connection would land where no turn looks. +DEFAULT_WORKSPACE_USER_ID = "open-tag" + +#: Apps whose data belongs to one person rather than to a team. Used only to +#: warn; it never changes what is allowed. +PERSONAL_TOOLKITS = frozenset( + {"gmail", "googlecalendar", "googledrive", "outlook", "slack", "dropbox"} +) + + +class ArcadeConfigError(ValueError): + """An operator set an Arcade variable to something unusable.""" + + +@dataclass(frozen=True) +class ArcadeConfig: + api_key: str + workspace_toolkits: tuple[str, ...] + user_toolkits: tuple[str, ...] + approvals: str + workspace_user_id: str + #: Prefix for every personal Arcade identity this deployment mints. Empty + #: when no personal app is configured, because then none are minted. + identity_namespace: str + #: Names dropped from `workspace_toolkits` because they are also personal. + #: Kept rather than discarded so the startup warning can be exact: once the + #: lists are resolved the overlap is gone, and a warning that cannot name + #: what it is about is one nobody acts on. + shared_overridden_by_personal: tuple[str, ...] = () + + +def _env(env: Mapping[str, str] | None) -> Mapping[str, str]: + return os.environ if env is None else env + + +def _value(source: Mapping[str, str], name: str) -> str: + return (source.get(name) or "").strip() + + +def _toolkit_list(raw: str) -> tuple[str, ...]: + """Split and trim, preserving case and order, dropping duplicates.""" + return tuple( + dict.fromkeys(item.strip() for item in raw.split(",") if item.strip()) + ) + + +def _approval_mode(raw: str) -> str: + value = raw.strip().lower() or "on" + if value not in APPROVAL_MODES: + raise ArcadeConfigError( + f'Invalid ARCADE_APPROVALS: "{raw.strip()}" — expected one of ' + + ", ".join(APPROVAL_MODES) + ) + return value + + +def read_arcade_config( + env: Mapping[str, str] | None = None, + *, + default_user_id: str, +) -> ArcadeConfig | None: + """Read the Arcade contract, or `None` when the feature is not configured. + + Raises `ArcadeConfigError` for a configuration that is present but unusable. + Pure: it collects no warnings and writes no logs, so the boot sequence + decides when those are said. See `startup_warnings`. + """ + source = _env(env) + api_key = _value(source, "ARCADE_API_KEY") + if not api_key: + return None + + workspace_toolkits = _toolkit_list(_value(source, "ARCADE_TOOLKITS")) + user_toolkits = _toolkit_list(_value(source, "ARCADE_USER_TOOLKITS")) + if not workspace_toolkits and not user_toolkits: + raise ArcadeConfigError( + "ARCADE_API_KEY is set but neither ARCADE_TOOLKITS nor " + "ARCADE_USER_TOOLKITS names an app, so there is nothing to reach. " + "Name at least one in either." + ) + + # Unconditional, and before the namespace check: a name in both lists is the + # operator saying it must run as the person, so the shared entry is dropped + # rather than left to be picked by iteration order. + overridden = tuple(name for name in workspace_toolkits if name in user_toolkits) + workspace_toolkits = tuple( + name for name in workspace_toolkits if name not in user_toolkits + ) + + identity_namespace = _value(source, "ARCADE_IDENTITY_NAMESPACE") + if user_toolkits and not identity_namespace: + raise ArcadeConfigError( + "ARCADE_USER_TOOLKITS names apps people connect for themselves, " + "which needs ARCADE_IDENTITY_NAMESPACE. Arcade identities are " + "global to a project, so without a namespace two deployments " + "sharing one project would share each other's connected accounts." + ) + + return ArcadeConfig( + api_key=api_key, + workspace_toolkits=workspace_toolkits, + user_toolkits=user_toolkits, + approvals=_approval_mode(_value(source, "ARCADE_APPROVALS")), + workspace_user_id=( + _value(source, "ARCADE_WORKSPACE_USER_ID") + or default_user_id.strip() + or DEFAULT_WORKSPACE_USER_ID + ), + identity_namespace=identity_namespace, + shared_overridden_by_personal=overridden, + ) + + +def startup_warnings(config: ArcadeConfig) -> tuple[str, ...]: + """Misconfigurations worth saying once, at boot rather than per turn.""" + warnings: list[str] = [] + + for name in config.shared_overridden_by_personal: + warnings.append( + f'"{name}" is in both ARCADE_TOOLKITS and ARCADE_USER_TOOLKITS. ' + "Using each person's own account; the shared one is ignored for " + "this app." + ) + + for name in config.workspace_toolkits: + if name.lower() in PERSONAL_TOOLKITS: + warnings.append( + f'"{name}" is in ARCADE_TOOLKITS (shared). Everyone will act ' + "through ONE account. If you meant each person to use their " + "own, move it to ARCADE_USER_TOOLKITS." + ) + + return tuple(warnings) diff --git a/agent/arcade_tools/effects.py b/agent/arcade_tools/effects.py new file mode 100644 index 00000000..dfec4791 --- /dev/null +++ b/agent/arcade_tools/effects.py @@ -0,0 +1,122 @@ +"""What an Arcade tool does, read from what the tool itself publishes. + +Arcade returns a `metadata.behavior` block carrying `read_only`, `destructive`, +`idempotent` and `open_world`. Two differences from the Composio path matter: + +* **All three bands are reachable.** MCP tags can say "read" or "destructive" + and nothing in between, which is why the Composio approval modes collapsed + into one. Arcade tools can declare `read_only: false, destructive: false` — + "this changes something and will not destroy anything" — so an ordinary write + need not be painted like a deletion. +* **Coverage is partial, and predictably so.** Whole toolkits publish no + behaviour block at all. Those are gated as destructive, which is correct and + also means every read against them asks a person. Configuration warns about + that rather than letting it be discovered one approval card at a time. + +Three things this module refuses to do, each of which reads as safe and is not: + +* Treat "nobody said" as "nothing dangerous". No block, an empty block, or a + block of the wrong shape all mean unclassified, and unclassified gates. +* Read a flag's presence as its value. `read_only: false` is a tool saying it + is **not** read-only. Only `True` — not merely truthy — is a claim. +* Infer from anything that is not a claim: the tool's name, its `operations` + list, or its idempotency. DELETE is idempotent. +""" + +from __future__ import annotations + +import logging +from collections.abc import Mapping +from typing import Any + +from composio_tools.classify import DESTRUCTIVE, READ, WRITE + +logger = logging.getLogger(__name__) + +#: Read straight off the payload rather than through a typed attribute. The +#: generated SDK models declare no metadata field at all; the value arrives +#: because the SDK's base model keeps fields it does not know about. A contract +#: test pins this against a recorded payload, because if a future release +#: tightens that model every tool silently becomes unclassified — which fails +#: safe, but fails safe by asking for approval on every read, and nothing else +#: would go red. +BEHAVIOR_PATH = ("metadata", "behavior") + + +def _behavior(definition: Any) -> Mapping[str, Any] | None: + """The behaviour block, or `None` when there is not one to read.""" + node: Any = definition + for step in BEHAVIOR_PATH: + if isinstance(node, Mapping): + node = node.get(step) + else: + # Also covers an SDK object rather than a dict: the payload is + # reached through the model's own mapping of extra fields. + node = getattr(node, step, None) + if node is None: + return None + return node if isinstance(node, Mapping) else None + + +def _claims(behavior: Mapping[str, Any], name: str) -> bool: + """Whether the block positively asserts `name`. + + `is True` rather than truthiness. These are booleans in every payload + observed, and a string like `"false"` is truthy — a shape nobody meant to + send must not be able to assert anything at all. + """ + return behavior.get(name) is True + + +def effect_of_definition(definition: Any) -> str: + """Classify one tool definition into `read`, `write` or `destructive`. + + Always answers. Unlike the Composio path, which returns `None` for + unclassified and lets its caller decide, the fail-safe choice is made here + because there is exactly one right answer to "nobody said": ask a person. + """ + behavior = _behavior(definition) + if behavior is None: + _warn_unclassified(definition, "publishes no behaviour metadata") + return DESTRUCTIVE + + destructive = _claims(behavior, "destructive") + read_only = _claims(behavior, "read_only") + + # Checked before the read, so a block claiming both lands on the reading + # that asks a person. No tool in the live catalogue claims both; this is + # here for the day one does. + if destructive: + return DESTRUCTIVE + if read_only: + return READ + + # Not a read. The write band needs the second, separate claim that the + # change is survivable — `destructive` explicitly false. A block that simply + # omits it has not made that claim. + if behavior.get("read_only") is False and behavior.get("destructive") is False: + return WRITE + + _warn_unclassified(definition, "claims neither read-only nor non-destructive") + return DESTRUCTIVE + + +def _warn_unclassified(definition: Any, because: str) -> None: + """Say why a tool is being gated, naming it. + + An operator meeting an approval card on an ordinary read needs the reason to + be findable. Without this the symptom gets blamed on the approval mode. + """ + name = None + if isinstance(definition, Mapping): + name = definition.get("qualified_name") or definition.get("name") + else: + name = getattr(definition, "qualified_name", None) or getattr( + definition, "name", None + ) + logger.warning( + "[arcade] %s %s, so it is gated as destructive rather than assumed " + "harmless.", + name or "an unnamed tool", + because, + ) diff --git a/agent/arcade_tools/outcomes.py b/agent/arcade_tools/outcomes.py new file mode 100644 index 00000000..47a15bb5 --- /dev/null +++ b/agent/arcade_tools/outcomes.py @@ -0,0 +1,192 @@ +"""One Arcade execute response, turned into one thing the caller can act on. + +Arcade publishes a typed error `kind` on every failure, so these outcomes are +read rather than pattern-matched out of message text. The kinds are grouped +because the groups lead somewhere different — the model can fix bad arguments +itself, a person must reconnect an expired account, and nobody should be told a +write failed when that is not established. + +The distinction the whole module exists for is **failed** versus **unknown**. +Failed means the call did not happen and saying so is safe. Unknown means the +request left and nothing came back, so whether it landed is exactly what cannot +be established — and replaying it is how one write becomes two. +""" + +from __future__ import annotations + +import enum +from collections.abc import Mapping +from dataclasses import dataclass +from typing import Any + + +class Outcome(enum.Enum): + SUCCEEDED = "succeeded" + NEEDS_AUTHORIZATION = "needs_authorization" + AUTHORIZATION_EXPIRED = "authorization_expired" + INVALID_ARGUMENTS = "invalid_arguments" + RATE_LIMITED = "rate_limited" + PROVIDER_UNAVAILABLE = "provider_unavailable" + FAILED = "failed" + UNKNOWN = "unknown" + + +#: Error kinds whose meaning this build knows. Anything absent is `UNKNOWN` +#: rather than folded into the nearest neighbour: a kind added by a future +#: Arcade release is something nobody here has reasoned about, and guessing at +#: it produces a confident sentence about somebody's data. +_KINDS: dict[str, Outcome] = { + "TOOL_REQUIREMENTS_NOT_MET": Outcome.NEEDS_AUTHORIZATION, + "UPSTREAM_RUNTIME_AUTH_ERROR": Outcome.AUTHORIZATION_EXPIRED, + "TOOL_RUNTIME_BAD_INPUT_VALUE": Outcome.INVALID_ARGUMENTS, + "UPSTREAM_RUNTIME_VALIDATION_ERROR": Outcome.INVALID_ARGUMENTS, + "UPSTREAM_RUNTIME_BAD_REQUEST": Outcome.INVALID_ARGUMENTS, + "UPSTREAM_RUNTIME_RATE_LIMIT": Outcome.RATE_LIMITED, + "UPSTREAM_RUNTIME_SERVER_ERROR": Outcome.PROVIDER_UNAVAILABLE, + "UPSTREAM_RUNTIME_NOT_FOUND": Outcome.FAILED, + "TOOLKIT_LOAD_FAILED": Outcome.PROVIDER_UNAVAILABLE, + "TOOL_DEFINITION_BAD_DEFINITION": Outcome.FAILED, + "TOOL_DEFINITION_BAD_INPUT_SCHEMA": Outcome.FAILED, + "TOOL_DEFINITION_BAD_OUTPUT_SCHEMA": Outcome.FAILED, + "TOOL_RUNTIME_BAD_OUTPUT_VALUE": Outcome.FAILED, + "TOOL_RUNTIME_CONTEXT_REQUIRED": Outcome.FAILED, + "TOOL_RUNTIME_FATAL": Outcome.FAILED, + "TOOL_RUNTIME_RETRY": Outcome.FAILED, + "UNKNOWN": Outcome.UNKNOWN, +} + +#: Only these may ever be retried without asking a person again, and only when +#: Arcade also said so. Everything else — including every `UNKNOWN` — is left +#: for a human to decide. +_RETRYABLE = frozenset({Outcome.FAILED, Outcome.RATE_LIMITED}) + + +@dataclass(frozen=True) +class OutcomeResult: + outcome: Outcome + #: The provider's own words, kept only where they help the reader. Never a + #: URL and never a token; see `_scrub`. + message: str = "" + value: Any = None + retry_after_ms: int | None = None + safe_to_retry: bool = False + #: Always `None`. Present so a caller reading this object cannot reach for + #: an authorization URL that it must not have: the link is a bearer + #: capability and is minted for one clicker, through the connect route. + authorization_url: None = None + + +def _mapping(value: Any) -> Mapping[str, Any] | None: + if isinstance(value, Mapping): + return value + if value is None or isinstance(value, (str, bytes, int, float, list, tuple)): + return None + # An SDK model rather than a dict. + dumped = getattr(value, "model_dump", None) + if callable(dumped): + try: + result = dumped() + except Exception: # noqa: BLE001 - model shapes vary + return None + return result if isinstance(result, Mapping) else None + return None + + +def _scrub(text: Any) -> str: + """Provider text with anything link-shaped removed. + + An authorization URL arriving inside an error message is the same leak as + one arriving in its own field, and the message is the part that gets shown. + """ + if not isinstance(text, str): + return "" + words = [ + "[link removed]" if "://" in word else word for word in text.split() + ] + return " ".join(words) + + +def outcome_of_response(response: Any) -> OutcomeResult: + """Classify one `tools.execute` response.""" + body = _mapping(response) + if body is None: + return OutcomeResult(outcome=Outcome.UNKNOWN) + + output = _mapping(body.get("output")) + if body.get("success") is True: + value = None if output is None else output.get("value") + return OutcomeResult(outcome=Outcome.SUCCEEDED, value=value) + + if output is None: + return OutcomeResult(outcome=Outcome.UNKNOWN) + + error = _mapping(output.get("error")) + if error is None: + # Not successful, and no error saying why. Nothing here establishes + # that the call did not happen. + return OutcomeResult(outcome=Outcome.UNKNOWN) + + kind = error.get("kind") + outcome = _KINDS.get(kind) if isinstance(kind, str) else None + if outcome is None: + outcome = Outcome.UNKNOWN + + retry_after = error.get("retry_after_ms") + if not isinstance(retry_after, int): + retry_after = None + + # Arcade's opinion, and only where this module already agrees the outcome is + # determinate. `can_retry` on an outcome nobody can determine is how a + # single approved write gets sent twice. + safe_to_retry = outcome in _RETRYABLE and error.get("can_retry") is True + + return OutcomeResult( + outcome=outcome, + message=_scrub(error.get("message")), + retry_after_ms=retry_after, + safe_to_retry=safe_to_retry, + ) + + +def outcome_of_transport_error(error: BaseException) -> OutcomeResult: + """Classify a call that raised instead of answering. + + Always `UNKNOWN`. The request may have been received, acted on, and only the + reply lost — so this is the one case where the honest answer is that nobody + knows. + """ + return OutcomeResult(outcome=Outcome.UNKNOWN, message=_scrub(str(error))) + + +#: What the model is told. Written for a reader deciding what to do next rather +#: than describing the provider's internals. +_SENTENCES = { + Outcome.NEEDS_AUTHORIZATION: ( + "{action} needs that account connected first. Ask the person to connect " + "it, then try again." + ), + Outcome.AUTHORIZATION_EXPIRED: ( + "{action} could not run because the connected account is no longer " + "valid — it may have been revoked or expired. It needs connecting again." + ), + Outcome.INVALID_ARGUMENTS: "{action} was rejected as invalid: {message}", + Outcome.RATE_LIMITED: ( + "{action} was rate limited by the provider. Wait before trying again." + ), + Outcome.PROVIDER_UNAVAILABLE: ( + "{action} could not run because the provider is unavailable. Nothing " + "was changed." + ), + Outcome.FAILED: "{action} failed: {message}", + Outcome.UNKNOWN: ( + "{action} returned no usable answer, so the outcome is unknown — it may " + "already have been applied. Check before trying it again." + ), +} + + +def describe_outcome(result: OutcomeResult, *, action: str) -> str: + """One sentence for the model, naming the action the way the card did.""" + template = _SENTENCES.get(result.outcome, _SENTENCES[Outcome.UNKNOWN]) + message = result.message or "no reason was given" + return template.format(action=action, message=message) diff --git a/agent/pyproject.toml b/agent/pyproject.toml index 8b381df4..91e3ed60 100644 --- a/agent/pyproject.toml +++ b/agent/pyproject.toml @@ -5,6 +5,12 @@ description = "OpenTag general-purpose team knowledge-work agent — CopilotKit requires-python = ">=3.12" dependencies = [ "ag-ui-langgraph>=0.0.23", + # Effect classification reads `metadata.behavior`, which no generated type + # declares — it survives because the SDK's base model keeps unknown fields. + # 1.10.0 is the release that was verified to do so against the live API; a + # lower resolution has not been checked and could drop the block silently, + # which would gate every read behind an approval card. + "arcadepy>=1.10.0", # 0.17.0 is the first release whose client exposes `.sessions`, and everything # in `composio_tools/sessions.py` goes through it. Below that the SDK offers # `tool_router` and no alias, so a lower resolution installs, imports, and @@ -32,7 +38,7 @@ dependencies = [ dev = ["pytest>=8.0.0"] [tool.setuptools] -packages = ["prompts", "coding", "composio_tools"] +packages = ["prompts", "coding", "composio_tools", "arcade_tools"] py-modules = [ "agent", "agent_auth", diff --git a/agent/tests/fixtures/arcade_catalogue.json b/agent/tests/fixtures/arcade_catalogue.json new file mode 100644 index 00000000..c62edbcc --- /dev/null +++ b/agent/tests/fixtures/arcade_catalogue.json @@ -0,0 +1,392 @@ +{ + "note": "Captured from the live Arcade catalogue on 2026-09-15, read-only. Shapes are verbatim; identifiers that could be project-specific are redacted. Regenerate rather than hand-edit.", + "tools": { + "read": { + "fully_qualified_name": "Apollo.EnrichOrganization@0.1.0", + "qualified_name": "Apollo.EnrichOrganization", + "name": "EnrichOrganization", + "description": "Turn a company domain into firmographics (industry, size, revenue, funding,\nlocation) so a rep can qualify and size an account. Consumes one enrichment\ncredit on a match; when the plan is out of credits the result reports\nstatus=insufficient_credits rather than failing.", + "toolkit": { + "name": "Apollo", + "description": "Arcade tools designed for LLMs to interact with Apollo.io sales intelligence", + "version": "0.1.0" + }, + "input": { + "parameters": [ + { + "name": "domain", + "required": true, + "description": "Company domain to look up, such as apollo.io. A full URL or www prefix is accepted and normalized.", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + } + ] + }, + "output": { + "available_modes": [ + "value", + "error" + ], + "description": "The matched company, or found=False when no match.", + "value_schema": { + "val_type": "json", + "properties": { + "status": { + "val_type": "string", + "enum": [ + "ok", + "insufficient_credits" + ] + }, + "found": { + "val_type": "boolean" + }, + "id": { + "val_type": "string" + }, + "name": { + "val_type": "string" + }, + "domain": { + "val_type": "string" + }, + "industry": { + "val_type": "string" + }, + "estimated_num_employees": { + "val_type": "integer" + }, + "annual_revenue": { + "val_type": "integer", + "nullable": true + }, + "total_funding": { + "val_type": "integer", + "nullable": true + }, + "founded_year": { + "val_type": "integer", + "nullable": true + }, + "location": { + "val_type": "string" + }, + "phone": { + "val_type": "string", + "nullable": true + } + }, + "required_keys": [ + "annual_revenue", + "domain", + "estimated_num_employees", + "found", + "founded_year", + "id", + "industry", + "location", + "name", + "phone", + "status", + "total_funding" + ] + } + }, + "requirements": { + "met": false, + "secrets": [ + { + "key": "APOLLO_API_KEY", + "met": false, + "status_reason": "Secret APOLLO_API_KEY not found" + } + ] + }, + "metadata": { + "classification": { + "service_domains": [ + "crm" + ] + }, + "behavior": { + "operations": [ + "read" + ], + "read_only": true, + "destructive": false, + "idempotent": true, + "open_world": true + } + } + }, + "write": { + "fully_qualified_name": "Asana.AttachFileToTask@1.2.2", + "qualified_name": "Asana.AttachFileToTask", + "name": "AttachFileToTask", + "description": "Attaches a file to an Asana task\n\nProvide exactly one of file_content_str, file_content_base64, or file_content_url, never more\nthan one.\n\n- Use file_content_str for text files (will be encoded using file_encoding)\n- Use file_content_base64 for binary files like images, PDFs, etc.\n- Use file_content_url if the file is hosted on an external URL", + "toolkit": { + "name": "Asana", + "description": "Arcade tools designed for LLMs to interact with Asana", + "version": "1.2.2" + }, + "input": { + "parameters": [ + { + "name": "task_id", + "required": true, + "description": "The ID of the task to attach the file to.", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + }, + { + "name": "file_name", + "required": true, + "description": "The name of the file to attach with format extension. E.g. 'Image.png' or 'Report.pdf'.", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + }, + { + "name": "file_content_str", + "required": false, + "description": "The string contents of the file to attach. Use this if the file is a text file. Defaults to None.", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + }, + { + "name": "file_content_base64", + "required": false, + "description": "The base64-encoded binary contents of the file. Use this for binary files like images or PDFs. Defaults to None.", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + }, + { + "name": "file_content_url", + "required": false, + "description": "The URL of the file to attach. Use this if the file is hosted on an external URL. Defaults to None.", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + }, + { + "name": "file_encoding", + "required": false, + "description": "The encoding of the file to attach. Only used with file_content_str. Defaults to 'utf-8'.", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + } + ] + }, + "output": { + "available_modes": [ + "value", + "error" + ], + "description": "The task with the file attached.", + "value_schema": { + "val_type": "json" + } + }, + "requirements": { + "met": true, + "authorization": { + "id": "arcade-asana", + "provider_id": "asana", + "provider_type": "oauth2", + "oauth2": { + "scopes": [ + "default" + ] + }, + "status": "active" + } + }, + "metadata": { + "classification": { + "service_domains": [ + "project_management" + ] + }, + "behavior": { + "operations": [ + "create" + ], + "read_only": false, + "destructive": false, + "idempotent": false, + "open_world": true + } + } + }, + "destructive": { + "fully_qualified_name": "Attio.RemoveFromList@1.1.3", + "qualified_name": "Attio.RemoveFromList", + "name": "RemoveFromList", + "description": "Remove a record from an Attio list.\n\nNote: Use the entry_id, not the record_id. Get entry_id from get_list_entries.", + "toolkit": { + "name": "Attio", + "description": "Arcade tools designed for LLMs to interact with Attio CRM", + "version": "1.1.3" + }, + "input": { + "parameters": [ + { + "name": "list_id", + "required": true, + "description": "List UUID", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + }, + { + "name": "entry_id", + "required": true, + "description": "Entry UUID (not record ID)", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + } + ] + }, + "output": { + "available_modes": [ + "value", + "error" + ], + "description": "Removal confirmation", + "value_schema": { + "val_type": "json" + } + }, + "requirements": { + "met": true, + "authorization": { + "id": "arcade-attio", + "provider_id": "attio", + "provider_type": "oauth2", + "oauth2": { + "scopes": [ + "call_recording:read", + "list_configuration:read-write", + "list_entry:read-write", + "meeting:read", + "note:read-write", + "object_configuration:read-write", + "record_permission:read-write", + "task:read-write", + "user_management:read" + ] + }, + "status": "active" + } + }, + "metadata": { + "classification": { + "service_domains": [ + "crm" + ] + }, + "behavior": { + "operations": [ + "delete" + ], + "read_only": false, + "destructive": true, + "idempotent": true, + "open_world": true + } + } + }, + "unclassified": { + "fully_qualified_name": "AirtableApi.AddBaseCollaborator@0.4.1", + "qualified_name": "AirtableApi.AddBaseCollaborator", + "name": "AddBaseCollaborator", + "description": "Add a collaborator to an Airtable base.\n\n Use this tool to add a new collaborator to a specified Airtable base. It facilitates inviting one collaborator at a time.\n\n Note: Understanding the request schema is necessary to properly create\n the stringified JSON input object for execution.\n\nThis operation also requires path parameters.\n\n Modes:\n - GET_REQUEST_SCHEMA: Returns the schema. Only call if you don't\n already have it. Do NOT call repeatedly if you already received\n the schema.\n - EXECUTE: Performs the operation with the provided request body\n JSON.\n Note: You must also provide the required path parameters when executing.\n\n If you need the schema, call with mode='get_request_schema' ONCE, then execute.\n ", + "toolkit": { + "name": "AirtableApi", + "description": "Tools that enable LLMs to interact directly with the airtable API.", + "version": "0.4.1" + }, + "input": { + "parameters": [ + { + "name": "mode", + "required": true, + "description": "Operation mode: 'get_request_schema' returns the OpenAPI spec for the request body, 'execute' performs the actual operation", + "value_schema": { + "val_type": "string", + "enum": [ + "get_request_schema", + "execute" + ] + }, + "inferrable": true + }, + { + "name": "base_id", + "required": false, + "description": "The ID of the Airtable base to which the collaborator will be added. Required when mode is 'execute', ignored when mode is 'get_request_schema'.", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + }, + { + "name": "request_body", + "required": false, + "description": "Stringified JSON representing the request body. Required when mode is 'execute', ignored when mode is 'get_request_schema'", + "value_schema": { + "val_type": "string" + }, + "inferrable": true + } + ] + }, + "output": { + "available_modes": [ + "value", + "error" + ], + "description": "Response from the API endpoint 'add-base-collaborator'.", + "value_schema": { + "val_type": "json" + } + }, + "requirements": { + "met": false, + "authorization": { + "id": "arcade-airtable", + "provider_type": "oauth2", + "oauth2": { + "scopes": [ + "workspacesAndBases:write" + ] + }, + "status": "inactive", + "status_reason": "Provider not found" + } + } + } + }, + "page_envelope": { + "limit": 2, + "offset": 0, + "page_count": 2, + "total_count": 8258, + "items": [] + } +} diff --git a/agent/tests/test_arcade_config.py b/agent/tests/test_arcade_config.py new file mode 100644 index 00000000..31a5e33f --- /dev/null +++ b/agent/tests/test_arcade_config.py @@ -0,0 +1,242 @@ +"""The Arcade environment contract. + +Deliberately not a copy of the Composio contract with the prefix changed. Two +things differ because Arcade differs: a key naming no apps is an error rather +than a shrug, and personal apps need a namespace because Arcade identities are +global to a project rather than scoped to a deployment. +""" + +from __future__ import annotations + +import pytest + +from arcade_tools.config import ( + ArcadeConfigError, + DEFAULT_WORKSPACE_USER_ID, + read_arcade_config, + startup_warnings, +) + + +def test_no_api_key_reports_unconfigured(): + assert read_arcade_config({}, default_user_id="open-tag") is None + + +def test_a_key_with_no_apps_is_an_error_rather_than_a_shrug(): + # Composio treats this as unconfigured for compatibility with deployments + # that already ship it. Arcade is new, so there is nothing to be compatible + # with, and a key naming nothing is a half-finished setup that should be + # said out loud at boot rather than discovered when the agent claims it has + # no apps. + with pytest.raises(ArcadeConfigError) as raised: + read_arcade_config({"ARCADE_API_KEY": "arc_test"}, default_user_id="open-tag") + + assert "ARCADE_TOOLKITS" in str(raised.value) + assert "ARCADE_USER_TOOLKITS" in str(raised.value) + + +def test_toolkit_lists_are_split_and_trimmed(): + config = read_arcade_config( + { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": " Github , Asana ,, ", + "ARCADE_USER_TOOLKITS": "Gmail", + "ARCADE_IDENTITY_NAMESPACE": "acme", + }, + default_user_id="open-tag", + ) + assert config is not None + assert config.workspace_toolkits == ("Github", "Asana") + assert config.user_toolkits == ("Gmail",) + + +def test_toolkit_names_keep_their_case(): + # Composio slugs are lowercase; Arcade's are not. `Github` and `github` are + # not interchangeable in a qualified tool name, so lowercasing the way the + # Composio reader does would produce identifiers that resolve to nothing. + config = read_arcade_config( + { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "GoogleCalendar", + "ARCADE_IDENTITY_NAMESPACE": "acme", + }, + default_user_id="open-tag", + ) + assert config is not None + assert config.workspace_toolkits == ("GoogleCalendar",) + + +def test_an_app_listed_in_both_scopes_is_personal_only(): + # Otherwise one name resolves to two identities and which one a call runs + # under depends on iteration order. + config = read_arcade_config( + { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github,Asana", + "ARCADE_USER_TOOLKITS": "Asana", + "ARCADE_IDENTITY_NAMESPACE": "acme", + }, + default_user_id="open-tag", + ) + assert config is not None + assert config.workspace_toolkits == ("Github",) + assert config.user_toolkits == ("Asana",) + + +def test_personal_apps_require_an_identity_namespace(): + # Arcade user ids are global within a project. Two deployments sharing a + # project and both calling somebody `slack:U1` would share that person's + # connected accounts across deployments without either one asking. + with pytest.raises(ArcadeConfigError) as raised: + read_arcade_config( + {"ARCADE_API_KEY": "arc_test", "ARCADE_USER_TOOLKITS": "Gmail"}, + default_user_id="open-tag", + ) + + assert "ARCADE_IDENTITY_NAMESPACE" in str(raised.value) + + +def test_shared_only_deployments_need_no_namespace(): + config = read_arcade_config( + {"ARCADE_API_KEY": "arc_test", "ARCADE_TOOLKITS": "Github"}, + default_user_id="open-tag", + ) + assert config is not None + assert config.identity_namespace == "" + + +def test_the_workspace_identity_falls_back_through_the_channel_name(): + config = read_arcade_config( + {"ARCADE_API_KEY": "arc_test", "ARCADE_TOOLKITS": "Github"}, + default_user_id="my-channel", + ) + assert config is not None + assert config.workspace_user_id == "my-channel" + + +def test_a_blank_channel_name_does_not_become_the_workspace_identity(): + # `INTELLIGENCE_CHANNEL_NAME=` is routine. An empty user id is a real + # Arcade identity that nothing else resolves to, so the shared connection + # would land where no turn looks. + config = read_arcade_config( + {"ARCADE_API_KEY": "arc_test", "ARCADE_TOOLKITS": "Github"}, + default_user_id=" ", + ) + assert config is not None + assert config.workspace_user_id == DEFAULT_WORKSPACE_USER_ID + + +@pytest.mark.parametrize("mode", ["on", "off", "ON", " off "]) +def test_approval_modes_are_read_case_and_space_insensitively(mode): + config = read_arcade_config( + { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + "ARCADE_APPROVALS": mode, + }, + default_user_id="open-tag", + ) + assert config is not None + assert config.approvals == mode.strip().lower() + + +def test_an_unset_approval_mode_defaults_to_on(): + config = read_arcade_config( + { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + "ARCADE_APPROVALS": "", + }, + default_user_id="open-tag", + ) + assert config is not None + assert config.approvals == "on" + + +def test_the_composio_approval_aliases_are_not_accepted_here(): + # `destructive` and `writes` exist on the Composio side only because + # refusing them would fail deployments that already ship them. Arcade has no + # such history, and accepting a word that means something specific about + # MCP tags would be a lie about what the gate does. + with pytest.raises(ArcadeConfigError): + read_arcade_config( + { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + "ARCADE_APPROVALS": "destructive", + }, + default_user_id="open-tag", + ) + + +def test_an_invalid_approval_mode_names_what_is_accepted(): + with pytest.raises(ArcadeConfigError) as raised: + read_arcade_config( + { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + "ARCADE_APPROVALS": "nonsense", + }, + default_user_id="open-tag", + ) + + assert "on" in str(raised.value) + assert "off" in str(raised.value) + + +def test_a_config_error_never_quotes_the_api_key(): + secret = "arc_live_secret_value" + with pytest.raises(ArcadeConfigError) as raised: + read_arcade_config( + {"ARCADE_API_KEY": secret, "ARCADE_APPROVALS": "nonsense"}, + default_user_id="open-tag", + ) + + assert secret not in str(raised.value) + + +def test_a_personal_data_app_configured_as_shared_is_warned_about(): + # Following the Composio convention: warnings are collected rather than + # logged during parsing, so the reader stays pure and the boot sequence + # decides when to say them. + # + # The case itself: a shared identity means one account everybody's calls run + # through, which for a mailbox is somebody's mail being read by the whole + # workspace. + config = read_arcade_config( + {"ARCADE_API_KEY": "arc_test", "ARCADE_TOOLKITS": "Gmail"}, + default_user_id="open-tag", + ) + assert config is not None + + warnings = startup_warnings(config) + + assert any("Gmail" in warning for warning in warnings) + assert any("ARCADE_USER_TOOLKITS" in warning for warning in warnings) + + +def test_an_app_in_both_scopes_is_warned_about(): + config = read_arcade_config( + { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Asana", + "ARCADE_USER_TOOLKITS": "Asana", + "ARCADE_IDENTITY_NAMESPACE": "acme", + }, + default_user_id="open-tag", + ) + assert config is not None + + warnings = startup_warnings(config) + + assert any("Asana" in warning for warning in warnings) + + +def test_a_clean_configuration_warns_about_nothing(): + config = read_arcade_config( + {"ARCADE_API_KEY": "arc_test", "ARCADE_TOOLKITS": "Github"}, + default_user_id="open-tag", + ) + assert config is not None + + assert startup_warnings(config) == () diff --git a/agent/tests/test_arcade_effects.py b/agent/tests/test_arcade_effects.py new file mode 100644 index 00000000..16c19a4d --- /dev/null +++ b/agent/tests/test_arcade_effects.py @@ -0,0 +1,146 @@ +"""Effect classification from Arcade's published behaviour metadata. + +The vocabulary is Arcade's own: `metadata.behavior` carries `read_only`, +`destructive`, `idempotent` and `open_world`. Unlike Composio's MCP tags, it can +express a write that is not destructive, so all three bands are reachable here. + +Every shape below was taken from the live catalogue on 2026-09-15 or is a +deliberate corruption of one. +""" + +from __future__ import annotations + +import json +import logging +from pathlib import Path + +import pytest + +from arcade_tools.effects import effect_of_definition +from composio_tools.classify import DESTRUCTIVE, READ, WRITE + +FIXTURE = Path(__file__).parent / "fixtures" / "arcade_catalogue.json" + + +def catalogue() -> dict: + return json.loads(FIXTURE.read_text())["tools"] + + +def test_a_real_read_only_tool_classifies_as_read(): + assert effect_of_definition(catalogue()["read"]) == READ + + +def test_a_real_non_destructive_write_classifies_as_write(): + # The band Composio could not express. `Asana.AttachFileToTask` declares + # read_only false and destructive false, which is a tool saying it changes + # something and will not destroy anything. + assert effect_of_definition(catalogue()["write"]) == WRITE + + +def test_a_real_destructive_tool_classifies_as_destructive(): + assert effect_of_definition(catalogue()["destructive"]) == DESTRUCTIVE + + +def test_a_real_tool_publishing_no_metadata_is_gated_as_destructive(): + # Whole families of the catalogue publish nothing. Unclassified is not + # "harmless" — it is "nobody said", and the only safe reading of that is the + # one that asks a person. + assert effect_of_definition(catalogue()["unclassified"]) == DESTRUCTIVE + + +def _definition(behavior) -> dict: + return {"qualified_name": "Test.Thing", "metadata": {"behavior": behavior}} + + +def test_destructive_wins_over_a_contradictory_read_only_claim(): + # No tool in the live catalogue claims both. If one ever does, the claim + # that leads to asking a person is the one to believe. + assert effect_of_definition( + _definition({"read_only": True, "destructive": True}) + ) == DESTRUCTIVE + + +def test_read_only_without_an_explicit_destructive_flag_is_still_a_read(): + assert effect_of_definition(_definition({"read_only": True})) == READ + + +def test_a_non_read_that_does_not_deny_destructiveness_is_destructive(): + # `read_only: False` alone says "this changes something". It does not say + # the change is survivable, and the write band requires that second claim. + assert effect_of_definition(_definition({"read_only": False})) == DESTRUCTIVE + + +def test_an_empty_behavior_block_is_destructive(): + assert effect_of_definition(_definition({})) == DESTRUCTIVE + + +def test_absent_metadata_is_destructive(): + assert effect_of_definition({"qualified_name": "Test.Thing"}) == DESTRUCTIVE + + +@pytest.mark.parametrize("junk", [None, "read_only", 42, [], ["read_only"]]) +def test_a_behavior_block_of_the_wrong_shape_is_destructive(junk): + assert effect_of_definition(_definition(junk)) == DESTRUCTIVE + + +@pytest.mark.parametrize("junk", [None, "behavior", 42, []]) +def test_metadata_of_the_wrong_shape_is_destructive(junk): + assert effect_of_definition({"metadata": junk}) == DESTRUCTIVE + + +@pytest.mark.parametrize("truthy", ["true", "True", 1, "yes"]) +def test_a_flag_that_is_not_a_boolean_asserts_nothing(truthy): + # Only `True` is a claim. A string is a shape nobody meant to send, and it + # must not be able to talk this module down to `read`. + assert effect_of_definition(_definition({"read_only": truthy})) == DESTRUCTIVE + + +@pytest.mark.parametrize("falsey", ["false", "False", 0, ""]) +def test_a_non_boolean_destructive_flag_does_not_earn_the_write_band(falsey): + assert ( + effect_of_definition(_definition({"read_only": False, "destructive": falsey})) + == DESTRUCTIVE + ) + + +def test_idempotency_never_stands_in_for_safety(): + # DELETE is idempotent. The plan forbids this inference and so does this. + assert ( + effect_of_definition( + _definition({"idempotent": True, "open_world": False}) + ) + == DESTRUCTIVE + ) + + +def test_the_operations_list_is_never_read_as_a_classification(): + # `operations: ["read"]` sits right next to the flags and looks like an + # answer. It is a description, not a claim, and believing it would let a + # tool be classified as a read while its own flags say otherwise. + assert ( + effect_of_definition( + _definition({"operations": ["read"], "read_only": False, "destructive": True}) + ) + == DESTRUCTIVE + ) + assert ( + effect_of_definition(_definition({"operations": ["read"]})) == DESTRUCTIVE + ) + + +def test_the_tool_name_is_never_read_as_a_classification(): + # A name is not a contract. Two tools whose names say "get" and "delete", + # both publishing nothing, get the same answer. + getter = {"qualified_name": "Test.GetThing"} + deleter = {"qualified_name": "Test.DeleteEverything"} + assert effect_of_definition(getter) == effect_of_definition(deleter) == DESTRUCTIVE + + +def test_an_unclassified_tool_says_so_once_in_the_log(caplog): + # An operator seeing an approval card on every read needs the reason to be + # findable. The warning names the tool and what was done about it. + with caplog.at_level(logging.WARNING): + effect_of_definition({"qualified_name": "AirtableApi.AddBaseCollaborator"}) + + assert "AirtableApi.AddBaseCollaborator" in caplog.text + assert "destructive" in caplog.text.lower() diff --git a/agent/tests/test_arcade_outcomes.py b/agent/tests/test_arcade_outcomes.py new file mode 100644 index 00000000..cd95bce8 --- /dev/null +++ b/agent/tests/test_arcade_outcomes.py @@ -0,0 +1,183 @@ +"""Turning one Arcade execute response into one thing we can act on. + +The plan requires six outcomes to stay distinct: needs authorization, revoked or +expired authorization, provider unavailable, invalid arguments, a definite +failure, and an outcome nobody can determine. Arcade publishes a typed error +kind, so these are read rather than guessed from message text. + +The outcome that matters most is the last one. A write whose result is unknown +must never be replayed automatically, however cheerfully the provider says it +may be retried. +""" + +from __future__ import annotations + +import pytest + +from arcade_tools.outcomes import ( + Outcome, + describe_outcome, + outcome_of_response, + outcome_of_transport_error, +) + + +def response(**output): + return {"success": False, "output": output} + + +def error(kind, **fields): + return response(error={"kind": kind, "message": "provider text", **fields}) + + +def test_a_successful_call_is_a_success(): + result = outcome_of_response({"success": True, "output": {"value": {"ok": 1}}}) + + assert result.outcome is Outcome.SUCCEEDED + assert result.value == {"ok": 1} + + +def test_unmet_requirements_read_as_needing_authorization(): + result = outcome_of_response(error("TOOL_REQUIREMENTS_NOT_MET")) + + assert result.outcome is Outcome.NEEDS_AUTHORIZATION + + +def test_an_upstream_auth_error_reads_as_revoked_rather_than_never_connected(): + # Different sentence to the person: "connect your account" is wrong advice + # for somebody who connected it last month and had the token revoked. + result = outcome_of_response(error("UPSTREAM_RUNTIME_AUTH_ERROR")) + + assert result.outcome is Outcome.AUTHORIZATION_EXPIRED + + +@pytest.mark.parametrize( + "kind", + ["TOOL_RUNTIME_BAD_INPUT_VALUE", "UPSTREAM_RUNTIME_VALIDATION_ERROR", + "UPSTREAM_RUNTIME_BAD_REQUEST"], +) +def test_bad_arguments_are_their_own_outcome(kind): + # The model can fix this one itself, and telling it "the provider is + # unavailable" would make it wait instead. + assert outcome_of_response(error(kind)).outcome is Outcome.INVALID_ARGUMENTS + + +def test_rate_limiting_carries_how_long_to_wait(): + result = outcome_of_response( + error("UPSTREAM_RUNTIME_RATE_LIMIT", retry_after_ms=4500) + ) + + assert result.outcome is Outcome.RATE_LIMITED + assert result.retry_after_ms == 4500 + + +@pytest.mark.parametrize( + "kind", ["UPSTREAM_RUNTIME_SERVER_ERROR", "TOOLKIT_LOAD_FAILED"] +) +def test_provider_trouble_is_unavailable_rather_than_a_tool_failure(kind): + assert outcome_of_response(error(kind)).outcome is Outcome.PROVIDER_UNAVAILABLE + + +def test_a_fatal_runtime_error_is_a_definite_failure(): + # Definite matters: the call did not happen, so saying so is safe. + assert outcome_of_response(error("TOOL_RUNTIME_FATAL")).outcome is Outcome.FAILED + + +def test_an_unknown_error_kind_is_not_guessed_at(): + # A kind this build has never heard of is an outcome nobody can determine, + # not a failure. Calling it a failure would tell an approver their write did + # not happen, which is a claim nothing here can support. + assert outcome_of_response(error("SOMETHING_NEW")).outcome is Outcome.UNKNOWN + + +def test_a_response_with_no_output_at_all_is_unknown(): + assert outcome_of_response({"success": False}).outcome is Outcome.UNKNOWN + + +def test_a_response_that_is_not_a_response_is_unknown(): + for junk in (None, "ok", 42, []): + assert outcome_of_response(junk).outcome is Outcome.UNKNOWN + + +def test_success_false_with_no_error_is_unknown_rather_than_failed(): + assert outcome_of_response(response()).outcome is Outcome.UNKNOWN + + +def test_a_transport_failure_is_unknown_not_failed(): + # The request left; nothing came back. Whether the write landed is exactly + # what cannot be established. + result = outcome_of_transport_error(TimeoutError("read timed out")) + + assert result.outcome is Outcome.UNKNOWN + + +def test_the_providers_retry_opinion_never_reaches_an_unknown_outcome(): + # `can_retry` is Arcade's opinion about its own error. For an outcome nobody + # can determine, replaying is how one write becomes two. + result = outcome_of_response(error("SOMETHING_NEW", can_retry=True)) + + assert result.outcome is Outcome.UNKNOWN + assert result.safe_to_retry is False + + +def test_only_a_definite_non_write_failure_is_ever_retryable(): + retryable = outcome_of_response(error("TOOL_RUNTIME_RETRY", can_retry=True)) + + assert retryable.outcome is Outcome.FAILED + assert retryable.safe_to_retry is True + + +def test_an_authorization_url_never_survives_into_the_outcome(): + # `output.authorization` carries a bearer capability. It reaches the model + # as a tool result unless something removes it here. + raw = response( + error={"kind": "TOOL_REQUIREMENTS_NOT_MET", "message": "connect first"}, + authorization={ + "id": "auth_1", + "url": "https://arcade.example/authorize?secret=abcd", + "status": "pending", + }, + ) + + result = outcome_of_response(raw) + rendered = describe_outcome(result, action="Send an email") + + assert result.outcome is Outcome.NEEDS_AUTHORIZATION + assert "https://" not in rendered + assert "abcd" not in rendered + assert result.authorization_url is None + + +def test_the_provider_message_reaches_the_model_for_a_fixable_error(): + # A validation message is the one case where the provider's own words help: + # the model needs to know which argument it got wrong. + result = outcome_of_response( + { + "success": False, + "output": { + "error": { + "kind": "TOOL_RUNTIME_BAD_INPUT_VALUE", + "message": "field 'to' must be an email address", + } + }, + } + ) + rendered = describe_outcome(result, action="Send an email") + + assert "must be an email address" in rendered + + +def test_an_unknown_outcome_says_the_write_may_already_have_happened(): + rendered = describe_outcome( + outcome_of_transport_error(TimeoutError()), action="Send an email" + ) + + assert "may already" in rendered.lower() + + +def test_a_definite_failure_does_not_cast_doubt_on_a_write_that_never_ran(): + rendered = describe_outcome( + outcome_of_response(error("TOOL_RUNTIME_FATAL")), action="Send an email" + ) + + assert "may already" not in rendered.lower() diff --git a/agent/tests/test_arcade_sdk_contract.py b/agent/tests/test_arcade_sdk_contract.py new file mode 100644 index 00000000..741d3fe2 --- /dev/null +++ b/agent/tests/test_arcade_sdk_contract.py @@ -0,0 +1,92 @@ +"""Do we read the installed Arcade SDK the way it is actually shaped? + +One reading in this provider is load-bearing and undocumented: effect +classification comes from `metadata.behavior`, and no generated type declares a +`metadata` field. It survives because the SDK's base model keeps fields it does +not know about. + +That is a property of the SDK's configuration rather than a promise. If a +release tightens it, every tool silently becomes unclassified — which fails +safe, but fails safe by putting an approval card in front of every read, and +nothing else in the suite would notice. These tests read the real installed +classes so that day fails here instead. +""" + +from __future__ import annotations + +import inspect +import json +from pathlib import Path + +from arcadepy.types import ToolDefinition +from arcadepy.resources.tools.tools import ToolsResource + +from arcade_tools.effects import effect_of_definition +from composio_tools.classify import DESTRUCTIVE, READ + +FIXTURE = Path(__file__).parent / "fixtures" / "arcade_catalogue.json" + + +def catalogue() -> dict: + return json.loads(FIXTURE.read_text())["tools"] + + +def parameters(method) -> dict[str, inspect.Parameter]: + return dict(inspect.signature(method).parameters) + + +def test_the_tool_model_still_keeps_fields_it_does_not_declare(): + # The whole classification path depends on this. `ToolDefinition` declares + # no `metadata`, so a model that dropped unknown fields would parse the + # payload into an object with the behaviour block missing. + assert ToolDefinition.model_config.get("extra") == "allow" + + +def test_behaviour_metadata_survives_parsing_by_the_real_model(): + # Asserted through the SDK's own constructor rather than on the raw dict, + # because the raw dict cannot tell us what the model does with it. + parsed = ToolDefinition.model_construct(**catalogue()["read"]) + + assert effect_of_definition(parsed) == READ + + +def test_a_parsed_tool_without_metadata_still_gates(): + parsed = ToolDefinition.model_construct(**catalogue()["unclassified"]) + + assert effect_of_definition(parsed) == DESTRUCTIVE + + +def test_the_tool_model_declares_no_behaviour_field_of_its_own(): + # If a future release starts declaring one, the extra-field read should be + # replaced by the typed attribute rather than left to shadow it. This test + # going red is that prompt, and is not a failure of the deployment. + declared = set(ToolDefinition.model_fields) + + assert "metadata" not in declared + assert "behavior" not in declared + + +def test_listing_accepts_the_paging_and_toolkit_arguments_we_pass(): + names = parameters(ToolsResource.list) + + assert "limit" in names + assert "offset" in names + assert "toolkit" in names + assert "user_id" in names + + +def test_authorization_is_requested_per_tool_and_per_person(): + # The plan's per-action scope rule depends on `tool_name` being the unit of + # authorization rather than a toolkit. + names = parameters(ToolsResource.authorize) + + assert "tool_name" in names + assert "user_id" in names + + +def test_execution_names_the_tool_the_person_and_the_arguments(): + names = parameters(ToolsResource.execute) + + assert "tool_name" in names + assert "user_id" in names + assert "input" in names diff --git a/agent/uv.lock b/agent/uv.lock index 57faa3f6..3671175e 100644 --- a/agent/uv.lock +++ b/agent/uv.lock @@ -242,6 +242,23 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/da/35/f2287558c17e29fafc8ef3daf819bb9834061cfa43bff8014f7df7f63bdc/anyio-4.14.2-py3-none-any.whl", hash = "sha256:9f505dda5ac9f0c8309b5e8bd445a8c2bf7246f3ce950121e45ea15bc41d1494", size = 125813, upload-time = "2026-07-12T20:29:05.763Z" }, ] +[[package]] +name = "arcadepy" +version = "1.10.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "anyio" }, + { name = "distro" }, + { name = "httpx" }, + { name = "pydantic" }, + { name = "sniffio" }, + { name = "typing-extensions" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/73/e0/f78e6e41f69a232ef71cc14c4ef177d9a1ac575bac834ee1ce4b9494e252/arcadepy-1.10.0.tar.gz", hash = "sha256:8dc3e44f19bc4d1cca43ae977b9a4decfac4ad6639af84be489b311d0efc8381", size = 127394, upload-time = "2025-11-06T23:59:07.955Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/fd/6a/9fb347181090878484d8889b8271f7c9b7cb57864548704d85c077c2b270/arcadepy-1.10.0-py3-none-any.whl", hash = "sha256:8e596edb27c84a4c58200c58a2e89446969916e7653c7ac6789ab4da1e152924", size = 118651, upload-time = "2025-11-06T23:59:06.499Z" }, +] + [[package]] name = "attrs" version = "26.1.0" @@ -1520,6 +1537,7 @@ version = "0.1.0" source = { editable = "." } dependencies = [ { name = "ag-ui-langgraph" }, + { name = "arcadepy" }, { name = "composio" }, { name = "copilotkit" }, { name = "daytona" }, @@ -1544,6 +1562,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "ag-ui-langgraph", specifier = ">=0.0.23" }, + { name = "arcadepy", specifier = ">=1.10.0" }, { name = "composio", specifier = ">=0.17.0" }, { name = "copilotkit", specifier = ">=0.1.76" }, { name = "daytona", specifier = ">=0.204.0" }, diff --git a/deployment/docker/agent.Dockerfile b/deployment/docker/agent.Dockerfile index 280866b9..8d6e4876 100644 --- a/deployment/docker/agent.Dockerfile +++ b/deployment/docker/agent.Dockerfile @@ -18,6 +18,7 @@ COPY agent/*.py ./ COPY agent/prompts ./prompts COPY agent/coding ./coding COPY agent/composio_tools ./composio_tools +COPY agent/arcade_tools ./arcade_tools RUN --mount=type=cache,target=/root/.cache/uv \ uv sync --frozen --no-dev \ && useradd --uid 10001 --create-home --home-dir /home/opentag opentag diff --git a/docs/superpowers/plans/2026-09-15-arcade-stage-1-findings.md b/docs/superpowers/plans/2026-09-15-arcade-stage-1-findings.md index 878e6601..a94cbcce 100644 --- a/docs/superpowers/plans/2026-09-15-arcade-stage-1-findings.md +++ b/docs/superpowers/plans/2026-09-15-arcade-stage-1-findings.md @@ -98,10 +98,19 @@ including reads. So this is a configuration concern, not a classification one. **Arcade config validation should warn at startup when a configured toolkit publishes no -behaviour metadata**, naming the curated toolkit to use instead where one exists -under the same name minus the `Api` suffix. That warning belongs with the other -startup warnings in Stage 3, and it is cheap: the answer is already in the -listing the adapter has to fetch anyway. +behaviour metadata**, and say what follows from that: every call to it will ask +for approval, including reads. + +The condition is measured, never guessed. It is "this toolkit's listing came +back carrying no behaviour metadata" — the same listing the adapter already +fetches — so a toolkit that starts publishing metadata stops warning without a +code change. + +Deliberately **not** keyed on the toolkit's name. The `*Api` suffix above +describes how today's catalogue happens to be split; it is Arcade's naming +convention, not a contract, and a rule reading it would be a guess wearing a +measurement's clothes. The warning does not suggest a replacement toolkit for +the same reason. ## Production browser verification @@ -120,9 +129,9 @@ expressible here. Two additions for Stage 3, both small: 1. **Warn on a toolkit that publishes no behaviour metadata**, at configuration - time, naming the curated equivalent where one exists. Without it, a deployer - who names a `*Api` toolkit gets an approval card on every read and no - explanation. + time, saying that every call to it will ask for approval. Measured from the + listing, never inferred from the toolkit's name. Without it, a deployer gets + an approval card on every read and no explanation. 2. **Read `metadata.behavior` off the undeclared field rather than a typed attribute**, and pin that with a contract test against a recorded payload. It is reachable because the SDK's base model allows extra fields, which is a From 1024da373955148775473cf6b66eb6ba3b517788 Mon Sep 17 00:00:00 2001 From: Maxim Date: Tue, 15 Sep 2026 22:57:31 +0200 Subject: [PATCH 04/15] feat(arcade): register the connected-app tools for an Arcade deployment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discovery, execution, identity and the prompt. An Arcade deployment now registers the same two tool names a Composio one does, which is safe precisely because only one provider is ever registered. The catalogue keeps two kinds of fact apart. A tool's schema and behaviour do not change between calls, so the listing is fetched once per toolkit and every page of it is followed — the first page of a toolkit is not the toolkit. Whether one person is connected changes the moment they click a link, so it is never cached and every check names the person. That separation is not tidiness. Listing without a user id reports requirements as met, which means "satisfiable in principle" rather than "this person is connected" — proved against the live API, where an identity nobody has ever used comes back met=false only when it is sent. A cache holding a listing fetched for one person would report everybody as connected to everything, so the listing never sends an identity and the authorization check refuses to run without one. Identity reuses the existing actor reader rather than growing a second one. Arcade adds a namespace on top, because its user ids are global within a project: without it, two deployments sharing one project and both spelling somebody slack:U1 would hand each other that person's connected accounts. A namespace containing the separator is refused, or the collision returns through the encoding. Ownership is checked again at execution rather than trusted from the search that produced the name. A model naming an action is not authorization to run it, and the two are separate calls of which only one changes anything. Mutation-checked, six ways: removing the execution-time allowlist, letting an anonymous turn reach personal apps through the shared account, ignoring the authorization check, ignoring a declined card, naming the speaker as approver on a shared call, and treating every action as a read. Each turns the suite red. A cross-provider contract suite pins what the two implementations must agree on — tool names, that neither takes an identity or an approver from the model, that an unclassified action is destructive on both sides, and that both reach the one approval card and the one actor reader by identity rather than by copy. Drift in either provider now fails there. The two interim tests from the selection stage are replaced: Arcade selection no longer warns that it is unimplemented, it registers tools and is never handed the Composio absence statement. A tripwire pins the in-memory checkpointer. An approval cannot outlive a provider switch today, because a switch means a restart and a restart discards every pending approval. A durable checkpointer would silently open that gap, so the failure message says what to build first: stamp the provider into each approval and refuse a resume under a different one. Co-Authored-By: Claude Opus 5.5 --- agent/agent.py | 43 +- agent/arcade_tools/catalog.py | 232 +++++++++ agent/arcade_tools/identity.py | 82 ++++ agent/arcade_tools/runtime.py | 107 +++++ agent/arcade_tools/tools.py | 283 +++++++++++ agent/prompts/__init__.py | 2 + agent/prompts/tools.py | 51 ++ agent/tests/test_arcade_catalog.py | 298 ++++++++++++ agent/tests/test_arcade_identity.py | 119 +++++ agent/tests/test_arcade_prompt.py | 93 ++++ agent/tests/test_arcade_tools.py | 504 ++++++++++++++++++++ agent/tests/test_connected_app_contract.py | 144 ++++++ agent/tests/test_connected_app_selection.py | 79 ++- 13 files changed, 2009 insertions(+), 28 deletions(-) create mode 100644 agent/arcade_tools/catalog.py create mode 100644 agent/arcade_tools/identity.py create mode 100644 agent/arcade_tools/runtime.py create mode 100644 agent/arcade_tools/tools.py create mode 100644 agent/tests/test_arcade_catalog.py create mode 100644 agent/tests/test_arcade_identity.py create mode 100644 agent/tests/test_arcade_prompt.py create mode 100644 agent/tests/test_arcade_tools.py create mode 100644 agent/tests/test_connected_app_contract.py diff --git a/agent/agent.py b/agent/agent.py index 44772516..ff74d902 100644 --- a/agent/agent.py +++ b/agent/agent.py @@ -29,6 +29,8 @@ from ag_ui_langgraph import CustomEventNames from langchain_core.callbacks.manager import adispatch_custom_event from langchain_core.runnables.config import ensure_config +from arcade_tools.runtime import arcade_runtime +from arcade_tools.tools import build_arcade_tools from connected_app_provider import ( PROVIDER_ARCADE, PROVIDER_COMPOSIO, @@ -48,6 +50,7 @@ CODING_ON_ADDENDUM, current_date_prompt, build_base_system_prompt, + arcade_addendum, composio_addendum, ) from tools import web_search @@ -225,25 +228,32 @@ def build_agent(): if provider == PROVIDER_COMPOSIO else None ) - if provider == PROVIDER_ARCADE: - # Said out loud rather than registering nothing in silence. An Arcade - # key is a deployer asking for connected apps, and answering that with - # the same behaviour as an unconfigured agent would read as "Arcade is - # set up and has nothing", which is a different and wrong statement. - logger.warning( - "[arcade] ARCADE_API_KEY selects Arcade, but the Arcade provider is " - "not implemented yet, so no connected-app tools are registered." + arcade = ( + arcade_runtime( + default_user_id=os.environ.get( + "INTELLIGENCE_CHANNEL_NAME", DEFAULT_WORKSPACE_USER_ID + ), ) - composio_tools: list = ( - [] - if composio is None - else build_composio_tools(composio.config, composio.cache, composio.effects) + if provider == PROVIDER_ARCADE + else None ) + # One pair of connected-app tools, from whichever provider is selected. The + # two builders produce the same two names, which is safe precisely because + # only one of them is ever called. + connected_app_tools: list = [] + if composio is not None: + connected_app_tools = build_composio_tools( + composio.config, composio.cache, composio.effects + ) + elif arcade is not None: + connected_app_tools = build_arcade_tools( + arcade.config, arcade.catalog, arcade.client_factory + ) main_tools = ( - [web_search, *internal_tools, *composio_tools] + [web_search, *internal_tools, *connected_app_tools] if has_web_search - else [*internal_tools, *composio_tools] + else [*internal_tools, *connected_app_tools] ) agent_display_name = ( @@ -268,9 +278,12 @@ def build_agent(): # Which apps exist is known here and was never passed on, so the model # answered questions about its own reach by guessing. It names apps only; # `search_my_tools` still owns which actions each one has. + # Only the selected provider's statement. An Arcade deployment handed the + # Composio one would be told it has no connected apps, which is false and + # is how a configured agent stops looking. system_prompt = system_prompt + composio_addendum( composio.config if composio is not None else None - ) + ) + arcade_addendum(arcade.config if arcade is not None else None) checkpointer = MemorySaver() create_kwargs = { diff --git a/agent/arcade_tools/catalog.py b/agent/arcade_tools/catalog.py new file mode 100644 index 00000000..688c1dc4 --- /dev/null +++ b/agent/arcade_tools/catalog.py @@ -0,0 +1,232 @@ +"""Which Arcade actions this deployment can reach, and who is connected to them. + +Two kinds of fact live here and they are kept apart on purpose. + +**Static facts cache.** A tool's schema, description and behaviour flags do not +change between calls, so the catalogue for a toolkit is fetched once per process +and every page of it is followed — the first page of a toolkit is not the +toolkit. + +**Authorization never caches.** Whether one person is connected changes the +moment they click a link, and a remembered "not connected" would outlive the +click that fixed it and keep telling them to connect an account they just +connected. Every check is a fresh call, and it names the person. + +The trap that shaped this module: listing without a `user_id` reports +`requirements.met` as true. That does not mean anybody is connected — it means +the tool's requirements are satisfiable in principle. Believing it would report +everybody as connected to everything, so the catalogue listing never sends an +identity, and the authorization check refuses to run without one. +""" + +from __future__ import annotations + +import logging +from collections.abc import Mapping, Sequence +from dataclasses import dataclass +from typing import Any + +from arcade_tools.config import ArcadeConfig + +logger = logging.getLogger(__name__) + +#: Arcade's maximum page size. Fewer, larger pages for the same tools. +PAGE_SIZE = 100 + +#: A guard against a paging bug turning into an unbounded loop against a +#: provider. Far above any real toolkit — the largest observed is under 800. +MAX_PAGES = 100 + +#: How many matches one search returns. The model reads these; a hundred rows +#: is not a better answer than ten, it is the same answer plus noise. +DEFAULT_SEARCH_LIMIT = 10 + + +def _toolkit_of(qualified_name: Any) -> str: + """The toolkit half of `Toolkit.Action`, or empty when there is not one. + + A bare `Action` owns nothing. Attributing it to the first configured toolkit + would let a caller reach a tool by leaving the prefix off. + """ + if not isinstance(qualified_name, str): + return "" + head, separator, _tail = qualified_name.partition(".") + return head if separator and head else "" + + +def owns(toolkits: Sequence[str], qualified_name: Any) -> bool: + """Whether one of these toolkits provides this action. + + Compared whole and case-insensitively. Whole, because `GithubEnterprise.X` + must not pass an allowlist naming `Github`; case-insensitively, because an + operator typing `github` means the same toolkit Arcade calls `Github`. + """ + toolkit = _toolkit_of(qualified_name).lower() + if not toolkit: + return False + return any(name.lower() == toolkit for name in toolkits) + + +@dataclass(frozen=True) +class AuthorizationState: + """What one person may do with one action, right now.""" + + connected: bool + #: True when this person has never begun connecting, as opposed to having a + #: connection that stopped working. They get different sentences. + never_started: bool = False + + +class Catalog: + """The configured toolkits' actions, fetched once, plus live auth checks.""" + + def __init__(self, client_factory, config: ArcadeConfig) -> None: + self._client_factory = client_factory + self._config = config + self._definitions: dict[str, list[dict[str, Any]]] = {} + + @property + def _allowed(self) -> tuple[str, ...]: + return (*self._config.workspace_toolkits, *self._config.user_toolkits) + + def _require_allowed(self, toolkit: str) -> None: + if not any(name.lower() == toolkit.lower() for name in self._allowed): + raise LookupError( + f'"{toolkit}" is not one of this deployment\'s configured apps.' + ) + + def definitions(self, toolkit: str) -> list[dict[str, Any]]: + """Every action in one configured toolkit, following every page.""" + self._require_allowed(toolkit) + cached = self._definitions.get(toolkit.lower()) + if cached is not None: + return cached + + collected: list[dict[str, Any]] = [] + offset = 0 + for _page in range(MAX_PAGES): + # No `user_id`. The cache is shared by everybody, so a listing must + # not be able to carry one person's authorization state into it. + response = self._client_factory().tools.list( + toolkit=toolkit, limit=PAGE_SIZE, offset=offset + ) + body = _as_mapping(response) or {} + items = body.get("items") + if not isinstance(items, list) or not items: + break + collected.extend( + dict(item) + for item in (_as_mapping(entry) for entry in items) + if item + ) + offset += len(items) + total = body.get("total_count") + if isinstance(total, int) and offset >= total: + break + else: + logger.warning( + "[arcade] stopped paging %s after %d pages; the listing may be " + "incomplete.", + toolkit, + MAX_PAGES, + ) + + self._definitions[toolkit.lower()] = collected + return collected + + def search( + self, + query: str, + toolkits: Sequence[str], + *, + limit: int = DEFAULT_SEARCH_LIMIT, + ) -> list[dict[str, Any]]: + """Actions in `toolkits` whose name or description match `query`. + + A plain substring index rather than a semantic one. The plan allows it + for a first version, and the allowlist keeps the haystack small enough + that it is a reasonable answer rather than a placeholder. + """ + needle = query.strip().lower() + matches: list[dict[str, Any]] = [] + for toolkit in toolkits: + for item in self.definitions(toolkit): + if len(matches) >= limit: + return matches + haystack = " ".join( + str(item.get(field) or "") + for field in ("qualified_name", "name", "description") + ).lower() + if not needle or needle in haystack: + matches.append(item) + return matches + + def lookup(self, qualified_name: str) -> dict[str, Any] | None: + """One action's definition, or `None` when this deployment has no such action.""" + toolkit = _toolkit_of(qualified_name) + if not toolkit: + return None + try: + self._require_allowed(toolkit) + except LookupError: + return None + for item in self.definitions(toolkit): + if item.get("qualified_name") == qualified_name: + return item + return None + + def authorization_for( + self, qualified_name: str, user_id: str | None + ) -> AuthorizationState: + """Whether `user_id` may run this action, asked fresh every time. + + Refuses an empty identity rather than asking anyway. Without a `user_id` + the provider answers about the tool rather than about a person, and that + answer reads as "connected" for everybody. + """ + identity = (user_id or "").strip() + if not identity: + raise ValueError( + "An authorization check needs the identity it is checking. " + "Asking without one reports the tool's own requirements, which " + "is not a statement about anybody's account." + ) + toolkit = _toolkit_of(qualified_name) + if not toolkit: + raise LookupError(f'"{qualified_name}" names no toolkit.') + self._require_allowed(toolkit) + + definition = self._client_factory().tools.get( + qualified_name, user_id=identity + ) + requirements = _as_mapping(_get(definition, "requirements")) + if requirements is None: + # Absent is not permission. + return AuthorizationState(connected=False) + + authorization = _as_mapping(requirements.get("authorization")) or {} + return AuthorizationState( + connected=requirements.get("met") is True, + never_started=authorization.get("token_status") == "not_started", + ) + + +def _get(node: Any, key: str) -> Any: + if isinstance(node, Mapping): + return node.get(key) + return getattr(node, key, None) + + +def _as_mapping(value: Any) -> Mapping[str, Any] | None: + if isinstance(value, Mapping): + return value + if value is None or isinstance(value, (str, bytes, int, float, list, tuple)): + return None + dumped = getattr(value, "model_dump", None) + if callable(dumped): + try: + result = dumped() + except Exception: # noqa: BLE001 - model shapes vary + return None + return result if isinstance(result, Mapping) else None + return None diff --git a/agent/arcade_tools/identity.py b/agent/arcade_tools/identity.py new file mode 100644 index 00000000..b1450774 --- /dev/null +++ b/agent/arcade_tools/identity.py @@ -0,0 +1,82 @@ +"""Which Arcade identity a turn acts as, per configured app. + +The trusted-actor rules are not reimplemented here. Reading who spoke, refusing +a caller-supplied identity, and clearing an anonymous turn all stay in +`composio_tools.state`, which owns them for both providers — its package name is +not a reason to move the identity boundary while adding a second provider. + +What this module adds is the one thing Arcade needs and Composio does not: a +namespace. Arcade user ids are global within a project, so two deployments +sharing one project and both spelling somebody `slack:U1` would hand each other +that person's connected accounts. The namespace is what keeps them apart, and +`ARCADE_IDENTITY_NAMESPACE` is required whenever a personal app is configured. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + +from arcade_tools.config import ArcadeConfig + +#: Separates the namespace from the actor key. Chosen because no actor key can +#: contain it: keys are `platform:id`, the platforms are a closed set, and none +#: of them contains a slash. +SEPARATOR = "/" + + +def arcade_user_id(namespace: str, actor_key: str) -> str: + """The Arcade identity for one person in one deployment. + + Refuses a namespace containing the separator. Without that check + `("ac/me", "slack:U1")` and `("ac", "me/slack:U1")` produce one string, and + the collision the namespace exists to prevent returns through the encoding. + """ + if SEPARATOR in namespace: + raise ValueError( + f"ARCADE_IDENTITY_NAMESPACE must not contain {SEPARATOR!r}: two " + "different namespaces could otherwise produce one identity." + ) + if not namespace: + raise ValueError("A personal Arcade identity needs a namespace.") + return f"{namespace}{SEPARATOR}{actor_key}" + + +@dataclass(frozen=True) +class ResolvedIdentities: + """Who this turn is, per app.""" + + #: Toolkit name -> the Arcade identity this turn uses for it. Empty when the + #: turn named nobody: a personal app never falls back to the shared account. + personal: dict[str, str] = field(default_factory=dict) + shared_user_id: str = "" + toolkits_for_shared: tuple[str, ...] = () + + +def resolve_identities( + config: ArcadeConfig, + *, + actor_key: str | None, +) -> ResolvedIdentities: + """Every identity applicable to this turn. + + One turn can be both the shared team identity and the person who spoke, so + this answers with all of them rather than the first match. + + An anonymous turn keeps its shared apps and loses its personal ones. Naming + a toolkit in `ARCADE_USER_TOOLKITS` is the operator saying it must run as + the person, so an unidentified turn gets no access to it rather than + quietly spending the shared account. + """ + personal: dict[str, str] = {} + if actor_key and config.user_toolkits: + for toolkit in config.user_toolkits: + personal[toolkit] = arcade_user_id(config.identity_namespace, actor_key) + + return ResolvedIdentities( + personal=personal, + # Never namespaced. The operator configures this one and it may already + # exist in their Arcade project; prefixing it would silently point at a + # different, empty account. + shared_user_id=config.workspace_user_id, + toolkits_for_shared=config.workspace_toolkits, + ) diff --git a/agent/arcade_tools/runtime.py b/agent/arcade_tools/runtime.py new file mode 100644 index 00000000..f37aada7 --- /dev/null +++ b/agent/arcade_tools/runtime.py @@ -0,0 +1,107 @@ +"""One Arcade setup per process, shared by the graph and the HTTP surface. + +Mirrors the Composio runtime's shape for the same reason it exists there: the +graph needs it to register tools and the connect route needs it to start an +authorization, and both must be the same object. Two catalogues would mean two +caches of the same listing and twice the cold-start cost on every restart. + +Cached including the `None` answer, so an unconfigured deployment does not +re-read the environment and re-log on every request. +""" + +from __future__ import annotations + +import logging +import threading +from collections.abc import Mapping +from dataclasses import dataclass +from typing import Any + +from arcade_tools.catalog import Catalog +from arcade_tools.config import ArcadeConfig, read_arcade_config, startup_warnings + +logger = logging.getLogger(__name__) + +_runtime: ArcadeRuntime | None = None +_built = False +#: The arguments the cached answer was built from. A cache that ignores what it +#: was called with is not a cache, it is a wrong answer that is right once. +_built_from: Any = None + +#: Reentrant, and for the same reason the Composio one is: the graph builds the +#: runtime while the connect route serves a click, and FastAPI runs a sync route +#: in a threadpool, so two callers reach here at once in an ordinary deployment. +_lock = threading.RLock() + + +@dataclass(frozen=True) +class ArcadeRuntime: + config: ArcadeConfig + catalog: Catalog + client_factory: Any + + +def _build_client(api_key: str): + """The SDK client, imported here rather than at module scope. + + A deployment that selected Composio never constructs this and should not pay + for the import either — and `connected_app_provider` asserts that selection + itself imports no provider package. + """ + from arcadepy import Arcade + + return Arcade(api_key=api_key) + + +def build_arcade_runtime( + env: Mapping[str, str] | None = None, + *, + default_user_id: str, +) -> ArcadeRuntime | None: + """Read the configuration and construct the shared pieces, or `None`.""" + config = read_arcade_config(env, default_user_id=default_user_id) + if config is None: + return None + + for warning in startup_warnings(config): + logger.warning("[arcade] %s", warning) + + client: Any = None + + def client_factory(): + nonlocal client + if client is None: + client = _build_client(config.api_key) + return client + + return ArcadeRuntime( + config=config, + catalog=Catalog(client_factory, config), + client_factory=client_factory, + ) + + +def arcade_runtime( + env: Mapping[str, str] | None = None, + *, + default_user_id: str = "open-tag", +) -> ArcadeRuntime | None: + """The process-wide runtime, built on first use.""" + global _runtime, _built, _built_from + signature = (id(env) if env is not None else None, default_user_id) + with _lock: + if _built and _built_from == signature: + return _runtime + _runtime = build_arcade_runtime(env, default_user_id=default_user_id) + _built = True + _built_from = signature + return _runtime + + +def reset_arcade_runtime() -> None: + """Drop the cached runtime. For tests, which vary the environment.""" + global _runtime, _built, _built_from + with _lock: + _runtime = None + _built = False + _built_from = None diff --git a/agent/arcade_tools/tools.py b/agent/arcade_tools/tools.py new file mode 100644 index 00000000..1b9e1ace --- /dev/null +++ b/agent/arcade_tools/tools.py @@ -0,0 +1,283 @@ +"""The two tools an Arcade deployment registers. + +Deliberately its own pair rather than a generalisation of the Composio pair. +Only one provider is ever registered, so the names are free, and the two +providers reach their catalogues through different calls with different failure +shapes — a shared implementation would be a parameterised branch everywhere and +a shared abstraction nowhere. + +What is *not* duplicated: identity comes from `composio_tools.state`, the +approval card from `write_confirmation`, and the effect vocabulary from +`composio_tools.classify`. Those are the parts a second copy would make +dangerous, so there is one of each. + +Three rules hold, and each has a test that goes red without it: + +* The model naming an action is not authorization to run it. Ownership is + checked again at execution, against the configured allowlist. +* Nobody's account is chosen by the model. Neither tool takes an identity, and + the only identity considered is the one the Channel forwarded. +* Connecting an account is not approval to write. They are different questions + and both get asked. +""" + +from __future__ import annotations + +import logging +from typing import Annotated, Any + +from langchain_core.tools import tool +from langgraph.prebuilt import InjectedState + +from arcade_tools.catalog import Catalog, owns +from arcade_tools.config import ArcadeConfig +from arcade_tools.effects import effect_of_definition +from arcade_tools.identity import resolve_identities +from arcade_tools.outcomes import ( + Outcome, + describe_outcome, + outcome_of_response, + outcome_of_transport_error, +) +from composio_tools.classify import needs_approval +from composio_tools.state import actor_key, actor_of +from write_confirmation import ( + emit_write_failure, + require_write_confirmation, + summarize_args, +) + +logger = logging.getLogger(__name__) + + +def humanize(qualified_name: str) -> str: + """`Github.CreateIssue` -> `Create issue` — what the approval card says.""" + _toolkit, _dot, action = qualified_name.partition(".") + action = action or qualified_name + spaced = "" + for index, character in enumerate(action): + if character.isupper() and index and not action[index - 1].isupper(): + spaced += " " + spaced += character + spaced = spaced.replace("_", " ").strip() + if not spaced: + return qualified_name + return spaced[:1].upper() + spaced[1:].lower() + + +def build_arcade_tools( + config: ArcadeConfig, + catalog: Catalog, + client_factory, +) -> list[Any]: + """The Arcade tools for this deployment.""" + + def turn_identities(state: dict[str, Any] | None): + """Who this turn is. Read from the forwarded actor and nowhere else.""" + # `actor_of` already refuses a caller-supplied value and anything that + # is not a person, so a bot posting into a thread cannot spend somebody + # else's connected account. + key = actor_key(actor_of(state)) + return key, resolve_identities(config, actor_key=key) + + def reachable_toolkits(identities) -> tuple[str, ...]: + return (*identities.toolkits_for_shared, *identities.personal) + + def anonymous_note() -> str: + return ( + "Apps that run as each person (" + + ", ".join(config.user_toolkits) + + ") are unavailable on this turn, because it did not say who is " + "speaking. This is not a missing connection." + ) + + @tool + def search_my_tools( + query: str, + state: Annotated[dict[str, Any], InjectedState], + ) -> dict[str, Any] | str: + """Find actions available in the connected apps. Call this before run_my_tool. + + Args: + query: What you want to do, in plain words, e.g. 'create an issue'. + """ + key, identities = turn_identities(state) + toolkits = reachable_toolkits(identities) + if not toolkits: + return ( + anonymous_note() + if key is None and config.user_toolkits + else "No connected apps are configured on this deployment." + ) + + try: + found = catalog.search(query, toolkits) + except Exception as error: # noqa: BLE001 - provider errors vary + logger.warning("[arcade] search failed: %s", error) + return "The connected apps could not be reached just now." + + payload: dict[str, Any] = { + "actions": [ + { + "qualifiedName": item.get("qualified_name"), + "description": item.get("description") or "", + "arguments": _argument_schema(item), + # So the model can tell the person what will happen before + # it calls, rather than the card being the first they hear. + "effect": effect_of_definition(item), + } + for item in found + ] + } + if key is None and config.user_toolkits: + payload["personalAppsUnavailable"] = anonymous_note() + return payload + + @tool + def run_my_tool( + qualified_name: str, + arguments: dict[str, Any], + state: Annotated[dict[str, Any], InjectedState], + ) -> Any: + """Run one action found by search_my_tools. + + Args: + qualified_name: The action from search_my_tools, e.g. 'Github.CreateIssue'. + arguments: Arguments matching that action's input schema. + """ + key, identities = turn_identities(state) + + # Ownership, decided here rather than trusted from the search that + # produced the name. A name the model produced is a name, not a grant. + personal_toolkits = tuple(identities.personal) + if owns(personal_toolkits, qualified_name): + user_id = identities.personal[_toolkit_of(qualified_name, personal_toolkits)] + approver = key + elif owns(identities.toolkits_for_shared, qualified_name): + user_id = identities.shared_user_id + # Runs as the team account, so it spends nobody's personal access + # and anybody may answer the card. + approver = None + elif owns(config.user_toolkits, qualified_name): + # Configured, and very probably connected. What is missing is the + # person, so "no app provides this" would send them to fix a setup + # that is not broken. + return f"{qualified_name} did not run. {anonymous_note()}" + else: + return ( + f"No connected app here provides {qualified_name}. " + "Call search_my_tools and use a name it returned." + ) + + definition = catalog.lookup(qualified_name) + if definition is None: + return ( + f"{qualified_name} is not an action this deployment can reach. " + "Call search_my_tools and use a name it returned." + ) + + # Asked before the card. Approving something that was never going to run + # spends the person's attention and teaches them the card means less + # than it does. + try: + authorization = catalog.authorization_for(qualified_name, user_id) + except Exception as error: # noqa: BLE001 - provider errors vary + logger.warning( + "[arcade] could not check authorization for %s: %s", + qualified_name, + error, + ) + return ( + f"{qualified_name} did not run: the account it needs could not " + "be checked just now." + ) + if not authorization.connected: + return ( + f"{qualified_name} needs that account connected first. " + "Ask to connect it, then try again." + ) + + effect = effect_of_definition(definition) + label = humanize(qualified_name) + gated = needs_approval(effect, config.approvals) + if gated: + # The same card, and the same pause, that already gate a Linear or + # Notion write. Connecting an account answered a different question. + approved = require_write_confirmation( + action=label, + fields=summarize_args(arguments), + extra_args={"approver": approver, "effect": effect}, + ) + if not approved: + return f"{label} was declined, so nothing ran." + + try: + response = client_factory().tools.execute( + tool_name=qualified_name, + user_id=user_id, + input=arguments, + ) + except (TypeError, AttributeError): + # A build whose SDK calls no longer land, not a tool that failed. + # Raised rather than turned into a result the model reads as "try + # again", which would hide it behind a retry loop. + raise + except Exception as error: # noqa: BLE001 - provider errors vary + result = outcome_of_transport_error(error) + return _report(result, label, qualified_name, gated) + + result = outcome_of_response(response) + if result.outcome is Outcome.SUCCEEDED: + return result.value + return _report(result, label, qualified_name, gated) + + def _report(result, label: str, qualified_name: str, gated: bool) -> str: + """One failure, told to everyone waiting on it. + + The thread hears it only when there was a card: an approver whose last + sight of this action was "running" has no other way to learn it did not. + """ + sentence = describe_outcome(result, action=label) + logger.warning("[arcade] %s: %s", qualified_name, sentence) + if gated: + emit_write_failure(label, sentence) + return sentence + + return [search_my_tools, run_my_tool] + + +def _toolkit_of(qualified_name: str, candidates: tuple[str, ...]) -> str: + """The configured spelling of this action's toolkit. + + Matched case-insensitively and answered with the operator's own spelling, + because that is the key the identity map is built under. + """ + head = qualified_name.partition(".")[0].lower() + for name in candidates: + if name.lower() == head: + return name + raise LookupError(qualified_name) + + +def _argument_schema(definition: Any) -> list[dict[str, Any]]: + """The action's declared inputs, flattened for the model to read.""" + node = definition.get("input") if isinstance(definition, dict) else None + parameters = (node or {}).get("parameters") if isinstance(node, dict) else None + if not isinstance(parameters, list): + return [] + flattened = [] + for parameter in parameters: + if not isinstance(parameter, dict): + continue + schema = parameter.get("value_schema") + flattened.append( + { + "name": parameter.get("name"), + "required": parameter.get("required") is True, + "description": parameter.get("description") or "", + "type": (schema or {}).get("val_type") + if isinstance(schema, dict) + else None, + } + ) + return flattened diff --git a/agent/prompts/__init__.py b/agent/prompts/__init__.py index 7d1a2f9e..173b7b58 100644 --- a/agent/prompts/__init__.py +++ b/agent/prompts/__init__.py @@ -12,6 +12,7 @@ from .tools import ( TOOLS_PROMPT, DEFAULT_INTERNAL_SOURCES, + arcade_addendum, composio_addendum, tools_prompt, CODING_ON_ADDENDUM, @@ -55,6 +56,7 @@ def build_base_system_prompt( "build_base_system_prompt", "build_system_prompt", "tools_prompt", + "arcade_addendum", "composio_addendum", "current_date_context", "current_date_prompt", diff --git a/agent/prompts/tools.py b/agent/prompts/tools.py index 7635a9e0..536eae8a 100644 --- a/agent/prompts/tools.py +++ b/agent/prompts/tools.py @@ -180,3 +180,54 @@ def composio_addendum(config) -> str: " until search_my_tools has returned it" ) return "\n".join(lines) + "\n" + + +def arcade_addendum(config) -> str: + """What an Arcade deployment is told about the apps it can reach. + + Same job as `composio_addendum`, and the same two rules: name the apps, + never the actions. A model told nothing about which apps exist answers from + belief rather than searching, and a model left holding an app name invents + plausible action names from it. + + Written separately rather than parameterised. The two configs carry + different shapes, and only one provider is ever registered so there is no + caller that needs both. What must not differ is the behaviour, which the + two prompt suites assert side by side. + + Nothing here says "no connected apps". That sentence belongs to a + deployment that has none; telling a configured Arcade deployment it has + none is how an agent stops looking. + """ + if config is None: + return "" + + # As the runtime actually routes them. A name in both lists resolves to the + # personal identity only, and the reader has already removed it from the + # shared list — advertising it as shared would promise everybody access to + # something that runs only for whoever is speaking. + shared = tuple(getattr(config, "workspace_toolkits", ()) or ()) + personal = tuple(getattr(config, "user_toolkits", ()) or ()) + if not shared and not personal: + return "" + + lines = [ + "\n- Connected apps are available. Call search_my_tools to find an" + " action in them before answering whether you can do something" + ] + if shared: + lines.append( + f"- Shared with everyone here: {_named(shared)}. These are connected" + " once for the whole workspace" + ) + if personal: + lines.append( + f"- Each person's own: {_named(personal)}. These run in the account" + " of whoever is speaking, and do nothing until that person connects" + " them" + ) + lines.append( + "- This names apps, not actions. Never claim a specific action exists" + " until search_my_tools has returned it" + ) + return "\n".join(lines) + "\n" diff --git a/agent/tests/test_arcade_catalog.py b/agent/tests/test_arcade_catalog.py new file mode 100644 index 00000000..c3b2d22c --- /dev/null +++ b/agent/tests/test_arcade_catalog.py @@ -0,0 +1,298 @@ +"""Discovery over the configured Arcade apps, and who is connected to what. + +Two rules hold this module together: + +* **Static facts cache, authorization never does.** A tool's schema and its + behaviour flags do not change between calls. Whether somebody is connected + changes the moment they connect, and a cached "no" would outlive the click + that fixed it. +* **The allowlist is enforced where the call happens**, not only where the + search happens. A model naming a tool is not authorization to run it. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from arcade_tools.catalog import Catalog, owns +from arcade_tools.config import read_arcade_config + +FIXTURE = Path(__file__).parent / "fixtures" / "arcade_catalogue.json" + + +def definition(qualified_name, *, toolkit=None, behavior=None, description=""): + toolkit = toolkit or qualified_name.split(".")[0] + node = { + "qualified_name": qualified_name, + "fully_qualified_name": f"{qualified_name}@1.0.0", + "name": qualified_name.split(".")[-1], + "description": description, + "toolkit": {"name": toolkit, "version": "1.0.0"}, + "input": {"parameters": []}, + } + if behavior is not None: + node["metadata"] = {"behavior": behavior} + return node + + +class FakeTools: + """Stands in for `client.tools`, counting what was asked of it.""" + + def __init__(self, pages=None, single=None): + self.pages = pages or {} + self.single = single or {} + self.list_calls = [] + self.get_calls = [] + + def list(self, **kwargs): + self.list_calls.append(kwargs) + toolkit = kwargs.get("toolkit") + offset = kwargs.get("offset", 0) + limit = kwargs.get("limit", 100) + items = self.pages.get(toolkit, []) + window = items[offset : offset + limit] + return { + "items": window, + "limit": limit, + "offset": offset, + "page_count": len(window), + "total_count": len(items), + } + + def get(self, name, **kwargs): + self.get_calls.append((name, kwargs)) + if name not in self.single: + raise LookupError(name) + return self.single[name] + + +class FakeClient: + """What the factory hands back: a client whose `.tools` is the resource.""" + + def __init__(self, tools): + self.tools = tools + + +def catalog_for(pages=None, single=None, **overrides): + env = { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + **overrides, + } + config = read_arcade_config(env, default_user_id="open-tag") + assert config is not None + tools = FakeTools(pages=pages, single=single) + return Catalog(lambda: FakeClient(tools), config), tools + + +def test_listing_follows_every_page(): + # The first page of a large toolkit is not the toolkit. Github alone has 43 + # tools against a default page size that does not reach them. + many = [definition(f"Github.Tool{index}") for index in range(250)] + catalog, tools = catalog_for(pages={"Github": many}) + + found = catalog.definitions("Github") + + assert len(found) == 250 + assert len({item["qualified_name"] for item in found}) == 250 + assert len(tools.list_calls) > 1 + + +def test_a_second_read_of_the_same_toolkit_costs_no_call(): + catalog, tools = catalog_for(pages={"Github": [definition("Github.One")]}) + + catalog.definitions("Github") + before = len(tools.list_calls) + catalog.definitions("Github") + + assert len(tools.list_calls) == before + + +def test_listing_never_carries_a_user_id(): + # The cache is shared across everybody, so a listing fetched for one person + # must not be able to hold that person's authorization state. Asking without + # an id also makes `met` meaningless, which is the point of the next test. + catalog, tools = catalog_for(pages={"Github": [definition("Github.One")]}) + + catalog.definitions("Github") + + assert all("user_id" not in call for call in tools.list_calls) + + +def test_an_unconfigured_toolkit_is_never_listed(): + catalog, tools = catalog_for(pages={"Asana": [definition("Asana.One")]}) + + with pytest.raises(LookupError): + catalog.definitions("Asana") + + assert tools.list_calls == [] + + +def test_search_matches_names_and_descriptions(): + catalog, _tools = catalog_for( + pages={ + "Github": [ + definition("Github.CreateIssue", description="Open a new issue"), + definition("Github.ListCommits", description="Recent commits"), + ] + } + ) + + found = catalog.search("issue", ("Github",)) + + assert [item["qualified_name"] for item in found] == ["Github.CreateIssue"] + + +def test_search_is_bounded(): + many = [definition(f"Github.Thing{index}", description="issue") for index in range(80)] + catalog, _tools = catalog_for(pages={"Github": many}) + + found = catalog.search("issue", ("Github",), limit=10) + + assert len(found) == 10 + + +def test_search_only_looks_inside_the_scopes_it_was_given(): + catalog, _tools = catalog_for( + pages={ + "Github": [definition("Github.CreateIssue", description="issue")], + "Asana": [definition("Asana.CreateTask", description="issue")], + }, + ARCADE_TOOLKITS="Github,Asana", + ) + + found = catalog.search("issue", ("Github",)) + + assert [item["qualified_name"] for item in found] == ["Github.CreateIssue"] + + +def test_ownership_is_decided_by_the_toolkit_not_the_whole_name(): + assert owns(("Github",), "Github.CreateIssue") is True + assert owns(("Github",), "Asana.CreateTask") is False + + +def test_ownership_ignores_the_case_an_operator_typed(): + # Arcade's names are not lowercase slugs, and an operator typing `github` + # means the same toolkit. Matching is case-insensitive; the canonical name + # still comes from Arcade. + assert owns(("github",), "Github.CreateIssue") is True + assert owns(("GITHUB",), "Github.CreateIssue") is True + + +def test_a_name_with_no_toolkit_owns_nothing(): + # A bare name cannot be attributed, and attributing it to the first + # configured toolkit would let a model reach a tool by leaving the prefix + # off. + assert owns(("Github",), "CreateIssue") is False + assert owns(("Github",), "") is False + + +def test_a_name_whose_prefix_merely_starts_the_same_owns_nothing(): + # `GithubEnterprise.X` must not pass an allowlist naming `Github`. + assert owns(("Github",), "GithubEnterprise.CreateIssue") is False + + +def test_authorization_is_asked_for_the_named_person(): + catalog, tools = catalog_for( + single={ + "Github.CreateIssue": { + "qualified_name": "Github.CreateIssue", + "requirements": {"met": True, "authorization": {"status": "active"}}, + } + } + ) + + state = catalog.authorization_for("Github.CreateIssue", "slack:U1") + + assert state.connected is True + assert tools.get_calls == [("Github.CreateIssue", {"user_id": "slack:U1"})] + + +def test_authorization_is_never_cached(): + # Somebody connects between two calls. A cached "not connected" would + # survive the click that fixed it and keep telling them to connect again. + catalog, tools = catalog_for( + single={ + "Github.CreateIssue": { + "qualified_name": "Github.CreateIssue", + "requirements": {"met": False}, + } + } + ) + + catalog.authorization_for("Github.CreateIssue", "slack:U1") + catalog.authorization_for("Github.CreateIssue", "slack:U1") + + assert len(tools.get_calls) == 2 + + +def test_an_unmet_requirement_reports_not_connected(): + catalog, _tools = catalog_for( + single={ + "Github.CreateIssue": { + "qualified_name": "Github.CreateIssue", + "requirements": { + "met": False, + "authorization": {"token_status": "not_started"}, + }, + } + } + ) + + state = catalog.authorization_for("Github.CreateIssue", "slack:U1") + + assert state.connected is False + assert state.never_started is True + + +def test_a_missing_requirements_block_is_not_read_as_connected(): + # Absent is not permission. A tool whose requirements did not come back + # tells us nothing about whether this person may call it. + catalog, _tools = catalog_for( + single={"Github.CreateIssue": {"qualified_name": "Github.CreateIssue"}} + ) + + state = catalog.authorization_for("Github.CreateIssue", "slack:U1") + + assert state.connected is False + + +def test_an_unconfigured_toolkit_is_never_authorization_checked(): + catalog, tools = catalog_for(single={"Asana.CreateTask": {"requirements": {"met": True}}}) + + with pytest.raises(LookupError): + catalog.authorization_for("Asana.CreateTask", "slack:U1") + + assert tools.get_calls == [] + + +def test_an_anonymous_authorization_check_is_refused_rather_than_asked(): + # `met` comes back true when no user id is sent — it means "this tool's + # requirements are satisfiable", not "this person is connected". Asking + # without an identity and believing the answer would report everybody as + # connected to everything. + catalog, tools = catalog_for( + single={"Github.CreateIssue": {"requirements": {"met": True}}} + ) + + for missing in (None, "", " "): + with pytest.raises(ValueError): + catalog.authorization_for("Github.CreateIssue", missing) + + assert tools.get_calls == [] + + +def test_the_real_catalogue_shapes_survive_the_reader(): + # The fixtures are verbatim payloads. Anything the reader assumes about + # their shape is asserted against the real thing rather than a fake. + tools = json.loads(FIXTURE.read_text())["tools"] + catalog, _fake = catalog_for( + pages={"Apollo": [tools["read"]]}, ARCADE_TOOLKITS="Apollo" + ) + + found = catalog.definitions("Apollo") + + assert found[0]["qualified_name"] == tools["read"]["qualified_name"] diff --git a/agent/tests/test_arcade_identity.py b/agent/tests/test_arcade_identity.py new file mode 100644 index 00000000..0948c5fd --- /dev/null +++ b/agent/tests/test_arcade_identity.py @@ -0,0 +1,119 @@ +"""Whose Arcade account a turn acts as. + +The rule is the one the Composio path already enforces, reused rather than +reimplemented: identity comes from what the Channel forwarded, never from +anything the model produced. Arcade adds one requirement on top — its user ids +are global to a project, so a namespace keeps two deployments sharing a project +from sharing each other's people. +""" + +from __future__ import annotations + +import pytest + +from arcade_tools.config import read_arcade_config +from arcade_tools.identity import arcade_user_id, resolve_identities + + +def config_for(**overrides): + env = { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + "ARCADE_USER_TOOLKITS": "Gmail", + "ARCADE_IDENTITY_NAMESPACE": "acme", + **overrides, + } + config = read_arcade_config(env, default_user_id="open-tag") + assert config is not None + return config + + +def test_a_personal_identity_carries_the_namespace_and_the_platform(): + assert arcade_user_id("acme", "slack:U1") == "acme/slack:U1" + + +def test_two_namespaces_never_produce_the_same_identity(): + # The whole reason the namespace exists. Two deployments in one Arcade + # project must not both resolve `slack:U1` to the same connected account. + assert arcade_user_id("acme", "slack:U1") != arcade_user_id("beta", "slack:U1") + + +def test_two_platforms_never_produce_the_same_identity(): + # A provider id is unique only within its provider. One deployment serving + # Slack and Teams would otherwise give `U1` on either the same account. + assert arcade_user_id("acme", "slack:U1") != arcade_user_id("acme", "teams:U1") + + +@pytest.mark.parametrize("namespace", ["ac/me", "acme/", "/acme"]) +def test_a_namespace_containing_the_separator_is_refused(namespace): + # Otherwise `acme/slack:U1` could be produced by two different namespace and + # actor pairs, and the collision the namespace exists to prevent comes back + # through the encoding instead. + with pytest.raises(ValueError): + arcade_user_id(namespace, "slack:U1") + + +def test_an_anonymous_turn_reaches_no_personal_app(): + identities = resolve_identities(config_for(), actor_key=None) + + assert identities.personal == {} + assert identities.shared_user_id == "open-tag" + + +def test_an_anonymous_turn_still_reaches_shared_apps(): + identities = resolve_identities(config_for(), actor_key=None) + + assert "Github" in identities.toolkits_for_shared + + +def test_a_named_turn_reaches_its_own_personal_apps(): + identities = resolve_identities(config_for(), actor_key="slack:U1") + + assert identities.personal["Gmail"] == "acme/slack:U1" + + +def test_a_personal_app_never_falls_back_to_the_shared_account(): + # Naming a toolkit personal is the operator saying it must run as the + # person. An unidentified turn gets no access rather than quietly spending + # the shared account. + identities = resolve_identities(config_for(), actor_key=None) + + assert "Gmail" not in identities.toolkits_for_shared + assert "Gmail" not in identities.personal + + +def test_the_identity_for_one_person_is_stable_across_turns(): + first = resolve_identities(config_for(), actor_key="slack:U1") + second = resolve_identities(config_for(), actor_key="slack:U1") + + assert first.personal == second.personal + + +def test_two_people_never_share_an_identity(): + one = resolve_identities(config_for(), actor_key="slack:U1") + two = resolve_identities(config_for(), actor_key="slack:U2") + + assert one.personal["Gmail"] != two.personal["Gmail"] + + +def test_a_deployment_with_no_personal_apps_needs_no_namespace(): + config = read_arcade_config( + {"ARCADE_API_KEY": "arc_test", "ARCADE_TOOLKITS": "Github"}, + default_user_id="open-tag", + ) + assert config is not None + + identities = resolve_identities(config, actor_key="slack:U1") + + assert identities.personal == {} + assert identities.toolkits_for_shared == ("Github",) + + +def test_the_shared_identity_is_never_namespaced(): + # It is configured by the operator and may already exist in their project. + # Prefixing it would silently point at a different, empty account. + identities = resolve_identities( + config_for(ARCADE_WORKSPACE_USER_ID="team-bot"), actor_key="slack:U1" + ) + + assert identities.shared_user_id == "team-bot" diff --git a/agent/tests/test_arcade_prompt.py b/agent/tests/test_arcade_prompt.py new file mode 100644 index 00000000..33e07811 --- /dev/null +++ b/agent/tests/test_arcade_prompt.py @@ -0,0 +1,93 @@ +"""What an Arcade deployment is told about the apps it can reach. + +The defect this guards against is documented on the Composio side and is not +provider-specific: a model told nothing about which apps exist answers from +belief instead of searching. It said it could not see Linear — which was +configured — without ever calling the search tool. + +The Arcade-specific half is the one the plan calls out: an Arcade deployment +must never be handed the Composio absence statement, because "you have no +connected apps" is false and makes the model stop looking. +""" + +from __future__ import annotations + +from arcade_tools.config import read_arcade_config +from prompts import arcade_addendum, composio_addendum + + +def config_for(**overrides): + env = { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + "ARCADE_USER_TOOLKITS": "Gmail", + "ARCADE_IDENTITY_NAMESPACE": "acme", + **overrides, + } + config = read_arcade_config(env, default_user_id="open-tag") + assert config is not None + return config + + +def test_the_configured_apps_are_named(): + text = arcade_addendum(config_for()) + + assert "Github" in text + assert "Gmail" in text + + +def test_shared_and_personal_are_told_apart(): + # They fail differently: a shared app is connected once by an operator, + # a personal one does nothing until that person connects it. That is the + # difference between "try again later" and "press the Connect button". + text = arcade_addendum(config_for()) + + shared_line = next(line for line in text.splitlines() if "Github" in line) + personal_line = next(line for line in text.splitlines() if "Gmail" in line) + + assert shared_line != personal_line + + +def test_the_search_tool_is_named_so_the_model_knows_to_look(): + assert "search_my_tools" in arcade_addendum(config_for()) + + +def test_apps_are_named_but_actions_never_are(): + # A model left holding an app name invents plausible action names from it, + # then tells somebody those actions exist. + text = arcade_addendum(config_for()) + + assert "never claim" in text.lower() or "not actions" in text.lower() + + +def test_no_provider_produces_no_addendum(): + assert arcade_addendum(None) == "" + + +def test_an_arcade_deployment_is_never_told_it_has_no_connected_apps(): + # The plan's explicit requirement. The Composio addendum is the one that + # would say this, and it must not be generated for an Arcade deployment. + arcade_text = arcade_addendum(config_for()) + composio_text = composio_addendum(None) + + assert "no connected apps" not in arcade_text.lower() + assert "no connected apps" not in composio_text.lower() + + +def test_a_long_app_list_admits_that_it_was_cut_short(): + many = ",".join(f"App{index}" for index in range(30)) + text = arcade_addendum(config_for(ARCADE_TOOLKITS=many)) + + assert "more" in text + + +def test_an_app_in_both_scopes_is_advertised_as_personal_only(): + # It routes as personal, so advertising it as shared would promise + # everybody access to something that runs only for whoever is speaking. + text = arcade_addendum( + config_for(ARCADE_TOOLKITS="Asana", ARCADE_USER_TOOLKITS="Asana") + ) + + lines = [line for line in text.splitlines() if "Asana" in line] + assert len(lines) == 1 + assert "own" in lines[0] diff --git a/agent/tests/test_arcade_tools.py b/agent/tests/test_arcade_tools.py new file mode 100644 index 00000000..7b8cefe1 --- /dev/null +++ b/agent/tests/test_arcade_tools.py @@ -0,0 +1,504 @@ +"""The two Arcade tools, and everything that must hold before one runs. + +The properties under test are the ones a review round would go looking for: a +model naming a tool is not authorization to run it, nobody's account is chosen +by the model, a declined card executes nothing, and an approval is spent once. +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from arcade_tools.catalog import Catalog +from arcade_tools.config import read_arcade_config +from arcade_tools.tools import build_arcade_tools +from composio_tools.state import ACTOR_STATE_KEY + + +def definition(qualified_name, *, behavior=None, description="", parameters=None): + node: dict[str, Any] = { + "qualified_name": qualified_name, + "fully_qualified_name": f"{qualified_name}@1.0.0", + "name": qualified_name.split(".")[-1], + "description": description, + "toolkit": {"name": qualified_name.split(".")[0], "version": "1.0.0"}, + "input": {"parameters": parameters or []}, + } + if behavior is not None: + node["metadata"] = {"behavior": behavior} + return node + + +READ_BEHAVIOR = {"read_only": True, "destructive": False} +WRITE_BEHAVIOR = {"read_only": False, "destructive": False} +DESTRUCTIVE_BEHAVIOR = {"read_only": False, "destructive": True} + + +class FakeTools: + def __init__(self, pages, requirements_met=True): + self.pages = pages + self.requirements_met = requirements_met + self.executed: list[dict] = [] + self.get_calls: list[tuple] = [] + + def list(self, **kwargs): + items = self.pages.get(kwargs.get("toolkit"), []) + return {"items": items, "total_count": len(items), "offset": 0} + + def get(self, name, **kwargs): + self.get_calls.append((name, kwargs)) + return { + "qualified_name": name, + "requirements": { + "met": self.requirements_met, + "authorization": { + "token_status": "completed" if self.requirements_met else "not_started" + }, + }, + } + + def execute(self, **kwargs): + self.executed.append(kwargs) + return {"success": True, "output": {"value": {"ok": True}}} + + +class FakeClient: + def __init__(self, tools): + self.tools = tools + + +def build(pages, *, requirements_met=True, approvals="on", **overrides): + env = { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + "ARCADE_USER_TOOLKITS": "Gmail", + "ARCADE_IDENTITY_NAMESPACE": "acme", + "ARCADE_APPROVALS": approvals, + **overrides, + } + config = read_arcade_config(env, default_user_id="open-tag") + assert config is not None + tools = FakeTools(pages, requirements_met=requirements_met) + catalog = Catalog(lambda: FakeClient(tools), config) + built = build_arcade_tools(config, catalog, lambda: FakeClient(tools)) + by_name = {item.name: item for item in built} + return by_name, tools + + +def state(actor_id="U1", platform="slack", kind="human"): + if actor_id is None: + return {} + return {ACTOR_STATE_KEY: {"id": actor_id, "platform": platform, "kind": kind}} + + +def invoke(tool, **kwargs): + return tool.invoke(kwargs) + + +# --- the tools exist and are named what the prompt says they are --- + + +def test_the_provider_registers_exactly_the_two_expected_tools(): + built, _tools = build({"Github": [definition("Github.ListIssues")]}) + + assert set(built) == {"search_my_tools", "run_my_tool"} + + +def test_neither_tool_lets_the_model_choose_whose_account_to_use(): + # The defect this prevents: an identity the model fills in is an identity it + # can change, and the first thing it would be asked to change is whose + # mailbox to open. + built, _tools = build({"Github": [definition("Github.ListIssues")]}) + + for tool in built.values(): + fields = set(tool.args_schema.model_fields) + assert "user_id" not in fields + assert "account" not in fields + assert "actor" not in fields + assert "provider" not in fields + + +# --- discovery --- + + +def test_search_returns_actions_from_configured_apps(): + built, _tools = build( + {"Github": [definition("Github.ListIssues", description="list issues")]} + ) + + found = invoke(built["search_my_tools"], query="issues", state=state()) + + assert "Github.ListIssues" in str(found) + + +def test_search_never_offers_an_unconfigured_app(): + built, _tools = build( + { + "Github": [definition("Github.ListIssues", description="issues")], + "Asana": [definition("Asana.ListTasks", description="issues")], + } + ) + + found = invoke(built["search_my_tools"], query="issues", state=state()) + + assert "Asana" not in str(found) + + +def test_an_anonymous_turn_sees_no_personal_actions(): + built, _tools = build( + { + "Github": [definition("Github.ListIssues", description="thing")], + "Gmail": [definition("Gmail.ListMail", description="thing")], + } + ) + + found = invoke(built["search_my_tools"], query="thing", state=state(actor_id=None)) + + assert "Gmail.ListMail" not in str(found) + assert "Github.ListIssues" in str(found) + + +def test_an_anonymous_turn_is_told_why_its_personal_apps_are_missing(): + # Otherwise the symptom reads to the person as "the app is not connected", + # and they go and connect an account that was never the problem. + built, _tools = build({"Gmail": [definition("Gmail.ListMail")]}) + + found = invoke(built["search_my_tools"], query="mail", state=state(actor_id=None)) + + assert "Gmail" in str(found) + + +def test_discovery_never_starts_an_account_connection(): + # A search must not mint anything. The link is a bearer capability and is + # minted for one clicker, through the connect route. + built, tools = build( + {"Github": [definition("Github.ListIssues", description="x")]}, + requirements_met=False, + ) + + invoke(built["search_my_tools"], query="x", state=state()) + + assert tools.executed == [] + + +# --- execution: the allowlist --- + + +def test_running_an_action_from_an_unconfigured_app_is_refused(): + # The model naming it is not authorization. This is checked at execution and + # not only at search, because the two are separate calls and only one of + # them is the one that changes anything. + built, tools = build( + { + "Github": [definition("Github.ListIssues")], + "Asana": [definition("Asana.DeleteProject")], + } + ) + + result = invoke( + built["run_my_tool"], + qualified_name="Asana.DeleteProject", + arguments={}, + state=state(), + ) + + assert tools.executed == [] + assert "Asana.DeleteProject" in str(result) + + +def test_running_an_action_that_does_not_exist_is_refused(): + built, tools = build({"Github": [definition("Github.ListIssues")]}) + + invoke( + built["run_my_tool"], + qualified_name="Github.MadeUpAction", + arguments={}, + state=state(), + ) + + assert tools.executed == [] + + +def test_a_personal_action_never_runs_for_an_anonymous_turn(): + built, tools = build( + {"Gmail": [definition("Gmail.SendMail", behavior=READ_BEHAVIOR)]} + ) + + invoke( + built["run_my_tool"], + qualified_name="Gmail.SendMail", + arguments={}, + state=state(actor_id=None), + ) + + assert tools.executed == [] + + +def test_a_bot_posting_as_a_person_never_spends_a_personal_account(): + built, tools = build( + {"Gmail": [definition("Gmail.ListMail", behavior=READ_BEHAVIOR)]} + ) + + invoke( + built["run_my_tool"], + qualified_name="Gmail.ListMail", + arguments={}, + state=state(kind="bot"), + ) + + assert tools.executed == [] + + +# --- execution: identity --- + + +def test_a_personal_action_runs_as_the_person_who_spoke(): + built, tools = build( + {"Gmail": [definition("Gmail.ListMail", behavior=READ_BEHAVIOR)]} + ) + + invoke( + built["run_my_tool"], + qualified_name="Gmail.ListMail", + arguments={}, + state=state(actor_id="U1"), + ) + + assert tools.executed[0]["user_id"] == "acme/slack:U1" + + +def test_two_people_run_as_themselves(): + built, tools = build( + {"Gmail": [definition("Gmail.ListMail", behavior=READ_BEHAVIOR)]} + ) + + invoke(built["run_my_tool"], qualified_name="Gmail.ListMail", arguments={}, + state=state(actor_id="U1")) + invoke(built["run_my_tool"], qualified_name="Gmail.ListMail", arguments={}, + state=state(actor_id="U2")) + + assert tools.executed[0]["user_id"] != tools.executed[1]["user_id"] + + +def test_a_shared_action_runs_as_the_workspace_identity(): + built, tools = build( + {"Github": [definition("Github.ListIssues", behavior=READ_BEHAVIOR)]} + ) + + invoke(built["run_my_tool"], qualified_name="Github.ListIssues", arguments={}, + state=state()) + + assert tools.executed[0]["user_id"] == "open-tag" + + +def test_an_identity_supplied_by_the_caller_is_ignored(): + # Belt and braces over the schema check above: even if something reached the + # tool with an identity in state, only the forwarded actor decides. + built, tools = build( + {"Gmail": [definition("Gmail.ListMail", behavior=READ_BEHAVIOR)]} + ) + poisoned = state(actor_id="U1") + poisoned["user_id"] = "acme/slack:VICTIM" + poisoned["arcade_user_id"] = "acme/slack:VICTIM" + + invoke(built["run_my_tool"], qualified_name="Gmail.ListMail", arguments={}, + state=poisoned) + + assert tools.executed[0]["user_id"] == "acme/slack:U1" + + +# --- execution: authorization --- + + +def test_an_unconnected_account_is_reported_rather_than_executed(): + built, tools = build( + {"Github": [definition("Github.ListIssues", behavior=READ_BEHAVIOR)]}, + requirements_met=False, + ) + + result = invoke(built["run_my_tool"], qualified_name="Github.ListIssues", + arguments={}, state=state()) + + assert tools.executed == [] + assert "connect" in str(result).lower() + + +def test_the_authorization_check_does_not_execute_anything(): + built, tools = build( + {"Github": [definition("Github.ListIssues", behavior=READ_BEHAVIOR)]}, + requirements_met=False, + ) + + invoke(built["run_my_tool"], qualified_name="Github.ListIssues", arguments={}, + state=state()) + + assert tools.get_calls # it was checked + assert tools.executed == [] # and checking did not run it + + +def test_authorization_is_rechecked_for_the_action_being_run(): + # A prior successful read does not establish access to send or delete. + built, tools = build( + { + "Github": [ + definition("Github.ListIssues", behavior=READ_BEHAVIOR), + definition("Github.DeleteRepo", behavior=READ_BEHAVIOR), + ] + } + ) + + invoke(built["run_my_tool"], qualified_name="Github.ListIssues", arguments={}, + state=state()) + invoke(built["run_my_tool"], qualified_name="Github.DeleteRepo", arguments={}, + state=state()) + + checked = [name for name, _kwargs in tools.get_calls] + assert "Github.ListIssues" in checked + assert "Github.DeleteRepo" in checked + + +# --- execution: the approval gate --- + + +def test_a_read_runs_without_asking_anybody(monkeypatch): + asked = [] + monkeypatch.setattr( + "arcade_tools.tools.require_write_confirmation", + lambda **kwargs: asked.append(kwargs) or True, + ) + built, tools = build( + {"Github": [definition("Github.ListIssues", behavior=READ_BEHAVIOR)]} + ) + + invoke(built["run_my_tool"], qualified_name="Github.ListIssues", arguments={}, + state=state()) + + assert asked == [] + assert len(tools.executed) == 1 + + +@pytest.mark.parametrize( + "behavior,expected", + [(WRITE_BEHAVIOR, "write"), (DESTRUCTIVE_BEHAVIOR, "destructive"), (None, "destructive")], +) +def test_a_non_read_asks_and_carries_its_effect_to_the_card( + monkeypatch, behavior, expected +): + asked = [] + monkeypatch.setattr( + "arcade_tools.tools.require_write_confirmation", + lambda **kwargs: asked.append(kwargs) or True, + ) + built, _tools = build( + {"Github": [definition("Github.DoThing", behavior=behavior)]} + ) + + invoke(built["run_my_tool"], qualified_name="Github.DoThing", arguments={}, + state=state()) + + assert len(asked) == 1 + assert asked[0]["extra_args"]["effect"] == expected + + +def test_a_declined_card_executes_nothing(monkeypatch): + monkeypatch.setattr( + "arcade_tools.tools.require_write_confirmation", lambda **kwargs: False + ) + built, tools = build( + {"Github": [definition("Github.DoThing", behavior=DESTRUCTIVE_BEHAVIOR)]} + ) + + result = invoke(built["run_my_tool"], qualified_name="Github.DoThing", + arguments={}, state=state()) + + assert tools.executed == [] + assert "declined" in str(result).lower() + + +def test_an_approved_card_executes_exactly_once(monkeypatch): + monkeypatch.setattr( + "arcade_tools.tools.require_write_confirmation", lambda **kwargs: True + ) + built, tools = build( + {"Github": [definition("Github.DoThing", behavior=DESTRUCTIVE_BEHAVIOR)]} + ) + + invoke(built["run_my_tool"], qualified_name="Github.DoThing", arguments={}, + state=state()) + + assert len(tools.executed) == 1 + + +def test_a_personal_call_names_its_approver(monkeypatch): + # A colleague approving somebody else's call would spend that person's + # access. The agent can only say whose call it is; the surface enforces it. + asked = [] + monkeypatch.setattr( + "arcade_tools.tools.require_write_confirmation", + lambda **kwargs: asked.append(kwargs) or True, + ) + built, _tools = build( + {"Gmail": [definition("Gmail.SendMail", behavior=DESTRUCTIVE_BEHAVIOR)]} + ) + + invoke(built["run_my_tool"], qualified_name="Gmail.SendMail", arguments={}, + state=state(actor_id="U1")) + + assert asked[0]["extra_args"]["approver"] == "slack:U1" + + +def test_a_shared_call_names_no_approver(monkeypatch): + # Anybody may approve an action that runs as the team account, because it + # spends nobody's personal access. + asked = [] + monkeypatch.setattr( + "arcade_tools.tools.require_write_confirmation", + lambda **kwargs: asked.append(kwargs) or True, + ) + built, _tools = build( + {"Github": [definition("Github.DoThing", behavior=DESTRUCTIVE_BEHAVIOR)]} + ) + + invoke(built["run_my_tool"], qualified_name="Github.DoThing", arguments={}, + state=state()) + + assert asked[0]["extra_args"]["approver"] is None + + +def test_approvals_off_runs_a_destructive_action_without_a_card(monkeypatch): + asked = [] + monkeypatch.setattr( + "arcade_tools.tools.require_write_confirmation", + lambda **kwargs: asked.append(kwargs) or True, + ) + built, tools = build( + {"Github": [definition("Github.DoThing", behavior=DESTRUCTIVE_BEHAVIOR)]}, + approvals="off", + ) + + invoke(built["run_my_tool"], qualified_name="Github.DoThing", arguments={}, + state=state()) + + assert asked == [] + assert len(tools.executed) == 1 + + +def test_connecting_an_account_is_not_approval_to_write(monkeypatch): + # The two questions are different: "may this agent use your account" and + # "do this specific thing now". A connected account still gets a card. + asked = [] + monkeypatch.setattr( + "arcade_tools.tools.require_write_confirmation", + lambda **kwargs: asked.append(kwargs) or True, + ) + built, _tools = build( + {"Gmail": [definition("Gmail.SendMail", behavior=DESTRUCTIVE_BEHAVIOR)]}, + requirements_met=True, + ) + + invoke(built["run_my_tool"], qualified_name="Gmail.SendMail", arguments={}, + state=state(actor_id="U1")) + + assert len(asked) == 1 diff --git a/agent/tests/test_connected_app_contract.py b/agent/tests/test_connected_app_contract.py new file mode 100644 index 00000000..2cd0035e --- /dev/null +++ b/agent/tests/test_connected_app_contract.py @@ -0,0 +1,144 @@ +"""Rules both connected-app providers must obey, asserted against both. + +There are two implementations of the same two tools, on purpose: the providers +reach their catalogues through different calls with different failure shapes, +and a single implementation would be a parameterised branch everywhere and a +shared abstraction nowhere. + +The cost of that choice is drift, and this file is the insurance. The rules +below are the ones where drift is dangerous rather than untidy — what a model is +allowed to decide, whose account a call runs in, and what happens to an action +nobody classified. A change to one provider that quietly relaxes one of these +goes red here rather than being discovered by whoever it happens to. + +It reuses each provider's own fakes rather than introducing a third set, so a +rule asserted here is asserted against the same objects that suite already +trusts. It deliberately does not introduce a shared base class: the point is to +pin what the two must agree on without making them share code they should not. +""" + +from __future__ import annotations + +import pytest + +from composio_tools.classify import DESTRUCTIVE, READ, WRITE, needs_approval + +from tests import test_arcade_tools as arcade_suite +from tests import test_composio_tools as composio_suite + +PROVIDERS = ("composio", "arcade") + + +def pair_for(provider: str) -> dict: + """Both tools from one provider, by name, with a minimal configuration.""" + if provider == "arcade": + built, _tools = arcade_suite.build( + {"Github": [arcade_suite.definition("Github.DoThing")]} + ) + return built + search, run, _client = composio_suite.tools_for({}) + return {"search_my_tools": search, "run_my_tool": run} + + +@pytest.mark.parametrize("provider", PROVIDERS) +def test_both_providers_register_the_same_two_tool_names(provider): + # The system prompt names these, and only one provider is ever registered. + # If the names diverged, the prompt would be right for one deployment and + # wrong for the other, with nothing to say which. + assert set(pair_for(provider)) == {"search_my_tools", "run_my_tool"} + + +@pytest.mark.parametrize("provider", PROVIDERS) +def test_neither_provider_lets_the_model_choose_an_identity(provider): + # The defect this prevents is the same on both sides: an identity the model + # fills in is an identity it can be talked into changing, and the first + # thing it would be asked to change is whose mailbox to open. + for tool in pair_for(provider).values(): + fields = set(tool.args_schema.model_fields) + assert not fields & {"user_id", "account", "actor", "approver", "provider"} + + +@pytest.mark.parametrize("provider", PROVIDERS) +def test_neither_provider_takes_its_approver_from_the_model(provider): + # Who may answer a card decides whose access a colleague can spend. It is + # derived from the forwarded actor on both sides, never from an argument. + assert "approver" not in set( + pair_for(provider)["run_my_tool"].args_schema.model_fields + ) + + +def test_an_unclassified_action_is_destructive_on_both_sides(): + # "Nobody said" is not "nothing dangerous". Both providers answer + # `destructive` for an action carrying no behaviour information, and a + # provider that started calling it a write would run it unasked under the + # default mode. + from arcade_tools.effects import effect_of_definition + + arcade_answer = effect_of_definition({"qualified_name": "Thing.Unclassified"}) + composio_answer = composio_suite.FakeEffects().effect_for("UNCLASSIFIED_SLUG") + + assert arcade_answer == composio_answer == DESTRUCTIVE + + +def test_the_gate_rule_is_shared_rather_than_reimplemented(): + # Both providers route through this one function. Asserted directly so a + # second copy of the rule cannot appear in one provider without this going + # red alongside it. + assert needs_approval(READ, "on") is False + assert needs_approval(WRITE, "on") is True + assert needs_approval(DESTRUCTIVE, "on") is True + assert needs_approval(DESTRUCTIVE, "off") is False + assert needs_approval(READ, "off") is False + + +def test_both_providers_import_the_one_approval_card(): + # A second card would look almost the same and behave slightly differently, + # which is the failure the first Composio branch was rewritten to avoid. + import arcade_tools.tools as arcade_module + import composio_tools.tools as composio_module + import write_confirmation + + assert ( + arcade_module.require_write_confirmation + is write_confirmation.require_write_confirmation + ) + assert ( + composio_module.require_write_confirmation + is write_confirmation.require_write_confirmation + ) + + +def test_both_providers_read_identity_through_the_one_actor_reader(): + # The identity boundary stays in one place while a second provider is + # added. Two readers would mean two answers to "who spoke", and the + # dangerous one is whichever is consulted second. + import arcade_tools.tools as arcade_module + import composio_tools.state as state_module + import composio_tools.tools as composio_module + + assert arcade_module.actor_of is state_module.actor_of + assert composio_module.actor_of is state_module.actor_of + assert arcade_module.actor_key is state_module.actor_key + assert composio_module.actor_key is state_module.actor_key + + +def test_only_arcade_can_express_an_ordinary_write(): + # Not a rule both obey — a difference worth pinning so it stays a decision + # rather than becoming a surprise. Composio's MCP tags cannot say "changes + # something, destroys nothing", which is why its approval modes collapsed + # into one; Arcade tools can, so an ordinary write need not be painted like + # a deletion. + from arcade_tools.effects import effect_of_definition + from composio_tools.classify import effect_of + + arcade_write = effect_of_definition( + { + "qualified_name": "Thing.Update", + "metadata": {"behavior": {"read_only": False, "destructive": False}}, + } + ) + + assert arcade_write == WRITE + # The Composio vocabulary has no input that produces it. + assert effect_of({"readOnlyHint": False}) is None + assert effect_of({"destructiveHint": False}) is None diff --git a/agent/tests/test_connected_app_selection.py b/agent/tests/test_connected_app_selection.py index aa6f7731..fc106c62 100644 --- a/agent/tests/test_connected_app_selection.py +++ b/agent/tests/test_connected_app_selection.py @@ -162,29 +162,43 @@ def test_both_keys_fail_the_boot_before_a_provider_client_exists(monkeypatch): _build_agent_recording_runtime_calls(monkeypatch) -def test_an_arcade_deployment_builds_no_composio_runtime(monkeypatch): +def _select_arcade(monkeypatch): monkeypatch.delenv("COMPOSIO_API_KEY", raising=False) monkeypatch.setenv("ARCADE_API_KEY", "arc_test") + monkeypatch.setenv("ARCADE_TOOLKITS", "Github") + + +def test_an_arcade_deployment_builds_no_composio_runtime(monkeypatch): + _select_arcade(monkeypatch) captured = _build_agent_recording_runtime_calls(monkeypatch) assert captured["runtime_calls"] == 0 -def test_an_arcade_deployment_says_its_provider_is_not_implemented_yet( - monkeypatch, caplog -): - # The interim state between selecting Arcade and implementing it. Registering - # nothing in silence would be indistinguishable from an agent that has no - # connected apps configured at all, which is a different and wrong answer to - # give a deployer who just set a key. - monkeypatch.delenv("COMPOSIO_API_KEY", raising=False) - monkeypatch.setenv("ARCADE_API_KEY", "arc_test") +def test_an_arcade_deployment_registers_the_connected_app_tools(monkeypatch): + # The pair is named identically by both providers, which is safe only + # because exactly one of them is ever registered. This asserts the Arcade + # half actually arrives rather than the agent quietly having none. + _select_arcade(monkeypatch) - with caplog.at_level(logging.WARNING): - _build_agent_recording_runtime_calls(monkeypatch) + captured = _build_agent_recording_runtime_calls(monkeypatch) - assert "not implemented yet" in caplog.text + registered = {getattr(item, "name", None) for item in captured["agent"]["tools"]} + assert {"search_my_tools", "run_my_tool"} <= registered + + +def test_an_arcade_deployment_is_never_told_it_has_no_connected_apps(monkeypatch): + # The plan's explicit requirement: the Composio absence statement must not + # reach an Arcade deployment, because it is false and stops the model + # looking. + _select_arcade(monkeypatch) + + captured = _build_agent_recording_runtime_calls(monkeypatch) + prompt = captured["agent"]["system_prompt"] + + assert "Github" in prompt + assert "no connected apps" not in prompt.lower() def test_a_composio_deployment_still_builds_its_runtime(monkeypatch): @@ -237,3 +251,42 @@ def test_selection_touches_no_provider_sdk(): assert "composio_tools" not in imported assert "arcadepy" not in imported assert "arcade_tools" not in imported + + +# --- why a pending approval cannot outlive a provider switch ----------------- +# +# The plan asked that provider binding reach the graph, not only the Connect +# card: an approval paused under one provider must never resume against the +# other. In this architecture that cannot happen, for two reasons that hold +# together and neither of which holds alone: +# +# * the provider is chosen once, when the graph is built, so switching it means +# a restart; and +# * pending approvals live in an in-memory checkpointer, so a restart discards +# every one of them. +# +# Nothing resumable survives the switch. The first half is structural — the +# tool list is handed to the graph once and the graph does not re-read it — so +# the test below pins the second, which is the one a plausible change breaks. +# The day someone swaps in a durable checkpointer, it fails, and that is the +# moment an approval needs to carry the provider that paused it and refuse to +# resume under another. + + +def test_pending_approvals_do_not_survive_a_restart(monkeypatch): + """Tripwire: read the block above before changing what this asserts. + + If this fails because the checkpointer is now durable, pending approvals + survive a restart — and therefore survive a provider switch. Before making + this pass again, stamp the provider into each approval and refuse a resume + whose provider is not the one currently selected. + """ + from langgraph.checkpoint.memory import MemorySaver + + _select_arcade(monkeypatch) + captured = _build_agent_recording_runtime_calls(monkeypatch) + + assert isinstance(captured["agent"]["checkpointer"], MemorySaver), ( + "The checkpointer is no longer in-memory, so a pending approval can now " + "outlive a provider switch. See the comment above these tests." + ) From b39c04e648400a68d16c9a09a964b866d63e8e66 Mon Sep 17 00:00:00 2001 From: Maxim Date: Wed, 23 Sep 2026 03:06:07 +0200 Subject: [PATCH 05/15] feat(arcade): connect a person's account from the Connect button Arcade's own sign-in only works for members of the Arcade project, which the colleagues this feature exists for are not. So OpenTag runs its own verifier: Arcade redirects the browser back to us and we tell it whose authorization this is. The browser arrives with no Slack context, so the identity comes from what was recorded when the flow started. That record is the whole security of the feature. It is kept server-side, never carried in a URL, never taken from the request, expires, is bounded, and is answered once. Arcade's verifier flow id cannot be mapped back to the authorization it belongs to, so the browser gets a session. The link a person receives points at OpenTag; passing through, the browser collects an opaque HttpOnly, Secure, SameSite=Lax cookie in exchange for a single-use ticket, and that cookie answers the question on the way back. Lax is required: the return leg is a cross-site navigation and Strict withholds the cookie. Proven end to end with a person outside the Arcade project. The browser-facing pages live on the runtime, so the process holding the provider keys keeps no public entry point beyond its health probe. What crosses the private network is an opaque handle; only the agent resolves it to an identity. The Connect button carries an Arcade action, since Arcade authorizes per action rather than per app, validated separately from a Composio app name because the string comes from the model and is rendered into a public post. The card records which provider minted it, so a card that outlives a provider change refuses itself. The search reports which personal apps still need connecting, asked once per app, so the model has a reason to offer the button before an action fails. The connect route requires the shared secret unconditionally; the verifier route is public because it carries no authority. Co-Authored-By: Claude Opus 5.5 --- agent/agent_auth.py | 5 + agent/arcade_tools/connect.py | 146 ++++++ agent/arcade_tools/runtime.py | 8 +- agent/arcade_tools/tools.py | 91 +++- agent/arcade_tools/verify.py | 266 +++++++++++ agent/main.py | 224 +++++++++- agent/tests/test_arcade_connect.py | 255 +++++++++++ agent/tests/test_arcade_routes.py | 415 ++++++++++++++++++ agent/tests/test_arcade_tools.py | 87 ++++ agent/tests/test_arcade_verify.py | 335 ++++++++++++++ app/__tests__/arcade-browser-routes.test.ts | 346 +++++++++++++++ app/arcade-browser-routes.ts | 267 +++++++++++ app/env.test.ts | 1 + app/env.ts | 30 ++ .../__tests__/connect-account.test.tsx | 4 +- app/human-in-the-loop/connect-account.tsx | 69 ++- app/runtime-host.ts | 48 +- app/tools/__tests__/arcade-connect.test.ts | 256 +++++++++++ app/tools/__tests__/connect-app.test.tsx | 6 +- .../__tests__/connect-arcade-path.test.tsx | 197 +++++++++ app/tools/__tests__/connect-click.test.tsx | 49 ++- app/tools/arcade-connect.ts | 201 +++++++++ app/tools/connect-app.tsx | 47 +- app/tools/connect-click.tsx | 68 ++- 24 files changed, 3332 insertions(+), 89 deletions(-) create mode 100644 agent/arcade_tools/connect.py create mode 100644 agent/arcade_tools/verify.py create mode 100644 agent/tests/test_arcade_connect.py create mode 100644 agent/tests/test_arcade_routes.py create mode 100644 agent/tests/test_arcade_verify.py create mode 100644 app/__tests__/arcade-browser-routes.test.ts create mode 100644 app/arcade-browser-routes.ts create mode 100644 app/tools/__tests__/arcade-connect.test.ts create mode 100644 app/tools/__tests__/connect-arcade-path.test.tsx create mode 100644 app/tools/arcade-connect.ts diff --git a/agent/agent_auth.py b/agent/agent_auth.py index 0f3d9182..c1ea422d 100644 --- a/agent/agent_auth.py +++ b/agent/agent_auth.py @@ -35,6 +35,11 @@ #: as a secret was configured. Listed rather than reached by collapsing #: repeated slashes in `_public_path`, because that would quietly make every #: path with a doubled slash a different path from the one written here. +#: +#: Still only the health probe. The Arcade connect flow needs two routes a +#: browser can reach, and they live on the surface rather than here — so this +#: service keeps no public entry point, and the process holding the provider +#: keys is not the one exposed to the internet. PUBLIC_PATHS = frozenset({"/health", "//health"}) diff --git a/agent/arcade_tools/connect.py b/agent/arcade_tools/connect.py new file mode 100644 index 00000000..df411f48 --- /dev/null +++ b/agent/arcade_tools/connect.py @@ -0,0 +1,146 @@ +"""Starting one person's account connection. + +A connect link is a bearer capability: whoever opens it binds their account to +the identity it was minted for. So it is minted per clicker, on demand, handed +straight back to the surface for private delivery, and never shown to the model +or written to a log. + +Two things differ from the Composio path, both because Arcade does: + +* **Authorization is per action.** `Gmail.SendMail` and `Gmail.ListMail` are + separate grants, which is the point — a person connecting so the agent can + read their mail is not thereby agreeing it may send any. The target therefore + names an action, and the scopes Arcade asks for are that action's. +* **Starting a flow records who it belongs to.** Arcade will later send a + browser back to the verifier carrying nothing but a flow id, and that record + is the only thing that can say whose authorization it is. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from typing import Any + +from arcade_tools.catalog import owns +from arcade_tools.config import ArcadeConfig +from arcade_tools.identity import arcade_user_id +from arcade_tools.verify import PendingFlows + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class ConnectRefused: + """Why no link was minted, in words an operator or a person can act on. + + Carries no `url` field at all, so a caller cannot reach for one on the + refusal path and find something stale. + """ + + reason: str + + +@dataclass(frozen=True) +class ConnectStarted: + """A flow that is now waiting on somebody.""" + + #: The opaque ticket that goes in the link handed to that person. `None` + #: when the account was already connected and no link was needed. + #: + #: Not the provider's URL. The person is sent to us first so their browser + #: can collect the cookie that identifies them on the way back — see + #: `arcade_tools/verify.py` for why that hop is not optional. + ticket: str | None + already_connected: bool = False + + +def start_connection( + config: ArcadeConfig, + client_factory, + pending: PendingFlows, + *, + identity: Any, + target: Any, +) -> ConnectStarted | ConnectRefused: + """Begin connecting `identity`'s own account for the action `target`. + + `identity` is the platform-namespaced actor key of whoever clicked — the + same value a turn uses to pick that person's account. A link minted against + anything else connects an account the agent will never look at again. + """ + actor = identity.strip() if isinstance(identity, str) else "" + if not actor: + # Found on the Composio side by a live run rather than a unit test: a + # blank id minted a real link bound to an identity nothing resolves to. + return ConnectRefused(reason="No person was named.") + + action = target.strip() if isinstance(target, str) else "" + if not action or "." not in action: + return ConnectRefused(reason="No action was named.") + + if not owns(config.user_toolkits, action): + if owns(config.workspace_toolkits, action): + return ConnectRefused( + reason=( + "That app is shared by the whole workspace, so it is " + "connected once by whoever runs this deployment — not from " + "here." + ) + ) + return ConnectRefused( + reason="That is not one of the apps people connect for themselves." + ) + + user_id = arcade_user_id(config.identity_namespace, actor) + + try: + authorization = client_factory().tools.authorize( + tool_name=action, user_id=user_id + ) + except Exception as error: # noqa: BLE001 - provider errors vary + # The action and the failure, never the identity and never a link. + logger.warning( + "[arcade] could not start a %s connection: %s", action, error + ) + return ConnectRefused( + reason=f"Could not start the {action.split('.')[0]} connection. " + "Try again shortly." + ) + + status = _field(authorization, "status") + url = _text(_field(authorization, "url")) + + if status == "completed": + # Already connected. Sending them round the provider again would work, + # but it asks somebody to do something they have already done. + return ConnectStarted(ticket=None, already_connected=True) + + if not url: + logger.warning("[arcade] %s authorization returned no link", action) + return ConnectRefused( + reason=f"Could not start the {action.split('.')[0]} connection. " + "Try again shortly." + ) + + # The authorization's own id is deliberately not recorded against anything. + # It is not the id the verifier is later given — that was established + # against the live API — so a record keyed on it would never be found. + try: + ticket = pending.issue_ticket(identity=user_id, provider_url=url) + except ValueError as error: + return ConnectRefused(reason=str(error)) + + # The provider URL is never handed out and never logged. It stays here, and + # the person is sent to us with nothing but an opaque ticket. + return ConnectStarted(ticket=ticket) + + +def _field(node: Any, key: str) -> Any: + if isinstance(node, dict): + return node.get(key) + return getattr(node, key, None) + + +def _text(value: Any) -> str: + return value.strip() if isinstance(value, str) else "" diff --git a/agent/arcade_tools/runtime.py b/agent/arcade_tools/runtime.py index f37aada7..6a808805 100644 --- a/agent/arcade_tools/runtime.py +++ b/agent/arcade_tools/runtime.py @@ -14,11 +14,12 @@ import logging import threading from collections.abc import Mapping -from dataclasses import dataclass +from dataclasses import dataclass, field from typing import Any from arcade_tools.catalog import Catalog from arcade_tools.config import ArcadeConfig, read_arcade_config, startup_warnings +from arcade_tools.verify import PendingFlows logger = logging.getLogger(__name__) @@ -39,6 +40,11 @@ class ArcadeRuntime: config: ArcadeConfig catalog: Catalog client_factory: Any + #: Flows started but not yet finished at the provider. Lives on the runtime + #: for the same reason the runtime exists: the connect route records a flow + #: and the verifier route resolves it, and they must be looking at one set. + #: Two would mean every connection failing verification. + pending_flows: PendingFlows = field(default_factory=PendingFlows) def _build_client(api_key: str): diff --git a/agent/arcade_tools/tools.py b/agent/arcade_tools/tools.py index 1b9e1ace..7e7db0c9 100644 --- a/agent/arcade_tools/tools.py +++ b/agent/arcade_tools/tools.py @@ -98,6 +98,10 @@ def search_my_tools( ) -> dict[str, Any] | str: """Find actions available in the connected apps. Call this before run_my_tool. + If the result has `needsConnection`, those apps are not connected for + this person yet. Call `connect_app` naming one action from that app, and + do not call `run_my_tool` for it until they say they have connected. + Args: query: What you want to do, in plain words, e.g. 'create an issue'. """ @@ -116,23 +120,72 @@ def search_my_tools( logger.warning("[arcade] search failed: %s", error) return "The connected apps could not be reached just now." - payload: dict[str, Any] = { - "actions": [ - { - "qualifiedName": item.get("qualified_name"), - "description": item.get("description") or "", - "arguments": _argument_schema(item), - # So the model can tell the person what will happen before - # it calls, rather than the card being the first they hear. - "effect": effect_of_definition(item), - } - for item in found - ] - } + actions = [ + { + "qualifiedName": item.get("qualified_name"), + "description": item.get("description") or "", + "arguments": _argument_schema(item), + # So the model can tell the person what will happen before it + # calls, rather than the card being the first they hear. + "effect": effect_of_definition(item), + } + for item in found + ] + payload: dict[str, Any] = {"actions": actions} + + # Which of these the person has not connected yet, named here rather + # than discovered by running one and failing. Without it the model has + # no reason to offer the Connect button, so somebody's first sign that + # an app needs connecting is an action that refuses to run. + needs_connection = _unconnected_toolkits(identities, actions) + if needs_connection: + payload["needsConnection"] = needs_connection + if key is None and config.user_toolkits: payload["personalAppsUnavailable"] = anonymous_note() return payload + def _unconnected_toolkits(identities, actions: list[dict]) -> list[str]: + """Personal apps among these results that this person has not connected. + + Asked once per app rather than once per action. Authorization itself is + per action, but "has never begun connecting" is a fact about a person + and a provider, so one probe answers for every action in the app — and + a search returning twenty actions must not cost twenty round trips. + + Shared apps are never reported: nobody presses Connect for those, and + offering the button would send somebody to bind their own account where + every call runs as the team. + """ + personal = identities.personal + if not personal: + return [] + + seen: dict[str, str] = {} + for action in actions: + name = action.get("qualifiedName") + toolkit = _toolkit_of_optional(name, tuple(personal)) + if toolkit is not None and toolkit not in seen and name: + seen[toolkit] = name + + unconnected: list[str] = [] + for toolkit, probe in seen.items(): + try: + state = catalog.authorization_for(probe, personal[toolkit]) + except Exception as error: # noqa: BLE001 - provider errors vary + # Not knowing is not the same as not connected. Saying "connect + # this" to somebody who already has would send them round a + # flow they have already completed. + logger.warning( + "[arcade] could not check whether %s is connected: %s", + toolkit, + error, + ) + continue + if not state.connected: + unconnected.append(toolkit) + return unconnected + @tool def run_my_tool( qualified_name: str, @@ -246,6 +299,18 @@ def _report(result, label: str, qualified_name: str, gated: bool) -> str: return [search_my_tools, run_my_tool] +def _toolkit_of_optional( + qualified_name: Any, candidates: tuple[str, ...] +) -> str | None: + """`_toolkit_of` for callers filtering rather than routing.""" + if not isinstance(qualified_name, str): + return None + try: + return _toolkit_of(qualified_name, candidates) + except LookupError: + return None + + def _toolkit_of(qualified_name: str, candidates: tuple[str, ...]) -> str: """The configured spelling of this action's toolkit. diff --git a/agent/arcade_tools/verify.py b/agent/arcade_tools/verify.py new file mode 100644 index 00000000..3f0c6398 --- /dev/null +++ b/agent/arcade_tools/verify.py @@ -0,0 +1,266 @@ +"""Knowing who is at the browser when Arcade sends somebody back. + +Arcade's own sign-in only works for members of the Arcade project, which the +colleagues this feature exists for are not. Production needs a verifier of ours: +Arcade redirects the browser to us carrying a flow id, and we tell it whose +authorization that is. + +**Why this is not simply a lookup.** Established against the live API rather +than assumed: the id Arcade hands us when we start an authorization and the flow +id it later hands the verifier are different id spaces. Asking Arcade about the +flow id is refused as an invalid authorization id, and no endpoint maps one to +the other. The custom verifier is built for ordinary web apps, where the answer +comes from a logged-in session. OpenTag has no sessions; it lives in Slack. + +**So the flow gains one hop and the browser gains a session.** The link handed +to a person points at us, not at the provider. Passing through, their browser +collects a cookie, and then they go on to the provider. When Arcade sends them +back, the cookie is still there — same browser, same site — and that is what +answers the question. + +Two short-lived stores, each keyed on an opaque random value and neither holding +anything a reader could learn from: + +* **Tickets** are the value in the link. Single use: spent on the way out, in + exchange for the cookie. +* **Browsers** are the value in the cookie. It never appears in a URL, so it is + not in history, not in a server log, and not forwardable by pasting. + +Nothing identifying travels in either. The mapping to a person stays here. +""" + +from __future__ import annotations + +import enum +import logging +import secrets +import threading +import time +from collections import OrderedDict +from collections.abc import Mapping +from dataclasses import dataclass +from typing import Any + +logger = logging.getLogger(__name__) + +#: How long somebody has between clicking Connect and finishing at the provider. +#: Long enough to find a password, short enough that an abandoned attempt is not +#: a claim on an identity that sits around all afternoon. +DEFAULT_TTL_SECONDS = 15 * 60 + +#: Bounded because this lives in process memory: whoever can start connections +#: must not be able to grow it without limit. +DEFAULT_MAX_PENDING = 512 + +#: Boring, and says nothing about who. Its value is random; its name should not +#: be the part that leaks a purpose. +COOKIE_NAME = "otc_session" + +#: Scoped, so the cookie is not attached to every other request this service +#: serves — only to the two routes that need it. +COOKIE_PATH = "/arcade" + + +def _token() -> str: + return secrets.token_urlsafe(32) + + +@dataclass(frozen=True) +class PendingConnection: + """One person, part-way through connecting one account.""" + + identity: str + #: Where to send them next. Held here rather than in the link so the link + #: carries nothing but an opaque ticket. + provider_url: str + + +class _Expiring: + """A small bounded store whose entries go stale. + + Deliberately in memory. The lifetime is minutes, the agent is one process, + and a record that survived a restart would be a claim on an identity + outliving every other trace of the click that made it. + + The honest cost: restart mid-flow and whoever is at the provider comes back + to something nobody remembers. They are told to start again, which is the + right answer — the alternative is guessing whose account to bind. + """ + + def __init__(self, *, clock, ttl_seconds: int, max_pending: int) -> None: + self._clock = clock + self._ttl = ttl_seconds + self._max = max_pending + self._items: OrderedDict[str, tuple[Any, float]] = OrderedDict() + # Two clicks land in a FastAPI threadpool at once in ordinary use. + self._lock = threading.Lock() + + def put(self, key: str, value: Any) -> None: + with self._lock: + self._drop_expired() + existing = self._items.get(key) + if existing is not None and existing[0] != value: + raise ValueError("That connection is already pending for somebody else.") + self._items[key] = (value, self._clock() + self._ttl) + self._items.move_to_end(key) + while len(self._items) > self._max: + # Oldest first: the longest-waiting is the most likely abandoned. + self._items.popitem(last=False) + + def take(self, key: Any) -> Any: + """Read and consume. `None` when there is nothing to read. + + Consumed on read rather than on success, throughout. A failure whose + cause cannot be established is indistinguishable from a success whose + reply was lost, so leaving a value spendable would permit a replay in + exactly the case nobody can reason about. + """ + if not isinstance(key, str) or not key: + return None + with self._lock: + self._drop_expired() + entry = self._items.pop(key, None) + return None if entry is None else entry[0] + + def _drop_expired(self) -> None: + now = self._clock() + for key in [k for k, (_v, deadline) in self._items.items() if deadline <= now]: + del self._items[key] + + +class PendingFlows: + """Everything in flight: links handed out, and browsers part-way through.""" + + def __init__( + self, + *, + clock=time.monotonic, + ttl_seconds: int = DEFAULT_TTL_SECONDS, + max_pending: int = DEFAULT_MAX_PENDING, + ) -> None: + shape = {"clock": clock, "ttl_seconds": ttl_seconds, "max_pending": max_pending} + self._tickets = _Expiring(**shape) + self._browsers = _Expiring(**shape) + + # --- the link handed to one person --- + + def issue_ticket(self, *, identity: str, provider_url: str) -> str: + """Mint the opaque value that goes in the link.""" + ticket = _token() + self._tickets.put( + ticket, PendingConnection(identity=identity, provider_url=provider_url) + ) + return ticket + + def claim_ticket(self, ticket: Any) -> PendingConnection | None: + """Spend a ticket on the way out. One use only.""" + return self._tickets.take(ticket) + + # --- the cookie their browser carries --- + + def remember_browser(self, identity: str) -> str: + """Mint the cookie value for a browser now on its way to the provider.""" + handle = _token() + self._browsers.put(handle, identity) + return handle + + def identity_for_browser(self, handle: Any) -> str | None: + """Whose browser this is, consuming it. `None` when we do not know.""" + return self._browsers.take(handle) + + +class VerifyOutcome(enum.Enum): + CONFIRMED = "confirmed" + #: No cookie, an expired one, or no flow id at all. One outcome, because to + #: whoever is looking they are one thing: this is not your connection. + UNKNOWN = "unknown" + FAILED = "failed" + + +@dataclass(frozen=True) +class VerifyResult: + outcome: VerifyOutcome + redirect_to: str | None = None + #: Shown to whoever arrived, who may not be the person the flow belongs to, + #: so it names nobody and no account. + message: str = "" + #: True once the cookie has done its job and should be cleared. + clear_cookie: bool = False + + +_MESSAGES = { + VerifyOutcome.CONFIRMED: "Connected. You can close this tab and go back to the chat.", + VerifyOutcome.UNKNOWN: ( + "This connection could not be completed in this browser. Ask again in " + "the chat and open the new link in the same browser you finish in." + ), + VerifyOutcome.FAILED: ( + "Something went wrong finishing this connection. Ask again in the chat " + "to start over." + ), +} + + +def verify_flow( + pending: PendingFlows, + client_factory, + *, + flow_id: Any, + browser_handle: Any, +) -> VerifyResult: + """Confirm to Arcade whose authorization `flow_id` is. + + Takes the flow id from Arcade and the identity from the browser's own + cookie. There is no argument for an identity a caller could supply, and a + test asserts one never appears: a verifier that accepts a user id from its + request lets anybody bind anybody's account by editing a URL. + """ + identifier = flow_id.strip() if isinstance(flow_id, str) else "" + identity = pending.identity_for_browser(browser_handle) + + if not identifier or identity is None: + # Ordinary enough to log without alarm: an expired attempt, a refreshed + # tab, a different browser, or somebody who found the route. + logger.info( + "[arcade] a verifier request could not be matched to a browser we " + "are waiting on" + ) + return _result(VerifyOutcome.UNKNOWN, clear_cookie=True) + + try: + response = client_factory().auth.confirm_user( + flow_id=identifier, user_id=identity + ) + except Exception as error: # noqa: BLE001 - provider errors vary + # The failure, never the identity: whose account it was is not needed to + # act on a provider error. + logger.warning("[arcade] could not confirm a verifier flow: %s", error) + return _result(VerifyOutcome.FAILED, clear_cookie=True) + + return _result( + VerifyOutcome.CONFIRMED, + redirect_to=_next_uri(response), + clear_cookie=True, + ) + + +def _next_uri(response: Any) -> str | None: + if isinstance(response, Mapping): + value = response.get("next_uri") + else: + value = getattr(response, "next_uri", None) + return value if isinstance(value, str) and value else None + + +def _result( + outcome: VerifyOutcome, + *, + redirect_to: str | None = None, + clear_cookie: bool = False, +) -> VerifyResult: + return VerifyResult( + outcome=outcome, + redirect_to=redirect_to, + message=_MESSAGES[outcome], + clear_cookie=clear_cookie, + ) diff --git a/agent/main.py b/agent/main.py index 18bac36b..744edc55 100644 --- a/agent/main.py +++ b/agent/main.py @@ -14,7 +14,15 @@ from agent import build_agent from agent_auth import authorizes_capability, configured_secret, is_authorized from agui import AGENT_DESCRIPTION, AGENT_NAME, build_agui_agent -from connected_app_provider import PROVIDER_COMPOSIO, selected_provider +from arcade_tools.connect import ConnectRefused as ArcadeConnectRefused +from arcade_tools.connect import start_connection +from arcade_tools.runtime import arcade_runtime +from arcade_tools.verify import verify_flow +from connected_app_provider import ( + PROVIDER_ARCADE, + PROVIDER_COMPOSIO, + selected_provider, +) from composio_tools.config import DEFAULT_WORKSPACE_USER_ID from composio_tools.connect import ConnectRefused, connect_link from composio_tools.runtime import composio_runtime @@ -179,6 +187,220 @@ def composio_connect(body: ConnectRequest, request: Request): return {"redirectUrl": result.url} +class ArcadeConnectRequest(BaseModel): + """One person, one action. No link comes in; exactly one goes out. + + `target` rather than `toolkit`, because Arcade authorizes per action: the + scopes it asks for are the ones that action needs, not everything the app + could ever do. + """ + + actor_id: str + platform: str + target: str + #: The clicker's `ProviderActor.kind`, refused when absent for the same + #: reason as the Composio route: a runtime too old to send it cannot say + #: whether a person clicked, and "I could not tell" is not a reason to mint + #: a bearer capability. + kind: str | None = None + + +@app.post("/arcade/connect") +def arcade_connect(body: ArcadeConnectRequest, request: Request): + """Start connecting one person's own account. + + The same shape and the same rules as the Composio route beside it, because + what makes a connect link dangerous is not which provider minted it. + """ + if configured_secret() is None: + print( + "[ERROR] /arcade/connect refused: no AGENT_AUTH_HEADER is set on " + "the agent, so it has no secret to check and will mint nothing", + file=sys.stderr, + ) + return JSONResponse( + { + "error": "Connecting your own account needs a shared secret set " + "on both this app and its agent, and the agent has not set one. " + "Ask whoever runs this deployment." + }, + status_code=503, + ) + if not authorizes_capability(request.headers.get("authorization")): + return JSONResponse({"error": "unauthorized"}, status_code=401) + + runtime = _selected_arcade_runtime() + if runtime is None: + return JSONResponse( + {"error": "Arcade is not configured on this deployment."}, + status_code=503, + ) + + actor = { + "id": body.actor_id, + "platform": body.platform, + "kind": body.kind, + } + identity = actor_key(actor) if is_personal_kind(actor) else None + if identity is None: + return JSONResponse({"error": actor_refusal(actor)}, status_code=400) + + result = start_connection( + runtime.config, + runtime.client_factory, + runtime.pending_flows, + identity=identity, + target=body.target, + ) + if isinstance(result, ArcadeConnectRefused): + return JSONResponse({"error": result.reason}, status_code=400) + if result.already_connected: + return {"alreadyConnected": True} + # The ticket only. The link is built by the surface, which is the half that + # knows its own public address — this service does not have one, and should + # not need to learn one to hand out a ticket. + return {"ticket": result.ticket} + + +@app.get("/connected-apps/provider") +def connected_app_provider_route(request: Request): + """Which connected-app provider this deployment runs, if any. + + The surface asks so a Connect card can record which provider minted it, and + refuse itself later if the deployment has since switched. Selection stays + here — this reports the answer, it does not take one. + + Behind the shared secret like everything else, though it is not a + capability: the name is not a secret, but an unauthenticated endpoint that + describes a deployment is a free reconnaissance answer. + """ + if not is_authorized(request.url.path, request.headers.get("authorization")): + return JSONResponse({"error": "unauthorized"}, status_code=401) + return {"provider": selected_provider()} + + +class ArcadeClaimRequest(BaseModel): + """One ticket, handed back by the browser that was given the link.""" + + ticket: str + + +@app.post("/arcade/claim") +def arcade_claim(body: ArcadeClaimRequest, request: Request): + """Spend a ticket, and say where that person is going next. + + Called by the surface, not by a browser. The surface owns the public + address and the cookie; this service owns who the ticket belongs to and + never tells anybody — it answers with an opaque handle instead, so the + identity does not cross the wire and cannot be replayed at the next step. + """ + refusal = _capability_guard("/arcade/claim") + if refusal is not None: + return refusal + if not authorizes_capability(request.headers.get("authorization")): + return JSONResponse({"error": "unauthorized"}, status_code=401) + + runtime = _selected_arcade_runtime() + if runtime is None: + return JSONResponse( + {"error": "Arcade is not configured on this deployment."}, + status_code=503, + ) + + claimed = runtime.pending_flows.claim_ticket(body.ticket) + if claimed is None: + return JSONResponse({"error": "unknown_ticket"}, status_code=404) + + return { + "providerUrl": claimed.provider_url, + "browserHandle": runtime.pending_flows.remember_browser(claimed.identity), + } + + +class ArcadeConfirmRequest(BaseModel): + """One browser coming back, named only by the handle it was issued.""" + + browser_handle: str | None = None + flow_id: str | None = None + + +@app.post("/arcade/confirm") +def arcade_confirm(body: ArcadeConfirmRequest, request: Request): + """Tell Arcade whose authorization just completed. + + The identity is resolved here, from a handle this service issued, and is + never accepted from the caller. That is the same rule the graph follows + before running a personal tool: the surface can say which browser came + back, but only this service can say who that is. + """ + refusal = _capability_guard("/arcade/confirm") + if refusal is not None: + return refusal + if not authorizes_capability(request.headers.get("authorization")): + return JSONResponse({"error": "unauthorized"}, status_code=401) + + runtime = _selected_arcade_runtime() + if runtime is None: + return JSONResponse( + {"error": "Arcade is not configured on this deployment."}, + status_code=503, + ) + + result = verify_flow( + runtime.pending_flows, + runtime.client_factory, + flow_id=body.flow_id, + browser_handle=body.browser_handle, + ) + return { + "outcome": result.outcome.value, + "redirectTo": result.redirect_to, + "message": result.message, + "clearCookie": result.clear_cookie, + } + + +def _capability_guard(route: str): + """Refuse a capability route that has no secret to check. + + Checked by each such route rather than by the middleware, because the + middleware enforces only when a secret is configured — and there is no + configuration in which handing a capability to an unauthenticated caller + is intended. + """ + if configured_secret() is not None: + return None + print( + f"[ERROR] {route} refused: no AGENT_AUTH_HEADER is set on the agent, " + "so it has no secret to check", + file=sys.stderr, + ) + return JSONResponse( + { + "error": "Connecting your own account needs a shared secret set on " + "both this app and its agent, and the agent has not set one. Ask " + "whoever runs this deployment." + }, + status_code=503, + ) + + +def _selected_arcade_runtime(): + """The Arcade runtime, but only when Arcade is the selected provider. + + Asked explicitly rather than left to fall out of an absent key. The case it + guards is a link or a card that outlived a provider change: minted under + Arcade, opened after the deployment switched to Composio. + """ + if selected_provider() != PROVIDER_ARCADE: + return None + return arcade_runtime( + default_user_id=os.environ.get( + "INTELLIGENCE_CHANNEL_NAME", DEFAULT_WORKSPACE_USER_ID + ) + ) + + def actor_refusal(actor: Mapping[str, Any]) -> str: """Why the gate above refused this actor, in words for whoever clicked. diff --git a/agent/tests/test_arcade_connect.py b/agent/tests/test_arcade_connect.py new file mode 100644 index 00000000..c4b4e22a --- /dev/null +++ b/agent/tests/test_arcade_connect.py @@ -0,0 +1,255 @@ +"""Starting one person's account connection. + +A connect link is a bearer capability: whoever opens it binds their account to +the identity it was minted for. So it is minted per clicker, on demand, handed +back to the surface for private delivery, and never shown to the model. + +Authorization on Arcade is per action rather than per app, so the target names +the action — that is what lets Arcade ask for the scopes the action actually +needs instead of everything the app could ever do. +""" + +from __future__ import annotations + +import pytest + +from arcade_tools.config import read_arcade_config +from arcade_tools.connect import ConnectRefused, ConnectStarted, start_connection +from arcade_tools.verify import PendingFlows + + +class FakeTools: + def __init__(self, url="https://provider.example/oauth?state=abc", flow_id="flow_1"): + self.url = url + self.flow_id = flow_id + self.authorized: list[dict] = [] + + def authorize(self, **kwargs): + self.authorized.append(kwargs) + return { + "id": self.flow_id, + "url": self.url, + "status": "pending", + "user_id": kwargs.get("user_id"), + } + + +class FakeClient: + def __init__(self, tools): + self.tools = tools + + +def setup(**overrides): + env = { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + "ARCADE_USER_TOOLKITS": "Gmail", + "ARCADE_IDENTITY_NAMESPACE": "acme", + **overrides, + } + config = read_arcade_config(env, default_user_id="open-tag") + assert config is not None + tools = FakeTools() + pending = PendingFlows(clock=lambda: 1000.0) + return config, tools, pending + + +def start(config, tools, pending, *, identity="slack:U1", target="Gmail.ListMail"): + return start_connection( + config, + lambda: FakeClient(tools), + pending, + identity=identity, + target=target, + ) + + +def test_a_ticket_is_minted_for_the_person_who_clicked(): + config, tools, pending = setup() + + result = start(config, tools, pending) + + assert isinstance(result, ConnectStarted) + assert result.ticket + assert tools.authorized[0]["user_id"] == "acme/slack:U1" + + +def test_the_provider_url_is_never_handed_out(): + # The person goes to us first, so their browser can collect the cookie that + # identifies it on the way back. Handing them the provider URL directly + # would skip that hop and there would be nothing to answer Arcade with. + config, tools, pending = setup() + + result = start(config, tools, pending) + + assert isinstance(result, ConnectStarted) + assert tools.url not in str(result) + + +def test_the_target_action_is_what_arcade_is_asked_about(): + # Per action, not per app. Asking about the app would request every scope + # the app has rather than the ones this action needs. + config, tools, pending = setup() + + start(config, tools, pending, target="Gmail.SendMail") + + assert tools.authorized[0]["tool_name"] == "Gmail.SendMail" + + +def test_the_ticket_resolves_to_the_person_and_their_destination(): + # This is what lets the verifier answer without anything identifying + # travelling in the link. + config, tools, pending = setup() + + result = start(config, tools, pending) + assert isinstance(result, ConnectStarted) + + claimed = pending.claim_ticket(result.ticket) + assert claimed is not None + assert claimed.identity == "acme/slack:U1" + assert claimed.provider_url == tools.url + + +def test_two_people_get_two_tickets_resolving_to_themselves(): + config, tools, pending = setup() + one = start(config, tools, pending, identity="slack:U1") + two = start(config, tools, pending, identity="slack:U2") + assert isinstance(one, ConnectStarted) and isinstance(two, ConnectStarted) + + assert one.ticket != two.ticket + first = pending.claim_ticket(one.ticket) + second = pending.claim_ticket(two.ticket) + assert first is not None and second is not None + assert first.identity == "acme/slack:U1" + assert second.identity == "acme/slack:U2" + + +def test_a_shared_app_is_refused_rather_than_connected_personally(): + # A shared app runs as one workspace identity. A link minted for a clicker + # would connect an account no shared call ever uses — the operator command + # exists for exactly this. + config, tools, pending = setup() + + result = start(config, tools, pending, target="Github.CreateIssue") + + assert isinstance(result, ConnectRefused) + assert tools.authorized == [] + + +def test_an_unconfigured_app_is_refused(): + config, tools, pending = setup() + + result = start(config, tools, pending, target="Asana.CreateTask") + + assert isinstance(result, ConnectRefused) + assert tools.authorized == [] + + +@pytest.mark.parametrize("target", ["", " ", "NoToolkit", None, 42]) +def test_a_target_naming_no_action_is_refused(target): + config, tools, pending = setup() + + result = start(config, tools, pending, target=target) + + assert isinstance(result, ConnectRefused) + assert tools.authorized == [] + + +@pytest.mark.parametrize("identity", [None, "", " "]) +def test_a_request_naming_nobody_mints_nothing(identity): + # Found on the Composio side by a live run rather than a unit test: a blank + # id minted a real link bound to an identity no turn would ever look up. + config, tools, pending = setup() + + result = start(config, tools, pending, identity=identity) + + assert isinstance(result, ConnectRefused) + assert tools.authorized == [] + + +def test_a_provider_failure_is_a_refusal_rather_than_a_crash(): + config, _tools, pending = setup() + + class Failing: + def authorize(self, **kwargs): + raise RuntimeError("provider said no") + + result = start_connection( + config, + lambda: FakeClient(Failing()), + pending, + identity="slack:U1", + target="Gmail.ListMail", + ) + + assert isinstance(result, ConnectRefused) + + +def test_an_authorization_with_no_link_is_a_refusal(): + config, tools, pending = setup() + tools.url = "" + + result = start(config, tools, pending) + + assert isinstance(result, ConnectRefused) + + +def test_the_authorizations_own_id_is_not_what_gets_recorded(): + # Established against the live API: the id `authorize` returns and the flow + # id the verifier is later given are different id spaces. A record keyed on + # the former would never be found, which is the defect this whole hop + # exists to avoid. + config, tools, pending = setup() + tools.flow_id = "ar_somethingopaque" + + result = start(config, tools, pending) + assert isinstance(result, ConnectStarted) + + assert result.ticket != "ar_somethingopaque" + + +def test_a_refusal_never_carries_a_ticket(): + config, tools, pending = setup() + + result = start(config, tools, pending, target="Github.CreateIssue") + + assert not hasattr(result, "ticket") + + +def test_the_link_is_never_logged(caplog): + import logging + + config, tools, pending = setup() + + with caplog.at_level(logging.DEBUG): + result = start(config, tools, pending) + + # Establish a link was actually minted first, or this passes for the wrong + # reason on a refusal. + assert isinstance(result, ConnectStarted) + assert tools.url not in caplog.text + + +def test_an_already_connected_person_is_told_so_rather_than_sent_round_again(): + config, tools, pending = setup() + + class AlreadyDone: + def __init__(self): + self.authorized = [] + + def authorize(self, **kwargs): + self.authorized.append(kwargs) + return {"id": "flow_1", "status": "completed", "url": None} + + already = AlreadyDone() + result = start_connection( + config, + lambda: FakeClient(already), + pending, + identity="slack:U1", + target="Gmail.ListMail", + ) + + assert isinstance(result, ConnectStarted) + assert result.already_connected is True + assert result.ticket is None diff --git a/agent/tests/test_arcade_routes.py b/agent/tests/test_arcade_routes.py new file mode 100644 index 00000000..75c835e4 --- /dev/null +++ b/agent/tests/test_arcade_routes.py @@ -0,0 +1,415 @@ +"""The three private routes the Arcade connect flow needs from this service. + +None of them is public. The browser-facing half of the flow lives on the +surface, which is the half that knows its own address and owns cookies — so the +process holding the provider keys keeps no public entry point. + +What this service keeps is the part only it can do: minting a ticket for one +person, and saying whose authorization is completing. The identity is resolved +here and never accepted from the caller, which is the same rule the graph +follows before it runs a personal tool. +""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +import arcade_tools.runtime as runtime_mod +from arcade_tools.config import read_arcade_config +from arcade_tools.runtime import ArcadeRuntime, reset_arcade_runtime +from arcade_tools.verify import PendingFlows + +SECRET = "Bearer s3cret" + + +class FakeTools: + def __init__(self): + self.authorized: list[dict] = [] + + def authorize(self, **kwargs): + self.authorized.append(kwargs) + return { + "id": "ar_opaque", + "url": "https://provider.example/oauth?state=abc", + "status": "pending", + } + + +class FakeAuth: + def __init__(self): + self.confirmed: list[dict] = [] + + def confirm_user(self, *, flow_id, user_id): + self.confirmed.append({"flow_id": flow_id, "user_id": user_id}) + return {"auth_id": "auth_1", "next_uri": "https://arcade.example/done"} + + +class FakeClient: + def __init__(self): + self.tools = FakeTools() + self.auth = FakeAuth() + + +@pytest.fixture +def client(monkeypatch): + reset_arcade_runtime() + monkeypatch.setenv("OPENAI_API_KEY", "sk-test") + import main + + yield TestClient(main.app, raise_server_exceptions=False) + reset_arcade_runtime() + + +def install(monkeypatch, **overrides): + """Select Arcade and install a runtime whose client is a fake.""" + monkeypatch.setenv("ARCADE_API_KEY", "arc_test") + monkeypatch.delenv("COMPOSIO_API_KEY", raising=False) + env = { + "ARCADE_API_KEY": "arc_test", + "ARCADE_TOOLKITS": "Github", + "ARCADE_USER_TOOLKITS": "Gmail", + "ARCADE_IDENTITY_NAMESPACE": "acme", + **overrides, + } + config = read_arcade_config(env, default_user_id="open-tag") + assert config is not None + fake = FakeClient() + from arcade_tools.catalog import Catalog + + runtime = ArcadeRuntime( + config=config, + catalog=Catalog(lambda: fake, config), + client_factory=lambda: fake, + pending_flows=PendingFlows(clock=lambda: 1000.0), + ) + monkeypatch.setattr(runtime_mod, "build_arcade_runtime", lambda *a, **k: runtime) + reset_arcade_runtime() + return runtime, fake + + +CONNECT_BODY = { + "actor_id": "U1", + "kind": "human", + "platform": "slack", + "target": "Gmail.ListMail", +} + +CAPABILITY_ROUTES = [ + ("/arcade/connect", CONNECT_BODY), + ("/arcade/claim", {"ticket": "anything"}), + ("/arcade/confirm", {"browser_handle": "x", "flow_id": "y"}), +] + + +def mint(client) -> str: + response = client.post( + "/arcade/connect", json=CONNECT_BODY, headers={"Authorization": SECRET} + ) + assert response.status_code == 200 + return response.json()["ticket"] + + +# --- every route that hands out or spends a capability --- + + +@pytest.mark.parametrize("path,body", CAPABILITY_ROUTES) +def test_a_capability_route_refuses_without_a_configured_secret( + client, monkeypatch, path, body +): + # There is no configuration in which serving one of these to an + # unauthenticated caller is intended, so with no secret they report + # themselves unavailable rather than serving. + monkeypatch.delenv("AGENT_AUTH_HEADER", raising=False) + _runtime, fake = install(monkeypatch) + + assert client.post(path, json=body).status_code == 503 + assert fake.tools.authorized == [] + assert fake.auth.confirmed == [] + + +@pytest.mark.parametrize("path,body", CAPABILITY_ROUTES) +def test_a_capability_route_refuses_a_wrong_secret(client, monkeypatch, path, body): + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, fake = install(monkeypatch) + + response = client.post( + path, json=body, headers={"Authorization": "Bearer wrong"} + ) + + assert response.status_code == 401 + assert fake.tools.authorized == [] + assert fake.auth.confirmed == [] + + +@pytest.mark.parametrize("path,body", CAPABILITY_ROUTES) +def test_a_capability_route_checks_the_secret_itself_not_only_the_middleware( + client, monkeypatch, path, body +): + # Found by deleting a route's own check and watching every test stay green: + # the middleware refuses unauthenticated traffic first, so a route-level + # check is invisible to a test that only goes through the front door. It is + # not redundant — the middleware exempts public paths, and one careless edit + # to that set is all it takes for the front door to stop covering these. + import agent_auth + + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + monkeypatch.setattr( + agent_auth, "PUBLIC_PATHS", frozenset({*agent_auth.PUBLIC_PATHS, path}) + ) + _runtime, fake = install(monkeypatch) + + assert agent_auth.is_authorized(path, None) is True + assert client.post(path, json=body).status_code == 401 + assert fake.tools.authorized == [] + assert fake.auth.confirmed == [] + + +def test_this_service_has_no_public_entry_point_beyond_its_health_probe(monkeypatch): + # The whole point of the browser half living on the surface. A public path + # here would put the process holding the provider keys on the internet. + from agent_auth import PUBLIC_PATHS, is_authorized + + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + + assert PUBLIC_PATHS == frozenset({"/health", "//health"}) + for closed in ("/arcade/connect", "/arcade/claim", "/arcade/confirm", "/arcade"): + assert is_authorized(closed, None) is False, closed + + +# --- minting --- + + +def test_the_connect_route_returns_a_ticket_and_no_link(client, monkeypatch): + # The surface builds the link, because it is the half that knows its own + # public address. This service does not have one and should not learn one. + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, fake = install(monkeypatch) + + response = client.post( + "/arcade/connect", json=CONNECT_BODY, headers={"Authorization": SECRET} + ) + + assert response.status_code == 200 + body = response.json() + assert body["ticket"] + assert "redirectUrl" not in body + assert "provider.example" not in str(body) + assert fake.tools.authorized[0]["user_id"] == "acme/slack:U1" + + +def test_the_connect_route_mints_nothing_for_something_posting_as_a_person( + client, monkeypatch +): + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, fake = install(monkeypatch) + + for kind in ("bot", "app", "system", "unknown", None): + response = client.post( + "/arcade/connect", + json={**CONNECT_BODY, "kind": kind}, + headers={"Authorization": SECRET}, + ) + assert response.status_code == 400 + + assert fake.tools.authorized == [] + + +@pytest.mark.parametrize("actor_id", ["", " "]) +def test_the_connect_route_refuses_a_request_naming_nobody( + client, monkeypatch, actor_id +): + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, fake = install(monkeypatch) + + response = client.post( + "/arcade/connect", + json={**CONNECT_BODY, "actor_id": actor_id}, + headers={"Authorization": SECRET}, + ) + + assert response.status_code == 400 + assert fake.tools.authorized == [] + + +# --- spending a ticket --- + + +def test_claiming_a_ticket_says_where_to_go_and_names_nobody(client, monkeypatch): + # The identity does not cross the wire. The surface gets an opaque handle, + # which it can only exchange back here. + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, _fake = install(monkeypatch) + + response = client.post( + "/arcade/claim", + json={"ticket": mint(client)}, + headers={"Authorization": SECRET}, + ) + + assert response.status_code == 200 + body = response.json() + assert body["providerUrl"].startswith("https://provider.example/") + assert body["browserHandle"] + assert "slack:U1" not in str(body) + assert "acme" not in str(body) + + +def test_a_ticket_cannot_be_spent_twice(client, monkeypatch): + # A forwarded link finds nothing, and a double-click starts one session. + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, _fake = install(monkeypatch) + ticket = mint(client) + + first = client.post( + "/arcade/claim", json={"ticket": ticket}, headers={"Authorization": SECRET} + ) + second = client.post( + "/arcade/claim", json={"ticket": ticket}, headers={"Authorization": SECRET} + ) + + assert first.status_code == 200 + assert second.status_code == 404 + + +def test_an_unknown_ticket_is_refused(client, monkeypatch): + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, _fake = install(monkeypatch) + + response = client.post( + "/arcade/claim", + json={"ticket": "not-a-ticket"}, + headers={"Authorization": SECRET}, + ) + + assert response.status_code == 404 + + +def test_the_browser_handle_is_not_the_ticket(client, monkeypatch): + # The ticket travelled in a URL, so it is in browser history and in logs. + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, _fake = install(monkeypatch) + ticket = mint(client) + + claimed = client.post( + "/arcade/claim", json={"ticket": ticket}, headers={"Authorization": SECRET} + ) + + assert claimed.json()["browserHandle"] != ticket + + +# --- confirming --- + + +def test_the_whole_journey_confirms_the_person_who_clicked(client, monkeypatch): + # The end-to-end shape in one test, with the surface's part played by two + # ordinary calls: mint, claim, then come back with the handle. + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, fake = install(monkeypatch) + + claimed = client.post( + "/arcade/claim", + json={"ticket": mint(client)}, + headers={"Authorization": SECRET}, + ).json() + + confirmed = client.post( + "/arcade/confirm", + json={ + "browser_handle": claimed["browserHandle"], + # An id from a space this service cannot resolve, returned verbatim. + "flow_id": "ed83c08d-4e1f-450c-9feb-9d95d221780f", + }, + headers={"Authorization": SECRET}, + ) + + assert confirmed.status_code == 200 + assert confirmed.json()["outcome"] == "confirmed" + assert confirmed.json()["redirectTo"] == "https://arcade.example/done" + assert fake.auth.confirmed == [ + { + "flow_id": "ed83c08d-4e1f-450c-9feb-9d95d221780f", + "user_id": "acme/slack:U1", + } + ] + + +def test_an_identity_offered_by_the_surface_is_never_used(client, monkeypatch): + # The rule this service keeps for itself: the surface can say which browser + # came back, but only this service can say who that is. A body naming a user + # must change nothing. + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, fake = install(monkeypatch) + + response = client.post( + "/arcade/confirm", + json={ + "browser_handle": "not-ours", + "flow_id": "flow_1", + "user_id": "acme/slack:VICTIM", + }, + headers={"Authorization": SECRET}, + ) + + assert response.json()["outcome"] == "unknown" + assert fake.auth.confirmed == [] + + +def test_an_unrecognised_browser_confirms_nobody(client, monkeypatch): + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, fake = install(monkeypatch) + + response = client.post( + "/arcade/confirm", + json={"browser_handle": "never-issued", "flow_id": "flow_1"}, + headers={"Authorization": SECRET}, + ) + + assert response.json()["outcome"] == "unknown" + assert fake.auth.confirmed == [] + + +def test_what_comes_back_names_nobody(client, monkeypatch): + # The surface shows this to whoever arrived, who may not be the person the + # connection belongs to. + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + runtime, _fake = install(monkeypatch) + handle = runtime.pending_flows.remember_browser("acme/slack:U1") + + response = client.post( + "/arcade/confirm", + json={"browser_handle": handle, "flow_id": "flow_1"}, + headers={"Authorization": SECRET}, + ) + + assert "slack:U1" not in response.text + assert "acme" not in response.text + + +# --- provider selection --- + + +@pytest.mark.parametrize("path,body", CAPABILITY_ROUTES) +def test_an_arcade_route_refuses_when_composio_is_selected( + client, monkeypatch, path, body +): + # The case this guards is a link that outlived a provider change. + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + _runtime, fake = install(monkeypatch) + monkeypatch.delenv("ARCADE_API_KEY", raising=False) + monkeypatch.setenv("COMPOSIO_API_KEY", "ak_test") + + if path == "/arcade/connect": + # Composio's own route is the one that answers under Composio. + response = client.post(path, json=body, headers={"Authorization": SECRET}) + else: + response = client.post(path, json=body, headers={"Authorization": SECRET}) + + assert response.status_code == 503 + assert fake.tools.authorized == [] + assert fake.auth.confirmed == [] + + +def test_health_stays_reachable(client, monkeypatch): + monkeypatch.setenv("AGENT_AUTH_HEADER", SECRET) + assert client.get("/health").status_code == 200 diff --git a/agent/tests/test_arcade_tools.py b/agent/tests/test_arcade_tools.py index 7b8cefe1..1ddccb71 100644 --- a/agent/tests/test_arcade_tools.py +++ b/agent/tests/test_arcade_tools.py @@ -502,3 +502,90 @@ def test_connecting_an_account_is_not_approval_to_write(monkeypatch): state=state(actor_id="U1")) assert len(asked) == 1 + + +# --- telling the model an app needs connecting --- + + +def test_search_names_apps_this_person_has_not_connected(): + # Without this the model has no reason to offer the Connect button, so the + # first sign an app needs connecting is an action that refuses to run. + built, _tools = build( + {"Gmail": [definition("Gmail.ListMail", description="mail")]}, + requirements_met=False, + ) + + found = invoke(built["search_my_tools"], query="mail", state=state()) + + assert found["needsConnection"] == ["Gmail"] + + +def test_search_says_nothing_about_apps_that_are_connected(): + built, _tools = build( + {"Gmail": [definition("Gmail.ListMail", description="mail")]}, + requirements_met=True, + ) + + found = invoke(built["search_my_tools"], query="mail", state=state()) + + assert "needsConnection" not in found + + +def test_a_shared_app_is_never_offered_for_personal_connecting(): + # Nobody presses Connect for a shared app. Offering it would send somebody + # to bind their own account where every call runs as the team. + built, _tools = build( + {"Github": [definition("Github.ListIssues", description="issues")]}, + requirements_met=False, + ) + + found = invoke(built["search_my_tools"], query="issues", state=state()) + + assert "needsConnection" not in found + + +def test_one_app_is_probed_once_however_many_actions_it_returns(): + # A search returning twenty actions must not cost twenty round trips. + built, tools = build( + { + "Gmail": [ + definition(f"Gmail.Thing{index}", description="mail") + for index in range(8) + ] + }, + requirements_met=False, + ) + + invoke(built["search_my_tools"], query="mail", state=state()) + + assert len(tools.get_calls) == 1 + + +def test_a_failed_check_does_not_claim_the_app_is_unconnected(): + # Not knowing is not the same as not connected. Telling somebody to connect + # an account they already connected sends them round a flow twice. + built, tools = build( + {"Gmail": [definition("Gmail.ListMail", description="mail")]} + ) + + def failing(name, **kwargs): + raise RuntimeError("provider is down") + + tools.get = failing + found = invoke(built["search_my_tools"], query="mail", state=state()) + + assert "needsConnection" not in found + + +def test_an_anonymous_turn_is_never_told_to_connect_anything(): + # It has no personal apps at all, so there is nothing to connect. + built, _tools = build( + {"Gmail": [definition("Gmail.ListMail", description="mail")]}, + requirements_met=False, + ) + + found = invoke( + built["search_my_tools"], query="mail", state=state(actor_id=None) + ) + + assert "needsConnection" not in str(found) diff --git a/agent/tests/test_arcade_verify.py b/agent/tests/test_arcade_verify.py new file mode 100644 index 00000000..1e750d41 --- /dev/null +++ b/agent/tests/test_arcade_verify.py @@ -0,0 +1,335 @@ +"""Knowing who is at the browser when Arcade sends them back. + +Arcade hands the verifier a flow id from an id space we cannot resolve — that +was established against the live API, not assumed — so the identity has to come +from somewhere else. It comes from a cookie this service gave that browser on +its way out. + +Two stores, both short-lived and both keyed on opaque random values: + +* the ticket in the link, spent once on the way out; +* the cookie value, which never appears in a URL. + +What this module refuses to do: + +* Take the identity from the request. A query parameter naming a user is an + invitation to name somebody else. +* Remember anything forever, or without limit. +* Answer twice. A connection is completed once. +""" + +from __future__ import annotations + +import pytest + +from arcade_tools.verify import ( + PendingFlows, + VerifyOutcome, + verify_flow, +) + + +class FakeAuth: + def __init__(self, next_uri="https://arcade.example/done", error=None): + self.confirmed: list[dict] = [] + self._next_uri = next_uri + self._error = error + + def confirm_user(self, *, flow_id, user_id): + if self._error is not None: + raise self._error + self.confirmed.append({"flow_id": flow_id, "user_id": user_id}) + return {"auth_id": "auth_1", "next_uri": self._next_uri} + + +class FakeClient: + def __init__(self, auth=None): + self.auth = auth or FakeAuth() + + +def flows(now=1000.0, **kwargs): + return PendingFlows(clock=lambda: now, **kwargs) + + +# --- the ticket in the link --- + + +def test_a_ticket_resolves_to_the_person_and_where_they_are_going(): + pending = flows() + ticket = pending.issue_ticket( + identity="acme/slack:U1", provider_url="https://github.example/oauth" + ) + + claimed = pending.claim_ticket(ticket) + + assert claimed is not None + assert claimed.identity == "acme/slack:U1" + assert claimed.provider_url == "https://github.example/oauth" + + +def test_a_ticket_is_spent_once(): + # The link reaches one person privately. Opening it twice should not start + # two sessions, and a forwarded link should find nothing. + pending = flows() + ticket = pending.issue_ticket(identity="acme/slack:U1", provider_url="https://x") + + assert pending.claim_ticket(ticket) is not None + assert pending.claim_ticket(ticket) is None + + +def test_two_tickets_are_never_the_same_value(): + pending = flows() + first = pending.issue_ticket(identity="acme/slack:U1", provider_url="https://x") + second = pending.issue_ticket(identity="acme/slack:U2", provider_url="https://x") + + assert first != second + + +@pytest.mark.parametrize("junk", [None, "", " ", 42]) +def test_a_ticket_that_is_not_a_ticket_resolves_to_nothing(junk): + assert flows().claim_ticket(junk) is None + + +# --- the cookie the browser carries --- + + +def test_a_browser_handle_resolves_to_the_person_it_was_issued_for(): + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + + assert pending.identity_for_browser(handle) == "acme/slack:U1" + + +def test_a_browser_handle_is_spent_once(): + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + + assert pending.identity_for_browser(handle) == "acme/slack:U1" + assert pending.identity_for_browser(handle) is None + + +def test_an_unknown_browser_resolves_to_nobody(): + assert flows().identity_for_browser("never-issued") is None + + +def test_a_handle_never_equals_the_ticket_it_came_from(): + # The ticket travelled in a URL, so it is in history and in logs. The value + # the browser keeps must not be that same string. + pending = flows() + ticket = pending.issue_ticket(identity="acme/slack:U1", provider_url="https://x") + claimed = pending.claim_ticket(ticket) + assert claimed is not None + handle = pending.remember_browser(claimed.identity) + + assert handle != ticket + + +def test_two_browsers_never_share_a_handle(): + pending = flows() + + assert pending.remember_browser("acme/slack:U1") != pending.remember_browser( + "acme/slack:U2" + ) + + +# --- expiry and bounds, on both stores --- + + +def test_a_ticket_expires(): + clock = {"now": 1000.0} + pending = PendingFlows(clock=lambda: clock["now"], ttl_seconds=600) + ticket = pending.issue_ticket(identity="acme/slack:U1", provider_url="https://x") + + clock["now"] += 601 + + assert pending.claim_ticket(ticket) is None + + +def test_a_browser_handle_expires(): + clock = {"now": 1000.0} + pending = PendingFlows(clock=lambda: clock["now"], ttl_seconds=600) + handle = pending.remember_browser("acme/slack:U1") + + clock["now"] += 601 + + assert pending.identity_for_browser(handle) is None + + +def test_something_inside_its_window_still_resolves(): + clock = {"now": 1000.0} + pending = PendingFlows(clock=lambda: clock["now"], ttl_seconds=600) + handle = pending.remember_browser("acme/slack:U1") + + clock["now"] += 599 + + assert pending.identity_for_browser(handle) == "acme/slack:U1" + + +def test_pending_browsers_are_bounded(): + # This lives in memory for the life of the process. Unbounded, whoever can + # start connections can grow it at will. + pending = PendingFlows(clock=lambda: 1000.0, max_pending=4) + handles = [pending.remember_browser(f"acme/slack:U{i}") for i in range(10)] + + assert pending.identity_for_browser(handles[0]) is None + assert pending.identity_for_browser(handles[-1]) == "acme/slack:U9" + + +def test_pending_tickets_are_bounded(): + pending = PendingFlows(clock=lambda: 1000.0, max_pending=4) + tickets = [ + pending.issue_ticket(identity=f"acme/slack:U{i}", provider_url="https://x") + for i in range(10) + ] + + assert pending.claim_ticket(tickets[0]) is None + assert pending.claim_ticket(tickets[-1]) is not None + + +# --- verifying --- + + +def test_a_known_browser_confirms_the_person_it_belongs_to(): + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + client = FakeClient() + + result = verify_flow( + pending, lambda: client, flow_id="flow_1", browser_handle=handle + ) + + assert result.outcome is VerifyOutcome.CONFIRMED + assert client.auth.confirmed == [ + {"flow_id": "flow_1", "user_id": "acme/slack:U1"} + ] + + +def test_the_flow_id_arcade_sent_is_the_one_passed_back_to_it(): + # We cannot resolve Arcade's flow id, but we must return it verbatim — it is + # how Arcade knows which authorization we are answering about. + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + client = FakeClient() + + verify_flow( + pending, + lambda: client, + flow_id="ed83c08d-4e1f-450c-9feb-9d95d221780f", + browser_handle=handle, + ) + + assert client.auth.confirmed[0]["flow_id"] == ( + "ed83c08d-4e1f-450c-9feb-9d95d221780f" + ) + + +def test_a_confirmed_flow_sends_the_browser_where_arcade_asked(): + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + + result = verify_flow( + pending, lambda: FakeClient(), flow_id="flow_1", browser_handle=handle + ) + + assert result.redirect_to == "https://arcade.example/done" + + +def test_a_browser_with_no_cookie_confirms_nobody(): + # The real-world case: started on a laptop, finished on a phone. Also the + # adversarial one: somebody who simply found the route. + client = FakeClient() + + result = verify_flow( + flows(), lambda: client, flow_id="flow_1", browser_handle=None + ) + + assert result.outcome is VerifyOutcome.UNKNOWN + assert client.auth.confirmed == [] + + +def test_a_browser_with_an_unrecognised_cookie_confirms_nobody(): + client = FakeClient() + + result = verify_flow( + flows(), lambda: client, flow_id="flow_1", browser_handle="not-ours" + ) + + assert result.outcome is VerifyOutcome.UNKNOWN + assert client.auth.confirmed == [] + + +@pytest.mark.parametrize("missing", [None, "", " "]) +def test_a_request_carrying_no_flow_id_confirms_nobody(missing): + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + client = FakeClient() + + result = verify_flow( + pending, lambda: client, flow_id=missing, browser_handle=handle + ) + + assert result.outcome is VerifyOutcome.UNKNOWN + assert client.auth.confirmed == [] + + +def test_an_identity_offered_by_the_caller_is_never_used(): + import inspect + + signature = inspect.signature(verify_flow) + + assert "user_id" not in signature.parameters + assert "actor" not in signature.parameters + + +def test_a_provider_failure_is_reported_rather_than_treated_as_success(): + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + client = FakeClient(auth=FakeAuth(error=RuntimeError("upstream is down"))) + + result = verify_flow( + pending, lambda: client, flow_id="flow_1", browser_handle=handle + ) + + assert result.outcome is VerifyOutcome.FAILED + + +def test_a_failed_confirmation_does_not_leave_the_browser_spendable(): + # The cookie was consumed on the way in. A retry starts over rather than + # replays, because a failed confirm and a confirmed one whose reply was lost + # look identical from here. + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + client = FakeClient(auth=FakeAuth(error=RuntimeError("upstream is down"))) + + verify_flow(pending, lambda: client, flow_id="flow_1", browser_handle=handle) + + assert pending.identity_for_browser(handle) is None + + +def test_every_outcome_asks_for_the_cookie_to_be_cleared(): + # Spent either way. Left behind, it is a stale claim on an identity sitting + # in somebody's browser for the rest of its lifetime. + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + + confirmed = verify_flow( + pending, lambda: FakeClient(), flow_id="flow_1", browser_handle=handle + ) + unknown = verify_flow( + pending, lambda: FakeClient(), flow_id="flow_1", browser_handle="nope" + ) + + assert confirmed.clear_cookie is True + assert unknown.clear_cookie is True + + +def test_nothing_about_the_person_reaches_the_reader(): + pending = flows() + handle = pending.remember_browser("acme/slack:U1") + + result = verify_flow( + pending, lambda: FakeClient(), flow_id="flow_1", browser_handle=handle + ) + + assert "slack:U1" not in result.message + assert "acme" not in result.message diff --git a/app/__tests__/arcade-browser-routes.test.ts b/app/__tests__/arcade-browser-routes.test.ts new file mode 100644 index 00000000..286fca6e --- /dev/null +++ b/app/__tests__/arcade-browser-routes.test.ts @@ -0,0 +1,346 @@ +/** + * The browser half of the Arcade connect flow. + * + * The properties under test are the ones that decide whether somebody's + * account binds to the right person: the cookie has to survive a cross-site + * return, it must not be readable by script or carry an identity, and a + * browser we never issued one to must confirm nobody. + */ + +import { describe, expect, it, vi } from "vitest"; +import { IncomingMessage, ServerResponse } from "node:http"; +import { Socket } from "node:net"; + +import { + COOKIE_NAME, + createArcadeAgentClient, + handleArcadeBrowserRequest, + readSessionCookie, + type ArcadeAgentClient, +} from "../arcade-browser-routes.js"; + +function requestFor( + url: string, + headers: Record = {}, + method = "GET", +): IncomingMessage { + const request = new IncomingMessage(new Socket()); + request.url = url; + request.method = method; + Object.assign(request.headers, headers); + return request; +} + +function responseFor(request: IncomingMessage) { + const response = new ServerResponse(request); + const chunks: string[] = []; + const end = response.end.bind(response); + response.end = ((chunk?: unknown) => { + if (typeof chunk === "string") chunks.push(chunk); + return end(chunk as never); + }) as typeof response.end; + return { + response, + get body() { + return chunks.join(""); + }, + get cookie() { + const header = response.getHeader("Set-Cookie"); + return Array.isArray(header) ? header.join("; ") : String(header ?? ""); + }, + }; +} + +function clientWith(overrides: Partial = {}): ArcadeAgentClient { + return { + claimTicket: vi.fn(async () => ({ + providerUrl: "https://provider.example/oauth?state=abc", + browserHandle: "handle-1", + })), + confirmFlow: vi.fn(async () => ({ + outcome: "confirmed" as const, + redirectTo: "https://arcade.example/done", + message: "Connected.", + clearCookie: true, + })), + ...overrides, + }; +} + +describe("routing", () => { + it("leaves every other path to the rest of the application", async () => { + for (const path of ["/", "/api/copilotkit", "/arcade", "/arcade/connect"]) { + const request = requestFor(path); + const { response } = responseFor(request); + + expect( + await handleArcadeBrowserRequest(request, response, clientWith()), + ).toBe(false); + } + }); + + it("refuses a method a browser redirect never uses", async () => { + const request = requestFor("/arcade/verify", {}, "POST"); + const sink = responseFor(request); + + await handleArcadeBrowserRequest(request, sink.response, clientWith()); + + expect(sink.response.statusCode).toBe(405); + }); +}); + +describe("the outbound hop", () => { + it("spends the ticket and sends the browser to the provider", async () => { + const client = clientWith(); + const request = requestFor("/arcade/start?t=ticket-1"); + const sink = responseFor(request); + + await handleArcadeBrowserRequest(request, sink.response, client); + + expect(client.claimTicket).toHaveBeenCalledWith("ticket-1"); + expect(sink.response.statusCode).toBe(303); + expect(sink.response.getHeader("Location")).toBe( + "https://provider.example/oauth?state=abc", + ); + }); + + it("gives the browser a cookie no script can read", async () => { + const request = requestFor("/arcade/start?t=ticket-1"); + const sink = responseFor(request); + + await handleArcadeBrowserRequest(request, sink.response, clientWith()); + + expect(sink.cookie).toContain("HttpOnly"); + expect(sink.cookie).toContain("Secure"); + // Strict would withhold the cookie on the return leg, which is a + // navigation from Arcade's site to ours. That is the whole flow. + expect(sink.cookie).toContain("SameSite=Lax"); + expect(sink.cookie).toContain("Path=/arcade"); + }); + + it("puts nothing identifying in the cookie", async () => { + const request = requestFor("/arcade/start?t=ticket-1"); + const sink = responseFor(request); + + await handleArcadeBrowserRequest(request, sink.response, clientWith()); + + expect(sink.cookie).toContain("handle-1"); + expect(sink.cookie).not.toContain("slack"); + expect(sink.cookie).not.toContain("ticket-1"); + }); + + it("tells somebody with a spent link to ask again", async () => { + const request = requestFor("/arcade/start?t=used"); + const sink = responseFor(request); + + await handleArcadeBrowserRequest( + request, + sink.response, + clientWith({ claimTicket: vi.fn(async () => null) }), + ); + + expect(sink.response.statusCode).toBe(200); + expect(sink.body).toContain("already been used"); + expect(sink.response.getHeader("Set-Cookie")).toBeUndefined(); + }); +}); + +describe("the return leg", () => { + it("confirms using the browser's own cookie", async () => { + const client = clientWith(); + const request = requestFor("/arcade/verify?flow_id=uuid-1", { + cookie: `${COOKIE_NAME}=handle-1`, + }); + const sink = responseFor(request); + + await handleArcadeBrowserRequest(request, sink.response, client); + + expect(client.confirmFlow).toHaveBeenCalledWith({ + browserHandle: "handle-1", + flowId: "uuid-1", + }); + expect(sink.response.statusCode).toBe(303); + }); + + it("never takes an identity from the query string", async () => { + // The defect this prevents: binding anybody's account by editing a URL. + const client = clientWith(); + const request = requestFor( + "/arcade/verify?flow_id=uuid-1&user_id=acme/slack:VICTIM", + { cookie: `${COOKIE_NAME}=handle-1` }, + ); + const sink = responseFor(request); + + await handleArcadeBrowserRequest(request, sink.response, client); + + expect(client.confirmFlow).toHaveBeenCalledWith({ + browserHandle: "handle-1", + flowId: "uuid-1", + }); + }); + + it("asks about a browser carrying no cookie rather than assuming one", async () => { + // Started on a laptop, finished on a phone — and also somebody who simply + // found the route. The agent is the one that decides, and it says nobody. + const client = clientWith({ + confirmFlow: vi.fn(async () => ({ + outcome: "unknown" as const, + redirectTo: null, + message: "This connection could not be completed in this browser.", + clearCookie: true, + })), + }); + const request = requestFor("/arcade/verify?flow_id=uuid-1"); + const sink = responseFor(request); + + await handleArcadeBrowserRequest(request, sink.response, client); + + expect(client.confirmFlow).toHaveBeenCalledWith({ + browserHandle: null, + flowId: "uuid-1", + }); + expect(sink.response.statusCode).toBe(200); + expect(sink.body).toContain("could not be completed"); + }); + + it("clears the cookie once it has been spent", async () => { + const request = requestFor("/arcade/verify?flow_id=uuid-1", { + cookie: `${COOKIE_NAME}=handle-1`, + }); + const sink = responseFor(request); + + await handleArcadeBrowserRequest(request, sink.response, clientWith()); + + expect(sink.cookie).toContain("Max-Age=0"); + }); + + it("says something useful when the agent cannot be reached", async () => { + const request = requestFor("/arcade/verify?flow_id=uuid-1"); + const sink = responseFor(request); + + await handleArcadeBrowserRequest( + request, + sink.response, + clientWith({ confirmFlow: vi.fn(async () => null) }), + ); + + expect(sink.response.statusCode).toBe(200); + expect(sink.body).toContain("Ask again in the chat"); + }); + + it("never caches a connection page", async () => { + // A cached one would show the last person's outcome to the next. + const request = requestFor("/arcade/verify?flow_id=uuid-1"); + const sink = responseFor(request); + + await handleArcadeBrowserRequest(request, sink.response, clientWith()); + + expect(sink.response.getHeader("Cache-Control")).toBe("no-store"); + }); +}); + +describe("sitting in front of the rest of the application", () => { + it("keeps the Channel control the server waits on", async () => { + // Wrapping a function drops its properties. Without carrying them across, + // the process starts, serves, and never activates its Channel — which + // looks like a dead Slack app rather than like a missing property. + const { createOpenTagRuntime } = await import("../runtime-host.js"); + + const { listener } = createOpenTagRuntime({ + environment: { + agentUrl: "http://agent.internal:8123", + agentAuthHeader: "Bearer s3cret", + channelName: "test-channel", + agentDisplayName: "OpenTag", + intelligenceApiKey: "cpk-test", + } as never, + channels: [], + }); + + expect("channels" in listener).toBe(true); + }); +}); + +describe("reading the cookie", () => { + it("finds the value among others", () => { + expect(readSessionCookie(`a=1; ${COOKIE_NAME}=wanted; b=2`)).toBe("wanted"); + }); + + it("answers nothing when there is none", () => { + expect(readSessionCookie(undefined)).toBeNull(); + expect(readSessionCookie("")).toBeNull(); + expect(readSessionCookie("other=1")).toBeNull(); + expect(readSessionCookie(`${COOKIE_NAME}=`)).toBeNull(); + }); + + it("is not fooled by a name that merely ends the same", () => { + expect(readSessionCookie(`not_${COOKIE_NAME}=theirs`)).toBeNull(); + }); +}); + +describe("talking to the agent", () => { + it("sends the shared secret", async () => { + const fetchImpl = vi.fn(async () => + new Response(JSON.stringify({ providerUrl: "https://x", browserHandle: "h" }), { + status: 200, + }), + ); + const client = createArcadeAgentClient({ + agentUrl: "http://agent.internal:8123/", + agentAuthHeader: "Bearer s3cret", + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + await client.claimTicket("ticket-1"); + + const [url, init] = fetchImpl.mock.calls[0] as unknown as [ + string, + RequestInit, + ]; + expect(url).toBe("http://agent.internal:8123/arcade/claim"); + expect((init.headers as Record).Authorization).toBe( + "Bearer s3cret", + ); + }); + + it("treats a spent ticket as an ordinary answer, not a fault", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn(async () => new Response("", { status: 404 })); + const client = createArcadeAgentClient({ + agentUrl: "http://agent.internal:8123", + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect(await client.claimTicket("gone")).toBeNull(); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it("answers nothing when the agent is unreachable", async () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + const fetchImpl = vi.fn(async () => { + throw new Error("connection refused"); + }); + const client = createArcadeAgentClient({ + agentUrl: "http://agent.internal:8123", + fetchImpl: fetchImpl as unknown as typeof fetch, + }); + + expect( + await client.confirmFlow({ browserHandle: "h", flowId: "f" }), + ).toBeNull(); + warn.mockRestore(); + }); + + it("does not report a malformed agent url as something to retry", async () => { + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + const client = createArcadeAgentClient({ + agentUrl: "not a url", + fetchImpl: vi.fn() as unknown as typeof fetch, + }); + + expect(await client.claimTicket("t")).toBeNull(); + expect(error).toHaveBeenCalled(); + error.mockRestore(); + }); +}); diff --git a/app/arcade-browser-routes.ts b/app/arcade-browser-routes.ts new file mode 100644 index 00000000..8830e5aa --- /dev/null +++ b/app/arcade-browser-routes.ts @@ -0,0 +1,267 @@ +/** + * The two pages a browser sees while connecting an Arcade account. + * + * These live here rather than on the agent for one reason: they are the only + * part of this feature the public internet must reach, and the agent is the + * process holding the provider keys. Keeping the front door on this side means + * the service exposed to the world holds no credentials of its own — it asks + * the agent, over the private network and behind the shared secret, and the + * agent answers in opaque handles. + * + * The problem they solve, established against the live Arcade API rather than + * assumed: when somebody finishes authorizing, Arcade redirects their browser + * back carrying a flow id from an id space nothing on our side can resolve, and + * asks who they are. Arcade's verifier is built for ordinary web apps, where + * the answer comes from a logged-in session. Nobody logs into OpenTag. + * + * So the outbound hop gives the browser a session. A person is handed a link to + * `/arcade/start`; passing through, their browser collects an opaque cookie and + * is sent on to the provider. When Arcade returns them to `/arcade/verify`, + * that cookie is what answers the question. + * + * Nothing identifying travels in either direction. The link carries a ticket + * that means nothing without the agent; the cookie carries a handle that means + * nothing without the agent; and the identity is resolved only there. + */ + +import type { IncomingMessage, ServerResponse } from "node:http"; + +/** Name says nothing about purpose; the value is random and opaque anyway. */ +export const COOKIE_NAME = "otc_session"; + +/** + * Scoped to these routes, so the cookie is not attached to every other request + * this service serves. + */ +export const COOKIE_PATH = "/arcade"; + +/** Matches the agent's own window for a part-finished connection. */ +export const COOKIE_MAX_AGE_SECONDS = 15 * 60; + +export const START_PATH = "/arcade/start"; +export const VERIFY_PATH = "/arcade/verify"; + +/** How long a browser waits on the agent before being told to try again. */ +export const DEFAULT_TIMEOUT_MS = 10_000; + +export interface ClaimedTicket { + providerUrl: string; + browserHandle: string; +} + +export interface ConfirmedFlow { + outcome: "confirmed" | "unknown" | "failed"; + redirectTo: string | null; + message: string; + clearCookie: boolean; +} + +export interface ArcadeAgentClient { + claimTicket(ticket: string): Promise; + confirmFlow(input: { + browserHandle: string | null; + flowId: string | null; + }): Promise; +} + +/** + * Shown when the agent cannot be reached or answers unusably. + * + * Deliberately the same sentence for both: to whoever is looking they are one + * thing — this did not work, ask again — and the difference between them is an + * operator's business, which is why it goes to the log instead. + */ +const UNAVAILABLE = + "This connection could not be completed just now. Ask again in the chat to " + + "start over."; + +const ALREADY_SPENT = + "This connection link has already been used or has expired. Ask again in " + + "the chat for a fresh one."; + +function endpoint(agentUrl: string, path: string): string { + const base = agentUrl.endsWith("/") ? agentUrl : `${agentUrl}/`; + return new URL(path, base).toString(); +} + +/** + * The agent, reached over the private network with the shared secret. + * + * Derived from the URL this process already uses to run the agent rather than + * configured separately: two variables pointing at one service drift, and the + * second one is always the stale one. + */ +export function createArcadeAgentClient(options: { + agentUrl: string; + agentAuthHeader?: string; + fetchImpl?: typeof fetch; + timeoutMs?: number; +}): ArcadeAgentClient { + const fetchImpl = options.fetchImpl ?? fetch; + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + + async function post(path: string, body: unknown): Promise { + let url: string; + try { + url = endpoint(options.agentUrl, path); + } catch { + // A malformed agent URL is a configuration mistake that will never + // resolve itself. Separated from the fetch below so it is not reported + // as "try again". + console.error( + `[opentag] could not build the ${path} request; check AGENT_URL`, + ); + return null; + } + + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), timeoutMs); + try { + const response = await fetchImpl(url, { + method: "POST", + headers: { + "Content-Type": "application/json", + ...(options.agentAuthHeader + ? { Authorization: options.agentAuthHeader } + : {}), + }, + body: JSON.stringify(body), + signal: controller.signal, + }); + if (!response.ok) { + // 404 from the claim route is an ordinary spent ticket, not a fault. + if (response.status !== 404) { + console.warn( + `[opentag] the agent answered ${response.status} for ${path}`, + ); + } + return null; + } + return (await response.json()) as T; + } catch (error) { + console.warn(`[opentag] could not reach the agent for ${path}:`, error); + return null; + } finally { + clearTimeout(timer); + } + } + + return { + claimTicket: (ticket) => post("arcade/claim", { ticket }), + confirmFlow: ({ browserHandle, flowId }) => + post("arcade/confirm", { + browser_handle: browserHandle, + flow_id: flowId, + }), + }; +} + +/** The cookie this service issued, or `null` when the browser carries none. */ +export function readSessionCookie(header: string | undefined): string | null { + if (!header) return null; + for (const part of header.split(";")) { + const separator = part.indexOf("="); + if (separator === -1) continue; + if (part.slice(0, separator).trim() !== COOKIE_NAME) continue; + const value = part.slice(separator + 1).trim(); + return value || null; + } + return null; +} + +function setSessionCookie(response: ServerResponse, handle: string): void { + response.setHeader( + "Set-Cookie", + [ + `${COOKIE_NAME}=${handle}`, + `Path=${COOKIE_PATH}`, + `Max-Age=${COOKIE_MAX_AGE_SECONDS}`, + "HttpOnly", + "Secure", + // Lax, not Strict, and the difference decides whether any of this works: + // the journey back is a navigation from Arcade's site to ours, and Strict + // withholds the cookie on exactly that. Lax sends it for a top-level GET + // and nothing riskier. + "SameSite=Lax", + ].join("; "), + ); +} + +function clearSessionCookie(response: ServerResponse): void { + response.setHeader( + "Set-Cookie", + `${COOKIE_NAME}=; Path=${COOKIE_PATH}; Max-Age=0; HttpOnly; Secure; SameSite=Lax`, + ); +} + +function sendText( + response: ServerResponse, + status: number, + body: string, +): void { + response.statusCode = status; + response.setHeader("Content-Type", "text/plain; charset=utf-8"); + // A connection page is never worth caching, and a cached one would show the + // last person's outcome to the next. + response.setHeader("Cache-Control", "no-store"); + response.end(body); +} + +function redirect(response: ServerResponse, location: string): void { + response.statusCode = 303; + response.setHeader("Location", location); + response.setHeader("Cache-Control", "no-store"); + response.end(); +} + +/** + * Handle one browser request, or answer `false` so the caller can route it. + * + * Written as a wrapper rather than a framework route because this service + * mounts exactly one listener, and two pages do not justify a second one. + */ +export async function handleArcadeBrowserRequest( + request: IncomingMessage, + response: ServerResponse, + client: ArcadeAgentClient, +): Promise { + const url = new URL(request.url ?? "/", "http://placeholder"); + const path = url.pathname.replace(/\/+$/, "") || "/"; + + if (path !== START_PATH && path !== VERIFY_PATH) return false; + if (request.method !== "GET" && request.method !== "HEAD") { + sendText(response, 405, "Method not allowed."); + return true; + } + + if (path === START_PATH) { + const claimed = await client.claimTicket(url.searchParams.get("t") ?? ""); + if (claimed === null) { + sendText(response, 200, ALREADY_SPENT); + return true; + } + setSessionCookie(response, claimed.browserHandle); + redirect(response, claimed.providerUrl); + return true; + } + + const confirmed = await client.confirmFlow({ + browserHandle: readSessionCookie(request.headers.cookie), + flowId: url.searchParams.get("flow_id"), + }); + if (confirmed === null) { + sendText(response, 200, UNAVAILABLE); + return true; + } + if (confirmed.clearCookie) { + // Spent either way. Left behind it is a stale claim on an identity sitting + // in somebody's browser for the rest of its lifetime. + clearSessionCookie(response); + } + if (confirmed.redirectTo) { + redirect(response, confirmed.redirectTo); + return true; + } + sendText(response, 200, confirmed.message || UNAVAILABLE); + return true; +} diff --git a/app/env.test.ts b/app/env.test.ts index 3312d0da..4bfc5e84 100644 --- a/app/env.test.ts +++ b/app/env.test.ts @@ -223,6 +223,7 @@ describe("readEnvironment", () => { "intelligenceGatewayWsUrl", "learningContainerId", "port", + "publicUrl", ]); // Nothing carried the values through under a different shape either. expect(JSON.stringify(environment)).not.toContain("xoxb-unused"); diff --git a/app/env.ts b/app/env.ts index b3d70e8b..a20fc5a4 100644 --- a/app/env.ts +++ b/app/env.ts @@ -15,6 +15,35 @@ export interface AppEnvironment { learningContainerId?: string; channelName: string; port: number; + /** + * Where a browser can reach this service, without a trailing slash. + * + * Needed only for per-person Arcade connections, which is why it is optional: + * a deployment using Composio, or Arcade with shared apps only, never grows a + * public address and must keep starting without one. Railway supplies its own + * as `RAILWAY_PUBLIC_DOMAIN`, so that is read as a fallback rather than made + * a second thing to configure. + */ + publicUrl?: string; +} + +/** + * The address a browser can reach this service on, if there is one. + * + * An explicit setting wins, because a deployment may sit behind a domain the + * platform does not know about. Railway's own variable is a bare hostname with + * no scheme, so it is given one; without that the link built from it is not a + * URL a browser will open. + */ +function readPublicUrl(env: NodeJS.ProcessEnv): string | undefined { + const configured = env.PUBLIC_URL?.trim(); + if (configured) return configured.replace(/\/+$/, ""); + const railway = env.RAILWAY_PUBLIC_DOMAIN?.trim(); + if (!railway) return undefined; + const withScheme = /^https?:\/\//.test(railway) + ? railway + : `https://${railway}`; + return withScheme.replace(/\/+$/, ""); } /** Trimmed, and blank counts as missing — a deploy UI's "unset" is an empty string. */ @@ -73,5 +102,6 @@ export function readEnvironment( // left empty in a deploy UI aborted boot rather than falling back to the // default every other variable here falls back to. port: parsePort(env.PORT?.trim() || undefined), + publicUrl: readPublicUrl(env), }; } diff --git a/app/human-in-the-loop/__tests__/connect-account.test.tsx b/app/human-in-the-loop/__tests__/connect-account.test.tsx index f93f0220..11b4eb96 100644 --- a/app/human-in-the-loop/__tests__/connect-account.test.tsx +++ b/app/human-in-the-loop/__tests__/connect-account.test.tsx @@ -99,7 +99,7 @@ describe("ConnectAccount", () => { // incomplete deployment throws out of the click. Unguarded, that throw is // the dead button this card's whole design exists to prevent. vi.stubEnv("AGENT_URL", ""); - const press = connectButton(ConnectAccount({ toolkit: "gmail" })); + const press = connectButton(ConnectAccount({ request: { toolkit: "gmail", provider: "composio" } })); const { ctx, postEphemeral } = interaction({ id: "U1", kind: "human" }); await press(ctx); @@ -135,7 +135,7 @@ describe("the notice shown when the click could not even be handed over", () => const consoleError = vi .spyOn(console, "error") .mockImplementation(() => undefined); - const press = connectButton(ConnectAccount({ toolkit: "gmail" })); + const press = connectButton(ConnectAccount({ request: { toolkit: "gmail", provider: "composio" } })); const surface = interaction(actor, ephemeral); try { await press(surface.ctx); diff --git a/app/human-in-the-loop/connect-account.tsx b/app/human-in-the-loop/connect-account.tsx index 76d3475c..58593e70 100644 --- a/app/human-in-the-loop/connect-account.tsx +++ b/app/human-in-the-loop/connect-account.tsx @@ -15,10 +15,27 @@ import { } from "@copilotkit/channels"; import type { InteractionContext, Renderable } from "@copilotkit/channels"; import { reportRecoverableError } from "../channel-helpers.js"; +import { normalizeAction } from "../tools/arcade-connect.js"; import { normalizeToolkit } from "../tools/composio-connect.js"; -/** What the button carries. The toolkit only — never an id, never a link. */ -export type ConnectRequest = { toolkit: string }; +/** + * What the button carries. What to connect and who mints it — never an id, + * never a link. + * + * `provider` is recorded rather than looked up at click time so a card that + * outlived a provider change refuses itself, instead of being answered by + * whichever client still happens to be configured. A card from before this + * field existed has none, and is treated as Composio's — which is what every + * one of them was. + * + * `toolkit` is Composio's unit (an app, lowercase) and `target` is Arcade's (a + * qualified action). Exactly one is set, by the provider that minted the card. + */ +export type ConnectRequest = { + toolkit?: string; + target?: string; + provider?: "composio" | "arcade"; +}; /** * Mint and deliver the link for whoever clicked. @@ -42,11 +59,11 @@ export type ConnectRequest = { toolkit: string }; */ async function connect( interaction: InteractionContext, - toolkit: string, + request: ConnectRequest, ) { try { const { handleConnectClick } = await import("../tools/connect-click.js"); - await handleConnectClick(toolkit, interaction); + await handleConnectClick(request, interaction); } catch (error) { // The one outcome this card is shaped to avoid. `handleConnectClick` // reports the failures it can name — a request that came back refused, a @@ -59,7 +76,7 @@ async function connect( // below report non-delivery by *returning*, so the old fixed // `told_the_clicker_privately` was written into the log on the exact runs // where nobody was told. - const recovery = await tellTheClicker(interaction, toolkit); + const recovery = await tellTheClicker(interaction, request); reportRecoverableError(error, { operation: "connect_account_click", recovery, @@ -83,19 +100,23 @@ async function connect( */ async function tellTheClicker( interaction: InteractionContext, - toolkit: string, + request: ConnectRequest, ): Promise { // Rendered into a card, so it gets the treatment every value that reaches a - // rendered surface from outside this repository gets. The slug is checked at - // both entry points already; a card re-derived from stored props is a third - // route in, and this is the one place on it that renders the value. - const slug = normalizeToolkit(toolkit); + // rendered surface from outside this repository gets. Both units are checked + // at their entry points already; a card re-derived from stored props is a + // third route in, and this is the one place on it that renders the value. + const named = + request.provider === "arcade" + ? normalizeAction(request.target ?? "") !== null + : normalizeToolkit(request.toolkit ?? "") !== null; + const label = requestLabel(request); const notice: Renderable = ( ); @@ -135,8 +156,22 @@ async function tellTheClicker( } } -export function ConnectAccount({ toolkit }: { toolkit: string }) { - const label = toolkit.charAt(0).toUpperCase() + toolkit.slice(1); +/** + * The app a request is about, for showing somebody. + * + * Composio names an app directly; Arcade names an action, whose app is the half + * before the dot. Both are already validated by the time they reach a card — + * this only chooses which to read, it does not make anything safe. + */ +export function requestLabel(request: ConnectRequest): string { + const name = request.target + ? request.target.slice(0, request.target.indexOf(".")) + : (request.toolkit ?? ""); + return name.charAt(0).toUpperCase() + name.slice(1); +} + +export function ConnectAccount({ request }: { request: ConnectRequest }) { + const label = requestLabel(request); return (
{`🔗 Connect ${label}`}
@@ -145,10 +180,10 @@ export function ConnectAccount({ toolkit }: { toolkit: string }) {