diff --git a/coverage/matrix.json b/coverage/matrix.json index bf22562..a44f559 100644 --- a/coverage/matrix.json +++ b/coverage/matrix.json @@ -372,6 +372,22 @@ } ] }, + { + "id": "p3-should-unprefixed-command-list", + "principle": 3, + "level": "should", + "summary": "Command-list entries name the command directly rather than repeating the binary name as a prefix.", + "applicability": { + "kind": "conditional", + "condition": "CLI uses subcommands" + }, + "verifiers": [ + { + "audit_id": "p3-unprefixed-command-list", + "layer": "behavioral" + } + ] + }, { "id": "p3-may-examples-subcommand", "principle": 3, @@ -957,8 +973,8 @@ } ], "summary": { - "total": 59, - "covered": 56, + "total": 60, + "covered": 57, "uncovered": 3, "dual_layer": 10, "must": { @@ -966,8 +982,8 @@ "covered": 28 }, "should": { - "total": 21, - "covered": 18 + "total": 22, + "covered": 19 }, "may": { "total": 10, diff --git a/docs/coverage-matrix.md b/docs/coverage-matrix.md index e577184..e2db273 100644 --- a/docs/coverage-matrix.md +++ b/docs/coverage-matrix.md @@ -7,10 +7,10 @@ When a requirement has no verifier, the cell reads **UNCOVERED** and the reader ## Summary -- **Total**: 59 requirements (56 covered / 3 uncovered) -- **Dual-layer**: 10 of 56 covered requirements have verifiers in two layers (behavioral + source or project) +- **Total**: 60 requirements (57 covered / 3 uncovered) +- **Dual-layer**: 10 of 57 covered requirements have verifiers in two layers (behavioral + source or project) - **MUST**: 28 of 28 covered -- **SHOULD**: 18 of 21 covered +- **SHOULD**: 19 of 22 covered - **MAY**: 10 of 10 covered ## P1: Non-Interactive by Default @@ -50,6 +50,7 @@ When a requirement has no verifier, the cell reads **UNCOVERED** and the reader | `p3-should-version-short` | SHOULD | Universal | `p3-version` (behavioral) | A short version alias (`-V`, `-v`, or `-version`) accompanies `--version` for fast version probes. | | `p3-should-paired-examples` | SHOULD | Universal | `p3-paired-examples` (behavioral) | Examples show human and agent invocations side by side (text then `--output json` equivalent). | | `p3-should-about-long-about` | SHOULD | Universal | `p3-about-long-about` (behavioral) | Short `about` for command-list summaries; `long_about` reserved for detailed descriptions visible with `--help`. | +| `p3-should-unprefixed-command-list` | SHOULD | If: CLI uses subcommands | `p3-unprefixed-command-list` (behavioral) | Command-list entries name the command directly rather than repeating the binary name as a prefix. | | `p3-may-examples-subcommand` | MAY | Universal | `p3-examples-subcommand` (behavioral) | Dedicated `examples` subcommand or `--examples` flag for curated usage patterns. | ## P4: Fail Fast, Actionable Errors diff --git a/src/audits/behavioral/mod.rs b/src/audits/behavioral/mod.rs index 504ea84..eb09e2c 100644 --- a/src/audits/behavioral/mod.rs +++ b/src/audits/behavioral/mod.rs @@ -42,6 +42,7 @@ mod subcommand_examples; mod subcommand_help; mod subcommand_operations; mod timeout_behavioral; +mod unprefixed_command_list; mod verbose_flag; mod version; @@ -87,6 +88,7 @@ pub fn all_behavioral_audits() -> Vec> { Box::new(consistent_envelope::ConsistentEnvelopeAudit), Box::new(subcommand_examples::SubcommandExamplesAudit), Box::new(paired_examples::PairedExamplesAudit), + Box::new(unprefixed_command_list::UnprefixedCommandListAudit), Box::new(subcommand_operations::SubcommandOperationsAudit), Box::new(force_yes::ForceYesAudit), Box::new(read_write_distinction::ReadWriteDistinctionAudit), diff --git a/src/audits/behavioral/unprefixed_command_list.rs b/src/audits/behavioral/unprefixed_command_list.rs new file mode 100644 index 0000000..c868c75 --- /dev/null +++ b/src/audits/behavioral/unprefixed_command_list.rs @@ -0,0 +1,270 @@ +//! Audit: `p3-should-unprefixed-command-list`. +//! +//! A command list whose entries repeat the binary name (`tool status`, +//! `tool server stop`) makes a reader or agent strip the prefix before the +//! command token is visible. The audit reads the command blocks the help +//! probe parsed: a block whose entries all led with the tool name is +//! prefixed, and every entry that names a command beyond that bare prefix +//! is an offender. The bare-invocation entry, which documents what the tool +//! does with no arguments, is not one. +//! +//! Warn when any offender exists, Pass when every graded block is +//! unprefixed, NotApplicable when no subcommand names were parsed from the +//! help, the same gate `p3-subcommand-examples` and `p6-standard-names` use. +//! An examples block is parsed like any other command block but is not +//! graded; see [`is_graded`]. + +use crate::audit::Audit; +use crate::project::Project; +use crate::runner::{CommandBlock, HelpOutput}; +use crate::types::{AuditGroup, AuditLayer, AuditResult, AuditStatus, Confidence}; + +/// Offending entries quoted in the evidence; the rest are summarised as a count. +const EVIDENCE_ENTRY_LIMIT: usize = 5; + +pub struct UnprefixedCommandListAudit; + +impl Audit for UnprefixedCommandListAudit { + fn id(&self) -> &str { + "p3-unprefixed-command-list" + } + + fn label(&self) -> &'static str { + "Command-list entries name the command without repeating the binary name" + } + + fn group(&self) -> AuditGroup { + AuditGroup::P3 + } + + fn layer(&self) -> AuditLayer { + AuditLayer::Behavioral + } + + fn covers(&self) -> &'static [&'static str] { + &["p3-should-unprefixed-command-list"] + } + + fn applicable(&self, project: &Project) -> bool { + project.runner.is_some() + } + + fn run(&self, project: &Project) -> anyhow::Result { + let status = match project.help_output() { + None => AuditStatus::Skip("could not probe --help".into()), + Some(help) => audit_unprefixed_command_list(help), + }; + Ok(AuditResult { + id: self.id().to_string(), + label: self.label().into(), + group: self.group(), + layer: self.layer(), + status, + confidence: Confidence::Medium, + mitigation: None, + }) + } +} + +pub(crate) fn audit_unprefixed_command_list(help: &HelpOutput) -> AuditStatus { + if help.subcommands().is_empty() { + return AuditStatus::NotApplicable(format!( + "{}; the SHOULD applies to CLIs that list subcommands.", + help.missing_subcommands_reason() + )); + } + let offenders: Vec<(&str, &str)> = help + .command_blocks() + .iter() + .filter(|block| is_graded(block)) + .flat_map(prefixed_commands) + .collect(); + let Some((prefix, _)) = offenders.first() else { + return AuditStatus::Pass; + }; + let quoted: Vec = offenders + .iter() + .take(EVIDENCE_ENTRY_LIMIT) + .map(|(prefix, command)| format!("`{prefix} {command}`")) + .collect(); + let more = offenders.len().saturating_sub(EVIDENCE_ENTRY_LIMIT); + let rest = if more > 0 { + format!(" and {more} more") + } else { + String::new() + }; + AuditStatus::Warn(format!( + "command-list entries repeat the binary name `{prefix}`: {}{rest}. Drop the prefix \ + so a reader or agent scanning the block gets the command token directly instead \ + of reconstructing it from a repeated binary name.", + quoted.join(", ") + )) +} + +/// Whether a block's entries are graded. An examples section whose header +/// ends in `commands:` (`Example commands:`) parses as a command block, but +/// examples are the sanctioned home for the binary name: P3's examples +/// requirement asks for full invocations, so grading them would penalise +/// exactly what it asks for. +fn is_graded(block: &CommandBlock) -> bool { + !block.header.to_ascii_lowercase().contains("example") +} + +/// The prefix and command text of each entry in a prefixed block that names +/// a command beyond the bare prefix, as written up to the description gap +/// (`tool`, `status [server|client]`). +fn prefixed_commands(block: &CommandBlock) -> impl Iterator { + block.prefix.as_deref().into_iter().flat_map(move |prefix| { + block + .entries + .iter() + .map(|entry| block.command_text(entry)) + .filter(|command| !command.is_empty()) + .map(move |command| (prefix, command)) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + const PREFIXED_HELP: &str = "\ +Usage: herdr [options] + +Common commands: + herdr Launch or attach to the persistent session + herdr status [server|client] Show local client and running server status + herdr server stop Stop the running server via the API socket + herdr machine Manage saved SSH machines + +Advanced commands: + herdr server Run as headless server +"; + + const CLAP_HELP: &str = "\ +Usage: anc + +Commands: + audit Run audits against a CLI project or binary + completions Generate shell completions + help Print this message or the help of the given subcommand +"; + + #[test] + fn prefixed_block_warns_and_names_the_offending_lines() { + let help = HelpOutput::from_raw(PREFIXED_HELP); + match audit_unprefixed_command_list(&help) { + AuditStatus::Warn(msg) => { + assert!(msg.contains("`herdr status [server|client]`"), "{msg}"); + assert!(msg.contains("`herdr server stop`"), "{msg}"); + assert!(msg.contains("`herdr server`"), "{msg}"); + assert!( + !msg.contains("`herdr `"), + "bare invocation must not be listed: {msg}" + ); + assert!( + !msg.contains(" more"), + "four offenders fit within the quote limit, so no count suffix: {msg}" + ); + assert!(!msg.contains("Launch"), "{msg}"); + } + other => panic!("expected Warn, got {other:?}"), + } + } + + #[test] + fn evidence_caps_the_quoted_entries_and_counts_the_rest() { + let mut raw = String::from("Usage: tool\n\nCommands:\n"); + for i in 0..8 { + raw.push_str(&format!(" tool cmd{i} Do thing {i}\n")); + } + let help = HelpOutput::from_raw(raw); + match audit_unprefixed_command_list(&help) { + AuditStatus::Warn(msg) => { + assert!(msg.contains("`tool cmd4`"), "{msg}"); + assert!(!msg.contains("`tool cmd5`"), "{msg}"); + assert!(msg.contains("and 3 more"), "{msg}"); + } + other => panic!("expected Warn, got {other:?}"), + } + } + + #[test] + fn clap_block_passes() { + let help = HelpOutput::from_raw(CLAP_HELP); + assert_eq!(audit_unprefixed_command_list(&help), AuditStatus::Pass); + } + + #[test] + fn no_command_block_is_not_applicable() { + let help = + HelpOutput::from_raw("Usage: tool [OPTIONS]\n\nOptions:\n -h, --help Print help\n"); + match audit_unprefixed_command_list(&help) { + AuditStatus::NotApplicable(msg) => { + assert!(msg.contains("no command block found"), "{msg}"); + assert!( + msg.contains("the SHOULD applies to CLIs that list subcommands"), + "{msg}" + ); + } + other => panic!("expected NotApplicable, got {other:?}"), + } + } + + #[test] + fn block_with_no_parseable_names_is_not_applicable() { + let help = HelpOutput::from_raw( + "Usage: modes [options]\n\nCommands:\n modes --init Initialize\n modes --build Build\n", + ); + match audit_unprefixed_command_list(&help) { + AuditStatus::NotApplicable(msg) => { + assert!(msg.contains("but no subcommand names"), "{msg}"); + } + other => panic!("expected NotApplicable, got {other:?}"), + } + } + + #[test] + fn example_block_is_not_graded() { + let help = HelpOutput::from_raw( + "Usage: exa \n\nCommands:\n init Create\n build Compile\n\n\ + Example commands:\n exa init --name foo\n exa build --release\n", + ); + assert_eq!(audit_unprefixed_command_list(&help), AuditStatus::Pass); + } + + #[test] + fn mixed_blocks_grade_only_the_prefixed_one() { + let help = HelpOutput::from_raw( + "Usage: tool [options]\n\nCommands:\n status Show status\n\n\ + Advanced commands:\n tool server Run the server\n tool debug Debug it\n", + ); + match audit_unprefixed_command_list(&help) { + AuditStatus::Warn(msg) => { + assert!(msg.contains("`tool server`"), "{msg}"); + assert!(msg.contains("`tool debug`"), "{msg}"); + assert!(!msg.contains("status"), "{msg}"); + } + other => panic!("expected Warn, got {other:?}"), + } + } + + #[test] + fn entry_that_merely_starts_with_the_tool_name_does_not_warn() { + let help = HelpOutput::from_raw( + "Usage: tool \n\nCommands:\n tool Run the tool\n status Show status\n", + ); + assert_eq!(audit_unprefixed_command_list(&help), AuditStatus::Pass); + } + + #[test] + fn bare_invocation_alone_does_not_warn() { + let help = HelpOutput::from_raw( + "Usage: tool [options]\n\nCommands:\n tool Launch the interactive session\n", + ); + assert!(matches!( + audit_unprefixed_command_list(&help), + AuditStatus::NotApplicable(_) + )); + } +} diff --git a/src/principles/registry.rs b/src/principles/registry.rs index 24cd0e7..27b371c 100644 --- a/src/principles/registry.rs +++ b/src/principles/registry.rs @@ -374,15 +374,15 @@ mod tests { #[test] fn registry_size_matches_spec() { - // Spec snapshot 2026-05-21: 59 requirements across P1-P8. + // Spec snapshot 2026-09-16: 60 requirements across P1-P8. // Bumping this counter is a deliberate act; it means the spec grew. - assert_eq!(REQUIREMENTS.len(), 59); + assert_eq!(REQUIREMENTS.len(), 60); } #[test] fn level_counts_match_spec() { assert_eq!(count_at_level(Level::Must), 28); - assert_eq!(count_at_level(Level::Should), 21); + assert_eq!(count_at_level(Level::Should), 22); assert_eq!(count_at_level(Level::May), 10); } diff --git a/src/principles/spec/principles/p3-progressive-help-discovery.md b/src/principles/spec/principles/p3-progressive-help-discovery.md index cfe1725..6b8421e 100644 --- a/src/principles/spec/principles/p3-progressive-help-discovery.md +++ b/src/principles/spec/principles/p3-progressive-help-discovery.md @@ -1,7 +1,7 @@ --- id: p3 title: Progressive Help Discovery -last-revised: 2026-05-21 +last-revised: 2026-09-17 status: active requirements: - id: p3-must-subcommand-examples @@ -29,6 +29,11 @@ requirements: level: should applicability: universal summary: Short `about` for command-list summaries; `long_about` reserved for detailed descriptions visible with `--help`. + - id: p3-should-unprefixed-command-list + level: should + applicability: + if: CLI uses subcommands + summary: Command-list entries name the command directly rather than repeating the binary name as a prefix. - id: p3-may-examples-subcommand level: may applicability: universal @@ -72,6 +77,11 @@ trial-and-errors its way into a working call, burning tokens and sometimes landi `yarn`, `make`), or `-version` (Go's `flag` package). Any of the three forms is sufficient. Agents probing tool versions across many CLIs save token cost when they can pin against a one- or two-character flag; the long-only path forces an extra parse step. +- Entries in the command list SHOULD name the command directly (`status`, `server stop`) rather than repeat the binary + name on every line (`tool status`, `tool server stop`). A reader or agent scanning the block takes the command token + straight from the left column instead of reconstructing it from a prefix it already knows from the `Usage:` line. The + bare invocation line, which documents what the tool does with no arguments, is the one entry that legitimately carries + the binary name alone. **MAY:** @@ -84,6 +94,8 @@ trial-and-errors its way into a working call, burning tokens and sometimes landi - `after_help` attribute on every subcommand variant. - Example invocations in `after_help` text that include realistic arguments, not placeholder `` tokens. - Both `about` (short) and `after_help` (examples) present on each subcommand. +- The command list's left column holds bare command tokens; the binary name appears in the `Usage:` line and in + examples, not at the head of each command entry. ## Anti-Patterns @@ -92,6 +104,8 @@ trial-and-errors its way into a working call, burning tokens and sometimes landi - A single `about` string serving as both summary and usage documentation. - Examples buried in a README or man page but absent from `--help` output. - `after_help` text that describes the flags in prose instead of demonstrating them in code. +- A hand-written command list that prefixes every entry with the binary name, so the command token sits second on each + line and a scanner has to strip the prefix before it can read the command. Measured by audit IDs `p3-help`, `p3-after-help`, `p3-version`. Run `anc audit --principle 3 .` against the CLI under test to see each. diff --git a/src/runner/mod.rs b/src/runner/mod.rs index 5841de3..3112fca 100644 --- a/src/runner/mod.rs +++ b/src/runner/mod.rs @@ -1,6 +1,6 @@ pub mod help_probe; -pub use help_probe::HelpOutput; +pub use help_probe::{CommandBlock, HelpOutput}; use std::cell::RefCell; use std::collections::HashMap; diff --git a/tests/build_parser.rs b/tests/build_parser.rs index cf3988f..912a6e3 100644 --- a/tests/build_parser.rs +++ b/tests/build_parser.rs @@ -466,7 +466,7 @@ fn vendored_spec_parses_to_expected_requirement_count() { } let combined = aggregate(parsed_per_file).expect("no duplicates in vendored spec"); - assert_eq!(combined.len(), 59, "vendored spec ships 59 requirements"); + assert_eq!(combined.len(), 60, "vendored spec ships 60 requirements"); // First entry should still be p1-must-env-var — the order is filename- // sorted then spec-frontmatter-order, and v0.4.0 only appended new IDs. diff --git a/tests/integration.rs b/tests/integration.rs index 254f9c5..106aeb6 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -297,6 +297,17 @@ fn test_handwritten_help_fixture_reports_real_subcommands() { "tool name or description word leaked into names: {evidence}" ); assert_ne!(row("p6-standard-names")["status"], "skip"); + let prefix = row("p3-unprefixed-command-list"); + assert_eq!(prefix["status"], "warn", "{prefix}"); + let prefix_evidence = prefix["evidence"].as_str().expect("warn evidence"); + assert!( + prefix_evidence.contains("`tally count `") && !prefix_evidence.contains("`tally `"), + "prefix evidence should name prefixed entries and not the bare invocation: {prefix_evidence}" + ); + assert!( + prefix_evidence.contains("and 2 more"), + "seven offenders, five quoted: {prefix_evidence}" + ); assert_eq!( row("p5-force-yes")["status"], "skip",