diff --git a/app/web/setup_instructions.py b/app/web/setup_instructions.py index dc83229..839b488 100644 --- a/app/web/setup_instructions.py +++ b/app/web/setup_instructions.py @@ -74,6 +74,10 @@ practice and the design here exists to dodge each one: The numbered steps are arranged so that: - All installation work (CLI, plugins) happens first, in one go. + - `agnes init` is mandatory — it bundles auth, workspace bootstrap, + CLAUDE.md fetch, and Claude Code SessionStart/End hooks into one + non-interactive call. Replaces the old `agnes auth import-token` + + `agnes auth whoami` pair. - The interactive question (skills copy vs on-demand) is the LAST step before Confirm — by that point everything else is done, the user only needs to decide one thing, and the assistant blocks on their answer. @@ -83,9 +87,9 @@ The numbered steps are arranged so that: Layout (with marketplace plugins to install): 0 TLS trust block (only when ca_pem is supplied) 1 Install CLI - 2 Login - 3 Verify - 4 Git check + 2 agnes init (auth + workspace bootstrap) + 3 agnes catalog (smoke verify) + 4 Pre-flight: git + claude 5 Marketplace + plugins 6 Diagnose 7 Skills (interactive — assistant waits for user) @@ -103,8 +107,6 @@ permitted that fallback chain — it's not improvising-around-a-TLS-error. from __future__ import annotations -from typing import Literal - # Marketplace name as published by app.marketplace_server.packager. # Hard-coded here (rather than imported) to keep this module dependency-free # and trivially testable. If the value ever drifts, the regression test @@ -312,15 +314,38 @@ def _install_cli_lines(*, has_ca: bool, server_url_placeholder: str = "{server_u ] -# Steps 2-3: login + verify. Static — these always come right after install. -_LOGIN_VERIFY_LINES: list[str] = [ - "", - "2) Log in (also saves the server URL):", - " agnes auth import-token --token \"{token}\" --server \"{server_url}\"", - "", - "3) Verify the login:", - " agnes auth whoami", -] +def _init_lines(server_url_placeholder: str = "{server_url}") -> list[str]: + """Steps 2-3 — `agnes init` (auth + workspace bootstrap) + smoke verify. + + `agnes init` is the workspace-rails delivery mechanism for everyone: + it authenticates with the PAT, fetches CLAUDE.md (RBAC-filtered), + writes AGNES_WORKSPACE.md (human-facing docs), installs Claude Code + SessionStart/End hooks (auto-refresh), and runs an initial `agnes pull` + so DuckDB views are ready. Subsumes the legacy `agnes auth import-token` + + `agnes auth whoami` pair — `init` already verifies the PAT against + `/api/catalog/tables` internally, and `agnes catalog` then doubles as + a smoke verify of the data plane. + + The PAT minted by `/setup` is `general` scope with a 90 d TTL, so the + init call will succeed for the operator's whole 90 d window without + re-clicking "Generate prompt". + """ + return [ + "", + "2) Bootstrap your Agnes workspace in this directory:", + f" agnes init --server-url \"{server_url_placeholder}\" --token \"{{token}}\" --workspace .", + "", + " This authenticates with the PAT, fetches your CLAUDE.md (RBAC-filtered),", + " writes AGNES_WORKSPACE.md (human-facing docs), installs Claude Code", + " SessionStart/End hooks (auto-refresh), and runs an initial `agnes pull`", + " so your DuckDB views are ready.", + "", + "3) Verify the data is queryable:", + " agnes catalog", + "", + " This should list the tables your account has grants for. Empty list", + " means your admin hasn't granted you access yet — contact them.", + ] def _diagnose_skills_lines(*, diagnose_num: str, skills_num: str) -> list[str]: @@ -378,13 +403,15 @@ def _finale_lines(*, confirm_step_num: str, has_ca: bool, has_marketplace: bool) non-existent step. The CA-bundle-source bullet only makes sense when the trust block ran (`has_ca`); the marketplace direct-vs-clone bullet only makes sense when the marketplace block ran (`has_marketplace`). - Skills + diagnose + version + whoami always render, so their bullets - are unconditional.""" + Init + catalog + diagnose + skills + version always render, so their + bullets are unconditional.""" bullets = [ " - `agnes --version` output", - " - `agnes auth whoami` output (email + role)", - " - Whether skills were copied or left on-demand", + " - First few lines of `agnes catalog` (tables you can see)", + " - Confirmation that `./CLAUDE.md` and `./AGNES_WORKSPACE.md` exist", + " - Confirmation that `./.claude/settings.json` contains SessionStart/End hooks", " - The `agnes diagnose` overall status", + " - Whether skills were copied or left on-demand", ] if has_ca: bullets.append( @@ -398,7 +425,7 @@ def _finale_lines(*, confirm_step_num: str, has_ca: bool, has_marketplace: bool) ) return [ f"{confirm_step_num}) Confirm:", - " Tell me \"Agnes CLI is ready\" and summarize:", + " Tell me \"Agnes workspace is ready\" and summarize:", *bullets, ] @@ -608,51 +635,6 @@ def _preamble_lines(*, has_ca: bool) -> list[str]: return lines -def _analyst_init_lines(server_url_placeholder: str = "{server_url}") -> list[str]: - """Steps 2-3 — `agnes init` (auth + workspace bootstrap) + smoke verify. - - Replaces the admin-flow login + verify steps. `agnes init` is non-interactive: - `--token` carries the PAT, `--server-url` carries the origin. The bootstrap - PAT has a 1 h TTL — if the user takes longer than that to paste this prompt, - the init call returns 401 and the user re-clicks "Generate prompt" on the - install page. - """ - return [ - "", - "2) Bootstrap your analyst workspace in this directory:", - f" agnes init --server-url \"{server_url_placeholder}\" --token \"{{token}}\" --workspace .", - "", - " This authenticates with the PAT, fetches your CLAUDE.md (RBAC-filtered),", - " writes AGNES_WORKSPACE.md (human-facing docs), installs Claude Code", - " SessionStart/End hooks (auto-refresh), and runs an initial `agnes pull`", - " so your DuckDB views are ready.", - "", - "3) Verify the data is queryable:", - " agnes catalog", - "", - " This should list the tables your account has grants for. Empty list", - " means your admin hasn't granted you access yet — contact them.", - ] - - -def _analyst_finale_lines(confirm_step_num: str, has_ca: bool) -> list[str]: - """Final Confirm step for analyst role. Shorter than admin: no marketplace, no plugins, no skills.""" - bullets = [ - " - `agnes --version` output", - " - First few lines of `agnes catalog` (tables you can see)", - " - Confirmation that `./CLAUDE.md` and `./AGNES_WORKSPACE.md` exist", - " - Confirmation that `./.claude/settings.json` contains SessionStart/End hooks", - ] - if has_ca: - bullets.append(" - Which CA bundle source got picked in step 0(d)") - return [ - "", - f"{confirm_step_num}) Confirm:", - " Tell me \"Agnes analyst workspace is ready\" and summarize:", - *bullets, - ] - - def resolve_lines( wheel_filename: str, *, @@ -660,7 +642,6 @@ def resolve_lines( self_signed_tls: bool = False, server_host: str = "", ca_pem: str | None = None, - role: Literal["analyst", "admin"] = "admin", ) -> list[str]: """Return the template lines with server-side placeholders substituted. @@ -669,13 +650,13 @@ def resolve_lines( substitution (or for `render_setup_instructions()` below). When `plugin_install_names` is empty/None, the output matches the - original 6-step layout (Confirm = step 6). When non-empty, a step-6 - git-check + step-7 marketplace block are inserted and Confirm becomes - step 8. + six-step no-marketplace layout (Confirm = step 6). When non-empty, a + step-4 pre-flight + step-5 marketplace block are inserted and Confirm + becomes step 8. `ca_pem` (PEM-encoded fullchain of the Agnes server's TLS cert) gates the cross-platform step-0 trust-bootstrap block AND switches step 1 to - the curl-then-local-install pattern AND switches step 7 to the + the curl-then-local-install pattern AND switches step 5 to the platform-aware marketplace strategy. Caller decides whether the cert needs the bootstrap (typically: skip for publicly-trusted certs like Let's Encrypt, emit for self-signed or private corp CA). @@ -691,14 +672,7 @@ def resolve_lines( The resulting URL (`/cli/wheel/agnes.whl`) will 404 at download time, but the instruction text still renders so operators can see the snippet shape and diagnose the missing wheel on the server. - - `role="analyst"` short-circuits to the analyst-workspace layout - (`_resolve_analyst_lines`) — see that function for the layout. Default - `role="admin"` keeps the admin layout below byte-identical to before. """ - if role == "analyst": - return _resolve_analyst_lines(wheel_filename, ca_pem=ca_pem) - names = list(plugin_install_names or []) has_marketplace = bool(names) has_ca = bool(ca_pem and ca_pem.strip()) @@ -707,16 +681,16 @@ def resolve_lines( # trusts the host without disabling verification. effective_self_signed = self_signed_tls and not has_ca - # Step layout. Marketplace goes BEFORE diagnose/skills, so the human-loop - # skills question is the last step before Confirm. Numbers shift between - # the no-marketplace layout (only 4 = diagnose, 5 = skills, 6 = confirm) - # and the marketplace layout (4 = git, 5 = marketplace, 6 = diagnose, - # 7 = skills, 8 = confirm). + # Step layout. Marketplace (when emitted) goes BEFORE diagnose/skills, + # so the human-loop skills question is the last step before Confirm. + # Numbers shift between the no-marketplace layout (4 diagnose, 5 skills, + # 6 confirm) and the marketplace layout (4 preflight, 5 marketplace, + # 6 diagnose, 7 skills, 8 confirm). if has_marketplace: - git_step, marketplace_step = "4", "5" + preflight_step, marketplace_step = "4", "5" diagnose_step, skills_step, confirm_step = "6", "7", "8" else: - git_step = marketplace_step = "" # unused; here just for symmetry + preflight_step = marketplace_step = "" # unused; here just for symmetry diagnose_step, skills_step, confirm_step = "4", "5", "6" lines: list[str] = [] @@ -724,14 +698,14 @@ def resolve_lines( lines.extend(_tls_trust_block(ca_pem)) # type: ignore[arg-type] lines.extend(_preamble_lines(has_ca=has_ca)) lines.extend(_install_cli_lines(has_ca=has_ca)) # 1 - lines.extend(_LOGIN_VERIFY_LINES) # 2, 3 + lines.extend(_init_lines()) # 2, 3 if has_marketplace: - lines.extend(_git_check_block(git_step)) # 4 - lines.extend(_marketplace_block( # 5 + lines.extend(_git_check_block(preflight_step)) # 4 + lines.extend(_marketplace_block( # 5 names, effective_self_signed, has_ca=has_ca, step_num=marketplace_step, )) # Diagnose + skills come AFTER the marketplace block (or right after - # whoami if there's no marketplace step at all). + # the catalog smoke verify if there's no marketplace step at all). lines.extend(_diagnose_skills_lines( diagnose_num=diagnose_step, skills_num=skills_step, )) @@ -748,30 +722,6 @@ def resolve_lines( ] -def _resolve_analyst_lines(wheel_filename: str, *, ca_pem: str | None) -> list[str]: - """Analyst workspace-bootstrap layout. Self-contained — no admin-only steps. - - Drops marketplace, plugins, skills, diagnose, login, and whoami (all of - those are admin-only or subsumed by `agnes init`). Reuses the trust - block, preamble, and install-CLI helpers from the admin path. - """ - has_ca = bool(ca_pem and ca_pem.strip()) - confirm_step = "4" # numbering: 0 (TLS optional), 1, 2, 3, 4 - - lines: list[str] = [] - if has_ca: - lines.extend(_tls_trust_block(ca_pem)) # type: ignore[arg-type] - lines.extend(_preamble_lines(has_ca=has_ca)) - lines.extend(_install_cli_lines(has_ca=has_ca)) # step 1 - lines.extend(_analyst_init_lines()) # steps 2-3 - lines.extend(_analyst_finale_lines(confirm_step, has_ca=has_ca)) # step 4 - - return [ - line.replace("{wheel_filename}", wheel_filename) - for line in lines - ] - - def render_setup_instructions( server_url: str, token: str, @@ -781,14 +731,13 @@ def render_setup_instructions( self_signed_tls: bool = False, server_host: str = "", ca_pem: str | None = None, - role: Literal["analyst", "admin"] = "admin", ) -> str: """Render the setup instructions as a single string. Used server-side for tests and any non-JS rendering path. The browser clipboard flow uses the JS renderer embedded in the Jinja partial; both must produce byte-identical output for a given (server_url, token, - wheel, plugins, flag, host, ca_pem, role) tuple. + wheel, plugins, flag, host, ca_pem) tuple. """ lines = resolve_lines( wheel_filename, @@ -796,7 +745,6 @@ def render_setup_instructions( self_signed_tls=self_signed_tls, server_host=server_host, ca_pem=ca_pem, - role=role, ) text = "\n".join(lines) return text.replace("{server_url}", server_url).replace("{token}", token) diff --git a/docs/superpowers/plans/2026-05-04-unified-setup-prompt.md b/docs/superpowers/plans/2026-05-04-unified-setup-prompt.md new file mode 100644 index 0000000..3eb6397 --- /dev/null +++ b/docs/superpowers/plans/2026-05-04-unified-setup-prompt.md @@ -0,0 +1,135 @@ +# Unified `/setup` Prompt — Implementation Plan + +**Branch:** `zs/clean-analyst-bootstrap-spec` (PR #173) +**Goal:** Collapse the dual admin/analyst bash setup-prompt architecture into a single unified flow that's RBAC-resolved per user. + +## Summary of chosen approach + +Collapse `_resolve_analyst_lines` and the admin layout in `app/web/setup_instructions.py` into a single `resolve_lines()` whose content is gated by booleans (`has_marketplace`, `has_skills`, `has_ca`). `agnes init` becomes mandatory for everyone (the workspace-rails delivery mechanism), so the unified flow always emits: TLS trust → install CLI → `agnes init` → preflight (`git --version` + `claude --version`) → marketplace/plugins (iff `plugin_install_names` non-empty) → skills (always emits) → diagnose → confirm. RBAC resolution stays in `compute_default_agent_prompt`, but unconditionally — admin/non-admin alike pass through `resolve_allowed_plugins`; only users with grants get the marketplace block. PAT scope is unified to `general` 90 d for ALL callers (no scope-by-role split — see decision below). The `?role=` query param and admin tile both go away. The `welcome_template` admin override remains a single text blob (no DB schema change, no role-aware UI affordance). + +## PAT scope decision — uniform `general` 90d for everyone, NO new endpoint + +Original plan proposed a new `POST /auth/tokens/issue-for-setup` endpoint that would inspect `user.is_admin` and mint `general`/90d for admins, `bootstrap-analyst`/1h for non-admins. After review: + +- **No real security benefit**: non-admin users can ALREADY mint their own `general` 90d PATs via the existing `POST /auth/tokens` route from the `/tokens` UI. There's no admin gate on `general` scope. So routing the install button through a "role-locked" endpoint is ceremony without security value. + +- **Bootstrap-analyst 1h scope is broken for the install flow**: an analyst pasting the prompt at 10:00 mints a PAT expiring at 11:00. They run `agnes init` at 10:30 (saves PAT to `~/.config/agnes/token.json`). At 11:30 they open Claude Code; SessionStart hook runs `agnes pull` → 401 because the saved PAT expired. `agnes init` does not re-mint a long-lived token internally. So bootstrap-analyst PATs are effectively single-use-then-broken in this flow. + +Decision: install button mints `general` scope, `expires_in_days=90` for everyone. Single `fetch('/auth/tokens', { method: 'POST', body: JSON.stringify({ name: 'agnes-install-...', expires_in_days: 90 }) })` in JS. The `bootstrap-analyst` scope + clamp logic stays in the codebase (still useful for future flows, e.g. a one-shot CI bootstrap), just not invoked from `/setup`. Tracked as separate cleanup issue: redesign or retire `bootstrap-analyst` scope. + +## Legacy `?role=admin` URL — no redirect needed + +`?role=` query parameter was introduced in this PR (not on `main`). No production URLs reference it; bookmarks/runbooks don't exist yet. Just remove the param from the route signature; no `RedirectResponse` shim required. + +## Tasks + +### Task 1 — Drop `role` parameter from `setup_instructions.resolve_lines` +**Files:** `app/web/setup_instructions.py` +**What:** Remove `role: Literal["analyst", "admin"]` parameter from `resolve_lines` and `render_setup_instructions`. Delete `_resolve_analyst_lines`, `_analyst_init_lines`, `_analyst_finale_lines` entirely. Move `agnes init` step into a new helper `_init_lines(server_url_placeholder)` that always emits. Reuse `_finale_lines` (no parallel analyst version once layouts merge). +**Tests deleted:** `tests/test_setup_instructions_analyst.py` (entire file). +**Tests rewritten:** `tests/test_setup_instructions.py` — drop `role=` kwarg from any `resolve_lines(...)` calls; update admin layout assertions for unified numbering (Task 3). +**Commit:** `refactor(setup-instructions): drop role param; collapse analyst/admin into one layout` +**LOC budget:** ~150 (deletion-heavy). + +### Task 2 — Adopt unified step layout +**Files:** `app/web/setup_instructions.py` +**What:** Recompose `resolve_lines` to always emit: +- 0 (optional): TLS trust block +- 1: Install CLI +- 2: `agnes init --server-url ... --token ...` (NEW position — was admin's step 2/3 login+verify; analyst's step 2-3 init+catalog) +- 3: `agnes catalog` smoke verify (drop the admin-only `agnes auth whoami` — `agnes init` already verifies the PAT against `/api/catalog/tables`) +- 4 (iff has_marketplace): Pre-flight: `git --version` AND `claude --version` (Task 4) +- 5 (iff has_marketplace): Marketplace + plugins +- 6: Diagnose +- 7 (iff has_skills, default True): Skills +- 8 (or earliest): Confirm + +Renumbering helper: `_step_numbers(*, has_marketplace, has_skills)` returns the dict so helpers don't reimplement renumber logic. Update `_finale_lines` bullets to be conditional on `has_marketplace`/`has_skills`/`has_ca`. +**Tests rewritten:** `tests/test_setup_instructions.py::test_resolve_lines_no_plugins_keeps_six_step_layout` — rename + update assertions. Unified no-plugin layout: 1 install, 2 init, 3 catalog, 4 diagnose, 5 skills, 6 confirm. With plugins: 1, 2, 3, 4 preflight, 5 marketplace, 6 diagnose, 7 skills, 8 confirm. +**Commit:** `refactor(setup-instructions): unified layout with mandatory agnes init` +**LOC budget:** ~120. + +### Task 3 — Add `claude --version` to the pre-flight check +**Files:** `app/web/setup_instructions.py` (`_git_check_block` → `_preflight_block`) +**What:** Rename `_git_check_block` to `_preflight_block`. Inside, after `git --version`, add `claude --version || { ... install hint ... }`. Install hint: `npm i -g @anthropic-ai/claude-code` or directs the user to the platform installer (link to `https://docs.claude.com/claude-code`). Keep section header "Make sure git and claude are installed (required for the marketplace clone)". Step number stays parameterized. +**Tests added:** `tests/test_setup_instructions.py::test_preflight_checks_both_git_and_claude`. +**Commit:** `feat(setup-instructions): preflight checks both git and claude` +**LOC budget:** ~40. + +### Task 4 — Drop `role` from `compute_default_agent_prompt`; resolve plugins unconditionally +**Files:** `src/welcome_template.py` +**What:** Remove `role: Literal["analyst", "admin"] = "admin"` parameter from `compute_default_agent_prompt`. Always run `marketplace_filter.resolve_allowed_plugins(conn, user)` (currently gated on `role == "admin"`). Function returns `[]` for users with no grants — that's already the analyst case. Remove `role` from `render_agent_prompt_banner`'s tail (the `role = "admin" if user.is_admin else "analyst"` block deletes entirely). +**Tests rewritten:** `tests/test_welcome_template_renderer.py` — drop role-aware distinction. Update assertions to reflect unified output: `agnes init` always present, `agnes auth import-token` never present (replaced by init), `claude plugin marketplace add` only when caller has plugin grants. +**Commit:** `refactor(welcome-template): drop role param; resolve plugins per-user unconditionally` +**LOC budget:** ~60. + +### Task 5 — Strip `?role=` from `/setup` route; remove silent admin-downgrade +**Files:** `app/web/router.py` +**What:** Remove `role: Literal["analyst", "admin"] = Query(default="analyst")` from `setup_page`. Delete silent-downgrade block. Drop `role` from `compute_default_agent_prompt(...)` calls. Drop `role` from template ctx. **No redirect needed** — `?role=` was introduced in this PR, no existing URLs reference it. +**Tests deleted:** `tests/test_setup_page_roles.py` — entire file. +**Tests added:** `tests/test_setup_page_unified.py` — two small tests: `test_setup_page_renders_unified_layout`, `test_setup_page_renders_marketplace_for_user_with_grants`. +**Commit:** `refactor(setup-page): drop role query param` +**LOC budget:** ~70. + +### Task 6 — Drop the admin tile and JS scope ternary from `install.html` +**Files:** `app/web/templates/install.html` +**What:** Delete role-tiles `