fix(cli): omni resume lists only the caller's own sessions (#4709)
`omnigent resume` (no id) opened a cross-agent picker over GET /v1/sessions, which returns every session the caller can *access* — including ones merely shared with them. Resume is owner-only (the server rejects binding a runner to a session you don't own), so a shared row in the picker was a dead end. Resolve the caller's identity via a best-effort GET /v1/me in the resume dispatch and pass owner_user_id to pick_conversation_cross_agent_from_sdk, which now drops rows the caller does not own. An unresolved identity (unauthenticated / transient failure) or a permissionless single-user server (owner unset, no sharing) leaves the list unfiltered — resume never breaks. Co-authored-by: Isaac Signed-off-by: Edwin He <41037314+Edwinhe03@users.noreply.github.com>
This commit is contained in:
@@ -970,13 +970,27 @@ async def pick_conversation_by_wrapper_label_from_sdk(
|
||||
async def pick_conversation_cross_agent_from_sdk(
|
||||
client: OmnigentClient,
|
||||
*,
|
||||
owner_user_id: str | None = None,
|
||||
out: TextIO | None = None,
|
||||
in_: TextIO | None = None,
|
||||
) -> str | None:
|
||||
"""Cross-agent variant: lists every session the caller can see
|
||||
via ``/v1/sessions`` and renders runtime metadata for
|
||||
``omnigent resume``'s runtime-dispatch UX."""
|
||||
``omnigent resume``'s runtime-dispatch UX.
|
||||
|
||||
``/v1/sessions`` returns every session the caller can *access*,
|
||||
which includes ones merely shared with them. Resume is an
|
||||
owner-only action — the server rejects binding a runner to a
|
||||
session you don't own (``PATCH /v1/sessions/{id}`` → 403) — so a
|
||||
shared row in this picker is a dead end. When ``owner_user_id`` is
|
||||
set, drop the rows this caller does not own so ``omnigent resume``
|
||||
lists only their own sessions. ``None`` leaves the list unfiltered:
|
||||
the caller's identity could not be resolved, or the server runs
|
||||
without permissions (``owner`` unset, no sharing to filter).
|
||||
"""
|
||||
convos = await client.sessions.list(limit=200, agent_id=None, order="desc")
|
||||
if owner_user_id is not None:
|
||||
convos = [c for c in convos if c.owner == owner_user_id]
|
||||
previews = await _collect_previews_async(client, convos)
|
||||
# Header label is intentionally generic — "resume" describes the
|
||||
# action, not a single agent. Without overriding the legacy
|
||||
|
||||
@@ -114,6 +114,11 @@ def _pick_conversation_for_resume(
|
||||
|
||||
base_url = server.rstrip("/")
|
||||
headers = _remote_headers(server_url=base_url)
|
||||
# Resume is owner-only, so the picker lists only the caller's own
|
||||
# sessions — never ones merely shared with them. Resolve who the
|
||||
# caller is here so the picker can drop shared rows; best-effort, so
|
||||
# an unresolved identity just leaves the list unfiltered.
|
||||
owner_user_id = _resolve_current_user_id(base_url=base_url, headers=headers)
|
||||
|
||||
async def _drive() -> str | None:
|
||||
"""
|
||||
@@ -128,7 +133,9 @@ def _pick_conversation_for_resume(
|
||||
from omnigent_client import OmnigentClient
|
||||
|
||||
async with OmnigentClient(base_url=base_url, headers=headers) as client:
|
||||
return await pick_conversation_cross_agent_from_sdk(client)
|
||||
return await pick_conversation_cross_agent_from_sdk(
|
||||
client, owner_user_id=owner_user_id
|
||||
)
|
||||
|
||||
try:
|
||||
return asyncio.run(_drive())
|
||||
@@ -150,6 +157,44 @@ def _pick_conversation_for_resume(
|
||||
) from exc
|
||||
|
||||
|
||||
def _resolve_current_user_id(
|
||||
*,
|
||||
base_url: str,
|
||||
headers: dict[str, str],
|
||||
) -> str | None:
|
||||
"""
|
||||
Best-effort ``GET /v1/me`` to learn who the caller is.
|
||||
|
||||
The cross-agent resume picker uses this to drop sessions merely
|
||||
*shared* with the caller (resume is owner-only). It is deliberately
|
||||
best-effort: a permissionless single-user server answers
|
||||
``user_id: null`` (no sharing — nothing to filter), and a transient
|
||||
lookup failure must never break resume. Both fall back to ``None``,
|
||||
which the picker reads as "no owner filter". Reuses the same header
|
||||
chain the picker's own requests carry, so it authenticates
|
||||
identically.
|
||||
|
||||
:param base_url: Remote server URL without a trailing slash.
|
||||
:param headers: Request headers (auth) shared with the picker.
|
||||
:returns: The caller's ``user_id``, or ``None`` when it can't be
|
||||
resolved (unauthenticated, non-200, non-JSON, or unreachable).
|
||||
"""
|
||||
try:
|
||||
resp = httpx.get(f"{base_url}/v1/me", headers=headers, timeout=10.0)
|
||||
except httpx.HTTPError:
|
||||
return None
|
||||
if resp.status_code != 200:
|
||||
return None
|
||||
try:
|
||||
body = resp.json()
|
||||
except ValueError:
|
||||
return None
|
||||
if not isinstance(body, dict):
|
||||
return None
|
||||
user_id = body.get("user_id")
|
||||
return user_id if isinstance(user_id, str) and user_id else None
|
||||
|
||||
|
||||
def _dispatch_by_runtime(
|
||||
*,
|
||||
target: str,
|
||||
|
||||
@@ -39,6 +39,7 @@ from omnigent.repl._resume_picker import (
|
||||
_last_message_preview_from_entities,
|
||||
_Preview,
|
||||
pick_conversation,
|
||||
pick_conversation_cross_agent_from_sdk,
|
||||
pick_conversation_from_store,
|
||||
)
|
||||
from omnigent.stores.agent_store.sqlalchemy_store import SqlAlchemyAgentStore
|
||||
@@ -842,6 +843,9 @@ class _BadgeRow:
|
||||
title: str | None = "test"
|
||||
created_at: int = 0
|
||||
labels: Mapping[str, str] | None = None
|
||||
# The owner filter in the cross-agent picker reads ``owner``; the
|
||||
# badge tests ignore it. Defaulted so existing rows are unaffected.
|
||||
owner: str | None = None
|
||||
|
||||
|
||||
def test_runtime_badge_claude_native() -> None:
|
||||
@@ -1282,3 +1286,79 @@ def test_workspace_metadata_omits_unrecorded_workspace_segment(
|
||||
assert selected == "eadade68b1f6e5f2f5e0c57a00d8d378"
|
||||
assert "Workspace" not in rendered
|
||||
assert "—" not in rendered
|
||||
|
||||
|
||||
# ── Cross-agent picker — owner filter ────────────────────
|
||||
#
|
||||
# ``omnigent resume`` (no id) lists the server's ACL-scoped sessions,
|
||||
# which include ones merely shared with the caller. Resume is
|
||||
# owner-only, so the picker drops rows the caller does not own. Reuses
|
||||
# the ``_FakeAPClient`` / ``_BadgeRow`` fakes above.
|
||||
|
||||
|
||||
def _owned_and_shared_rows() -> list[_BadgeRow]:
|
||||
"""A shared row (owned by someone else) then the caller's own row."""
|
||||
return [
|
||||
_BadgeRow(
|
||||
id="5bcf1e3b9a1c4d2e8f0a1b2c3d4e5f60",
|
||||
title="bob's shared chat",
|
||||
owner="bob@example.com",
|
||||
labels={},
|
||||
),
|
||||
_BadgeRow(
|
||||
id="a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6",
|
||||
title="my own chat",
|
||||
owner="me@example.com",
|
||||
labels={},
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
async def test_cross_agent_picker_drops_sessions_not_owned_by_caller() -> None:
|
||||
"""
|
||||
With ``owner_user_id`` set, sessions merely shared with the caller are
|
||||
dropped — only their own sessions are listed and selectable. Resume is
|
||||
owner-only, so a shared row would be a dead end.
|
||||
"""
|
||||
client = _FakeAPClient(rows=_owned_and_shared_rows())
|
||||
out = io.StringIO()
|
||||
|
||||
# After filtering to "me@example.com" only the owned row survives, so
|
||||
# row 1 is that owned session.
|
||||
selected = await pick_conversation_cross_agent_from_sdk(
|
||||
client,
|
||||
owner_user_id="me@example.com",
|
||||
out=out,
|
||||
in_=io.StringIO("1\n"),
|
||||
)
|
||||
|
||||
assert selected == "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6"
|
||||
rendered = out.getvalue()
|
||||
assert "my own chat" in rendered
|
||||
# The shared row must not appear at all.
|
||||
assert "bob's shared chat" not in rendered
|
||||
assert "5bcf1e3b9a1c4d2e8f0a1b2c3d4e5f60" not in rendered
|
||||
|
||||
|
||||
async def test_cross_agent_picker_lists_everything_when_owner_unknown() -> None:
|
||||
"""
|
||||
``owner_user_id=None`` (identity unresolved, or a permissionless
|
||||
single-user server) leaves the list unfiltered — the pre-existing
|
||||
behavior. Both rows are listed, so resume never silently hides
|
||||
sessions when it can't tell who the caller is.
|
||||
"""
|
||||
client = _FakeAPClient(rows=_owned_and_shared_rows())
|
||||
out = io.StringIO()
|
||||
|
||||
# No filter → row 1 is still the (shared) row the server returned first.
|
||||
selected = await pick_conversation_cross_agent_from_sdk(
|
||||
client,
|
||||
owner_user_id=None,
|
||||
out=out,
|
||||
in_=io.StringIO("1\n"),
|
||||
)
|
||||
|
||||
assert selected == "5bcf1e3b9a1c4d2e8f0a1b2c3d4e5f60"
|
||||
rendered = out.getvalue()
|
||||
assert "bob's shared chat" in rendered
|
||||
assert "my own chat" in rendered
|
||||
|
||||
@@ -690,3 +690,96 @@ def test_read_wrapper_label_remote_raises_on_404(
|
||||
)
|
||||
assert "5eca720dc2bc6cdc3a99028d7bd0f917" in excinfo.value.message
|
||||
assert "not found" in excinfo.value.message
|
||||
|
||||
|
||||
# ── _resolve_current_user_id ──────────────────────────────
|
||||
|
||||
|
||||
def test_resolve_current_user_id_returns_user_id(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""
|
||||
Happy path: ``GET /v1/me`` answers 200 with a ``user_id`` → return it.
|
||||
|
||||
The cross-agent picker feeds this to its owner filter so ``omnigent
|
||||
resume`` lists only the caller's own sessions.
|
||||
"""
|
||||
|
||||
def _fake_get(url: str, *, headers: dict[str, str], timeout: float) -> httpx.Response:
|
||||
"""Return a canned ``GET /v1/me`` identity."""
|
||||
del headers, timeout
|
||||
assert url.endswith("/v1/me"), url
|
||||
return httpx.Response(200, json={"user_id": "alice@example.com", "is_admin": False})
|
||||
|
||||
monkeypatch.setattr(httpx, "get", _fake_get)
|
||||
|
||||
result = resume_dispatch._resolve_current_user_id(
|
||||
base_url="https://example.com",
|
||||
headers={},
|
||||
)
|
||||
assert result == "alice@example.com"
|
||||
|
||||
|
||||
def test_resolve_current_user_id_none_when_server_has_no_auth(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""
|
||||
A permissionless single-user server answers ``user_id: null`` → ``None``.
|
||||
|
||||
There is no sharing on such a server, so the picker must fall back to
|
||||
listing everything (no owner filter) rather than hiding all rows.
|
||||
"""
|
||||
|
||||
def _fake_get(url: str, *, headers: dict[str, str], timeout: float) -> httpx.Response:
|
||||
"""Return the unauthenticated-identity shape."""
|
||||
del url, headers, timeout
|
||||
return httpx.Response(200, json={"user_id": None, "is_admin": False})
|
||||
|
||||
monkeypatch.setattr(httpx, "get", _fake_get)
|
||||
|
||||
result = resume_dispatch._resolve_current_user_id(
|
||||
base_url="https://example.com",
|
||||
headers={},
|
||||
)
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_resolve_current_user_id_none_on_non_200(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""
|
||||
A non-200 (e.g. OIDC 401 with a ``login_url``) yields ``None`` — the
|
||||
picker lists everything rather than failing. Resume stays usable even
|
||||
when identity can't be resolved.
|
||||
"""
|
||||
|
||||
def _fake_get(url: str, *, headers: dict[str, str], timeout: float) -> httpx.Response:
|
||||
"""Return a 401 login-required response."""
|
||||
del url, headers, timeout
|
||||
return httpx.Response(401, json={"user_id": None, "login_url": "/login"})
|
||||
|
||||
monkeypatch.setattr(httpx, "get", _fake_get)
|
||||
|
||||
result = resume_dispatch._resolve_current_user_id(
|
||||
base_url="https://example.com",
|
||||
headers={},
|
||||
)
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_resolve_current_user_id_none_on_transport_error(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""
|
||||
A transport failure must NEVER break resume — it degrades to ``None``
|
||||
(no owner filter), same as any other unresolved-identity case.
|
||||
"""
|
||||
|
||||
def _fake_get(url: str, *, headers: dict[str, str], timeout: float) -> httpx.Response:
|
||||
"""Raise the network error the picker must swallow."""
|
||||
del url, headers, timeout
|
||||
raise httpx.ConnectError("connection refused")
|
||||
|
||||
monkeypatch.setattr(httpx, "get", _fake_get)
|
||||
|
||||
result = resume_dispatch._resolve_current_user_id(
|
||||
base_url="https://example.com",
|
||||
headers={},
|
||||
)
|
||||
assert result is None
|
||||
|
||||
Reference in New Issue
Block a user