From 8d8d2c219e0983b6b77006cd19dd46ffbdca0f81 Mon Sep 17 00:00:00 2001 From: ZdenekSrotyr Date: Tue, 5 May 2026 13:49:18 +0200 Subject: [PATCH] =?UTF-8?q?refactor(cli-store):=20pull/info=20=E2=86=92=20?= =?UTF-8?q?agnes=20admin=20store;=20add=20agnes=20store=20mine?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backup-orchestration commands were split across two namespaces (pull in agnes store, push in agnes admin store), which broke the operator mental model — pull/push are a paired operation and should sit together. Move pull + info into agnes admin store so all bulk operations share one help screen. Add agnes store mine as the user-facing equivalent — calls the same /api/store/bundle.zip endpoint with ?owner=me, which the server resolves to the caller's user_id. Authors can archive their own uploads without admin role; whole-Store bulk reads stay admin-flavored as a discoverability hint. Server: 3-line addition to export_bundle handles owner='me' as a magic alias for the caller. No new endpoint. Tests updated: pull/info expectations move from agnes store to agnes admin store; new tests cover agnes store mine and the ?owner=me server resolution. 69/69 store tests green locally. --- CHANGELOG.md | 17 +++-- app/api/store.py | 5 ++ cli/commands/admin_store.py | 138 ++++++++++++++++++++++++++++++++++-- cli/commands/store.py | 107 ++++++---------------------- tests/test_cli_store.py | 84 +++++++++++++++------- tests/test_store_api.py | 21 ++++++ 6 files changed, 248 insertions(+), 124 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f85e4f..62a51da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,11 +51,18 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C `cli/v2_client.py` (`api_post_multipart` / `api_put_multipart` / `api_get_stream`) so future multipart and binary-download endpoints don't have to roll their own httpx wiring. -- **CLI: `agnes admin store push`** — admin-only Store bulk restore. - Wraps `POST /api/store/import-bundle` with mode=merge|replace|skip and - client-side zipping when the source is a directory (so a backup git - repo's working tree can go straight back into Agnes via a single - command). +- **CLI: `agnes admin store {pull,push,info}`** — operator-flavored + bulk Store ops. ``pull`` and ``info`` share the open + `GET /api/store/bundle.zip` / `/entities` endpoints; ``push`` wraps + the admin-gated `POST /api/store/import-bundle`. ``push`` accepts + either a *.zip file or a directory containing `manifest.json` + + `entities/` (CLI zips a directory client-side, so a backup git + repo's working tree round-trips straight back into Agnes via a + single command). +- **CLI: `agnes store mine`** — analyst-facing self-bundle. Same + endpoint as `admin store pull`, scoped via `?owner=me` (server + resolves the magic value to the caller's user_id) so authors can + archive their own uploads without admin role. - **REST: `GET /api/store/bundle.zip`** — deterministic ZIP of all (filtered) Store entities for whole-Store backup. Layout: `manifest.json` at the top with per-entity metadata + `owner_email` diff --git a/app/api/store.py b/app/api/store.py index 837243d..96478cf 100644 --- a/app/api/store.py +++ b/app/api/store.py @@ -1249,6 +1249,11 @@ async def export_bundle( """ if type and type not in _VALID_TYPES: raise HTTPException(status_code=400, detail="invalid_type") + # `owner=me` magic value resolves to the caller's user id — used by + # `agnes store mine` so analysts can pull a bundle of just their own + # uploads without needing to look up their own user_id first. + if owner == "me": + owner = user["id"] repo = StoreEntitiesRepository(conn) # Page through everything. The 100/req limit on `list` is a UI # pagination affordance, not a backup constraint — for a bulk export diff --git a/cli/commands/admin_store.py b/cli/commands/admin_store.py index 6bb2ddd..a81d569 100644 --- a/cli/commands/admin_store.py +++ b/cli/commands/admin_store.py @@ -1,9 +1,10 @@ -"""`agnes admin store push` — admin-only Store bulk restore. +"""`agnes admin store {pull,push,info}` — operator-flavored bulk Store ops. -Wraps ``POST /api/store/import-bundle`` (admin-gated). Read paths -(``pull`` / ``info``) live under user-namespace ``agnes store`` because the -server endpoint for the export is open to any authenticated user (the -Store is community-readable). +Read direction (``pull`` / ``info``) lives here too even though the server +endpoint is open to any authenticated user, so all backup-orchestration +commands sit in one namespace. Analyst-facing per-entity browse stays in +``agnes store``; analysts who want to download just their OWN uploads +have ``agnes store mine``. """ from __future__ import annotations @@ -17,9 +18,132 @@ from typing import Optional import typer -from cli.v2_client import V2ClientError, api_post_multipart +from cli.v2_client import ( + V2ClientError, + api_get_json, + api_get_stream, + api_post_multipart, +) -admin_store_app = typer.Typer(help="Admin: Store bulk restore (push)") +admin_store_app = typer.Typer(help="Admin: bulk Store ops (pull / push / info)") + + +@admin_store_app.command("pull") +def pull_bundle( + type: Optional[str] = typer.Option(None, "--type", help="skill | agent | plugin"), + category: Optional[str] = typer.Option(None, "--category"), + owner: Optional[str] = typer.Option(None, "--owner", help="Filter by owner user_id"), + search: Optional[str] = typer.Option(None, "--search", "-q"), + out: Path = typer.Option( + Path("agnes-store-bundle.zip"), "-o", "--out", + help="Where to save the ZIP (default: ./agnes-store-bundle.zip)", + ), + unpack: Optional[Path] = typer.Option( + None, "--unpack", + help="Instead of saving the ZIP, unpack it into this directory. " + "Useful for committing a snapshot to a backup git repo: " + "`agnes admin store pull --unpack ./backup/ && cd backup && git add .`", + ), +): + """Download the whole Store as a deterministic ZIP. + + With ``--unpack DIR`` the ZIP is streamed and immediately extracted + into ``DIR`` (the directory is wiped first so re-runs leave a clean + diff). Bundle layout:: + + manifest.json + entities// + ├── plugin/... + └── assets/... + + Every entity matching the given filters is included; no filters = + everything in the Store. Server endpoint is open (any authenticated + user can call it) — this command lives under ``admin store`` only by + operational convention; analysts wanting their OWN uploads use + ``agnes store mine``. + """ + params: dict = {} + if type: + params["type"] = type + if category: + params["category"] = category + if owner: + params["owner"] = owner + if search: + params["search"] = search + + if unpack: + scratch = Path(tempfile.mkdtemp(prefix="agnes_store_pull_")) + zip_path = scratch / "bundle.zip" + try: + try: + api_get_stream("/api/store/bundle.zip", str(zip_path), **params) + except V2ClientError as e: + typer.echo(str(e), err=True) + raise typer.Exit(1) + if unpack.exists(): + shutil.rmtree(unpack) + unpack.mkdir(parents=True, exist_ok=True) + with zipfile.ZipFile(zip_path, "r") as zf: + zf.extractall(unpack) + finally: + shutil.rmtree(scratch, ignore_errors=True) + typer.echo(f"Unpacked Store bundle → {unpack}") + return + + out.parent.mkdir(parents=True, exist_ok=True) + try: + size = api_get_stream("/api/store/bundle.zip", str(out), **params) + except V2ClientError as e: + typer.echo(str(e), err=True) + raise typer.Exit(1) + typer.echo(f"Wrote {size:,} bytes → {out}") + + +@admin_store_app.command("info") +def store_info( + json_out: bool = typer.Option(False, "--json"), +): + """Summary of the Store: total entities, breakdown by type, total size. + + Assembled client-side from a paginated /entities sweep so it stays + in sync with what `pull` would emit. + """ + skip = 0 + page = 100 + by_type: dict = {} + total_entities = 0 + total_size = 0 + while True: + try: + body = api_get_json( + "/api/store/entities", limit=page, skip=skip, + ) + except V2ClientError as e: + typer.echo(str(e), err=True) + raise typer.Exit(1) + items = body.get("items", []) + if not items: + break + for it in items: + total_entities += 1 + total_size += int(it.get("file_size") or 0) + by_type[it["type"]] = by_type.get(it["type"], 0) + 1 + if len(items) < page: + break + skip += page + + summary = { + "total_entities": total_entities, + "total_file_size_bytes": total_size, + "by_type": by_type, + } + if json_out: + typer.echo(json.dumps(summary, indent=2)) + return + typer.echo(f"Store: {total_entities} entit, {total_size:,} bytes total") + for t in sorted(by_type): + typer.echo(f" {t:8s} {by_type[t]}") @admin_store_app.command("push") diff --git a/cli/commands/store.py b/cli/commands/store.py index 95a4654..8f35974 100644 --- a/cli/commands/store.py +++ b/cli/commands/store.py @@ -216,64 +216,43 @@ def update_entity( # --------------------------------------------------------------------------- -# Bundle: pull + info (read paths, any authenticated user). -# Bulk restore (push) lives under `agnes admin store push` because the -# server-side endpoint is admin-only. +# `agnes store mine` — bundle of the caller's OWN entities (creator scope). +# +# Whole-Store bulk reads (`pull` / `info`) live under `agnes admin store` +# because operationally they're backup tooling for operators. This stays +# in user namespace because every authenticated user is allowed to grab +# a backup of their own creations (offline archive, leaving the org, +# moving to another instance). # --------------------------------------------------------------------------- -@store_app.command("pull") -def pull_bundle( - type: Optional[str] = typer.Option(None, "--type", help="skill | agent | plugin"), - category: Optional[str] = typer.Option(None, "--category"), - owner: Optional[str] = typer.Option(None, "--owner", help="Filter by owner user_id"), - search: Optional[str] = typer.Option(None, "--search", "-q"), +@store_app.command("mine") +def pull_my_entities( out: Path = typer.Option( - Path("agnes-store-bundle.zip"), "-o", "--out", - help="Where to save the ZIP (default: ./agnes-store-bundle.zip)", + Path("my-store-entities.zip"), "-o", "--out", + help="Where to save the ZIP (default: ./my-store-entities.zip)", ), unpack: Optional[Path] = typer.Option( None, "--unpack", - help="Instead of saving the ZIP, unpack it into this directory. " - "Useful for committing a snapshot to a backup git repo: " - "`agnes store pull --unpack ./backup/ && cd backup && git add .`", + help="Instead of saving the ZIP, unpack it into this directory.", ), ): - """Download the whole Store as a deterministic ZIP. + """Download a bundle of every Store entity you own (created). - With ``--unpack DIR`` the ZIP is streamed and immediately extracted - into ``DIR`` (the directory is wiped first so re-runs leave a clean - diff). The bundle layout:: - - manifest.json - entities// - ├── plugin/... - └── assets/... - - Every entity matching the given filters is included; no filters = - everything in the Store. + Server-side this is the same ``GET /api/store/bundle.zip`` endpoint + that `agnes admin store pull` uses, scoped to the caller via + ``?owner=me`` (the server resolves the magic value to your user_id). """ import shutil as _shutil import tempfile as _tempfile import zipfile as _zipfile - params: dict = {} - if type: - params["type"] = type - if category: - params["category"] = category - if owner: - params["owner"] = owner - if search: - params["search"] = search - if unpack: - # Stream into a temp file, then unpack into `unpack` (wiped first). - scratch = Path(_tempfile.mkdtemp(prefix="agnes_store_pull_")) + scratch = Path(_tempfile.mkdtemp(prefix="agnes_store_mine_")) zip_path = scratch / "bundle.zip" try: try: - api_get_stream("/api/store/bundle.zip", str(zip_path), **params) + api_get_stream("/api/store/bundle.zip", str(zip_path), owner="me") except V2ClientError as e: typer.echo(str(e), err=True) raise typer.Exit(1) @@ -284,59 +263,13 @@ def pull_bundle( zf.extractall(unpack) finally: _shutil.rmtree(scratch, ignore_errors=True) - typer.echo(f"Unpacked Store bundle → {unpack}") + typer.echo(f"Unpacked your Store entities → {unpack}") return out.parent.mkdir(parents=True, exist_ok=True) try: - size = api_get_stream("/api/store/bundle.zip", str(out), **params) + size = api_get_stream("/api/store/bundle.zip", str(out), owner="me") except V2ClientError as e: typer.echo(str(e), err=True) raise typer.Exit(1) typer.echo(f"Wrote {size:,} bytes → {out}") - - -@store_app.command("info") -def store_info( - json_out: bool = typer.Option(False, "--json"), -): - """Summary of the Store: total entities, breakdown by type, total size. - - No new endpoint — assembled client-side from a paginated /entities - sweep so it stays in sync with what `pull` would emit. - """ - skip = 0 - page = 100 - by_type: dict = {} - total_entities = 0 - total_size = 0 - while True: - try: - body = api_get_json( - "/api/store/entities", limit=page, skip=skip, - ) - except V2ClientError as e: - typer.echo(str(e), err=True) - raise typer.Exit(1) - items = body.get("items", []) - if not items: - break - for it in items: - total_entities += 1 - total_size += int(it.get("file_size") or 0) - by_type[it["type"]] = by_type.get(it["type"], 0) + 1 - if len(items) < page: - break - skip += page - - summary = { - "total_entities": total_entities, - "total_file_size_bytes": total_size, - "by_type": by_type, - } - if json_out: - typer.echo(json.dumps(summary, indent=2)) - return - typer.echo(f"Store: {total_entities} entit, {total_size:,} bytes total") - for t in sorted(by_type): - typer.echo(f" {t:8s} {by_type[t]}") diff --git a/tests/test_cli_store.py b/tests/test_cli_store.py index d2773d8..b763de2 100644 --- a/tests/test_cli_store.py +++ b/tests/test_cli_store.py @@ -30,10 +30,19 @@ def test_store_help_lists_subcommands(): r = runner.invoke(store_app, ["--help"]) assert r.exit_code == 0 out = _clean(r.output) - for cmd in ("list", "show", "install", "uninstall", "upload", "delete"): + for cmd in ("list", "show", "install", "uninstall", "upload", "update", "delete", "mine"): assert cmd in out, f"missing subcommand {cmd!r} in help" +def test_admin_store_help_lists_subcommands(): + from cli.commands.admin_store import admin_store_app + r = runner.invoke(admin_store_app, ["--help"]) + assert r.exit_code == 0 + out = _clean(r.output) + for cmd in ("pull", "push", "info"): + assert cmd in out + + def test_my_stack_help_lists_subcommands(): r = runner.invoke(my_stack_app, ["--help"]) assert r.exit_code == 0 @@ -233,57 +242,85 @@ def test_store_update_sends_put_multipart(monkeypatch): # --------------------------------------------------------------------------- -def test_store_pull_writes_zip(monkeypatch, tmp_path): +def test_admin_store_pull_writes_zip(monkeypatch, tmp_path): + """Bulk pull of all Store entities lives under `agnes admin store pull`.""" + from cli.commands.admin import admin_app + from cli.commands import admin_store as admin_store_mod + captured: dict = {} def _stream(path, dest, **params): captured["path"] = path captured["params"] = params - captured["dest"] = dest - # Write a placeholder so the size message looks plausible. with open(dest, "wb") as f: f.write(b"PK\x03\x04fakezip") return 9 - import cli.commands.store as store_mod - monkeypatch.setattr(store_mod, "api_get_stream", _stream) + monkeypatch.setattr(admin_store_mod, "api_get_stream", _stream) out = tmp_path / "store.zip" - r = runner.invoke(store_app, ["pull", "-o", str(out)]) + r = runner.invoke(admin_app, ["store", "pull", "-o", str(out)]) assert r.exit_code == 0, r.output assert captured["path"] == "/api/store/bundle.zip" + # `mine` uses owner=me; bulk pull does NOT. + assert "owner" not in captured["params"] assert "Wrote 9 bytes" in _clean(r.output) assert out.exists() -def test_store_pull_unpack(monkeypatch, tmp_path): - """`--unpack DIR` streams to a temp ZIP and extracts into DIR.""" +def test_admin_store_pull_unpack(monkeypatch, tmp_path): + """`agnes admin store pull --unpack DIR` streams + extracts.""" import zipfile + from cli.commands.admin import admin_app + from cli.commands import admin_store as admin_store_mod - # Build a fake bundle in-memory and write it as the streamed payload. fake_zip_path = tmp_path / "_fake.zip" with zipfile.ZipFile(fake_zip_path, "w") as zf: zf.writestr("manifest.json", '{"format":1,"entries":[]}') zf.writestr("entities/abc/plugin/.claude-plugin/plugin.json", '{}') def _stream(path, dest, **params): - # Copy fake zip bytes into the streamed dest. from pathlib import Path as _P with open(dest, "wb") as fh: fh.write(_P(fake_zip_path).read_bytes()) return _P(dest).stat().st_size - import cli.commands.store as store_mod - monkeypatch.setattr(store_mod, "api_get_stream", _stream) + monkeypatch.setattr(admin_store_mod, "api_get_stream", _stream) target = tmp_path / "unpacked" - r = runner.invoke(store_app, ["pull", "--unpack", str(target)]) + r = runner.invoke(admin_app, ["store", "pull", "--unpack", str(target)]) assert r.exit_code == 0, r.output assert (target / "manifest.json").is_file() assert (target / "entities/abc/plugin/.claude-plugin/plugin.json").is_file() -def test_store_info_summarizes(monkeypatch): +def test_store_mine_uses_owner_me_param(monkeypatch, tmp_path): + """`agnes store mine` is the user-facing variant — same endpoint with + `?owner=me` so server can scope to caller's own entities.""" + captured: dict = {} + + def _stream(path, dest, **params): + captured["path"] = path + captured["params"] = params + with open(dest, "wb") as f: + f.write(b"PK\x03\x04mine") + return 7 + + import cli.commands.store as store_mod + monkeypatch.setattr(store_mod, "api_get_stream", _stream) + + out = tmp_path / "mine.zip" + r = runner.invoke(store_app, ["mine", "-o", str(out)]) + assert r.exit_code == 0, r.output + assert captured["path"] == "/api/store/bundle.zip" + assert captured["params"] == {"owner": "me"} + assert out.exists() + + +def test_admin_store_info_summarizes(monkeypatch): + from cli.commands.admin import admin_app + from cli.commands import admin_store as admin_store_mod + page1 = { "items": [ {"type": "skill", "file_size": 1024}, @@ -295,13 +332,9 @@ def test_store_info_summarizes(monkeypatch): empty = {"items": [], "total": 3, "skip": 100, "limit": 100} pages = [page1, empty] - def _get(path, **params): - return pages.pop(0) + monkeypatch.setattr(admin_store_mod, "api_get_json", lambda *a, **kw: pages.pop(0)) - import cli.commands.store as store_mod - monkeypatch.setattr(store_mod, "api_get_json", _get) - - r = runner.invoke(store_app, ["info"]) + r = runner.invoke(admin_app, ["store", "info"]) assert r.exit_code == 0, r.output out = _clean(r.output) assert "3 entit" in out @@ -309,16 +342,17 @@ def test_store_info_summarizes(monkeypatch): assert "agent" in out and "1" in out -def test_store_info_json(monkeypatch): +def test_admin_store_info_json(monkeypatch): + from cli.commands.admin import admin_app + from cli.commands import admin_store as admin_store_mod one = { "items": [{"type": "plugin", "file_size": 999}], "total": 1, "skip": 0, "limit": 100, } pages = [one, {"items": [], "total": 1, "skip": 100, "limit": 100}] - import cli.commands.store as store_mod - monkeypatch.setattr(store_mod, "api_get_json", lambda *a, **kw: pages.pop(0)) + monkeypatch.setattr(admin_store_mod, "api_get_json", lambda *a, **kw: pages.pop(0)) - r = runner.invoke(store_app, ["info", "--json"]) + r = runner.invoke(admin_app, ["store", "info", "--json"]) assert r.exit_code == 0, r.output import json as _json body = _json.loads(_clean(r.output)) diff --git a/tests/test_store_api.py b/tests/test_store_api.py index d8d7ae5..5b73627 100644 --- a/tests/test_store_api.py +++ b/tests/test_store_api.py @@ -670,6 +670,27 @@ class TestStoreBundle: assert entries_by_id[eid_a]["owner_email"] == "owner-bundle@x.com" assert entries_by_id[eid_a]["name"] == "bundle-a" + def test_bundle_zip_owner_me_resolves_to_caller(self, web_client): + """`?owner=me` magic value resolves server-side to the caller's + user_id, so `agnes store mine` can pull a self-bundle without + having to look up its own id first.""" + _, alice_cookies = _create_user(web_client, "mine-a@x.com") + _, bob_cookies = _create_user(web_client, "mine-b@x.com") + self._upload_skill(web_client, alice_cookies, name="alice-1") + self._upload_skill(web_client, alice_cookies, name="alice-2") + self._upload_skill(web_client, bob_cookies, name="bob-1") + + # Alice asks for owner=me → only her two entities. + r = web_client.get("/api/store/bundle.zip?owner=me", cookies=alice_cookies) + assert r.status_code == 200 + assert r.headers["x-bundle-entry-count"] == "2" + + with zipfile.ZipFile(io.BytesIO(r.content)) as zf: + names = zf.namelist() + assert any("alice-1-by-mine-a" in n for n in names) + assert any("alice-2-by-mine-a" in n for n in names) + assert not any("bob-1-by-mine-b" in n for n in names) + def test_bundle_zip_filters(self, web_client): _, cookies = _create_user(web_client, "filter@x.com") self._upload_skill(web_client, cookies, name="keep-this")