diff --git a/app/web/router.py b/app/web/router.py index fdae031..5faa03b 100644 --- a/app/web/router.py +++ b/app/web/router.py @@ -717,18 +717,19 @@ async def activity_center( @router.get("/setup", response_class=HTMLResponse) async def setup_page( request: Request, - role: Literal["analyst", "admin"] = Query(default="admin"), + role: Literal["analyst", "admin"] = Query(default="analyst"), user: Optional[dict] = Depends(get_optional_user), conn: duckdb.DuckDBPyConnection = Depends(_get_db), ): """Setup instructions for the local agent (CLI + Claude Code). The `role` query param picks the layout: - - `admin` (default) → full marketplace + skills + diagnose flow, - byte-identical to the pre-Task-4 page for any caller that doesn't - pass `?role=`. - - `analyst` → trimmed workspace-bootstrap flow rendered by + - `analyst` (default) → trimmed workspace-bootstrap flow rendered by `_resolve_analyst_lines` (no marketplace, no plugins). + - `admin` → full marketplace + skills + diagnose flow. Admin-only: + non-admin callers asking for `?role=admin` are silently downgraded + to `analyst` so the page never shows them instructions they can't + execute. When an admin override is saved, the override replaces the auto-generated setup_instructions output everywhere (both the /setup page display and the @@ -739,6 +740,13 @@ async def setup_page( from src.welcome_template import compute_default_agent_prompt, _sanitize_banner_html from jinja2 import Environment, StrictUndefined, TemplateError + # Gate the admin layout on user.is_admin. Non-admins requesting `?role=admin` + # silently fall back to analyst — admin instructions reference admin-only + # endpoints (marketplace registration, skills install) that a non-admin + # PAT can't authenticate against. + if role == "admin" and not (user and user.get("is_admin")): + role = "analyst" + base_url = str(request.base_url).rstrip("/") # Determine the script text: override (Jinja2-rendered) or live default. diff --git a/app/web/templates/install.html b/app/web/templates/install.html index 768f571..de07f6d 100644 --- a/app/web/templates/install.html +++ b/app/web/templates/install.html @@ -704,6 +704,8 @@
+ {# Admin tile is admin-only — non-admins see the analyst tile alone. #} + {% set _show_admin_tile = user and user.is_admin %} diff --git a/src/welcome_template.py b/src/welcome_template.py index 545fe80..3f3b739 100644 --- a/src/welcome_template.py +++ b/src/welcome_template.py @@ -244,4 +244,11 @@ def render_agent_prompt_banner( # Fall through to default # No override (or broken override) — return live default bash script. - return compute_default_agent_prompt(conn, user=user, server_url=server_url) + # 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" + return compute_default_agent_prompt( + conn, user=user, server_url=server_url, role=role, + ) diff --git a/tests/test_setup_page_roles.py b/tests/test_setup_page_roles.py index 4f9bb08..adb44de 100644 --- a/tests/test_setup_page_roles.py +++ b/tests/test_setup_page_roles.py @@ -32,19 +32,16 @@ def client(tmp_path, monkeypatch): close_system_db() -def test_setup_page_default_role_is_admin(client): - """No `role` query param → admin layout (default, preserves existing flow).""" +def test_setup_page_default_role_is_analyst(client): + """No `role` query param → analyst layout (most users are analysts; + admin layout is opt-in via the admin tile, which only renders to admins).""" resp = client.get("/setup", follow_redirects=True) assert resp.status_code == 200 text = resp.text - # Both tiles present in markup; admin tile is the active one. - assert "role=analyst" in text - assert "role=admin" in text or 'href="/setup"' in text - # Active state lives on the admin tile when role=admin (default). - # Asserting the tile labels are both rendered keeps the assertion - # robust against future styling tweaks. + # Analyst tile rendered; analyst layout is what the unauthenticated / + # non-admin caller gets by default. assert "Analyst workspace" in text - assert "Admin CLI" in text + assert "role=analyst" in text def test_setup_page_analyst_role(client): @@ -53,12 +50,35 @@ def test_setup_page_analyst_role(client): assert resp.status_code == 200 text = resp.text assert "Analyst workspace" in text - assert "Admin CLI" in text # The page must reflect the analyst selection somewhere — either via # the active-state CSS class or the `role=analyst` link being rendered. assert "role=analyst" in text +def test_setup_page_admin_tile_hidden_for_non_admin(client): + """Non-admin caller (anonymous in this test) must NOT see the admin tile — + the admin paste prompt references admin-only endpoints (marketplace + registration, skills install) that a non-admin PAT can't authenticate + against, so showing it would lead to a confusing failure. + """ + resp = client.get("/setup", follow_redirects=True) + assert resp.status_code == 200 + assert "Admin CLI" not in resp.text + assert "role=admin" not in resp.text + + +def test_setup_page_admin_role_downgraded_for_non_admin(client): + """Non-admin requesting `?role=admin` is silently downgraded to analyst. + The page must NOT render admin instructions (no `claude plugin marketplace + add` in the rendered prompt) for someone who can't execute them.""" + resp = client.get("/setup?role=admin", follow_redirects=True) + assert resp.status_code == 200 + # Admin-only steps must NOT appear (would surface admin paste prompt). + assert "claude plugin marketplace add" not in resp.text + # Analyst-only step IS present (downgrade landed on analyst layout). + assert "agnes init" in resp.text + + def test_install_redirects_to_setup(client): """`/install` legacy path keeps redirecting to `/setup` (302/307).""" resp = client.get("/install", follow_redirects=False) @@ -168,13 +188,30 @@ def test_setup_page_analyst_clipboard_renders_analyst_layout(client): ) -def test_setup_page_admin_clipboard_renders_admin_layout(client): - """Counterpart to the analyst test — admin tile MUST keep the existing - full marketplace/plugins flow byte-equivalent (no regression from Task 4). +def test_setup_page_admin_clipboard_renders_admin_layout(client, monkeypatch): + """Counterpart to the analyst test — admin caller asking for `?role=admin` + sees the full marketplace/plugins flow. + + Admin layout is now admin-gated (non-admins are silently downgraded to + analyst). To exercise the admin path, monkeypatch `get_optional_user` to + return an admin user dict. This avoids spinning up a full session-cookie + fixture for one assertion. """ import re + from app.web.router import get_optional_user + from fastapi import Request + + async def _admin_user(request: Request): # type: ignore[no-redef] + return {"id": "admin-1", "email": "admin@example.com", + "is_admin": True, "name": "Admin"} + + # Override the FastAPI dependency on the running app. + client.app.dependency_overrides[get_optional_user] = _admin_user + try: + resp = client.get("/setup?role=admin", follow_redirects=True) + finally: + client.app.dependency_overrides.pop(get_optional_user, None) - resp = client.get("/setup?role=admin", follow_redirects=True) assert resp.status_code == 200 text = resp.text @@ -186,21 +223,14 @@ def test_setup_page_admin_clipboard_renders_admin_layout(client): assert match, "SETUP_INSTRUCTIONS_TEMPLATE array not found in rendered HTML" clipboard_block = match.group(1) - # Admin layout marker MUST be present. `agnes auth import-token` is the - # admin login step (analyst replaces it with `agnes init`); `agnes skills` - # is admin-only post-auth setup. Either one anchors the admin layout - # without depending on RBAC plugin grants (which the unauthenticated - # TestClient won't have). + # Admin layout marker MUST be present. assert "agnes auth import-token" in clipboard_block, ( - "Admin clipboard payload missing `agnes auth import-token` — " - "Task 4 should not have changed admin behavior." + "Admin clipboard payload missing `agnes auth import-token`" ) assert "agnes skills" in clipboard_block, ( "Admin clipboard payload missing the skills setup step" ) - # Analyst-only marker MUST NOT appear in admin layout. `agnes init` is - # the analyst-only auth + workspace bootstrap; admin uses - # `agnes auth import-token` instead. + # Analyst-only marker MUST NOT appear in admin layout. assert "agnes init" not in clipboard_block, ( "Admin clipboard block leaked the analyst `agnes init` step" ) diff --git a/tests/test_web_ui.py b/tests/test_web_ui.py index 22ddbca..92aba84 100644 --- a/tests/test_web_ui.py +++ b/tests/test_web_ui.py @@ -214,7 +214,9 @@ class TestClaudeSetupPreview: """ def test_install_preview_visible_for_signed_in_user(self, web_client, admin_cookie): - resp = web_client.get("/setup", cookies=admin_cookie) + # /setup defaults to ?role=analyst (the common visitor case). + # Admin can switch to ?role=admin to see the marketplace + plugins flow. + resp = web_client.get("/setup?role=admin", cookies=admin_cookie) assert resp.status_code == 200 body = resp.text # Preview card + placeholder token render @@ -227,12 +229,33 @@ class TestClaudeSetupPreview: # because it validates the PEP 427 filename in the URL before fetch). assert "/cli/wheel/" in body assert "/cli/agnes.whl" not in body - # New numbered headers + agnes diagnose step + # Admin layout: numbered headers + diagnose step assert "1) Install the CLI" in body assert "4) Run diagnostics" in body assert "agnes diagnose" in body assert "agnes auth whoami" in body + def test_install_preview_default_role_is_analyst(self, web_client, admin_cookie): + """Bare /setup defaults to analyst layout — even for admin users. + Admin opts in via the admin tile (?role=admin) when they want the + marketplace/plugins flow.""" + import re + resp = web_client.get("/setup", cookies=admin_cookie) + assert resp.status_code == 200 + body = resp.text + # The clipboard payload (SETUP_INSTRUCTIONS_TEMPLATE JS array) + # carries the analyst layout. Admin tile label "Admin CLI" still + # appears as a tile heading (admin user sees both tiles), but the + # default rendered prompt is the analyst flow. + match = re.search( + r"var\s+SETUP_INSTRUCTIONS_TEMPLATE\s*=\s*\[(.*?)\]\.join\(", + body, re.DOTALL, + ) + assert match, "SETUP_INSTRUCTIONS_TEMPLATE array missing" + clipboard = match.group(1) + assert "agnes init" in clipboard + assert "claude plugin marketplace add" not in clipboard + def test_dashboard_setup_cta_links_to_setup(self, web_client, admin_cookie): """Dashboard setup CTA shows env-setup-cta and a link to /setup instead of an inline collapsed preview."""