refactor(setup-page): drop role query param

The `/setup` route no longer accepts `?role=analyst|admin`. The route
signature drops the `Literal[...] = Query(...)` parameter and the
silent admin-downgrade block (`if role == "admin" and not is_admin:
role = "analyst"`). The `role` ctx variable threaded into install.html
also goes away — Task 6 cleans up the template's role-tile UI and the
JS PAT-mint ternary.

`?role=` is silently ignored by FastAPI for unknown query params, so
existing bookmarks (none in production — the param was added in this
PR and never shipped) just degrade to the unified layout. No
RedirectResponse shim needed.

Tests: drop the entire `tests/test_setup_page_roles.py` file (eight
role-branching tests that no longer apply) and add
`tests/test_setup_page_unified.py` with three tests:

  - `test_setup_page_renders_unified_layout`
  - `test_setup_page_ignores_role_query_param`
  - `test_setup_page_renders_marketplace_for_user_with_grants`
  - `test_install_legacy_path_redirects_to_setup`

Also replace the role-aware `test_install_preview_*` tests in
test_web_ui.py with unified-layout assertions.

Plan: docs/superpowers/plans/2026-05-04-unified-setup-prompt.md task 5.
This commit is contained in:
ZdenekSrotyr 2026-05-04 22:16:59 +02:00
parent 291079b1d2
commit 2ee529533f
4 changed files with 152 additions and 314 deletions

View file

@ -7,10 +7,10 @@ import logging
import os
from datetime import datetime
from pathlib import Path
from typing import Literal, Optional
from typing import Optional
from urllib.parse import quote
from fastapi import APIRouter, Depends, Query, Request, HTTPException
from fastapi import APIRouter, Depends, Request, HTTPException
from fastapi.responses import HTMLResponse, RedirectResponse
from fastapi.templating import Jinja2Templates
import duckdb
@ -717,42 +717,31 @@ async def activity_center(
@router.get("/setup", response_class=HTMLResponse)
async def setup_page(
request: Request,
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:
- `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.
Single unified flow for everyone admin-vs-analyst is no longer a
layout branch. The marketplace + plugins block appears iff the
caller has plugin grants in `resource_grants` (resolved inside
`compute_default_agent_prompt`).
When an admin override is saved, the override replaces the auto-generated
setup_instructions output everywhere (both the /setup page display and the
dashboard clipboard CTA). When no override is set, the live default from
When an admin override is saved, the override replaces the
auto-generated setup_instructions output everywhere (both the
/setup page display and the dashboard clipboard CTA). When no
override is set, the live default from
setup_instructions.resolve_lines() is used.
"""
from src.repositories.welcome_template import WelcomeTemplateRepository
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.
# The override is role-agnostic by design — admins who set an override are
# opting into the exact text they wrote, regardless of which tile the
# caller picked. Only the live default branches on `role`.
# The override is per-instance, applies to every caller — admins who set
# an override are opting into the exact text they wrote.
row = WelcomeTemplateRepository(conn).get()
override_content = row.get("content")
if override_content:
@ -789,7 +778,6 @@ async def setup_page(
# Override both variables so the partial and the JS array stay in sync.
setup_instructions_lines=setup_instructions_lines,
setup_script_text=setup_script_text,
role=role,
)
return templates.TemplateResponse(request, "install.html", ctx)

View file

@ -1,277 +0,0 @@
"""Tests for /setup role query-param branching.
Task 4 wires `?role=analyst|admin` through the /setup route handler so the
template can render two role tiles and the renderer can pick the right
layout (admin = full marketplace/skills/diagnose flow; analyst = trimmed
workspace-bootstrap flow). Default is `admin` to preserve existing behavior.
"""
import pytest
from fastapi.testclient import TestClient
@pytest.fixture
def client(tmp_path, monkeypatch):
"""TestClient against a freshly-built FastAPI app rooted at tmp_path.
Mirrors the `web_client` fixture in tests/test_web_ui.py we re-create
the app so the DuckDB singleton picks up the per-test DATA_DIR rather
than leaking state across tests on the same xdist worker.
"""
monkeypatch.setenv("DATA_DIR", str(tmp_path))
monkeypatch.setenv("TESTING", "1")
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-key-min-32-characters!!")
(tmp_path / "state").mkdir()
(tmp_path / "analytics").mkdir()
(tmp_path / "extracts").mkdir()
from src.db import close_system_db
close_system_db()
from app.main import create_app
app = create_app()
yield TestClient(app)
close_system_db()
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
# Analyst tile rendered; analyst layout is what the unauthenticated /
# non-admin caller gets by default.
assert "Analyst workspace" in text
assert "role=analyst" in text
def test_setup_page_analyst_role(client):
"""`?role=analyst` → analyst tile is the active one."""
resp = client.get("/setup?role=analyst", follow_redirects=True)
assert resp.status_code == 200
text = resp.text
assert "Analyst workspace" 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)
assert resp.status_code in (302, 307)
assert "/setup" in resp.headers["location"]
def test_setup_page_invalid_role_falls_back(client):
"""Invalid role values must NOT 500 — either FastAPI's Literal
validation rejects with 422, or the route quietly falls back to admin.
Both are acceptable; what's not acceptable is an unhandled exception.
"""
resp = client.get("/setup?role=hacker", follow_redirects=True)
assert resp.status_code in (200, 422)
def test_setup_page_analyst_js_uses_bootstrap_scope(client):
"""Analyst tile's setupNewClaude JS must mint bootstrap-analyst PATs.
The JS PAT mint must be role-aware: analyst gets a short-TTL
bootstrap-analyst-scoped PAT (server clamps ttl 3600s), not the
historical 90-day general PAT. Asserts the wiring at the rendered
template level so we catch any regression in either the Jinja ctx
plumbing or the JS branching.
"""
resp = client.get("/setup?role=analyst", follow_redirects=True)
assert resp.status_code == 200
text = resp.text
# The role variable must be set to analyst in JS scope.
assert (
'const ROLE = "analyst"' in text
or 'ROLE = "analyst"' in text
or 'data-role="analyst"' in text
)
# The bootstrap-analyst scope must appear in the JS PAT-mint body.
assert "bootstrap-analyst" in text
assert "ttl_seconds" in text
def test_setup_page_admin_js_uses_general_scope(client):
"""Admin tile's setupNewClaude JS must keep the existing 90-day
expires_in_days behavior byte-identical PAT mint shape so existing
admin flows don't regress.
"""
resp = client.get("/setup?role=admin", follow_redirects=True)
assert resp.status_code == 200
text = resp.text
assert "expires_in_days" in text # still present in the admin body
def test_setup_page_analyst_clipboard_renders_analyst_layout(client):
"""The clipboard text the analyst tile produces must be the analyst layout
(`agnes init` + `agnes catalog`), NOT the admin layout (`claude plugin
marketplace add` + admin-only flow).
The rendered HTML embeds the role-aware `setup_instructions_lines` text
into a JS array `SETUP_INSTRUCTIONS_TEMPLATE` (see
`_claude_setup_instructions.jinja`); `renderSetupInstructions` substitutes
`{server_url}` / `{token}` into that array at click time. So checking the
embedded array against the served HTML is sufficient if it carries the
analyst layout, the clipboard payload will too.
Pinning to the JS array block specifically (not the whole page) avoids
false positives from chrome / preview-mode <pre> renders elsewhere.
"""
import re
resp = client.get("/setup?role=analyst", follow_redirects=True)
assert resp.status_code == 200
text = resp.text
# Locate the JS array that holds the clipboard template body. The partial
# emits `var SETUP_INSTRUCTIONS_TEMPLATE = [...].join("\n");` — match
# everything between the `[` and the matching `]` so we can scope the
# assertions to *just* what gets pasted into Claude Code, ignoring the
# preview <pre> block that also embeds the lines.
match = re.search(
r"var\s+SETUP_INSTRUCTIONS_TEMPLATE\s*=\s*\[(.*?)\]\.join\(",
text,
re.DOTALL,
)
assert match, "SETUP_INSTRUCTIONS_TEMPLATE array not found in rendered HTML"
clipboard_block = match.group(1)
# Analyst layout markers MUST be in the clipboard block.
assert "agnes init" in clipboard_block, (
"Analyst clipboard payload missing `agnes init` — is "
"compute_default_agent_prompt(role=role) being threaded into "
"setup_instructions_lines on the /setup route?"
)
assert "agnes catalog" in clipboard_block, (
"Analyst clipboard payload missing `agnes catalog` smoke verify"
)
# Admin-only markers MUST NOT be in the analyst clipboard block.
# If they are, renderSetupInstructions is producing admin layout despite
# the analyst tile being selected — the bug this test guards against.
# `agnes auth import-token` is the admin login step (analyst uses
# `agnes init` which bundles auth + workspace bootstrap).
assert "agnes auth import-token" not in clipboard_block, (
"Analyst clipboard block contains admin-only `agnes auth import-token` "
"— role plumbing is broken; analyst sees admin instructions."
)
assert "agnes skills" not in clipboard_block, (
"Analyst clipboard block contains admin-only skills setup step — "
"analyst layout should not include skills management."
)
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)
assert resp.status_code == 200
text = resp.text
match = re.search(
r"var\s+SETUP_INSTRUCTIONS_TEMPLATE\s*=\s*\[(.*?)\]\.join\(",
text,
re.DOTALL,
)
assert match, "SETUP_INSTRUCTIONS_TEMPLATE array not found in rendered HTML"
clipboard_block = match.group(1)
# Admin layout marker MUST be present.
assert "agnes auth import-token" in clipboard_block, (
"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.
assert "agnes init" not in clipboard_block, (
"Admin clipboard block leaked the analyst `agnes init` step"
)
def test_setup_page_role_is_json_escaped(client):
"""The `ROLE` JS const must be injected via Jinja `tojson` (defense in
depth) not as a bare `"{{ role }}"` string interpolation. This makes
JS string-escaping explicit and removes the dependency on Jinja
autoescape semantics for JS contexts.
"""
resp = client.get("/setup?role=analyst", follow_redirects=True)
assert resp.status_code == 200
text = resp.text
# tojson always emits double-quoted JSON: the rendered output is exactly
# `const ROLE = "analyst";` (note: no extra space inside the quotes).
assert 'const ROLE = "analyst";' in text
def test_setup_page_js_ternary_keys_bootstrap_to_analyst(client):
"""Mutation-resistant assertion: the `bootstrap-analyst` scope must sit on
the truthy branch of `ROLE === "analyst"`, not on the falsy branch.
Without this, a silent inversion (`ROLE === "analyst"` `ROLE !== "analyst"`)
would let analyst tile mint a general PAT and admin tile mint a
bootstrap-scoped PAT with both substrings still present in served HTML.
The test passes whether the content is queried via either role URL since
the JS ternary itself is identical across role responses.
"""
import re
resp = client.get("/setup?role=admin", follow_redirects=True) # JS body is role-independent
assert resp.status_code == 200
text = resp.text
# Match the ternary `ROLE === "analyst" ? <truthy_branch> : <falsy_branch>`.
# Allow whitespace/newlines, ensure `bootstrap-analyst` is in the truthy branch.
pattern = re.compile(
r'ROLE\s*===\s*"analyst"\s*\?\s*\{[^}]*bootstrap-analyst[^}]*\}\s*:\s*\{[^}]*expires_in_days[^}]*\}',
re.DOTALL,
)
assert pattern.search(text), (
"JS PAT-mint ternary must have `bootstrap-analyst` on the analyst (truthy) "
"branch and `expires_in_days` on the admin (falsy) branch. A silent inversion "
"would let the analyst tile mint a general PAT — exactly the regression we want to catch."
)

View file

@ -0,0 +1,114 @@
"""Tests for the unified `/setup` route.
The previous `?role=analyst|admin` query parameter is gone. The route
renders a single layout for everyone admin-vs-analyst is no longer a
branch. The marketplace + plugins block is gated by per-user
`resource_grants` resolved inside `compute_default_agent_prompt`.
"""
import pytest
from fastapi.testclient import TestClient
@pytest.fixture
def client(tmp_path, monkeypatch):
"""TestClient against a freshly-built FastAPI app rooted at tmp_path.
Mirrors the `web_client` fixture in tests/test_web_ui.py we re-create
the app so the DuckDB singleton picks up the per-test DATA_DIR rather
than leaking state across tests on the same xdist worker.
"""
monkeypatch.setenv("DATA_DIR", str(tmp_path))
monkeypatch.setenv("TESTING", "1")
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-key-min-32-characters!!")
(tmp_path / "state").mkdir()
(tmp_path / "analytics").mkdir()
(tmp_path / "extracts").mkdir()
from src.db import close_system_db
close_system_db()
from app.main import create_app
app = create_app()
yield TestClient(app)
close_system_db()
def test_setup_page_renders_unified_layout(client):
"""Bare `/setup` (no query param) renders the unified flow:
- `agnes init` is mandatory (subsumes the old admin-only
`agnes auth import-token` + `agnes auth whoami` pair).
- Anonymous visitors with no plugin grants get the no-marketplace
layout (Confirm = step 6).
"""
resp = client.get("/setup", follow_redirects=True)
assert resp.status_code == 200
text = resp.text
# Unified flow markers.
assert "agnes init" in text
# Legacy admin-only login verbs are gone from the rendered prompt.
assert "agnes auth import-token" not in text
# No-marketplace layout: Confirm = step 6.
assert "6) Confirm:" in text
def test_setup_page_ignores_role_query_param(client):
"""`?role=...` is no longer accepted by the route signature. FastAPI
ignores unknown query params silently `/setup?role=admin` still
serves the unified layout. No 422, no redirect, no behavior delta
vs. bare `/setup`."""
bare = client.get("/setup", follow_redirects=True)
with_role = client.get("/setup?role=admin", follow_redirects=True)
assert bare.status_code == 200
assert with_role.status_code == 200
# Both responses contain the unified-flow marker.
assert "agnes init" in bare.text
assert "agnes init" in with_role.text
# Legacy admin-only login verbs are gone from both.
assert "agnes auth import-token" not in bare.text
assert "agnes auth import-token" not in with_role.text
def test_setup_page_renders_marketplace_for_user_with_grants(client, monkeypatch):
"""When the caller has plugin grants in `resource_grants`, the
unified flow inserts the marketplace + plugins block (step 5) and
Confirm shifts to step 8.
Stub `marketplace_filter.resolve_allowed_plugins` to return a
plugin so we don't have to seed the full marketplace plumbing in
this test we're verifying the layout switch, not the RBAC
resolver itself (covered by `test_marketplace_filter`)."""
from app.web.router import get_optional_user
from fastapi import Request
from src import marketplace_filter
async def _admin_user(request: Request): # type: ignore[no-redef]
return {"id": "admin-1", "email": "admin@example.com",
"is_admin": True, "name": "Admin", "groups": ["Admin"]}
monkeypatch.setattr(
marketplace_filter,
"resolve_allowed_plugins",
lambda conn, user: [{"manifest_name": "demo-plugin"}],
)
client.app.dependency_overrides[get_optional_user] = _admin_user
try:
resp = client.get("/setup", follow_redirects=True)
finally:
client.app.dependency_overrides.pop(get_optional_user, None)
assert resp.status_code == 200
text = resp.text
# Marketplace block markers.
assert "claude plugin install demo-plugin@agnes" in text
# Layout shift: Confirm is now step 8 (was 6 without marketplace).
assert "8) Confirm:" in text
# Pre-flight is in the rendered prompt at step 4.
assert "Make sure git and claude are installed" in text
def test_install_legacy_path_redirects_to_setup(client):
"""`/install` legacy path keeps redirecting to `/setup` (302/307)."""
resp = client.get("/install", follow_redirects=False)
assert resp.status_code in (302, 307)
assert "/setup" in resp.headers["location"]

View file

@ -214,9 +214,11 @@ class TestClaudeSetupPreview:
"""
def test_install_preview_visible_for_signed_in_user(self, web_client, 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)
# /setup is now a single unified flow regardless of caller's role.
# Admin sees the same layout as everyone else; the marketplace
# block appears iff the caller has plugin grants in
# `resource_grants` (the seeded admin in this fixture has none).
resp = web_client.get("/setup", cookies=admin_cookie)
assert resp.status_code == 200
body = resp.text
# Preview card + placeholder token render
@ -229,24 +231,29 @@ 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
# Admin layout: numbered headers + diagnose step
# Unified 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
# `agnes init` is now the mandatory bootstrap step.
assert "agnes init" in body
# The generated /setup prompt's "Log in" / "Verify the login"
# admin-only headers are gone (agnes init subsumes them).
# `agnes auth whoami` survives as a static manual-install
# example elsewhere on the page (not in the generated prompt).
assert "2) Log in" not in body
assert "3) Verify the login" not 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."""
def test_install_preview_unified_layout(self, web_client, admin_cookie):
"""The clipboard payload (SETUP_INSTRUCTIONS_TEMPLATE JS array)
carries the unified layout for every caller admin-vs-analyst
is no longer a layout branch. Without plugin grants, the
marketplace block is omitted (no `claude plugin marketplace
add` line)."""
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,
@ -255,6 +262,12 @@ class TestClaudeSetupPreview:
clipboard = match.group(1)
assert "agnes init" in clipboard
assert "claude plugin marketplace add" not in clipboard
# Legacy admin-only auth verbs are gone from the generated prompt.
assert "agnes auth import-token" not in clipboard
# `agnes auth whoami` was the old admin step 3; subsumed by
# `agnes init` + `agnes catalog` smoke verify.
assert "3) Verify the login" not in clipboard
assert "2) Log in" 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