Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/minds/changelog/mngr-speed-up-sharing.md
Original file line number Diff line number Diff line change
@@ -0,0 +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).
33 changes: 14 additions & 19 deletions apps/minds/imbue/minds/desktop_client/imbue_cloud_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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: ``<short_user>--<short_agent>``,
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)
Expand Down
33 changes: 33 additions & 0 deletions apps/minds/imbue/minds/desktop_client/imbue_cloud_cli_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions apps/remote_service_connector/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 (`<username>--<agent-prefix>`) 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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Added an O(1) `GET /tunnels/by-agent/{agent_id}` endpoint. Minds always knows the exact tunnel name it wants (`<username>--<agent-prefix>`), 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`).
Original file line number Diff line number Diff line change
Expand Up @@ -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
(``<username>--<agent-prefix>``), 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)
Expand Down Expand Up @@ -2360,6 +2380,31 @@ 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] | 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. 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)
return tunnel.model_dump() if tunnel is not None else None


@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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -410,6 +468,30 @@ 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_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 == 200
assert resp.json() is None


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())
Expand Down
3 changes: 3 additions & 0 deletions libs/mngr_imbue_cloud/changelog/mngr-speed-up-sharing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Added a `mngr imbue_cloud tunnels find-by-agent <agent_id>` 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 `<username>--<agent-prefix>` 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.
28 changes: 28 additions & 0 deletions libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Expand Down
16 changes: 16 additions & 0 deletions libs/mngr_imbue_cloud/imbue/mngr_imbue_cloud/cli/tunnels_test.py
Original file line number Diff line number Diff line change
@@ -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
Loading