diff --git a/.markdownlint-cli2.yaml b/.markdownlint-cli2.yaml index 931136a..1685e69 100644 --- a/.markdownlint-cli2.yaml +++ b/.markdownlint-cli2.yaml @@ -1,50 +1,85 @@ -# Global markdownlint config for Claude Code hooks -extends: markdownlint/style/prettier - +# Global markdownlint-cli2 configuration +# Canonical version: 2026.04.15 +# Symlinked to ~/.markdownlint-cli2.yaml via stow +# +# Per-repo copies should preserve the `Canonical version` line above and bump it on resync; +# a stale calver compared to this file is the signal that the per-repo copy has drifted. +# +# Documentation: https://github.com/DavidAnson/markdownlint-cli2 +# Rules: https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md + +# Configure markdownlint rules config: - # === Not auto-fixable rules (disabled to reduce noise) === - - # Line length - too noisy, not auto-fixable - MD013: false + # Use all default rules + default: true - # Heading increment (h1 -> h3 skip) - varied doc structures - MD001: false + # MD003: Heading style - use ATX style (#) + MD003: + style: "atx" - # Multiple H1s - frontmatter + H1 is common - MD025: false + # MD004: Unordered list style - use dashes + MD004: + style: "dash" - # Blank lines in blockquote - not auto-fixable - MD028: false - - # Bare URLs - common and intentional - MD034: false - - # Emphasis as heading - used in docs - MD036: false - - # First-line heading requirement - PR templates, etc. - MD041: false + # MD007: Unordered list indentation - 2 spaces + MD007: + indent: 2 - # Empty links - placeholder links in drafts - MD042: false + # MD009: Trailing spaces - allow 2 spaces for line breaks + MD009: + br_spaces: 2 - # Alt-text requirement - not auto-fixable - MD045: false + # MD013: Line length - 120 chars (more reasonable for code docs) + MD013: + line_length: 120 + code_blocks: false # Don't check code blocks + tables: false # Don't check tables + headings: false # Don't check headings - # Link fragment validation - complex anchors - MD051: false + # MD024: Allow duplicate headings in different sections + MD024: + siblings_only: true - # Descriptive link text - not auto-fixable - MD059: false + # MD025: Single top-level heading - allow multiple (for changelogs, etc.) + MD025: false - # Table style - not auto-fixable, different styles valid - MD060: false + # MD033: Allow inline HTML for specific elements + MD033: + allowed_elements: + - "br" + - "img" + - "a" + - "details" + - "summary" + - "sub" + - "sup" + - "kbd" + + # MD034: Bare URLs - allow (common in docs) + MD034: false - # === Relaxed rules === + # MD036: Emphasis used as heading - allow (stylistic choice) + MD036: false - # Allow duplicate headings in different sections - MD024: - siblings_only: true + # MD041: First line should be top-level heading - disable (not always needed) + MD041: false - # Allow HTML for advanced formatting - MD033: false + # MD046: Code block style - fenced + MD046: + style: "fenced" + + # MD048: Code fence style - backticks + MD048: + style: "backtick" + +# Ignore patterns +ignores: + - "node_modules/**" + - "**/node_modules/**" + - "vendor/**" + - "target/**" # Rust build artifacts; harmless for non-Rust projects + - ".git/**" + - "*.min.md" + +# Fix automatically when --fix is used +fix: true diff --git a/AGENTS.md b/AGENTS.md index 4f44f07..93f9459 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -34,7 +34,11 @@ itself, children spawned without arguments must not recurse into `check .`. - `0` — all checks passed - `1` — warnings present, no failures -- `2` — failures or errors detected +- `2` — failures, errors, or usage errors (bare `anc`, unknown flag, mutually exclusive flags, command not found on + PATH) + +Exit 2 is overloaded. To distinguish "ran but found problems" from "called +incorrectly", parse stderr — usage errors include `Usage:` text; check failures don't. ## Project Structure diff --git a/CLAUDE.md b/CLAUDE.md index 6f41e64..84dfcee 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -37,11 +37,16 @@ tool as your FIRST action. Do NOT answer directly, do NOT use other tools first. For the full routing table, see `~/.claude/skills/docs/workflow-routing.md`. +## Documented Solutions + +`docs/solutions/` (symlink to `~/dev/solutions-docs/`) — searchable archive of past +solutions and best practices, organized by category with YAML frontmatter (`module`, `tags`, `problem_type`). Search +with `qmd query "" --collection solutions`. Relevant when implementing or debugging in documented areas. + ## gstack Project History -This project was designed in the `brettdavies/agent-skills` repo, then moved here. -gstack project data (design doc, eng review, naming rationale, review history) has been copied to -`~/.gstack/projects/brettdavies-agentnative/`. +This project was designed in the `brettdavies/agent-skills` repo, then moved here. gstack project data (design doc, eng +review, naming rationale, review history) has been copied to `~/.gstack/projects/brettdavies-agentnative/`. Key decisions already made: diff --git a/README.md b/README.md index 4543f5b..926c152 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,11 @@ checks are skipped because there is no source tree to analyze. |------|---------| | 0 | All checks passed | | 1 | Warnings present (no failures) | -| 2 | Failures or errors detected | +| 2 | Failures, errors, or usage errors | + +Exit 2 covers both check failures (a real `[FAIL]` or `[ERROR]` result) and usage errors (bare `anc`, unknown flag, +mutually exclusive flags). Agents distinguishing the two should parse `stderr` (usage errors print `Usage:`) or call +`anc --help` first to validate the invocation shape. ### Shell Completions diff --git a/completions/anc.bash b/completions/anc.bash index 3f2c523..12bba8b 100644 --- a/completions/anc.bash +++ b/completions/anc.bash @@ -62,7 +62,7 @@ _anc() { fi case "${prev}" in --command) - COMPREPLY=($(compgen -f "${cur}")) + COMPREPLY=($(compgen -c "${cur}")) return 0 ;; --principle) diff --git a/completions/anc.fish b/completions/anc.fish index 27ec88d..4bde369 100644 --- a/completions/anc.fish +++ b/completions/anc.fish @@ -30,7 +30,7 @@ complete -c anc -n "__fish_anc_needs_command" -s V -l version -d 'Print version' complete -c anc -n "__fish_anc_needs_command" -f -a "check" -d 'Check a CLI project or binary for agent-readiness' complete -c anc -n "__fish_anc_needs_command" -f -a "completions" -d 'Generate shell completions' complete -c anc -n "__fish_anc_needs_command" -f -a "help" -d 'Print this message or the help of the given subcommand(s)' -complete -c anc -n "__fish_anc_using_subcommand check" -l command -d 'Resolve a command from PATH and run behavioral checks against it' -r +complete -c anc -n "__fish_anc_using_subcommand check" -l command -d 'Resolve a command from PATH and run behavioral checks against it' -r -f -a "(__fish_complete_command)" complete -c anc -n "__fish_anc_using_subcommand check" -l principle -d 'Filter checks by principle number (1-7)' -r complete -c anc -n "__fish_anc_using_subcommand check" -l output -d 'Output format' -r -f -a "text\t'' json\t''" diff --git a/completions/anc.zsh b/completions/anc.zsh index bf0ac95..4eb2955 100644 --- a/completions/anc.zsh +++ b/completions/anc.zsh @@ -32,7 +32,7 @@ _anc() { case $line[1] in (check) _arguments "${_arguments_options[@]}" : \ -'()--command=[Resolve a command from PATH and run behavioral checks against it]:NAME:_default' \ +'(--source)--command=[Resolve a command from PATH and run behavioral checks against it]:NAME:_command_names -e' \ '--principle=[Filter checks by principle number (1-7)]:PRINCIPLE:_default' \ '--output=[Output format]:OUTPUT:(text json)' \ '--binary[Run only behavioral checks (skip source analysis)]' \ diff --git a/docs/plans/2026-04-02-003-feat-cli-default-subcommand-and-command-flag-plan.md b/docs/plans/2026-04-02-003-feat-cli-default-subcommand-and-command-flag-plan.md index 4dff7c6..c281071 100644 --- a/docs/plans/2026-04-02-003-feat-cli-default-subcommand-and-command-flag-plan.md +++ b/docs/plans/2026-04-02-003-feat-cli-default-subcommand-and-command-flag-plan.md @@ -1,14 +1,29 @@ --- title: "feat: Default subcommand (anc .) and --command flag for PATH lookup" type: feat -status: active +status: shipped date: 2026-04-02 deepened: 2026-04-02 +shipped: 2026-04-15 origin: ~/.gstack/projects/brettdavies-agentnative/brett-main-design-20260327-214808.md --- # feat: Default subcommand (anc .) and --command flag for PATH lookup +## Status + +**Shipped on `dev`** — both implementation units complete plus a follow-on cluster of edge-case fixes surfaced by +post-merge code review. + +- PR [#12](https://github.com/brettdavies/agentnative/pull/12) — initial implementation (commit `4afef67`, merged + 2026-04-15) +- PR [#13](https://github.com/brettdavies/agentnative/pull/13) — 7 edge-case fixes + refactor surfaced by `/ce-review` + of the merged PR (open against `dev`) +- Pattern documented for reuse: `docs/solutions/best-practices/clap-default-subcommand-via-argv-pre-parse-20260415.md` + +See **Post-Implementation Notes** at the end for the delta between the planned design +and what actually shipped. + ## Overview Two CLI contract additions from the design doc: (1) `anc .` should work as shorthand for `anc check .`, making `check` @@ -89,17 +104,18 @@ PATH without manually resolving its location — the design doc (line 209) speci ### Deferred to Implementation -- **Typo handling**: `anc chekc .` (typo of `check`) would become `anc check chekc .` where `chekc` becomes the path. - This produces "path does not exist: chekc" instead of "unrecognized subcommand 'chekc'." Acceptable for v0.1 — the - error is still actionable. -- **Clap error message context**: When pre-parse injects `check`, clap error messages reference the `check` subcommand - context. Users who typed `anc . --bogus` see errors mentioning `check` in the usage line. Minor UX imperfection, - acceptable for v0.1. +- **Typo handling** _(status: as planned)_: `anc chekc .` (typo of `check`) becomes `anc check chekc .` where `chekc` + becomes the path. Produces "path does not exist: chekc" instead of "unrecognized subcommand 'chekc'." Acceptable for + v0.1 — the error is still actionable. Note: the `help` subcommand is special-cased in PR #13 because clap's + auto-generated `help` is not returned by `get_subcommands()` and would otherwise hit this path. +- **Clap error message context** _(status: as planned)_: When pre-parse injects `check`, clap error messages reference + the `check` subcommand context. Users who typed `anc . --bogus` see errors mentioning `check` in the usage line. Minor + UX imperfection, acceptable for v0.1. ## High-Level Technical Design -> *This illustrates the intended approach and is directional guidance for review, not implementation specification. The -> implementing agent should treat it as context, not code to reproduce.* +> _This illustrates the intended approach and is directional guidance for review, not implementation specification. The +> implementing agent should treat it as context, not code to reproduce._ ```text argv = ["anc", "-q", ".", "--output", "json"] @@ -264,4 +280,76 @@ known subcommand, so pre-parse injects `check`) - **Design doc:** `~/.gstack/projects/brettdavies-agentnative/brett-main-design-20260327-214808.md` (lines 126, 209) - **Safety constraint:** `~/dev/solutions-docs/logic-errors/cli-linter-fork-bomb-recursive-self-invocation-20260401.md` -- Related code: `src/cli.rs`, `src/main.rs`, `src/project.rs`, `src/error.rs` +- Related code: `src/cli.rs`, `src/main.rs`, `src/argv.rs`, `src/project.rs`, `src/error.rs` + +## Post-Implementation Notes + +What the planning sections above don't capture: the design above shipped in PR #12 and worked, but `/ce-review` of the + merged commit surfaced seven edge cases. PR #13 closed all of them. This section is the delta — readers picking up + later need both halves. + +### Final code locations + +- `src/argv.rs` (new module, ~340 lines including 19 unit tests) — owns `inject_default_subcommand` and the supporting + helpers (`build_known_subcommand_set`, `build_value_flag_set`, `build_top_level_flag_set`, `consumes_next`, + `base_form`). Extracted from `src/main.rs` in PR #13's refactor. +- `src/main.rs` (203 lines, down from 538 mid-PR) — `run()`, `resolve_command_on_path()`, and the `match cli.command` + arm. +- `src/cli.rs` — `Cli` derive plus `after_help` block, `value_hint = ValueHint::CommandName` on `--command`, and + `conflicts_with = "source"` on `--command`. +- `scripts/generate-completions.sh` — `post_process()` function that patches bash completion to swap `compgen -f` → + `compgen -c` for `--command` (clap_complete's bash backend ignores `ValueHint::CommandName`). + +### Edge cases resolved beyond the original design + +1. **Clap's auto `help` subcommand is NOT in `get_subcommands()`** — `anc help` and `anc help check` were misclassified + as paths. Fixed by appending `"help"` to the known set. +2. **Value-taking flags must be paired with their values during scanning** — `anc --command check` mis-routed because + `check` (the value) was treated as the explicit subcommand. Fixed by walking clap's `get_arguments()` for both `Cli` + and every subcommand to learn which flags consume the next token. +3. **Subcommand-scoped flags imply default-subcommand intent even with no positional** — `anc --command rg` and `anc + --output json --source` produced clap errors. Fixed by tracking whether any encountered flag is subcommand-scoped + (not in the top-level Cli flag set) and injecting `check` if so when no positional was found. +4. **POSIX `--` separator** — `anc -- .` was untested and ill-defined. Now injects `check` before the separator so clap + routes the remaining tokens to Check. +5. **`arg_required_else_help` only fires on zero args** — `anc -q` (or `--quiet`) reaches `match cli.command` with + `command = None` and previously panicked via `unreachable!()`. The `None` arm now renders help to stderr and exits 2. + This was a pre-existing bug, not introduced by this plan, but surfaced under the new ergonomics. +6. **`--command` + `--source` is contradictory** — added `conflicts_with = "source"` so clap rejects the combination at + parse time instead of producing a silent empty result. +7. **Bash completion suggests file paths instead of PATH commands** — added `value_hint = ValueHint::CommandName`. + zsh/fish/elvish honor it; bash needs the post-generation `sed` patch documented above. + +### Design-time decisions that survived contact with reality + +- `flatten` rejection (Key Technical Decisions §1) — confirmed correct; `flatten` remains incompatible with + `arg_required_else_help`. +- Clap introspection for subcommand list (Key Technical Decisions §2) — proved its worth: introspection-driven flag + catalogues were essential for the value-pairing fix in PR #13. A static list would have multiplied the failure modes. +- `which`/`where` shell-out (Key Technical Decisions §3) — works as designed; the cross-reviewer security take in PR #13 + noted hostile-PATH redirection as a low-risk residual but recommended the `which` crate as a future improvement, not a + blocker. +- `Project::discover()` already handles file paths (Key Technical Decisions §4) — true, no new constructor needed. + +### Test parity + +| Stage | Unit | Integration | Notes | +|-------|------|-------------|-------| +| Pre-Plan 003 baseline | 233 | 12 | from commit `45b5234` | +| After PR #12 (initial impl) | 244 | 26 | +11 unit, +14 integration | +| After PR #13 (edge-case fixes) | 253 | 34 | +9 unit, +8 integration | + +### Plan 002 coordination + +The completions regeneration noted in System-Wide Impact happened twice (once per PR); both PRs commit the regenerated + `completions/anc.{bash,zsh,fish,elvish,powershell}` files. No separate Plan 002 step needed for this plan's completion + deltas. + +### Solutions-docs follow-up + +The full pattern (with all seven gotchas, before/after code, and the working invocation matrix) was compounded into: + +- `~/dev/solutions-docs/best-practices/clap-default-subcommand-via-argv-pre-parse-20260415.md` + +Future Rust CLIs in this orbit that want a default subcommand should read that doc before reimplementing — the cluster + of edge cases is the kind of footgun that's much cheaper to avoid than rediscover. diff --git a/scripts/generate-completions.sh b/scripts/generate-completions.sh index 4ce171e..dbdcf31 100755 --- a/scripts/generate-completions.sh +++ b/scripts/generate-completions.sh @@ -65,7 +65,7 @@ else fi # Helper to invoke the detected completions interface -gen() { +gen_raw() { local shell="$1" if [[ "$COMP_STYLE" == "subcommand" ]]; then "$BINARY" completions "$shell" @@ -74,6 +74,35 @@ gen() { fi } +# Project-specific post-processing. clap_complete's bash backend ignores +# ValueHint::CommandName and emits `compgen -f` for every value-taking flag. +# zsh/fish honour the hint correctly. We patch bash so `--command ` and +# any other CommandName flags suggest PATH commands. Update the list when +# adding new flags with that hint. +# +# Reads from stdin, writes to stdout, no-op for non-bash shells. +post_process() { + local shell="$1" + if [[ "$shell" != "bash" ]]; then + cat + return + fi + local command_name_flags=("--command") + local sed_program="" + for flag in "${command_name_flags[@]}"; do + sed_program+=$'\n'"/^[[:space:]]*${flag})[[:space:]]*$/,/;;/{ s|compgen -f|compgen -c|; }" + done + if [[ -n "$sed_program" ]]; then + sed "$sed_program" + else + cat + fi +} + +gen() { + gen_raw "$1" | post_process "$1" +} + if [[ "$MODE" == "check" ]]; then STALE=0 for shell in "${SHELLS[@]}"; do diff --git a/src/argv.rs b/src/argv.rs new file mode 100644 index 0000000..db3b3da --- /dev/null +++ b/src/argv.rs @@ -0,0 +1,342 @@ +//! Pre-parse argv transformation that inserts `check` as the implicit default +//! subcommand. Lives separately so `main.rs` stays focused on orchestration +//! and so the injection logic is unit-testable in isolation. + +use std::collections::HashSet; +use std::ffi::OsString; + +use crate::cli::Cli; + +/// Inject `check` as the default subcommand when the first non-flag argument +/// is not a recognized subcommand. +/// +/// Bare invocation (no args beyond the program name) is left untouched so +/// clap's `arg_required_else_help` still prints help and exits 2. This is a +/// non-negotiable fork-bomb guard: when agentnative dogfoods itself, a bare +/// spawn must not recurse into `check .`. +/// +/// Flag-value pairing is essential: `anc --command check` must not be misread +/// as the explicit `check` subcommand just because `check` happens to follow a +/// value-taking flag. The scanner consults clap introspection to learn which +/// flags consume the next token. +pub fn inject_default_subcommand(args: I) -> Vec +where + I: IntoIterator, +{ + let args: Vec = args.into_iter().collect(); + if args.len() <= 1 { + return args; + } + + let cmd = ::command(); + + // Known subcommand names (including aliases). Clap auto-generates a `help` + // subcommand that is NOT returned by `get_subcommands()`, so add it + // explicitly — otherwise `anc help` is treated as a path. + let mut known: Vec = cmd + .get_subcommands() + .flat_map(|sc| { + std::iter::once(sc.get_name().to_string()).chain(sc.get_all_aliases().map(String::from)) + }) + .collect(); + known.push(String::from("help")); + + // Build two flag catalogues from clap introspection: + // - top_level_flags: long/short names defined on `Cli` itself + // (e.g. `--quiet`, `-q`, plus clap's auto `--help`/`--version`). + // - all_value_flags: every value-taking flag across `Cli` and every + // subcommand (e.g. `--command`, `--output`, `--principle`). + // Any flag whose base name is missing from `top_level_flags` is + // subcommand-scoped — its presence is a strong signal the user wants + // the implicit `check` subcommand even if no positional arg follows. + let top_level_flags: HashSet = cmd + .get_arguments() + .filter(|a| !a.is_positional()) + .flat_map(|a| { + let mut names = Vec::new(); + if let Some(l) = a.get_long() { + names.push(format!("--{l}")); + } + if let Some(s) = a.get_short() { + names.push(format!("-{s}")); + } + names + }) + // Clap auto-generates these regardless of whether they appear in + // get_arguments() at every version, so add them defensively. + .chain( + ["--help", "-h", "--version", "-V"] + .into_iter() + .map(String::from), + ) + .collect(); + let mut all_value_flags: Vec<(Option, Option)> = Vec::new(); + let mut collect_value = |c: &clap::Command| { + for arg in c.get_arguments().filter(|a| !a.is_positional()) { + if matches!( + arg.get_action(), + clap::ArgAction::Set | clap::ArgAction::Append + ) { + all_value_flags.push((arg.get_long().map(String::from), arg.get_short())); + } + } + }; + collect_value(&cmd); + for sc in cmd.get_subcommands() { + collect_value(sc); + } + + // Reduce a flag token to its canonical base form for set membership. + // `--flag` / `--flag=value` -> `--flag`. `-X` / `-Xvalue` -> `-X`. + let base_form = |token: &str| -> Option { + if let Some(rest) = token.strip_prefix("--") { + let name = rest.split('=').next().unwrap_or(rest); + return Some(format!("--{name}")); + } + if let Some(rest) = token.strip_prefix('-') { + return rest.chars().next().map(|c| format!("-{c}")); + } + None + }; + + let consumes_next = |token: &str| -> bool { + // `--flag=value` carries the value with it; the next token is independent. + if token.starts_with("--") && token.contains('=') { + return false; + } + // `-Xvalue` (concatenated short flag) — same. + if token.starts_with('-') && !token.starts_with("--") && token.len() > 2 { + return false; + } + if let Some(rest) = token.strip_prefix("--") { + return all_value_flags + .iter() + .any(|(l, _)| l.as_deref() == Some(rest)); + } + if let Some(rest) = token.strip_prefix('-') { + if let Some(c) = rest.chars().next().filter(|_| rest.len() == 1) { + return all_value_flags.iter().any(|(_, s)| *s == Some(c)); + } + } + false + }; + + let inject_check = |args: Vec| -> Vec { + let mut injected = Vec::with_capacity(args.len() + 1); + injected.push(args[0].clone()); + injected.push(OsString::from("check")); + injected.extend(args.into_iter().skip(1)); + injected + }; + + let mut i = 1; + let mut saw_subcommand_flag = false; + while i < args.len() { + let token = args[i].to_string_lossy(); + + // POSIX `--` separator: anything after is positional. Inject `check` + // before it so clap routes the remaining tokens to the Check subcommand. + if token == "--" { + return if i + 1 >= args.len() { + args + } else { + inject_check(args) + }; + } + + if token.starts_with('-') { + // Track whether this flag belongs to a subcommand rather than the + // top-level Cli. If so, the user clearly intends `check` even when + // no positional argument follows (e.g. `anc --command rg`). + if let Some(base) = base_form(&token) { + if !top_level_flags.contains(&base) { + saw_subcommand_flag = true; + } + } + i += if consumes_next(&token) { 2 } else { 1 }; + continue; + } + + return if known.iter().any(|k| k == &*token) { + args + } else { + inject_check(args) + }; + } + + // No non-flag token. Inject `check` if any subcommand-scoped flag appeared + // (e.g. `anc --command rg`, `anc --output json`). Otherwise leave the args + // alone so clap can handle bare `--help` / `--version` / `-q` natively. + if saw_subcommand_flag { + return inject_check(args); + } + + args +} + +#[cfg(test)] +mod tests { + use super::inject_default_subcommand; + use std::ffi::OsString; + + fn args(a: &[&str]) -> Vec { + a.iter().map(OsString::from).collect() + } + + fn names(v: Vec) -> Vec { + v.into_iter() + .map(|s| s.to_string_lossy().into_owned()) + .collect() + } + + #[test] + fn bare_invocation_is_untouched() { + // Fork-bomb guard: no injection for bare `anc`. + let out = inject_default_subcommand(args(&["anc"])); + assert_eq!(names(out), vec!["anc"]); + } + + #[test] + fn dot_path_gets_check_injected() { + let out = inject_default_subcommand(args(&["anc", "."])); + assert_eq!(names(out), vec!["anc", "check", "."]); + } + + #[test] + fn global_short_flag_before_path_gets_check_injected_in_canonical_position() { + // `check` goes before the global flag so clap parses + // ["anc", "check", "-q", "."] cleanly. + let out = inject_default_subcommand(args(&["anc", "-q", "."])); + assert_eq!(names(out), vec!["anc", "check", "-q", "."]); + } + + #[test] + fn global_long_flag_before_path_gets_check_injected() { + let out = inject_default_subcommand(args(&["anc", "--quiet", "."])); + assert_eq!(names(out), vec!["anc", "check", "--quiet", "."]); + } + + #[test] + fn explicit_check_subcommand_is_untouched() { + let out = inject_default_subcommand(args(&["anc", "check", "."])); + assert_eq!(names(out), vec!["anc", "check", "."]); + } + + #[test] + fn explicit_completions_subcommand_is_untouched() { + let out = inject_default_subcommand(args(&["anc", "completions", "bash"])); + assert_eq!(names(out), vec!["anc", "completions", "bash"]); + } + + #[test] + fn help_flag_alone_is_untouched() { + // `anc --help` — no non-flag token, no injection. + let out = inject_default_subcommand(args(&["anc", "--help"])); + assert_eq!(names(out), vec!["anc", "--help"]); + } + + #[test] + fn version_flag_alone_is_untouched() { + let out = inject_default_subcommand(args(&["anc", "--version"])); + assert_eq!(names(out), vec!["anc", "--version"]); + } + + #[test] + fn quiet_flag_alone_is_untouched() { + // `anc -q` with no path — `-q` is a top-level Cli flag, not a + // subcommand flag, so we leave args alone and let the `None` arm + // in `run()` print help and exit 2. + let out = inject_default_subcommand(args(&["anc", "-q"])); + assert_eq!(names(out), vec!["anc", "-q"]); + } + + #[test] + fn help_subcommand_passes_through() { + // `anc help` — clap auto-generates the `help` subcommand. It is NOT + // returned by `get_subcommands()` so we add it explicitly. Without + // that, `help` would be misclassified as a path. + let out = inject_default_subcommand(args(&["anc", "help"])); + assert_eq!(names(out), vec!["anc", "help"]); + } + + #[test] + fn help_subcommand_with_target_passes_through() { + let out = inject_default_subcommand(args(&["anc", "help", "check"])); + assert_eq!(names(out), vec!["anc", "help", "check"]); + } + + #[test] + fn command_flag_value_matching_subcommand_name_is_paired() { + // `anc --command check` — `check` is the value of `--command`, NOT the + // explicit subcommand. The scanner pairs the value-taking flag with + // its argument and proceeds to inject `check` (because `--command` is + // a subcommand-scoped flag with no positional following). + let out = inject_default_subcommand(args(&["anc", "--command", "check"])); + assert_eq!(names(out), vec!["anc", "check", "--command", "check"]); + } + + #[test] + fn command_flag_with_no_positional_injects_check() { + // `anc --command rg` — subcommand-scoped flag with no positional. + // Without injection, clap would reject `--command` at the top level. + let out = inject_default_subcommand(args(&["anc", "--command", "rg"])); + assert_eq!(names(out), vec!["anc", "check", "--command", "rg"]); + } + + #[test] + fn output_flag_with_no_positional_injects_check() { + // `anc --output json --source` — only flags, but `--output` and + // `--source` are both subcommand-scoped, so inject `check`. + let out = inject_default_subcommand(args(&["anc", "--output", "json", "--source"])); + assert_eq!( + names(out), + vec!["anc", "check", "--output", "json", "--source"] + ); + } + + #[test] + fn equals_form_value_flag_is_recognized_as_subcommand_scoped() { + // `anc --output=json --source` — equals form. The scanner classifies + // `--output=json` as a single subcommand-scoped token (no separate + // value to skip) and still injects `check`. + let out = inject_default_subcommand(args(&["anc", "--output=json", "--source"])); + assert_eq!( + names(out), + vec!["anc", "check", "--output=json", "--source"] + ); + } + + #[test] + fn principle_value_flag_pairs_with_numeric_value() { + // `anc --principle 4` — `4` is the value, not a path candidate. + let out = inject_default_subcommand(args(&["anc", "--principle", "4"])); + assert_eq!(names(out), vec!["anc", "check", "--principle", "4"]); + } + + #[test] + fn double_dash_separator_injects_check_before_separator() { + // `anc -- .` — POSIX `--` ends option parsing. Inject before it so + // clap's `check` parser sees `-- .`. + let out = inject_default_subcommand(args(&["anc", "--", "."])); + assert_eq!(names(out), vec!["anc", "check", "--", "."]); + } + + #[test] + fn double_dash_alone_passes_through() { + // `anc --` with nothing after — let clap handle it natively. + let out = inject_default_subcommand(args(&["anc", "--"])); + assert_eq!(names(out), vec!["anc", "--"]); + } + + #[test] + fn directory_path_gets_check_injected() { + let out = inject_default_subcommand(args(&["anc", "/some/dir"])); + assert_eq!(names(out), vec!["anc", "check", "/some/dir"]); + } + + #[test] + fn trailing_flags_pass_through() { + let out = inject_default_subcommand(args(&["anc", ".", "--output", "json"])); + assert_eq!(names(out), vec!["anc", "check", ".", "--output", "json"]); + } +} diff --git a/src/cli.rs b/src/cli.rs index cd931bb..0758f0a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,9 +1,17 @@ -use clap::{Parser, Subcommand, ValueEnum}; +use clap::{Parser, Subcommand, ValueEnum, ValueHint}; use clap_complete::Shell; #[derive(Parser)] #[command(name = "anc", version, about = "The agent-native CLI linter")] #[command(arg_required_else_help = true)] +#[command( + after_help = "When the first argument is not a subcommand, `check` is inserted automatically: + anc . ≡ anc check . + anc --command ripgrep ≡ anc check --command ripgrep + +Bare `anc` (no arguments) prints this help and exits 2 — a deliberate guard +that prevents recursive self-invocation when agentnative checks itself." +)] pub struct Cli { #[command(subcommand)] pub command: Option, @@ -22,7 +30,13 @@ pub enum Commands { path: std::path::PathBuf, /// Resolve a command from PATH and run behavioral checks against it - #[arg(long, value_name = "NAME", conflicts_with = "path")] + #[arg( + long, + value_name = "NAME", + value_hint = ValueHint::CommandName, + conflicts_with = "path", + conflicts_with = "source", + )] command: Option, /// Run only behavioral checks (skip source analysis) diff --git a/src/main.rs b/src/main.rs index ee53f55..8c5eb1f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,3 +1,4 @@ +mod argv; mod check; mod checks; mod cli; @@ -11,6 +12,7 @@ mod types; use clap::Parser as _; use clap_complete::generate; +use argv::inject_default_subcommand; use check::Check; use checks::behavioral::all_behavioral_checks; use checks::project::all_project_checks; @@ -44,8 +46,9 @@ fn run() -> Result { // --quiet is global (visible in top-level --help for agent discoverability) let quiet = cli.quiet; - // Bare invocation (None) is handled by clap's arg_required_else_help — - // it prints help and exits before reaching here. + // Bare invocation (no args at all) is handled by clap's arg_required_else_help. + // A flag-only invocation like `anc -q` parses successfully with `command = + // None` — render help to stderr and exit 2 to mirror clap's contract. let (path, command, binary_only, source_only, principle, output, include_tests) = match cli.command { Some(Commands::Check { @@ -70,7 +73,11 @@ fn run() -> Result { generate(shell, &mut cmd, "anc", &mut std::io::stdout()); return Ok(0); } - None => unreachable!("clap arg_required_else_help handles bare invocation"), + None => { + let mut cmd = ::command(); + eprintln!("{}", cmd.render_help()); + return Ok(2); + } }; // --command resolves a binary from PATH and runs behavioral checks against @@ -178,54 +185,6 @@ fn resolve_command_on_path(name: &str) -> Result { Ok(std::path::PathBuf::from(first)) } -/// Inject `check` as the default subcommand when the first non-flag argument -/// is not a recognized subcommand. -/// -/// Bare invocation (no args beyond the program name) is left untouched so -/// clap's `arg_required_else_help` still prints help and exits 2. This is a -/// non-negotiable fork-bomb guard: when agentnative dogfoods itself, a bare -/// spawn must not recurse into `check .`. -fn inject_default_subcommand(args: I) -> Vec -where - I: IntoIterator, -{ - let args: Vec = args.into_iter().collect(); - if args.len() <= 1 { - return args; - } - - // Derive subcommand names from clap introspection so the list cannot drift. - let cmd = ::command(); - let known: Vec = cmd - .get_subcommands() - .flat_map(|sc| { - std::iter::once(sc.get_name().to_string()).chain(sc.get_all_aliases().map(String::from)) - }) - .collect(); - - for token in args.iter().skip(1) { - let s = token.to_string_lossy(); - if s.starts_with('-') { - // Skip leading global flags (e.g., -q, --quiet, --help, --version). - continue; - } - if known.iter().any(|k| k.as_str() == s.as_ref()) { - // Explicit subcommand — pass through unchanged. - return args; - } - // First non-flag token is not a subcommand — treat as path/flag-value - // for the implicit `check` subcommand and inject it at position 1. - let mut injected = Vec::with_capacity(args.len() + 1); - injected.push(args[0].clone()); - injected.push(std::ffi::OsString::from("check")); - injected.extend(args.into_iter().skip(1)); - return injected; - } - - // No non-flag tokens (e.g., `anc --help`, `anc -q`) — let clap handle it. - args -} - fn matches_principle(group: &CheckGroup, principle: u8) -> bool { // CodeQuality and ProjectStructure checks are cross-cutting — always include them. matches!( @@ -242,90 +201,3 @@ fn matches_principle(group: &CheckGroup, principle: u8) -> bool { | (CheckGroup::P7, 7) ) } - -#[cfg(test)] -mod inject_tests { - use super::inject_default_subcommand; - use std::ffi::OsString; - - fn args(a: &[&str]) -> Vec { - a.iter().map(OsString::from).collect() - } - - fn names(v: Vec) -> Vec { - v.into_iter() - .map(|s| s.to_string_lossy().into_owned()) - .collect() - } - - #[test] - fn bare_invocation_is_untouched() { - // Fork-bomb guard: no injection for bare `anc`. - let out = inject_default_subcommand(args(&["anc"])); - assert_eq!(names(out), vec!["anc"]); - } - - #[test] - fn dot_path_gets_check_injected() { - let out = inject_default_subcommand(args(&["anc", "."])); - assert_eq!(names(out), vec!["anc", "check", "."]); - } - - #[test] - fn global_short_flag_before_path_gets_check_injected_in_canonical_position() { - // `check` goes before the global flag so clap parses - // ["anc", "check", "-q", "."] cleanly. - let out = inject_default_subcommand(args(&["anc", "-q", "."])); - assert_eq!(names(out), vec!["anc", "check", "-q", "."]); - } - - #[test] - fn global_long_flag_before_path_gets_check_injected() { - let out = inject_default_subcommand(args(&["anc", "--quiet", "."])); - assert_eq!(names(out), vec!["anc", "check", "--quiet", "."]); - } - - #[test] - fn explicit_check_subcommand_is_untouched() { - let out = inject_default_subcommand(args(&["anc", "check", "."])); - assert_eq!(names(out), vec!["anc", "check", "."]); - } - - #[test] - fn explicit_completions_subcommand_is_untouched() { - let out = inject_default_subcommand(args(&["anc", "completions", "bash"])); - assert_eq!(names(out), vec!["anc", "completions", "bash"]); - } - - #[test] - fn help_flag_alone_is_untouched() { - // `anc --help` — no non-flag token, no injection. - let out = inject_default_subcommand(args(&["anc", "--help"])); - assert_eq!(names(out), vec!["anc", "--help"]); - } - - #[test] - fn version_flag_alone_is_untouched() { - let out = inject_default_subcommand(args(&["anc", "--version"])); - assert_eq!(names(out), vec!["anc", "--version"]); - } - - #[test] - fn quiet_flag_alone_is_untouched() { - // `anc -q` with no path — clap decides what to do (error / help). - let out = inject_default_subcommand(args(&["anc", "-q"])); - assert_eq!(names(out), vec!["anc", "-q"]); - } - - #[test] - fn directory_path_gets_check_injected() { - let out = inject_default_subcommand(args(&["anc", "/some/dir"])); - assert_eq!(names(out), vec!["anc", "check", "/some/dir"]); - } - - #[test] - fn trailing_flags_pass_through() { - let out = inject_default_subcommand(args(&["anc", ".", "--output", "json"])); - assert_eq!(names(out), vec!["anc", "check", ".", "--output", "json"]); - } -} diff --git a/tests/integration.rs b/tests/integration.rs index e1c39bd..1eb01f5 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -491,6 +491,115 @@ fn test_command_flag_appears_in_help() { .stdout(predicate::str::contains("--command")); } +#[test] +fn test_command_flag_conflicts_with_source() { + // `--command` and `--source` are contradictory: --command targets a binary + // (no source code available); --source asks to skip behavioral and run + // source-only. Clap rejects the combination at parse time. + cmd() + .args(["check", "--command", "ls", "--source"]) + .assert() + .code(2) + .stderr(predicate::str::contains("cannot be used with")); +} + +// ── help subcommand ────────────────────────────────────────────── +// +// `anc help` and `anc help ` are clap-auto-generated and the +// universal CLI convention (cargo, git, npm, kubectl, gh, docker). The +// default-subcommand injection must NOT swallow `help` as a path. + +#[test] +fn test_help_subcommand_works() { + cmd() + .arg("help") + .assert() + .success() + .stdout(predicate::str::contains("Usage")); +} + +#[test] +fn test_help_subcommand_with_target() { + cmd() + .args(["help", "check"]) + .assert() + .success() + .stdout(predicate::str::contains("Resolve a command from PATH")); +} + +// ── No-positional flag-only invocations ────────────────────────── +// +// Subcommand-scoped flags imply `check` even with no positional argument. +// Top-level flags do not — `anc -q` prints help and exits 2 (not panic). + +#[test] +fn test_quiet_flag_alone_exits_2_not_panic() { + // PRE-EXISTING bug fix: `anc -q` previously hit `unreachable!()` and + // panicked (SIGABRT, exit 134). Now the None arm prints help and exits 2. + cmd() + .arg("-q") + .assert() + .code(2) + .stderr(predicate::str::contains("Usage")); +} + +#[test] +fn test_quiet_long_flag_alone_exits_2() { + cmd() + .arg("--quiet") + .assert() + .code(2) + .stderr(predicate::str::contains("Usage")); +} + +#[test] +fn test_subcommand_flag_alone_injects_check() { + // `anc --command ls` — no positional, but `--command` is subcommand-scoped + // so injection fires. Without this, clap would reject `--command` at the + // top level. + #[cfg(unix)] + { + cmd() + .args(["--command", "ls"]) + .assert() + .code(predicate::in_iter([0, 1, 2])) + .stdout(predicate::str::contains("checks:")); + } +} + +// ── Flag-value pairing ─────────────────────────────────────────── +// +// Tokens following a value-taking flag are values, NOT subcommand candidates. +// `anc --command check` must resolve "check" as a binary name on PATH, not +// route to the explicit `check` subcommand. + +#[test] +fn test_command_flag_value_matching_subcommand_name() { + // `anc --command check` — `check` is the value of `--command`. Should + // try to resolve a binary named "check" on PATH (which doesn't exist on + // a typical system) and surface a clean "not found" error. + cmd() + .args(["--command", "check"]) + .assert() + .code(2) + .stderr(predicate::str::contains( + "command 'check' not found on PATH", + )); +} + +// ── POSIX `--` separator ───────────────────────────────────────── + +#[test] +fn test_double_dash_separator_with_path() { + // `anc -- .` should run check against `.` (POSIX convention treats + // everything after `--` as positional). + cmd() + .args(["--", "."]) + .assert() + .code(predicate::in_iter([1, 2])) + .stdout(predicate::str::contains("checks:")); +} + #[test] fn test_explicit_completions_subcommand_still_works() { // `anc completions bash` — must pass through, not be treated as default subcommand.