refactor(setup-instructions): drop role param; collapse analyst/admin into one layout
Removes the `role: Literal["analyst", "admin"]` parameter from `resolve_lines` / `render_setup_instructions` and deletes the `_resolve_analyst_lines`, `_analyst_init_lines`, `_analyst_finale_lines` helpers. The unified flow now always emits `agnes init` (the workspace-rails delivery mechanism) in place of the legacy `agnes auth import-token` + `agnes auth whoami` pair, and uses `agnes catalog` as the smoke-verify step. `agnes init` already verifies the PAT internally, and `agnes catalog` doubles as a data-plane smoke check, so dropping `agnes auth whoami` costs no signal. Drops the now-redundant `tests/test_setup_instructions_analyst.py` and patches the one ordering test in `tests/test_setup_instructions.py` that referenced the old "Log in" / "Verify the login" headers. Also strips the `role=role` kwarg from `compute_default_agent_prompt`'s call into `resolve_lines` so the welcome-template render path keeps working; welcome_template.py's own role param is removed in a follow-up task. Plan: docs/superpowers/plans/2026-05-04-unified-setup-prompt.md task 1.
This commit is contained in:
ZdenekSrotyr
parent
8784f10a6b
commit
9334beed15
5 changed files with 200 additions and 199 deletions
|
|
@ -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)
|
||||
|
|
|
|||
135
docs/superpowers/plans/2026-05-04-unified-setup-prompt.md
Normal file
135
docs/superpowers/plans/2026-05-04-unified-setup-prompt.md
Normal file
|
|
@ -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 `<nav>` block. Drop `_show_admin_tile` flag. Delete `const ROLE = {{ role | tojson }};` line. Replace `tokenBody = ROLE === "analyst" ? {scope: "bootstrap-analyst", ttl_seconds: 3600} : {expires_in_days: 90}` ternary with a single body: `{name: defaultTokenName(), expires_in_days: 90}`. Continues to call existing `POST /auth/tokens` endpoint — no new endpoint needed (see PAT scope decision above). Keep "Valid 90 days" copy as-is (true for everyone now).
|
||||
**Tests rewritten:** `tests/test_web_ui.py::test_install_preview_*` — drop `?role=admin` from URLs; admin caller now sees unified layout. `tests/test_setup_page_roles.py::test_setup_page_analyst_js_uses_bootstrap_scope` and `test_setup_page_admin_js_uses_general_scope` are deleted as part of Task 5.
|
||||
**Commit:** `refactor(install.html): single tile, single PAT-mint body shape`
|
||||
**LOC budget:** ~50.
|
||||
|
||||
### Task 7 — Audit `_build_context` and dashboard-CTA path
|
||||
**Files:** `app/web/router.py` (`_build_context`)
|
||||
**What:** `_build_context` calls `render_agent_prompt_banner(conn, user=user, server_url=ctx_server_url)` already. Once `render_agent_prompt_banner` is role-free (Task 4), this just works. Verify the no-conn fallback path still works: passes `plugin_install_names=[]`, anonymous visitors see no-marketplace shape — same as today. **Audit only; if no edits needed, skip the commit.**
|
||||
**LOC budget:** 0 net (audit only).
|
||||
|
||||
### Task 8 — Delete dead test infrastructure
|
||||
**Files:** `tests/test_setup_instructions_analyst.py` (delete), `tests/test_setup_page_roles.py` (delete)
|
||||
**What:** Confirm no other tests `import` from these modules. If `tests/fixtures/analyst_bootstrap.py` references analyst-specific paths, audit and update.
|
||||
**Commit:** `chore(tests): drop split-flow test files; covered by unified suite`
|
||||
**LOC budget:** -358 (file deletions).
|
||||
|
||||
### Task 9 — CHANGELOG entry under `## [Unreleased]`
|
||||
**Files:** `CHANGELOG.md`
|
||||
**What:** Add bullets:
|
||||
- Under `### Changed`: `**BREAKING** /setup is now a single unified flow regardless of caller's role. The ?role= query parameter (introduced in this PR) is removed before merge — no migration needed. The admin tile is gone. PAT scope is uniform: every install-page mint uses scope=general with expires_in_days=90, calling the existing POST /auth/tokens endpoint. The bootstrap-analyst 1h-clamped scope is no longer used from /setup (see open issue for redesign). The marketplace + plugins block is emitted only when the caller has plugin grants in resource_grants. agnes init is now part of every setup flow (admin and analyst alike) — it's the workspace-rails delivery mechanism.`
|
||||
- Under `### Added`: pre-flight check now verifies `claude --version` in addition to `git --version`.
|
||||
- Under `### Removed`: `_resolve_analyst_lines` helper, `role` parameter on `compute_default_agent_prompt` and `resolve_lines`, `?role=` query param on `/setup`, admin tile in `install.html`.
|
||||
**Commit:** `docs(changelog): unified /setup flow under Unreleased`
|
||||
**LOC budget:** ~30.
|
||||
|
||||
### Task 10 — Final smoke test + invariant pin
|
||||
**Files:** none (verification) + `tests/test_setup_instructions.py`
|
||||
**What:** Verify orthogonal commits NOT regressed:
|
||||
- `agnes init --token` ContextVar override (commit `8784f10a`) — confirm unified flow's emitted `agnes init` line still passes `--token`.
|
||||
- Sub-agent's stale-`da` cleanup (commit `8233c3e3`) — verify unified prompt has no `da` verbs.
|
||||
**Tests added:** `tests/test_setup_instructions.py::test_unified_flow_uses_only_agnes_verbs` — `assert "da " not in resolve_lines(...)` (with space delimiter to avoid false-positive on `Darwin`/`adapter`).
|
||||
**Commit:** `test(setup-instructions): pin no-legacy-da-verbs invariant`
|
||||
**LOC budget:** ~25.
|
||||
|
||||
## Test impact summary
|
||||
|
||||
| File | Action | Reason |
|
||||
|---|---|---|
|
||||
| `tests/test_setup_instructions_analyst.py` | **DELETE (81 LOC)** | Dual-layout assertions; unified path makes them moot |
|
||||
| `tests/test_setup_page_roles.py` | **DELETE (277 LOC)** | All eight tests assert role-branching that's gone |
|
||||
| `tests/test_setup_instructions.py` | **REWRITE** | Drop `role=` kwargs; update step-number assertions; add preflight + no-da-verbs tests |
|
||||
| `tests/test_welcome_template_renderer.py` | **REWRITE** | Drop role-aware tests; assert unified default with conditional marketplace |
|
||||
| `tests/test_welcome_template_api.py` | **NO CHANGE** | API surface unchanged |
|
||||
| `tests/test_tokens_bootstrap_scope.py` | **NO CHANGE** | Underlying clamp logic preserved (no longer used from /setup, but kept for future reuse) |
|
||||
| `tests/test_setup_page_unified.py` | **NEW** | Cover single tile, no `?role=` param |
|
||||
| `tests/test_web_ui.py::test_install_preview_*` | **REWRITE** | Drop `?role=admin` from URLs |
|
||||
|
||||
## Resolved questions
|
||||
|
||||
All five open questions from the original Plan-agent draft have been resolved:
|
||||
1. **Skills always-on** — yes, no `has_skills` boolean.
|
||||
2. **`agnes init` workspace dir guard** — out of scope; user opted to drop. Documented assumption is "paste in your workspace dir" (no enforcement).
|
||||
3. **PAT mint endpoint** — no new endpoint; uniform `general` 90 d for everyone via existing `/auth/tokens` (see PAT scope decision section).
|
||||
4. **`?role=admin` redirect** — moot, `?role=` introduced in this PR, no production URLs to migrate.
|
||||
5. **Admin override copy** — no doc note; admin/analyst split deferred entirely (the codebase no longer encourages role-split UX).
|
||||
|
||||
## Out-of-scope follow-ups (file as separate issues after merge)
|
||||
|
||||
- Bootstrap-analyst scope is now unused from `/setup`. Either retire it or fix the design hole (1 h clamp breaks `agnes pull` after the install window). Tracked as separate issue.
|
||||
- Workspace-dir guard in `agnes init` — refuse-to-clobber-non-empty-home heuristic. Orthogonal to setup prompt.
|
||||
|
|
@ -188,7 +188,6 @@ def compute_default_agent_prompt(
|
|||
self_signed_tls=self_signed_tls,
|
||||
server_host=server_host,
|
||||
ca_pem=ca_pem,
|
||||
role=role,
|
||||
)
|
||||
return "\n".join(lines)
|
||||
except Exception:
|
||||
|
|
|
|||
|
|
@ -303,14 +303,14 @@ def test_resolve_lines_with_plugins_uses_install_first_diagnose_last_layout():
|
|||
assert stray not in joined
|
||||
# Crucial ordering invariants for the new layout.
|
||||
install_idx = joined.index("1) Install the CLI")
|
||||
login_idx = joined.index("2) Log in")
|
||||
verify_idx = joined.index("3) Verify the login:")
|
||||
init_idx = joined.index("2) Bootstrap your Agnes workspace")
|
||||
catalog_idx = joined.index("3) Verify the data is queryable:")
|
||||
git_idx = joined.index("4) Make sure git is installed")
|
||||
market_idx = joined.index("5) Register the Agnes Claude Code marketplace")
|
||||
diag_idx = joined.index("6) Run diagnostics:")
|
||||
skills_idx = joined.index("7) Skills")
|
||||
confirm_idx = joined.index("8) Confirm:")
|
||||
assert install_idx < login_idx < verify_idx < git_idx < market_idx < diag_idx < skills_idx < confirm_idx
|
||||
assert install_idx < init_idx < catalog_idx < git_idx < market_idx < diag_idx < skills_idx < confirm_idx
|
||||
# No git-config sslVerify=false line unless self_signed_tls is set.
|
||||
assert "git config --global" not in joined
|
||||
# server_host is server-side substituted; the placeholder must be gone.
|
||||
|
|
|
|||
|
|
@ -1,81 +0,0 @@
|
|||
"""Tests for analyst-branch rendering of /setup paste prompt."""
|
||||
|
||||
from app.web.setup_instructions import render_setup_instructions
|
||||
|
||||
|
||||
def test_render_analyst_role_basic():
|
||||
text = render_setup_instructions(
|
||||
server_url="https://agnes.example.com",
|
||||
token="agnes_pat_TEST",
|
||||
wheel_filename="agnes-0.32.0-py3-none-any.whl",
|
||||
role="analyst",
|
||||
)
|
||||
# Required content for analyst role:
|
||||
assert "uv tool install" in text
|
||||
assert "agnes init" in text
|
||||
assert "--token" in text and "agnes_pat_TEST" in text
|
||||
assert "--server-url" in text and "https://agnes.example.com" in text
|
||||
assert "agnes catalog" in text # smoke verify step
|
||||
# Forbidden content (admin-only):
|
||||
assert "marketplace" not in text
|
||||
assert "claude plugin install" not in text
|
||||
assert "agnes skills install" not in text
|
||||
assert "agnes diagnose" not in text
|
||||
|
||||
|
||||
def test_render_admin_role_unchanged():
|
||||
"""Default role=admin keeps the existing layout."""
|
||||
text = render_setup_instructions(
|
||||
server_url="https://agnes.example.com",
|
||||
token="agnes_pat_TEST",
|
||||
wheel_filename="agnes-0.32.0-py3-none-any.whl",
|
||||
# role omitted — defaults to "admin"
|
||||
)
|
||||
assert "agnes auth import-token" in text # admin uses import-token, not agnes init
|
||||
assert "agnes diagnose" in text # admin keeps diagnose
|
||||
|
||||
|
||||
def test_render_analyst_with_ca_pem():
|
||||
"""Analyst role + private CA → TLS trust block reused from admin path."""
|
||||
text = render_setup_instructions(
|
||||
server_url="https://agnes.example.com",
|
||||
token="agnes_pat_TEST",
|
||||
wheel_filename="agnes-0.32.0-py3-none-any.whl",
|
||||
role="analyst",
|
||||
ca_pem="-----BEGIN CERTIFICATE-----\nMIIBxxx\n-----END CERTIFICATE-----",
|
||||
)
|
||||
assert "AGNES_CA_PEM" in text # heredoc marker from trust block
|
||||
assert "ca-bundle.pem" in text
|
||||
assert "agnes init" in text # analyst-specific step still present
|
||||
|
||||
|
||||
def test_render_analyst_confirm_is_step_4():
|
||||
"""Pin the analyst Confirm step number so a future renumbering breaks the test
|
||||
instead of silently emitting `4) Confirm:` while step 3 has actually moved.
|
||||
Steps: 0 (TLS optional), 1 (install), 2 (init), 3 (verify), 4 (confirm).
|
||||
"""
|
||||
text = render_setup_instructions(
|
||||
server_url="https://agnes.example.com",
|
||||
token="agnes_pat_TEST",
|
||||
wheel_filename="agnes-0.32.0-py3-none-any.whl",
|
||||
role="analyst",
|
||||
)
|
||||
assert "4) Confirm:" in text
|
||||
# Also pin the init/verify step numbers
|
||||
assert "2) Bootstrap your analyst workspace" in text
|
||||
assert "3) Verify the data is queryable" in text
|
||||
|
||||
|
||||
def test_render_analyst_finale_mentions_workspace_md():
|
||||
"""Confirm bullets reference both CLAUDE.md and AGNES_WORKSPACE.md
|
||||
(which `agnes init` writes per Task 11). Init-step prose must also mention
|
||||
AGNES_WORKSPACE.md so the operator knows what to verify."""
|
||||
text = render_setup_instructions(
|
||||
server_url="https://agnes.example.com",
|
||||
token="agnes_pat_TEST",
|
||||
wheel_filename="agnes-0.32.0-py3-none-any.whl",
|
||||
role="analyst",
|
||||
)
|
||||
assert "AGNES_WORKSPACE.md" in text
|
||||
# Mentioned twice — once in the init prose, once in the confirm bullet
|
||||
assert text.count("AGNES_WORKSPACE.md") >= 2
|
||||
Loading…
Reference in a new issue