refactor(setup-instructions): unified layout with mandatory agnes init
Adds `_step_numbers(*, has_marketplace, has_skills)` so step numbering lives in one place instead of being split across three branches in `resolve_lines`. Pins the unified layout in the tests: No plugins: 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 `agnes auth import-token` / `agnes auth whoami` are now banned from the rendered prompt — `agnes init` subsumes them. The renamed `test_resolve_lines_no_plugins_unified_six_step_layout` asserts those strings are absent and that the new step headers (`Bootstrap your Agnes workspace`, `Verify the data is queryable`) are present. Plan: docs/superpowers/plans/2026-05-04-unified-setup-prompt.md task 2.
This commit is contained in:
ZdenekSrotyr
parent
9334beed15
commit
e16698c3cc
2 changed files with 66 additions and 20 deletions
|
|
@ -635,6 +635,44 @@ def _preamble_lines(*, has_ca: bool) -> list[str]:
|
|||
return lines
|
||||
|
||||
|
||||
def _step_numbers(*, has_marketplace: bool, has_skills: bool = True) -> dict[str, str]:
|
||||
"""Compute the step numbers for the unified layout based on which optional
|
||||
blocks are emitted.
|
||||
|
||||
Returns a dict keyed by logical step name; values are stringified
|
||||
1-based step numbers (preserving the existing string-based helper API
|
||||
so call sites stay diff-minimal).
|
||||
|
||||
Mandatory steps (always emitted): install (1), init (2), catalog (3),
|
||||
diagnose, confirm. Optional: preflight + marketplace (gated on
|
||||
has_marketplace), skills (gated on has_skills — default True; the
|
||||
Resolved-Question section in the plan settled on always-on, so the
|
||||
parameter is here purely to keep the helper general for future use,
|
||||
not to expose a real toggle).
|
||||
|
||||
Step-0 (TLS trust block) sits outside this numbering — it is gated by
|
||||
has_ca and has its own "0)" header rendered inside the trust block
|
||||
helper.
|
||||
"""
|
||||
n = 4
|
||||
preflight = marketplace = ""
|
||||
if has_marketplace:
|
||||
preflight = str(n); n += 1
|
||||
marketplace = str(n); n += 1
|
||||
diagnose = str(n); n += 1
|
||||
skills = str(n) if has_skills else ""
|
||||
if has_skills:
|
||||
n += 1
|
||||
confirm = str(n)
|
||||
return {
|
||||
"preflight": preflight,
|
||||
"marketplace": marketplace,
|
||||
"diagnose": diagnose,
|
||||
"skills": skills,
|
||||
"confirm": confirm,
|
||||
}
|
||||
|
||||
|
||||
def resolve_lines(
|
||||
wheel_filename: str,
|
||||
*,
|
||||
|
|
@ -683,15 +721,10 @@ def resolve_lines(
|
|||
|
||||
# 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:
|
||||
preflight_step, marketplace_step = "4", "5"
|
||||
diagnose_step, skills_step, confirm_step = "6", "7", "8"
|
||||
else:
|
||||
preflight_step = marketplace_step = "" # unused; here just for symmetry
|
||||
diagnose_step, skills_step, confirm_step = "4", "5", "6"
|
||||
# `_step_numbers` returns the renumbered step labels in one place — no
|
||||
# branch on every helper — so the layout is unambiguous and trivially
|
||||
# extendable when a future step is added.
|
||||
steps = _step_numbers(has_marketplace=has_marketplace, has_skills=True)
|
||||
|
||||
lines: list[str] = []
|
||||
if has_ca:
|
||||
|
|
@ -700,18 +733,18 @@ def resolve_lines(
|
|||
lines.extend(_install_cli_lines(has_ca=has_ca)) # 1
|
||||
lines.extend(_init_lines()) # 2, 3
|
||||
if has_marketplace:
|
||||
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,
|
||||
lines.extend(_git_check_block(steps["preflight"])) # 4
|
||||
lines.extend(_marketplace_block( # 5
|
||||
names, effective_self_signed, has_ca=has_ca, step_num=steps["marketplace"],
|
||||
))
|
||||
# Diagnose + skills come AFTER the marketplace block (or right after
|
||||
# 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,
|
||||
diagnose_num=steps["diagnose"], skills_num=steps["skills"],
|
||||
))
|
||||
lines.append("")
|
||||
lines.extend(_finale_lines(
|
||||
confirm_step_num=confirm_step,
|
||||
confirm_step_num=steps["confirm"],
|
||||
has_ca=has_ca,
|
||||
has_marketplace=has_marketplace,
|
||||
))
|
||||
|
|
|
|||
|
|
@ -47,16 +47,26 @@ def test_render_setup_instructions_wires_all_placeholders():
|
|||
assert "T-123" in out
|
||||
|
||||
|
||||
def test_resolve_lines_no_plugins_keeps_six_step_layout():
|
||||
"""Backwards-compat: empty plugin list → original 6-step layout, Confirm = 6."""
|
||||
def test_resolve_lines_no_plugins_unified_six_step_layout():
|
||||
"""Unified no-plugin layout: 1 install, 2 init, 3 catalog, 4 diagnose,
|
||||
5 skills, 6 confirm. No marketplace, no preflight."""
|
||||
from app.web.setup_instructions import resolve_lines
|
||||
|
||||
joined = "\n".join(resolve_lines("agnes.whl"))
|
||||
# Mandatory unified-flow steps.
|
||||
assert "1) Install the CLI" in joined
|
||||
assert "2) Bootstrap your Agnes workspace" in joined
|
||||
assert "3) Verify the data is queryable:" in joined
|
||||
assert "4) Run diagnostics:" in joined
|
||||
assert "5) Skills" in joined
|
||||
assert "6) Confirm:" in joined
|
||||
# No stray Confirms at other positions.
|
||||
assert "7) Confirm:" not in joined
|
||||
assert "8) Confirm:" not in joined
|
||||
assert "claude plugin marketplace add" not in joined
|
||||
assert "claude plugin install" not in joined
|
||||
# No preflight step when there's no marketplace block to gate.
|
||||
assert "Make sure git is installed" not in joined
|
||||
# Legacy `git config sslVerify=false` downgrade must NOT be emitted.
|
||||
# Match the specific config line, not the bare substring (which appears
|
||||
# in the preamble as a "don't do this" example).
|
||||
|
|
@ -69,6 +79,9 @@ def test_resolve_lines_no_plugins_keeps_six_step_layout():
|
|||
assert "step 0(d)" not in joined
|
||||
assert "Which CA bundle source got picked" not in joined
|
||||
assert "Whether the marketplace add went via direct HTTPS" not in joined
|
||||
# Legacy admin-only auth verbs are gone — `agnes init` subsumes them.
|
||||
assert "agnes auth import-token" not in joined
|
||||
assert "agnes auth whoami" not in joined
|
||||
|
||||
|
||||
def test_preamble_step_zero_d_reference_only_when_trust_block_emitted():
|
||||
|
|
@ -267,10 +280,10 @@ def test_trust_block_step_0c_does_not_reference_stale_step_number():
|
|||
|
||||
|
||||
def test_resolve_lines_with_plugins_uses_install_first_diagnose_last_layout():
|
||||
"""Marketplace layout puts install/login/git/marketplace BEFORE diagnose
|
||||
and skills, so the human-loop skills question is the final blocking
|
||||
step before Confirm. Step numbers: 4 git, 5 marketplace, 6 diagnose,
|
||||
7 skills, 8 confirm."""
|
||||
"""Marketplace layout puts install/init/catalog/preflight/marketplace
|
||||
BEFORE diagnose and skills, so the human-loop skills question is the
|
||||
final blocking step before Confirm. Step numbers: 4 preflight,
|
||||
5 marketplace, 6 diagnose, 7 skills, 8 confirm."""
|
||||
from app.web.setup_instructions import resolve_lines
|
||||
|
||||
lines = resolve_lines(
|
||||
|
|
|
|||
Loading…
Reference in a new issue