From e6c37aa3097ef60ff2a52e56d832a47d2b99a813 Mon Sep 17 00:00:00 2001 From: Hynek Urban Date: Fri, 10 Jul 2026 08:14:54 +0000 Subject: [PATCH 1/2] Add O(1) tunnel-for-agent lookup to speed up Cloudflare sharing Cloudflare sharing in minds fired 'mngr imbue_cloud tunnels list' to find an agent's tunnel, which enumerated every tunnel on the account and fetched each one's config server-side (O(n) Cloudflare calls) -- 8-11s on accounts with many tunnels. minds always knows the exact tunnel name it wants (--), so add: - Connector: GET /tunnels/by-agent/{agent_id} + ForwardingCtx.get_tunnel_for_agent, resolving the tunnel via Cloudflare's server-side name filter plus one config fetch (2 Cloudflare calls regardless of N); 404 when absent. - mngr_imbue_cloud: 'tunnels find-by-agent' CLI + client.find_tunnel_for_agent (emits/parses null when absent). - minds: point ImbueCloudCli.find_tunnel_for_agent at the new lookup. --- apps/minds/changelog/mngr-speed-up-sharing.md | 1 + .../minds/desktop_client/imbue_cloud_cli.py | 33 ++++---- .../desktop_client/imbue_cloud_cli_test.py | 33 ++++++++ apps/remote_service_connector/README.md | 1 + .../changelog/mngr-speed-up-sharing.md | 1 + .../imbue/remote_service_connector/app.py | 42 ++++++++++ .../remote_service_connector/app_test.py | 79 +++++++++++++++++++ .../changelog/mngr-speed-up-sharing.md | 1 + .../imbue/mngr_imbue_cloud/cli/tunnels.py | 28 +++++++ .../mngr_imbue_cloud/cli/tunnels_test.py | 16 ++++ .../mngr_imbue_cloud/connector/client.py | 21 +++++ .../mngr_imbue_cloud/connector/client_test.py | 20 +++++ 12 files changed, 257 insertions(+), 19 deletions(-) create mode 100644 apps/minds/changelog/mngr-speed-up-sharing.md create mode 100644 apps/remote_service_connector/changelog/mngr-speed-up-sharing.md create mode 100644 libs/mngr_imbue_cloud/changelog/mngr-speed-up-sharing.md create mode 100644 libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels_test.py diff --git a/apps/minds/changelog/mngr-speed-up-sharing.md b/apps/minds/changelog/mngr-speed-up-sharing.md new file mode 100644 index 0000000000..7a43f6effb --- /dev/null +++ b/apps/minds/changelog/mngr-speed-up-sharing.md @@ -0,0 +1 @@ +Cloudflare sharing is now much faster. `ImbueCloudCli.find_tunnel_for_agent` (used by the sharing-status and disable-sharing flows) now delegates to the connector's new O(1) `tunnels find-by-agent` lookup instead of running `tunnels list`, which enumerated every tunnel on the account and fetched each one's config server-side. On accounts with many tunnels this call had grown to 8-11 seconds; it now completes in well under a second regardless of how many tunnels exist. diff --git a/apps/minds/imbue/minds/desktop_client/imbue_cloud_cli.py b/apps/minds/imbue/minds/desktop_client/imbue_cloud_cli.py index 4f42557f78..e12e81e84c 100644 --- a/apps/minds/imbue/minds/desktop_client/imbue_cloud_cli.py +++ b/apps/minds/imbue/minds/desktop_client/imbue_cloud_cli.py @@ -44,13 +44,6 @@ # desktop client. _CONNECTOR_URL_SUBPROCESS_ENV: str = "MNGR__PROVIDERS__IMBUE_CLOUD__CONNECTOR_URL" -# Mirrors ``apps/remote_service_connector/.../app.py`` -- the connector uses -# the first 16 hex chars of the agent UUID (after stripping ``"agent-"``) as -# the trailing slug of every tunnel name. Used by ``find_tunnel_for_agent`` -# to filter ``list_tunnels`` output without having to know the per-account -# username prefix. -_AGENT_ID_PREFIX_LENGTH = 16 - class ImbueCloudCliError(MindError): """Raised when a `mngr imbue_cloud ...` invocation returns a non-zero exit code. @@ -559,23 +552,25 @@ def get_service_auth(self, account: str, tunnel_name: str, service_name: str) -> def find_tunnel_for_agent(self, account: str, agent_id: str) -> TunnelInfo | None: """Return the tunnel registered for ``agent_id`` under ``account``, or None. - Uses ``list_tunnels`` and matches on the trailing-slug convention the - connector uses for tunnel names: ``--``, - where ``short_agent`` is the first 16 hex chars of the agent UUID - (``"agent-"`` prefix stripped). Stable contract -- changing the - truncation length on the connector side requires updating - ``_AGENT_ID_PREFIX_LENGTH`` here in lockstep. + Delegates to the connector's O(1) ``tunnels find-by-agent`` lookup, + which resolves the exact tunnel via Cloudflare's server-side name + filter (2 Cloudflare calls) instead of enumerating every tunnel and + fetching each one's config -- the old ``list_tunnels`` path was O(n) + in the number of tunnels on the account and dominated the sharing + flow's latency. Returning ``None`` lets the sharing-status route distinguish "tunnel doesn't exist yet" (the user hasn't enabled sharing) from "tunnel exists but no service is registered for this name". """ - short_agent = agent_id.removeprefix("agent-")[:_AGENT_ID_PREFIX_LENGTH] - suffix = f"--{short_agent}" - for tunnel in self.list_tunnels(account): - if tunnel.tunnel_name.endswith(suffix): - return tunnel - return None + result = self._run( + ["tunnels", "find-by-agent", agent_id, "--account", account], + cg_name="imbue-cloud-tunnels-find-by-agent", + ) + body = self._expect_success(result, "tunnels find-by-agent") + if body is None: + return None + return TunnelInfo.model_validate(body) # ------------------------------------------------------------------ # R2 buckets (one per workspace; used to back up the host_dir via restic) diff --git a/apps/minds/imbue/minds/desktop_client/imbue_cloud_cli_test.py b/apps/minds/imbue/minds/desktop_client/imbue_cloud_cli_test.py index bd8eefe159..eb0f528325 100644 --- a/apps/minds/imbue/minds/desktop_client/imbue_cloud_cli_test.py +++ b/apps/minds/imbue/minds/desktop_client/imbue_cloud_cli_test.py @@ -53,3 +53,36 @@ def test_run_routes_through_mngr_caller_with_home_cwd_and_connector_env() -> Non assert recorded.cwd == Path.home() # The trailing slash is stripped so the plugin builds clean URLs. assert recorded.env_overrides == {_CONNECTOR_URL_SUBPROCESS_ENV: "https://connector.example"} + + +def test_find_tunnel_for_agent_uses_find_by_agent_subcommand() -> None: + """``find_tunnel_for_agent`` delegates to the connector's O(1) ``tunnels + find-by-agent`` lookup rather than listing every tunnel, and parses the + returned tunnel JSON.""" + tunnel_json = {"tunnel_name": "owner--abc123", "tunnel_id": "t-1", "services": ["web"]} + caller = RecordingMngrCaller(result=MngrCallResult(returncode=0, stdout=json.dumps(tunnel_json))) + cli = ImbueCloudCli(mngr_caller=caller, connector_url=AnyUrl("https://connector.example/")) + + tunnel = cli.find_tunnel_for_agent(account="owner@example.com", agent_id="agent-abc123") + + assert tunnel is not None + assert tunnel.tunnel_name == "owner--abc123" + assert tunnel.services == ("web",) + recorded = caller.recorded_calls[0] + assert recorded.argv == ( + "imbue_cloud", + "tunnels", + "find-by-agent", + "agent-abc123", + "--account", + "owner@example.com", + ) + + +def test_find_tunnel_for_agent_returns_none_when_plugin_emits_null() -> None: + """When no tunnel exists for the agent, the plugin emits the JSON literal + ``null`` and ``find_tunnel_for_agent`` maps it to ``None``.""" + caller = RecordingMngrCaller(result=MngrCallResult(returncode=0, stdout=json.dumps(None))) + cli = ImbueCloudCli(mngr_caller=caller, connector_url=AnyUrl("https://connector.example/")) + + assert cli.find_tunnel_for_agent(account="owner@example.com", agent_id="agent-abc123") is None diff --git a/apps/remote_service_connector/README.md b/apps/remote_service_connector/README.md index 27349385e0..10359fd6be 100644 --- a/apps/remote_service_connector/README.md +++ b/apps/remote_service_connector/README.md @@ -144,6 +144,7 @@ When `CLOUDFLARE_ALLOWED_IDPS` is set, Access Applications created for forwarded - `POST /tunnels` -- Create a tunnel. Body: `{"agent_id": "...", "default_auth_policy": ...}`. Returns tunnel info with token. - `GET /tunnels` -- List your tunnels with their configured services. +- `GET /tunnels/by-agent/{agent_id}` -- Resolve your tunnel for a single agent (O(1)): looks the tunnel up by its exact name (`--`) via Cloudflare's server-side name filter plus one config fetch, instead of enumerating every tunnel like `GET /tunnels`. Returns the tunnel info (no token) or 404 when no tunnel exists for that agent yet. - `DELETE /tunnels/{tunnel_name}` -- Delete a tunnel and all its DNS records, Access Applications, ingress rules, and KV entries. ### Services (admin or agent) diff --git a/apps/remote_service_connector/changelog/mngr-speed-up-sharing.md b/apps/remote_service_connector/changelog/mngr-speed-up-sharing.md new file mode 100644 index 0000000000..ffde76909f --- /dev/null +++ b/apps/remote_service_connector/changelog/mngr-speed-up-sharing.md @@ -0,0 +1 @@ +Added an O(1) `GET /tunnels/by-agent/{agent_id}` endpoint. minds always knows the exact tunnel name it wants (`--`), so this resolves the tunnel via Cloudflare's server-side name filter plus a single config fetch (2 Cloudflare calls regardless of how many tunnels the account owns) and returns 404 when no tunnel exists for the agent yet. Previously the only way to find an agent's tunnel was `GET /tunnels`, which enumerated every tunnel under the user prefix and fetched each one's config (O(n) Cloudflare calls), dominating the latency of the sharing flow. diff --git a/apps/remote_service_connector/imbue/remote_service_connector/app.py b/apps/remote_service_connector/imbue/remote_service_connector/app.py index d94214913f..38857b2948 100644 --- a/apps/remote_service_connector/imbue/remote_service_connector/app.py +++ b/apps/remote_service_connector/imbue/remote_service_connector/app.py @@ -1290,6 +1290,26 @@ def list_tunnels(self, username: str) -> list[TunnelInfo]: result.append(TunnelInfo(tunnel_name=name, tunnel_id=tid, services=services)) return result + def get_tunnel_for_agent(self, username: str, agent_id: str) -> TunnelInfo | None: + """Resolve the caller's tunnel for a single agent in O(1) Cloudflare calls. + + minds always knows the exact tunnel name it wants + (``--``), so this resolves the tunnel via + Cloudflare's server-side name filter (:func:`cf_get_tunnel_by_name`) + plus a single config fetch -- 2 Cloudflare calls regardless of how + many tunnels the account owns. Contrast with :meth:`list_tunnels`, + which enumerates every tunnel under the user prefix and fetches each + one's config (O(n) calls). Returns ``None`` when the user has no + tunnel for the agent yet. + """ + name = make_tunnel_name(username, agent_id) + tunnel = self.ops.get_tunnel_by_name(name) + if tunnel is None: + return None + tid = tunnel["id"] + services = self._list_services(tid, name, username) + return TunnelInfo(tunnel_name=name, tunnel_id=tid, services=services) + def delete_tunnel(self, tunnel_name: str, username: str) -> None: self.verify_ownership(tunnel_name, username) tunnel = self.get_tunnel_or_raise(tunnel_name) @@ -2360,6 +2380,28 @@ def list_tunnels(request: Request) -> list[dict[str, object]]: return [t.model_dump() for t in get_ctx().list_tunnels(admin.username)] +@web_app.get("/tunnels/by-agent/{agent_id}") +def get_tunnel_for_agent(request: Request, agent_id: str) -> dict[str, object]: + """Resolve the authenticated user's tunnel for ``agent_id`` (O(1) lookup). + + Uses Cloudflare's server-side name filter plus one config fetch (2 + Cloudflare calls) instead of the O(n) ``GET /tunnels`` path that + enumerates every tunnel and fetches each one's config. Returns 404 when + the user has no tunnel for the agent yet, letting the client distinguish + "sharing not enabled" from other failures. The static ``by-agent`` prefix + can never collide with a real ``{tunnel_name}`` (those always contain the + ``--`` separator), so there is no ambiguity with the other ``/tunnels/*`` + routes. + """ + with handle_endpoint_errors(): + auth = authenticate_request(request, get_ctx().ops) + admin = require_admin(auth) + tunnel = get_ctx().get_tunnel_for_agent(admin.username, agent_id) + if tunnel is None: + raise HTTPException(status_code=404, detail=f"No tunnel found for agent '{agent_id}'") + return tunnel.model_dump() + + @web_app.delete("/tunnels/{tunnel_name}") def delete_tunnel(request: Request, tunnel_name: str) -> dict[str, str]: """Delete a tunnel and all its associated DNS records, Access Applications, ingress rules, and KV entries. diff --git a/apps/remote_service_connector/imbue/remote_service_connector/app_test.py b/apps/remote_service_connector/imbue/remote_service_connector/app_test.py index 62bc05a7ea..d54399ef84 100644 --- a/apps/remote_service_connector/imbue/remote_service_connector/app_test.py +++ b/apps/remote_service_connector/imbue/remote_service_connector/app_test.py @@ -17,6 +17,7 @@ from imbue.remote_service_connector.app import AdminAuth from imbue.remote_service_connector.app import AuthPolicy from imbue.remote_service_connector.app import CloudflareApiError +from imbue.remote_service_connector.app import ForwardingCtx from imbue.remote_service_connector.app import HttpCloudflareOps from imbue.remote_service_connector.app import InvalidR2BucketNameError from imbue.remote_service_connector.app import InvalidTunnelComponentError @@ -214,6 +215,63 @@ def test_list_tunnels_filters_by_user() -> None: assert len(tunnels) == 2 +def test_get_tunnel_for_agent_returns_none_when_absent() -> None: + ctx = make_fake_forwarding_ctx() + assert ctx.get_tunnel_for_agent("alice", "agent1") is None + + +def test_get_tunnel_for_agent_returns_tunnel_with_services() -> None: + ctx = make_fake_forwarding_ctx() + ctx.create_tunnel("alice", "agent1") + ctx.add_service("alice--agent1", "alice", "web", "http://localhost:8080") + tunnel = ctx.get_tunnel_for_agent("alice", "agent1") + assert tunnel is not None + assert tunnel.tunnel_name == "alice--agent1" + assert [s.service_name for s in tunnel.services] == ["web"] + + +class _CallCountingCloudflareOps(FakeCloudflareOps): + """FakeCloudflareOps that counts the O(n)-prone tunnel calls. + + Used to assert the ``get_tunnel_for_agent`` fast path never enumerates the + account (``list_tunnels``) and fetches only the matched tunnel's config. + """ + + def __init__(self) -> None: + super().__init__() + self.list_tunnels_calls = 0 + self.get_tunnel_config_calls = 0 + + def list_tunnels(self, include_prefix: str = "") -> list[dict[str, Any]]: + self.list_tunnels_calls += 1 + return super().list_tunnels(include_prefix=include_prefix) + + def get_tunnel_config(self, tunnel_id: str) -> dict[str, Any]: + self.get_tunnel_config_calls += 1 + return super().get_tunnel_config(tunnel_id) + + +def test_get_tunnel_for_agent_targets_by_name_not_enumeration() -> None: + """The O(1) lookup must resolve the exact tunnel without enumerating the + account (``list_tunnels``) or fetching every tunnel's config. + + Creates many tunnels for the user, then counts the expensive calls: the + lookup must hit ``get_tunnel_config`` exactly once (for the matched + tunnel) and never call ``list_tunnels``. + """ + ops = _CallCountingCloudflareOps() + ctx = ForwardingCtx(ops=ops, domain="example.com") + for i in range(10): + ctx.create_tunnel("alice", f"agent{i}") + ops.get_tunnel_config_calls = 0 + ops.list_tunnels_calls = 0 + tunnel = ctx.get_tunnel_for_agent("alice", "agent7") + assert tunnel is not None + assert tunnel.tunnel_name == "alice--agent7" + assert ops.get_tunnel_config_calls == 1 + assert ops.list_tunnels_calls == 0 + + def test_delete_tunnel_cascades() -> None: ctx = make_fake_forwarding_ctx() ctx.create_tunnel("alice", "agent1") @@ -410,6 +468,27 @@ def test_route_list_tunnels_admin(monkeypatch: pytest.MonkeyPatch) -> None: assert len(resp.json()) == 1 +def test_route_get_tunnel_for_agent_admin(monkeypatch: pytest.MonkeyPatch) -> None: + client = _make_test_client(monkeypatch) + client.post("/tunnels", json={"agent_id": "agent1"}, headers=_admin_headers()) + resp = client.get("/tunnels/by-agent/agent1", headers=_admin_headers()) + assert resp.status_code == 200 + assert resp.json()["tunnel_name"] == "testuser--agent1" + + +def test_route_get_tunnel_for_agent_returns_404_when_absent(monkeypatch: pytest.MonkeyPatch) -> None: + client = _make_test_client(monkeypatch) + resp = client.get("/tunnels/by-agent/agent1", headers=_admin_headers()) + assert resp.status_code == 404 + + +def test_route_get_tunnel_for_agent_agent_forbidden(monkeypatch: pytest.MonkeyPatch) -> None: + client = _make_test_client(monkeypatch) + client.post("/tunnels", json={"agent_id": "agent1"}, headers=_admin_headers()) + resp = client.get("/tunnels/by-agent/agent1", headers=_agent_headers("tunnel-1")) + assert resp.status_code == 403 + + def test_route_add_service_admin(monkeypatch: pytest.MonkeyPatch) -> None: client = _make_test_client(monkeypatch) client.post("/tunnels", json={"agent_id": "agent1"}, headers=_admin_headers()) diff --git a/libs/mngr_imbue_cloud/changelog/mngr-speed-up-sharing.md b/libs/mngr_imbue_cloud/changelog/mngr-speed-up-sharing.md new file mode 100644 index 0000000000..1fb6a7c777 --- /dev/null +++ b/libs/mngr_imbue_cloud/changelog/mngr-speed-up-sharing.md @@ -0,0 +1 @@ +Added a `mngr imbue_cloud tunnels find-by-agent ` subcommand (and the backing `ImbueCloudConnectorClient.find_tunnel_for_agent`) that resolves the caller's tunnel for a single agent via the connector's new O(1) `GET /tunnels/by-agent/{agent_id}` lookup. It emits the tunnel JSON, or the JSON literal `null` when no tunnel exists for the agent yet. This replaces the previous approach of listing every tunnel on the account and filtering client-side, which grew linearly with the number of tunnels and made the sharing flow slow. diff --git a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels.py b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels.py index 6f2f2e0f4b..4d0ea22c7f 100644 --- a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels.py +++ b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels.py @@ -84,6 +84,34 @@ def list_tunnels(account: str | None, connector_url: str | None) -> None: ) +@tunnels.command(name="find-by-agent") +@click.argument("agent_id") +@click.option("--account", default=None, help="Account email (defaults to the active account)") +@click.option("--connector-url", default=None, help="Override connector URL") +@handle_imbue_cloud_errors +def find_by_agent(agent_id: str, account: str | None, connector_url: str | None) -> None: + """Resolve this account's tunnel for AGENT_ID via the connector's O(1) lookup. + + Emits the tunnel JSON, or the JSON literal ``null`` when no tunnel exists + for the agent yet. + """ + client = make_connector_client(connector_url) + store = make_session_store() + parsed_account = resolve_account_or_active(store, account) + token = get_active_token(store, client, parsed_account) + info = client.find_tunnel_for_agent(token, agent_id) + if info is None: + emit_json(None) + return + emit_json( + { + "tunnel_name": info.tunnel_name, + "tunnel_id": info.tunnel_id, + "services": list(info.services), + } + ) + + @tunnels.command(name="delete") @click.argument("tunnel_name") @click.option("--account", default=None, help="Account email (defaults to the active account)") diff --git a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels_test.py b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels_test.py new file mode 100644 index 0000000000..bd64de2e8a --- /dev/null +++ b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels_test.py @@ -0,0 +1,16 @@ +from click.testing import CliRunner + +from imbue.mngr_imbue_cloud.cli.tunnels import tunnels + + +def test_tunnels_group_lists_subcommands() -> None: + result = CliRunner().invoke(tunnels, ["--help"]) + assert result.exit_code == 0 + for name in ("create", "list", "find-by-agent", "delete", "services", "auth"): + assert name in result.output + + +def test_find_by_agent_help_documents_agent_argument() -> None: + result = CliRunner().invoke(tunnels, ["find-by-agent", "--help"]) + assert result.exit_code == 0 + assert "AGENT_ID" in result.output diff --git a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client.py b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client.py index bafa29a91c..74faddf645 100644 --- a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client.py +++ b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client.py @@ -509,6 +509,27 @@ def list_tunnels(self, access_token: SecretStr) -> list[TunnelInfo]: return [] return [_parse_tunnel_info(entry) for entry in body if isinstance(entry, dict)] + def find_tunnel_for_agent(self, access_token: SecretStr, agent_id: str) -> TunnelInfo | None: + """Resolve the caller's tunnel for ``agent_id`` via the O(1) connector lookup. + + Hits ``GET /tunnels/by-agent/{agent_id}``, which resolves the exact + tunnel through Cloudflare's server-side name filter (2 Cloudflare + calls) rather than enumerating every tunnel and fetching each one's + config. Returns ``None`` when the connector reports 404 (no tunnel for + the agent yet). + """ + response = self._send( + "GET", + self._url(f"/tunnels/by-agent/{agent_id}"), + exc_cls=ImbueCloudTunnelError, + headers=self._bearer(access_token), + timeout=self.timeout_seconds, + ) + if response.status_code == 404: + return None + body = self._check(response, ImbueCloudTunnelError) + return _parse_tunnel_info(body) + def delete_tunnel(self, access_token: SecretStr, tunnel_name: str) -> None: response = self._send( "DELETE", diff --git a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client_test.py b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client_test.py index 45307ed602..04c9fa3bc8 100644 --- a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client_test.py +++ b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client_test.py @@ -547,3 +547,23 @@ def handler(request: httpx.Request) -> httpx.Response: with pytest.raises(ImbueCloudTunnelError): client.list_tunnels(SecretStr("tok")) assert state["calls"] == 1 + + +def test_find_tunnel_for_agent_parses_tunnel(monkeypatch: pytest.MonkeyPatch) -> None: + def handler(request: httpx.Request) -> httpx.Response: + assert request.url.path == "/tunnels/by-agent/agent-abc123" + return httpx.Response(200, json={"tunnel_name": "owner--abc123", "tunnel_id": "t-1", "services": ["web"]}) + + client, _state = _install_flaky_httpx_get(monkeypatch, fail_times=0, handler=handler) + tunnel = client.find_tunnel_for_agent(SecretStr("tok"), "agent-abc123") + assert tunnel is not None + assert tunnel.tunnel_name == "owner--abc123" + assert tunnel.services == ("web",) + + +def test_find_tunnel_for_agent_returns_none_on_404(monkeypatch: pytest.MonkeyPatch) -> None: + def handler(request: httpx.Request) -> httpx.Response: + return httpx.Response(404, json={"detail": "No tunnel found for agent 'agent-abc123'"}) + + client, _state = _install_flaky_httpx_get(monkeypatch, fail_times=0, handler=handler) + assert client.find_tunnel_for_agent(SecretStr("tok"), "agent-abc123") is None From fc4c95f3e9908a40dc6469683cc2d131caf76958 Mon Sep 17 00:00:00 2001 From: Hynek Urban Date: Fri, 10 Jul 2026 08:36:22 +0000 Subject: [PATCH 2/2] Make find_tunnel_for_agent tolerate connectors without the by-agent endpoint Minds clients update independently of (and often ahead of) the deployed connector, so a client with the new 'tunnels find-by-agent' code can hit a connector that lacks GET /tunnels/by-agent/{agent_id}. The endpoint's unknown-route 404 was being mapped to 'no tunnel', so sharing status was stuck at enabled=false against an un-redeployed connector. - Connector: return HTTP 200 + null for 'no tunnel' (reserving 404 for 'endpoint absent'). - Client: on 404, transparently fall back to the O(n) GET /tunnels enumeration (matching the -- slug); on 200+null, return None with no fallback. Fast path is unchanged when the tunnel exists. --- apps/minds/changelog/mngr-speed-up-sharing.md | 2 + apps/remote_service_connector/README.md | 2 +- .../changelog/mngr-speed-up-sharing.md | 4 +- .../imbue/remote_service_connector/app.py | 23 +++++----- .../remote_service_connector/app_test.py | 7 ++- .../changelog/mngr-speed-up-sharing.md | 2 + .../mngr_imbue_cloud/connector/client.py | 44 ++++++++++++++++--- .../mngr_imbue_cloud/connector/client_test.py | 41 ++++++++++++++++- 8 files changed, 104 insertions(+), 21 deletions(-) diff --git a/apps/minds/changelog/mngr-speed-up-sharing.md b/apps/minds/changelog/mngr-speed-up-sharing.md index 7a43f6effb..689a57f5b8 100644 --- a/apps/minds/changelog/mngr-speed-up-sharing.md +++ b/apps/minds/changelog/mngr-speed-up-sharing.md @@ -1 +1,3 @@ Cloudflare sharing is now much faster. `ImbueCloudCli.find_tunnel_for_agent` (used by the sharing-status and disable-sharing flows) now delegates to the connector's new O(1) `tunnels find-by-agent` lookup instead of running `tunnels list`, which enumerated every tunnel on the account and fetched each one's config server-side. On accounts with many tunnels this call had grown to 8-11 seconds; it now completes in well under a second regardless of how many tunnels exist. + +The lookup degrades gracefully against a connector that has not been redeployed yet: because the desktop app updates independently of the connector, the underlying client falls back to the old enumeration when the new endpoint is absent, so sharing status keeps working during the rollout window (just at the old speed until the connector is redeployed). diff --git a/apps/remote_service_connector/README.md b/apps/remote_service_connector/README.md index 10359fd6be..16149c5e73 100644 --- a/apps/remote_service_connector/README.md +++ b/apps/remote_service_connector/README.md @@ -144,7 +144,7 @@ When `CLOUDFLARE_ALLOWED_IDPS` is set, Access Applications created for forwarded - `POST /tunnels` -- Create a tunnel. Body: `{"agent_id": "...", "default_auth_policy": ...}`. Returns tunnel info with token. - `GET /tunnels` -- List your tunnels with their configured services. -- `GET /tunnels/by-agent/{agent_id}` -- Resolve your tunnel for a single agent (O(1)): looks the tunnel up by its exact name (`--`) via Cloudflare's server-side name filter plus one config fetch, instead of enumerating every tunnel like `GET /tunnels`. Returns the tunnel info (no token) or 404 when no tunnel exists for that agent yet. +- `GET /tunnels/by-agent/{agent_id}` -- Resolve your tunnel for a single agent (O(1)): looks the tunnel up by its exact name (`--`) via Cloudflare's server-side name filter plus one config fetch, instead of enumerating every tunnel like `GET /tunnels`. Returns the tunnel info (no token), or HTTP 200 with `null` when no tunnel exists for that agent yet. 404 is reserved for "this connector predates the endpoint" (an unknown route), which lets clients that are newer than the connector fall back to enumerating `GET /tunnels`. - `DELETE /tunnels/{tunnel_name}` -- Delete a tunnel and all its DNS records, Access Applications, ingress rules, and KV entries. ### Services (admin or agent) diff --git a/apps/remote_service_connector/changelog/mngr-speed-up-sharing.md b/apps/remote_service_connector/changelog/mngr-speed-up-sharing.md index ffde76909f..3b57848b8f 100644 --- a/apps/remote_service_connector/changelog/mngr-speed-up-sharing.md +++ b/apps/remote_service_connector/changelog/mngr-speed-up-sharing.md @@ -1 +1,3 @@ -Added an O(1) `GET /tunnels/by-agent/{agent_id}` endpoint. minds always knows the exact tunnel name it wants (`--`), so this resolves the tunnel via Cloudflare's server-side name filter plus a single config fetch (2 Cloudflare calls regardless of how many tunnels the account owns) and returns 404 when no tunnel exists for the agent yet. Previously the only way to find an agent's tunnel was `GET /tunnels`, which enumerated every tunnel under the user prefix and fetched each one's config (O(n) Cloudflare calls), dominating the latency of the sharing flow. +Added an O(1) `GET /tunnels/by-agent/{agent_id}` endpoint. Minds always knows the exact tunnel name it wants (`--`), so this resolves the tunnel via Cloudflare's server-side name filter plus a single config fetch (2 Cloudflare calls regardless of how many tunnels the account owns) and returns HTTP 200 with `null` when no tunnel exists for the agent yet. Previously the only way to find an agent's tunnel was `GET /tunnels`, which enumerated every tunnel under the user prefix and fetched each one's config (O(n) Cloudflare calls), dominating the latency of the sharing flow. + +"No tunnel" is 200 + `null` rather than 404 on purpose: 404 is reserved for an unknown route, so a client that is newer than the connector can tell "this connector predates the endpoint" (404 -> fall back to enumerating `GET /tunnels`) apart from "the endpoint works and there is simply no tunnel yet" (200 + `null`). diff --git a/apps/remote_service_connector/imbue/remote_service_connector/app.py b/apps/remote_service_connector/imbue/remote_service_connector/app.py index 38857b2948..2438a44199 100644 --- a/apps/remote_service_connector/imbue/remote_service_connector/app.py +++ b/apps/remote_service_connector/imbue/remote_service_connector/app.py @@ -2381,25 +2381,28 @@ def list_tunnels(request: Request) -> list[dict[str, object]]: @web_app.get("/tunnels/by-agent/{agent_id}") -def get_tunnel_for_agent(request: Request, agent_id: str) -> dict[str, object]: +def get_tunnel_for_agent(request: Request, agent_id: str) -> dict[str, object] | None: """Resolve the authenticated user's tunnel for ``agent_id`` (O(1) lookup). Uses Cloudflare's server-side name filter plus one config fetch (2 Cloudflare calls) instead of the O(n) ``GET /tunnels`` path that - enumerates every tunnel and fetches each one's config. Returns 404 when - the user has no tunnel for the agent yet, letting the client distinguish - "sharing not enabled" from other failures. The static ``by-agent`` prefix - can never collide with a real ``{tunnel_name}`` (those always contain the - ``--`` separator), so there is no ambiguity with the other ``/tunnels/*`` - routes. + enumerates every tunnel and fetches each one's config. The static + ``by-agent`` prefix can never collide with a real ``{tunnel_name}`` + (those always contain the ``--`` separator), so there is no ambiguity + with the other ``/tunnels/*`` routes. + + Returns HTTP 200 with ``null`` when the user has no tunnel for the agent + yet (rather than 404). This is deliberate: a client hitting a connector + that predates this endpoint gets FastAPI's generic 404-for-unknown-route, + so reserving 404 exclusively for "endpoint absent" lets the client tell + "this connector is too old, fall back to enumerating ``GET /tunnels``" + apart from "the endpoint works and there is simply no tunnel" (200 null). """ with handle_endpoint_errors(): auth = authenticate_request(request, get_ctx().ops) admin = require_admin(auth) tunnel = get_ctx().get_tunnel_for_agent(admin.username, agent_id) - if tunnel is None: - raise HTTPException(status_code=404, detail=f"No tunnel found for agent '{agent_id}'") - return tunnel.model_dump() + return tunnel.model_dump() if tunnel is not None else None @web_app.delete("/tunnels/{tunnel_name}") diff --git a/apps/remote_service_connector/imbue/remote_service_connector/app_test.py b/apps/remote_service_connector/imbue/remote_service_connector/app_test.py index d54399ef84..4272292842 100644 --- a/apps/remote_service_connector/imbue/remote_service_connector/app_test.py +++ b/apps/remote_service_connector/imbue/remote_service_connector/app_test.py @@ -476,10 +476,13 @@ def test_route_get_tunnel_for_agent_admin(monkeypatch: pytest.MonkeyPatch) -> No assert resp.json()["tunnel_name"] == "testuser--agent1" -def test_route_get_tunnel_for_agent_returns_404_when_absent(monkeypatch: pytest.MonkeyPatch) -> None: +def test_route_get_tunnel_for_agent_returns_null_when_absent(monkeypatch: pytest.MonkeyPatch) -> None: + # 200 + null (not 404) so a client can tell "no tunnel" apart from + # "this connector predates the endpoint" (an unknown-route 404). client = _make_test_client(monkeypatch) resp = client.get("/tunnels/by-agent/agent1", headers=_admin_headers()) - assert resp.status_code == 404 + assert resp.status_code == 200 + assert resp.json() is None def test_route_get_tunnel_for_agent_agent_forbidden(monkeypatch: pytest.MonkeyPatch) -> None: diff --git a/libs/mngr_imbue_cloud/changelog/mngr-speed-up-sharing.md b/libs/mngr_imbue_cloud/changelog/mngr-speed-up-sharing.md index 1fb6a7c777..962f036be8 100644 --- a/libs/mngr_imbue_cloud/changelog/mngr-speed-up-sharing.md +++ b/libs/mngr_imbue_cloud/changelog/mngr-speed-up-sharing.md @@ -1 +1,3 @@ Added a `mngr imbue_cloud tunnels find-by-agent ` subcommand (and the backing `ImbueCloudConnectorClient.find_tunnel_for_agent`) that resolves the caller's tunnel for a single agent via the connector's new O(1) `GET /tunnels/by-agent/{agent_id}` lookup. It emits the tunnel JSON, or the JSON literal `null` when no tunnel exists for the agent yet. This replaces the previous approach of listing every tunnel on the account and filtering client-side, which grew linearly with the number of tunnels and made the sharing flow slow. + +The client is backward compatible with a connector that predates the endpoint: since Minds clients update independently of (and often ahead of) the connector, a 404 on `GET /tunnels/by-agent/{agent_id}` is treated as "this connector is too old" and the client transparently falls back to the old `GET /tunnels` enumeration (matching on the `--` name convention). A genuine "no tunnel yet" is signalled by HTTP 200 with `null`, so it never triggers the fallback. Once the connector is redeployed, every call takes the fast path. diff --git a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client.py b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client.py index 74faddf645..ff15bf1845 100644 --- a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client.py +++ b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client.py @@ -51,6 +51,17 @@ DEFAULT_TIMEOUT_SECONDS = 30.0 KEY_OP_TIMEOUT_SECONDS = 90.0 +# Tunnel-name convention mirrored from the connector +# (``apps/remote_service_connector/.../app.py``): every tunnel is named +# ``--``, where ```` is the first 16 hex +# chars of the agent UUID (``"agent-"`` prefix stripped). Used only by the +# ``find_tunnel_for_agent`` back-compat fallback, which enumerates tunnels and +# matches on this trailing slug when the connector lacks the O(1) by-agent +# endpoint. Keep in lockstep with the connector's ``TUNNEL_NAME_SEP`` / +# ``_AGENT_ID_PREFIX_LENGTH``. +_TUNNEL_NAME_SEP = "--" +_AGENT_ID_PREFIX_LENGTH = 16 + # Transient-transport retry policy for connector calls. The connector is a # Modal app that scales to zero, so a call hitting a cold/scaling instance can # fail at the transport layer (DNS "Name or service not known" -> ConnectError, @@ -510,13 +521,21 @@ def list_tunnels(self, access_token: SecretStr) -> list[TunnelInfo]: return [_parse_tunnel_info(entry) for entry in body if isinstance(entry, dict)] def find_tunnel_for_agent(self, access_token: SecretStr, agent_id: str) -> TunnelInfo | None: - """Resolve the caller's tunnel for ``agent_id`` via the O(1) connector lookup. + """Resolve the caller's tunnel for ``agent_id``, or ``None`` if there is none. - Hits ``GET /tunnels/by-agent/{agent_id}``, which resolves the exact + Fast path: ``GET /tunnels/by-agent/{agent_id}`` resolves the exact tunnel through Cloudflare's server-side name filter (2 Cloudflare calls) rather than enumerating every tunnel and fetching each one's - config. Returns ``None`` when the connector reports 404 (no tunnel for - the agent yet). + config. On that endpoint, HTTP 200 with ``null`` means "no tunnel for + this agent yet". + + Back-compat: a connector deployed before this endpoint existed answers + the unknown route with a generic 404. Clients update independently of + (and often ahead of) the connector, so a 404 here is treated as "this + connector is too old" and we transparently fall back to the O(n) + ``GET /tunnels`` enumeration, matching on the ``--`` + name convention. This keeps sharing working during the rollout window; + once the connector is redeployed, every call takes the fast path. """ response = self._send( "GET", @@ -526,10 +545,25 @@ def find_tunnel_for_agent(self, access_token: SecretStr, agent_id: str) -> Tunne timeout=self.timeout_seconds, ) if response.status_code == 404: - return None + return self._find_tunnel_for_agent_via_list(access_token, agent_id) body = self._check(response, ImbueCloudTunnelError) + if not body: + return None return _parse_tunnel_info(body) + def _find_tunnel_for_agent_via_list(self, access_token: SecretStr, agent_id: str) -> TunnelInfo | None: + """O(n) fallback for connectors without the ``by-agent`` endpoint. + + Enumerates the caller's tunnels and matches on the trailing + ``--`` slug the connector uses for tunnel names. + """ + short_agent = agent_id.removeprefix("agent-")[:_AGENT_ID_PREFIX_LENGTH] + suffix = f"{_TUNNEL_NAME_SEP}{short_agent}" + for tunnel in self.list_tunnels(access_token): + if tunnel.tunnel_name.endswith(suffix): + return tunnel + return None + def delete_tunnel(self, access_token: SecretStr, tunnel_name: str) -> None: response = self._send( "DELETE", diff --git a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client_test.py b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client_test.py index 04c9fa3bc8..3fc9e7bf7e 100644 --- a/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client_test.py +++ b/libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/connector/client_test.py @@ -561,9 +561,46 @@ def handler(request: httpx.Request) -> httpx.Response: assert tunnel.services == ("web",) -def test_find_tunnel_for_agent_returns_none_on_404(monkeypatch: pytest.MonkeyPatch) -> None: +def test_find_tunnel_for_agent_returns_none_on_200_null(monkeypatch: pytest.MonkeyPatch) -> None: + # The up-to-date connector answers "no tunnel" with 200 + null; no O(n) + # enumeration fallback is triggered. def handler(request: httpx.Request) -> httpx.Response: - return httpx.Response(404, json={"detail": "No tunnel found for agent 'agent-abc123'"}) + assert request.url.path == "/tunnels/by-agent/agent-abc123" + return httpx.Response(200, json=None) + + client, state = _install_flaky_httpx_get(monkeypatch, fail_times=0, handler=handler) + assert client.find_tunnel_for_agent(SecretStr("tok"), "agent-abc123") is None + assert state["calls"] == 1 + + +def test_find_tunnel_for_agent_falls_back_to_list_on_404(monkeypatch: pytest.MonkeyPatch) -> None: + # A connector that predates the by-agent endpoint 404s the unknown route; + # the client transparently falls back to enumerating GET /tunnels and + # matches on the trailing -- slug. + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/tunnels/by-agent/agent-abc123": + return httpx.Response(404, json={"detail": "Not Found"}) + assert request.url.path == "/tunnels" + return httpx.Response( + 200, + json=[ + {"tunnel_name": "owner--deadbeef", "tunnel_id": "t-0", "services": []}, + {"tunnel_name": "owner--abc123", "tunnel_id": "t-1", "services": ["web"]}, + ], + ) + + client, _state = _install_flaky_httpx_get(monkeypatch, fail_times=0, handler=handler) + tunnel = client.find_tunnel_for_agent(SecretStr("tok"), "agent-abc123") + assert tunnel is not None + assert tunnel.tunnel_name == "owner--abc123" + assert tunnel.services == ("web",) + + +def test_find_tunnel_for_agent_fallback_returns_none_when_no_match(monkeypatch: pytest.MonkeyPatch) -> None: + def handler(request: httpx.Request) -> httpx.Response: + if request.url.path == "/tunnels/by-agent/agent-abc123": + return httpx.Response(404, json={"detail": "Not Found"}) + return httpx.Response(200, json=[]) client, _state = _install_flaky_httpx_get(monkeypatch, fail_times=0, handler=handler) assert client.find_tunnel_for_agent(SecretStr("tok"), "agent-abc123") is None