Skip to content

feat(setup): one-command first-run flow + agent-narrowing defaults (closes #103)#104

Merged
liam-ai-reality merged 6 commits into
mainfrom
feat/issue-103-klasp-setup-defaults
May 8, 2026
Merged

feat(setup): one-command first-run flow + agent-narrowing defaults (closes #103)#104
liam-ai-reality merged 6 commits into
mainfrom
feat/issue-103-klasp-setup-defaults

Conversation

@liam-ai-reality
Copy link
Copy Markdown
Contributor

Summary

This PR implements all 4 changes from issue #103, landing in the suggested order (each as a separate commit).

Changes

  1. Default [gate].agents to detected agents (feat/adopt: default [gate].agents to detected agents) — new adopt/detect_agents.rs probes ~/.claude/, ~/.codex/, ~/.aider*; write_klasp_toml gains agents: Option<&[String]> param; klasp init --adopt --mode mirror uses the narrowed list.

  2. Auto-suffix duplicate check names (feat/adopt: auto-suffix duplicate check names on collision) — when two ProposedChecks collide on the same name, the second gets a gate-type suffix: lintlint-lefthook. First keeps bare name.

  3. klasp install warn-on-narrower (feat/install: warn when --agent <single> is narrower than [gate].agents) — single-agent install against a multi-agent klasp.toml emits a WARN to stderr; install still exits 0.

  4. klasp setup orchestrator (feat/setup: one-command first-run orchestrator) — new subcommand runs detect → narrow → write → install → doctor in one command. --interactive adds y/n prompts. --dry-run writes nothing. docs/setup.md added.

Acceptance criteria mapping

AC Commit Test
klasp setup exists and runs full sequence 4 (1e4d1c2) tests/setup.rs::setup_help_exits_successfully, claude_only_home_narrows_agents_to_claude_code
Fresh repo with ~/.claude/ only → agents = ["claude_code"] + doctor exits 0 4 (1e4d1c2) tests/setup.rs::claude_only_home_narrows_agents_to_claude_code (smoke-tested end-to-end manually)
--interactive prompts before write + install 4 (1e4d1c2) tests/setup.rs::interactive_n_to_mirror_skips_write, interactive_y_y_writes_file
--dry-run prints plan, writes nothing 4 (1e4d1c2) tests/setup.rs::dry_run_prints_plan_writes_nothing
Duplicate names → second gets gate suffix; unit test in writer.rs 2 (8687224) klasp/src/adopt/writer.rs::tests::duplicate_name_suffix_on_collision, first_occurrence_keeps_bare_name
klasp init --adopt --mode mirror with ~/.claude/ only → agents = ["claude_code"] 1 (06103b3) klasp/src/adopt/writer.rs::tests::agents_some_uses_narrowed_list; klasp/src/adopt/detect_agents.rs::tests::claude_only_home_returns_claude_code
klasp install --agent <single> against multi-agent config emits WARN; install succeeds 3 (5015221) tests/setup.rs::install_single_agent_warns_about_uncovered, install_all_does_not_warn_narrower
Integration tests: claude-only, all-three, #97 fixtures, dry-run, interactive Y/N 4 (1e4d1c2) tests/setup.rs (11 tests)
docs/setup.md documents new flow with copy-paste first-run example 4 (1e4d1c2) docs/setup.md

Test plan

  • cargo build --workspace — clean
  • cargo fmt --all -- --check — clean
  • cargo clippy --all-targets --workspace -- -D warnings — clean
  • cargo test --workspace693 passed (up from 682 before this PR, +11 new tests)
  • Manual smoke test: fresh repo + fake ~/.claude/ only → klasp setupdoctor: all checks passed, agents = ["claude_code"]

Non-goals (not changed)

🤖 Generated with Claude Code

liam-ai-reality and others added 5 commits May 8, 2026 12:39
…ange 1)

Add `adopt/detect_agents.rs` with machine-level agent probing (`~/.claude/`,
`~/.codex/`, `~/.aider*`). Update `writer::write_klasp_toml` to accept an
`agents: Option<&[String]>` param — `Some(list)` writes a narrowed list,
`None` falls through to today's three-agent fallback with an "edit me" comment.
Wire detection into `klasp init --adopt --mode mirror` via `cmd/init.rs` and
add `fs_util::home_dir()` helper. Unit tests cover claude-only, codex-only,
aider-conf, all-three, and fallback scenarios.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
 change 2)

When two `ProposedCheck`s from different gate detectors share the same name
(e.g. both Husky and Lefthook emit a check named "lint"), the second check is
suffixed with its gate type: "lint-husky", "lint-lefthook". The first keeps
the bare name. Collision detection uses a `HashMap<String, usize>` of seen
names; the `gate_suffix()` helper maps `GateType` variants to short strings.
Unit tests cover: collision produces suffix on second, single occurrence stays
bare, and the round-trip TOML parses cleanly after deduplication.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nts (closes #103 change 3)

When the user runs `klasp install --agent <name>` against a klasp.toml whose
`[gate].agents` includes additional agents, emit a WARN to stderr listing the
uncovered agents ("klasp.toml lists agents codex, aider that this install will
NOT cover"). The install itself still exits 0. The check is skipped for
`--agent all` (which already covers everything in the config). Wired as
`warn_if_narrower_than_config` called from `try_run` immediately after the
surface selection resolves.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…#103 change 4)

Add `klasp setup` subcommand that runs the full detect → narrow → write →
install → doctor sequence in order. Behaviour:
  - Non-interactive default: detect gates, detect machine agents, write
    klasp.toml with narrowed [gate].agents, install all declared surfaces,
    run doctor, print green summary or actionable failures.
  - `--interactive`: y/n prompts before write + install; multi-select for gate
    mirroring (answer n = skip gate adoption, write empty-checks config).
  - `--dry-run`: prints plan and computed agents list, writes nothing.

`klasp setup` is additive sugar: `init`, `install`, `doctor` remain unchanged
as scriptable primitives. Force-write (true) makes setup re-runnable on
existing repos.

Adds integration tests in `tests/setup.rs` covering: subcommand exists, dry-
run writes nothing, claude-only → narrowed to claude_code, all-three → all
three, Lefthook fixture, Husky fixture, interactive n (graceful abort),
interactive y/y (writes file), duplicate gate names get suffix, install warns
on narrower config, install-all does not warn.

Adds `docs/setup.md` with copy-paste first-run example, flag reference, agent
detection logic table, and duplicate-name suffix documentation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- dedup run_doctor_inline → reuse cmd::doctor checks (fixes missing error arms)
- dedup print_hook_warning → restore dropped actionable hint lines
- dedup install dispatch → extract install_one_surface
- hoist config load out of install loop (single read)
- fix narrowed_agents_arg dead None branch (restores AC #6 fallback)
- drop narrating module-level doc comments + redundant alias + duplicate step label
- replace stringly-typed agent IDs with AGENT_ID constants
- inline probe_aider iterator (drop Vec alloc)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread klasp/src/cmd/init.rs
None
} else {
Some(detected)
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Suggestion (cosmetic — AC #6 literal text): narrowed_agents_arg's None branch is unreachable from real callers. detect_installed_agents never returns an empty Vec — when home-dir lookup fails OR no probes match, it returns all_agents_fallback() (the 3-element ["claude_code", "codex", "aider"]). So this None arm only fires from the unit test on line 172.

User-visible effect: the no-detection path correctly writes the 3-agent default, but without the # Comment out any you don't use hint that the writer's None branch emits. AC #6 reads literally as "falls through to today's default with the 'edit me' comment" — the comment is missing.

Fix (smallest delta — keep detect_installed_agents returning a Vec but compare against the fallback at the call site):

use crate::adopt::detect_agents::{detect_installed_agents, ALL_AGENTS};

let detected_agents = detect_installed_agents(home.as_deref());
let agents_arg = if detected_agents.iter().map(String::as_str).eq(ALL_AGENTS.iter().copied()) {
    None
} else {
    Some(detected_agents.as_slice())
};

(Requires making ALL_AGENTS pub(crate) if not already.) Or change detect_installed_agents to return Option<Vec<String>> and have callers map None. Either way, also apply in cmd/setup.rs line 115 which has the same dead Some(...) wrap.

Low severity — functional 3-agent fallback works; just the helper comment is lost.

@liam-ai-reality
Copy link
Copy Markdown
Contributor Author

Code review

Summary

Reviewed 5 commits / 1600 LOC across 12 files: 4 feature commits implementing issue #103 (agent-narrowing defaults, duplicate-name suffix, install warn-on-narrower, klasp setup orchestrator + 11 integration tests + docs/setup.md) plus a follow-up simplify pass (b655deb) that deduplicates doctor-check logic, restores dropped actionable hint lines in print_hook_warning, hoists config load out of the install loop, and replaces stringly-typed agent IDs with AGENT_ID constants. Tests 693 → 695, clippy clean.

Critical Issues

File Line Issue
None

Suggestions

File Line Issue
klasp/src/cmd/init.rs 139–145 narrowed_agents_arg's None branch is unreachable from production callers. detect_installed_agents never returns an empty Vec — it falls back to all-three on no-detection. Result: AC #6's literal "falls through to today's default with the 'edit me' comment" is missed (3-agent default writes correctly, but without the # Comment out any you don't use hint from writer's None branch). Fix at the init.rs + setup.rs call sites by comparing against ALL_AGENTS and mapping equal → None. See inline comment for diff. Cosmetic; functional behaviour is correct.

What Looks Good

  • Doctor-check refactor (b655deb) cleanly extracted pub(crate) per-check fns; setup now reuses them, preventing the divergence the simplify-reviewer flagged (missing ConfigVersion/ConfigParse/Io arms).
  • install_one_surface extraction unifies the Codex-vs-non-Codex dispatch path between cmd::install::try_run and cmd::setup::run_install_all — single source of truth for surface dispatch.
  • print_hook_warning consolidation also restored 3 actionable hint lines that the original setup copy silently dropped — a quiet correctness win.
  • 11 integration tests in klasp/tests/setup.rs cover claude-only, all-three-detected, dry-run, interactive Y/N branches, and the install warn-on-narrower scenario. EOF on stdin returns "no" (correctly matched by the "" arm), confirmed by interactive_n_to_mirror_skips_write.
  • Atomic write of klasp.toml reuses the existing fs_util::atomic_write_text (NamedTempFile + persist) — no new TOCTOU surface.
  • All 8 issue klasp setup: one-command first-run flow + sensible install/adopt defaults #103 acceptance criteria are exercised by tests; mapping is auditable in the PR body.

Verdict

🟡 One non-blocking suggestion. The flagged narrowed_agents_arg issue is cosmetic (AC #6 literal-text drift, not functional regression). Safe to merge as-is or address in a 2-line follow-up.


Reviewed by /agentic-flow step 07-code-review on commit b655deb. 4 parallel reviewers (2 CLAUDE.md compliance, 2 bug-scan), 2 validators. Raw findings: 3 (1 stdin-bound CLAUDE.md flag — INVALID per validator; 1 narrowed_agents bug — VALID-low-severity, posted inline; 1 reviewer reported clean). High-signal validated: 1.

…t (AC #6)

`detect_installed_agents` now returns `(Vec<String>, bool)` where the bool
flags whether the result is the all-three fallback (no agents detected).
Callers thread the flag into `narrowed_agents_arg` so the writer's None-arm
fires only on real fallbacks — not when the user genuinely has all three
agents installed.

Closes the AC #6 literal-text drift flagged in PR #104 review:
#104 (comment)

- detect_installed_agents: return (list, fell_back) tuple
- detect_agents tests: assert fell_back flag in all 7 cases
- init.rs::narrowed_agents_arg: take fell_back flag instead of is_empty check
- init.rs tests: 3 cases (fallback, narrowed, user-with-all-three)
- setup.rs: destructure tuple, pass agents_arg=None on fallback, prefix the
  detected-agents print with "(none — falling back to ...)" on fallback

Tests 695 → 696, clippy + fmt clean.
@liam-ai-reality
Copy link
Copy Markdown
Contributor Author

Review remediation

Addressed inline in commit d1e5859.

Suggestion: AC #6 literal-text drift on narrowed_agents_arg None branch

Per discussion_r3208525826 — original narrowed_agents_arg checked detected.is_empty(), but detect_installed_agents always returned a non-empty Vec (3-agent fallback on no-detection), so the None arm was unreachable from production callers and the writer's "Comment out any you don't use" hint never fired in real runs.

Fix: disambiguated at the source rather than at the call site. detect_installed_agents now returns (Vec<String>, bool) where the bool flags fell_back. Callers thread this directly into narrowed_agents_arg(detected, fell_back). Side-effect: a user who genuinely has all three agents installed (~/.claude/ + ~/.codex/ + ~/.aider* all present) is no longer mis-classified as "fallback" — they get the narrowed-list-without-edit-me-comment behaviour they deserve.

Touched:

Tests: 695 → 696 (net +1 — three new init.rs assertions replacing two; remaining detect_agents cases now assert the fell_back flag too). cargo clippy --all-targets --workspace -- -D warnings clean. cargo fmt --all -- --check clean.

No deferred follow-ups. Ready to merge.

@liam-ai-reality liam-ai-reality merged commit fb61d78 into main May 8, 2026
7 checks passed
@liam-ai-reality liam-ai-reality deleted the feat/issue-103-klasp-setup-defaults branch May 8, 2026 13:31
liam-ai-reality added a commit that referenced this pull request May 8, 2026
- README.md: replace 3-command quickstart with `klasp setup`; add v0.4.0
  callout near top; mark #97 + #103 as shipped in feature table; link
  docs/setup.md from quickstart and Documentation sections; keep
  3-command flow as "scriptable / CI" sub-section.
- docs/recipes.md: add `klasp setup` as canonical first-run section
  above per-tool recipes; reference docs/setup.md; update version header
  to include v0.4.
- docs/roadmap.md: update status blurb to 2026-05-08; add v0.4.0 section
  with shipped deliverables for #97 (PR #101) and #103 (PR #104).
- docs/adopt.md: add Agent narrowing section reflecting that [gate].agents
  now defaults to detected agents (not 3-agent default); update example
  session Next: block accordingly; link to docs/setup.md.
- Cargo.toml + npm/pypi manifests: bump all versions to 0.4.0.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant