From 291079b1d2d2caaeb0c27f7f320bc537c607f438 Mon Sep 17 00:00:00 2001 From: ZdenekSrotyr Date: Mon, 4 May 2026 22:13:46 +0200 Subject: [PATCH] refactor(welcome-template): drop role param; resolve plugins per-user unconditionally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the `role: Literal["analyst", "admin"] = "admin"` parameter from `compute_default_agent_prompt`. The same RBAC pass (`marketplace_filter.resolve_allowed_plugins`) now runs for every user — admin or not. Users with no `resource_grants` rows get the no-marketplace layout; users with grants get the marketplace block inserted. Admin-vs-analyst is no longer a layout branch. `render_agent_prompt_banner` no longer derives a `role` from `user.is_admin`; it just delegates to `compute_default_agent_prompt`. Two `compute_default_agent_prompt(...role=role)` call sites in `app/web/router.py::setup_page` are updated to drop the keyword so the route keeps rendering — Task 5 will remove the `?role=` query parameter and the silent admin-downgrade block from the route signature itself. Tests: drop role-aware assertions from test_welcome_template_renderer and test_welcome_template_api. Both files now assert the unified default contains `agnes init` + `uv tool install` and bans the legacy `agnes auth import-token` / `agnes auth whoami` verbs. Plan: docs/superpowers/plans/2026-05-04-unified-setup-prompt.md task 4. --- app/web/router.py | 4 +-- src/welcome_template.py | 43 +++++++++++++------------ tests/test_welcome_template_api.py | 6 ++-- tests/test_welcome_template_renderer.py | 30 +++++++++-------- 4 files changed, 45 insertions(+), 38 deletions(-) diff --git a/app/web/router.py b/app/web/router.py index 5faa03b..842a7ce 100644 --- a/app/web/router.py +++ b/app/web/router.py @@ -768,11 +768,11 @@ async def setup_page( except (TemplateError, Exception) as exc: logger.warning("setup_page: override render failed (%s); falling back to default", exc) setup_script_text = compute_default_agent_prompt( - conn, user=user, server_url=base_url, role=role, + conn, user=user, server_url=base_url, ) else: setup_script_text = compute_default_agent_prompt( - conn, user=user, server_url=base_url, role=role, + conn, user=user, server_url=base_url, ) # Split for the legacy setup_instructions_lines list variable that the diff --git a/src/welcome_template.py b/src/welcome_template.py index b739ca1..3715514 100644 --- a/src/welcome_template.py +++ b/src/welcome_template.py @@ -24,7 +24,7 @@ import os import re from datetime import date, datetime, timezone from pathlib import Path -from typing import Any, Literal +from typing import Any from urllib.parse import urlparse import duckdb @@ -130,21 +130,22 @@ def compute_default_agent_prompt( *, user: dict[str, Any] | None, server_url: str, - role: Literal["analyst", "admin"] = "admin", ) -> str: """Return the live default setup script from setup_instructions.resolve_lines(). - This is the full bash bootstrap prompt that /setup shows when no admin - override is set. The returned string is bash (not HTML) — callers must - NOT pass it through _sanitize_banner_html. + This is the unified bash bootstrap prompt that /setup shows when no + admin override is set. The returned string is bash (not HTML) — + callers must NOT pass it through _sanitize_banner_html. ``conn`` and ``user`` are forwarded to resolve the RBAC-filtered plugin - install list (anonymous visitors / no conn get the no-marketplace layout). - ``server_url`` is used to derive the server host for the marketplace block. + install list. The same RBAC pass runs for everyone (admin and + non-admin alike): users with no plugin grants get the no-marketplace + layout (Confirm = step 6); users with grants get the marketplace + plugins + block inserted (Confirm = step 8). Anonymous visitors / no conn fall + through to the no-marketplace layout. - ``role`` selects the layout: ``"admin"`` (default) keeps the existing - full bootstrap, ``"analyst"`` short-circuits to the trimmed analyst - workspace flow (no marketplace, plugins forced empty). + ``server_url`` is used to derive the server host for the marketplace + block. """ try: from app.web.setup_instructions import resolve_lines @@ -153,12 +154,13 @@ def compute_default_agent_prompt( _wheel = _find_wheel() _wheel_filename = _wheel.name if _wheel else "agnes.whl" - # Analyst flow has no marketplace concept — skip the RBAC plugin - # resolution entirely so the analyst tile renders the same lines for - # everyone (and so resolve_lines's analyst short-circuit fires - # regardless of whether the caller has plugin grants). + # RBAC plugin resolution is unconditional — same code path for + # admin and non-admin. Users with no `resource_grants` rows get an + # empty list and the no-marketplace layout; users with grants get + # the marketplace block. Admin-vs-analyst is no longer a layout + # branch. plugin_install_names: list[str] = [] - if role == "admin" and user and conn is not None: + if user and conn is not None: try: from src import marketplace_filter plugin_install_names = [ @@ -243,11 +245,10 @@ def render_agent_prompt_banner( # Fall through to default # No override (or broken override) — return live default bash script. - # Pick role by user identity: admins get the full CLI install + marketplace - # flow (existing behavior). Everyone else gets the analyst workspace - # bootstrap. The dashboard CTA hits this path; without role-by-identity, - # analysts would get admin instructions they can't actually execute. - role = "admin" if (user and user.get("is_admin")) else "analyst" + # Same unified flow for everyone; admin-vs-analyst is no longer a + # layout branch. The marketplace block is gated by the caller's + # plugin grants in `resource_grants`, which `compute_default_agent_prompt` + # resolves unconditionally. return compute_default_agent_prompt( - conn, user=user, server_url=server_url, role=role, + conn, user=user, server_url=server_url, ) diff --git a/tests/test_welcome_template_api.py b/tests/test_welcome_template_api.py index a86a63d..c779dd7 100644 --- a/tests/test_welcome_template_api.py +++ b/tests/test_welcome_template_api.py @@ -38,9 +38,11 @@ def test_admin_get_template_initially_null(seeded_app): # default field must be present and contain the live setup script assert "default" in body assert body["default"] # non-empty - # Admin layout marker — `agnes auth import-token` is the login step. - assert "agnes auth" in body["default"] + # Unified layout markers — `agnes init` and `uv tool install` are + # mandatory; legacy `agnes auth import-token` is gone. + assert "agnes init" in body["default"] assert "uv tool install" in body["default"] + assert "agnes auth import-token" not in body["default"] # No legacy verb in the rendered default assert "da analyst setup" not in body["default"] diff --git a/tests/test_welcome_template_renderer.py b/tests/test_welcome_template_renderer.py index 47f1fd4..aa56584 100644 --- a/tests/test_welcome_template_renderer.py +++ b/tests/test_welcome_template_renderer.py @@ -39,35 +39,39 @@ def _user(email="alice@example.com"): def test_returns_default_script_when_no_override(conn): """When no override is set, render_agent_prompt_banner returns the live - setup script (not an empty string). - - A non-admin user gets the analyst layout: `agnes init` subsumes auth and - the trimmed flow has no `agnes auth` line. An admin user gets the full - CLI bootstrap with `agnes auth import-token`. + unified setup script (not an empty string). Every caller — admin or + non-admin — sees `agnes init` (the workspace-rails delivery + mechanism). Whether the marketplace block appears depends on the + caller's plugin grants in `resource_grants`, NOT on `is_admin`. """ out = render_agent_prompt_banner(conn, user=_user(), server_url="https://example.com") # Must be non-empty — the default IS the setup script assert out != "" - # Analyst layout: `agnes init` is the bootstrap step. + # Unified layout: `agnes init` is mandatory. assert "agnes init" in out + # Legacy admin-only auth verbs are gone — `agnes init` subsumes them. + assert "agnes auth import-token" not in out + assert "agnes auth whoami" not in out # No legacy verb anywhere in the rendered default assert "da analyst setup" not in out assert "da sync" not in out def test_compute_default_returns_setup_script(conn): - """compute_default_agent_prompt returns a non-empty string with setup - script markers including {server_url} and agnes commands. - - Default role is `admin`, which renders the full CLI install + login flow. + """compute_default_agent_prompt returns a non-empty string with the + unified setup-script markers, including the {server_url} placeholder + and the agnes init line. """ out = compute_default_agent_prompt(conn, user=_user(), server_url="https://example.com") assert out != "" # {server_url} placeholder must survive (not replaced by Jinja2) assert "{server_url}" in out - # Admin layout references the agnes CLI install + login flow - assert "agnes auth" in out + # Unified layout: install + init are always present. + assert "agnes init" in out assert "uv tool install" in out + # Admin-only auth verbs replaced by `agnes init`. + assert "agnes auth import-token" not in out + assert "agnes auth whoami" not in out # No legacy verb anywhere in the rendered default assert "da analyst setup" not in out @@ -237,7 +241,7 @@ def test_render_failure_falls_back_to_default_not_exception(conn): out = render_agent_prompt_banner(conn, user=_user(), server_url="https://example.com") # Must not raise — falls back to the live default script (non-empty) assert out != "" - # Non-admin → analyst layout: `agnes init` is the bootstrap step. + # Unified layout: `agnes init` is the bootstrap step regardless of role. assert "agnes init" in out assert "uv tool install" in out