Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/rulesets/protect-main.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
{
"type": "pull_request",
"parameters": {
"required_approving_review_count": 0,
"required_approving_review_count": 1,
"dismiss_stale_reviews_on_push": false,
"required_reviewers": [],
"require_code_owner_review": false,
Expand Down
13 changes: 13 additions & 0 deletions coverage/matrix.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
"kind": "universal"
},
"verifiers": [
{
"check_id": "p1-env-hints",
"layer": "behavioral"
},
{
"check_id": "p1-env-flags-source",
"layer": "source"
Expand All @@ -30,6 +34,10 @@
"check_id": "p1-non-interactive",
"layer": "behavioral"
},
{
"check_id": "p1-flag-existence",
"layer": "behavioral"
},
{
"check_id": "p1-non-interactive-source",
"layer": "project"
Expand Down Expand Up @@ -439,6 +447,10 @@
"condition": "CLI invokes a pager for output"
},
"verifiers": [
{
"check_id": "p6-no-pager-behavioral",
"layer": "behavioral"
},
{
"check_id": "p6-no-pager",
"layer": "source"
Expand Down Expand Up @@ -602,6 +614,7 @@
"total": 46,
"covered": 19,
"uncovered": 27,
"dual_layer": 7,
"must": {
"total": 23,
"covered": 17
Expand Down
7 changes: 4 additions & 3 deletions docs/coverage-matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ When a requirement has no verifier, the cell reads **UNCOVERED** and the reader
## Summary

- **Total**: 46 requirements (19 covered / 27 uncovered)
- **Dual-layer**: 7 of 19 covered requirements have verifiers in two layers (behavioral + source or project)
- **MUST**: 17 of 23 covered
- **SHOULD**: 2 of 16 covered
- **MAY**: 0 of 7 covered
Expand All @@ -16,8 +17,8 @@ When a requirement has no verifier, the cell reads **UNCOVERED** and the reader

| ID | Level | Applicability | Verifier(s) | Summary |
| --- | --- | --- | --- | --- |
| `p1-must-env-var` | MUST | Universal | `p1-env-flags-source` (source) | Every flag settable via environment variable (falsey-value parser for booleans). |
| `p1-must-no-interactive` | MUST | Universal | `p1-non-interactive` (behavioral)<br>`p1-non-interactive-source` (project) | `--no-interactive` flag gates every prompt library call; when set or stdin is not a TTY, use defaults/stdin or exit with an actionable error. |
| `p1-must-env-var` | MUST | Universal | `p1-env-hints` (behavioral)<br>`p1-env-flags-source` (source) | Every flag settable via environment variable (falsey-value parser for booleans). |
| `p1-must-no-interactive` | MUST | Universal | `p1-non-interactive` (behavioral)<br>`p1-flag-existence` (behavioral)<br>`p1-non-interactive-source` (project) | `--no-interactive` flag gates every prompt library call; when set or stdin is not a TTY, use defaults/stdin or exit with an actionable error. |
| `p1-must-no-browser` | MUST | If: CLI authenticates against a remote service | `p1-headless-auth` (source) | Headless authentication path (`--no-browser` / OAuth Device Authorization Grant). |
| `p1-should-tty-detection` | SHOULD | Universal | `p1-tty-detection-source` (source) | Auto-detect non-interactive context via TTY detection; suppress prompts when stderr is not a terminal. |
| `p1-should-defaults-in-help` | SHOULD | Universal | **UNCOVERED** | Document default values for prompted inputs in `--help` output. |
Expand Down Expand Up @@ -73,7 +74,7 @@ When a requirement has no verifier, the cell reads **UNCOVERED** and the reader
| `p6-must-no-color` | MUST | Universal | `p6-no-color-behavioral` (behavioral)<br>`p6-no-color` (source)<br>`p6-no-color` (source) | TTY detection plus support for `NO_COLOR` and `TERM=dumb` — color codes suppressed when stdout/stderr is not a terminal. |
| `p6-must-completions` | MUST | Universal | `p6-completions` (project) | Shell completions available via a `completions` subcommand (Tier 1 meta-command — needs no config/auth/network). |
| `p6-must-timeout-network` | MUST | If: CLI makes network calls | `p6-timeout` (source) | Network CLIs ship a `--timeout` flag with a sensible default (e.g., 30 seconds). |
| `p6-must-no-pager` | MUST | If: CLI invokes a pager for output | `p6-no-pager` (source) | If the CLI uses a pager (`less`, `more`, `$PAGER`), it supports `--no-pager` or respects `PAGER=""`. |
| `p6-must-no-pager` | MUST | If: CLI invokes a pager for output | `p6-no-pager-behavioral` (behavioral)<br>`p6-no-pager` (source) | If the CLI uses a pager (`less`, `more`, `$PAGER`), it supports `--no-pager` or respects `PAGER=""`. |
| `p6-must-global-flags` | MUST | If: CLI uses subcommands | `p6-global-flags` (source) | Agentic flags (`--output`, `--quiet`, `--no-interactive`, `--timeout`) are `global = true` so they propagate to every subcommand. |
| `p6-should-stdin-input` | SHOULD | If: CLI has commands that accept input data | **UNCOVERED** | Commands that accept input read from stdin when no file argument is provided. |
| `p6-should-consistent-naming` | SHOULD | If: CLI uses subcommands | **UNCOVERED** | Subcommand naming follows a consistent `noun verb` or `verb noun` convention throughout the tool. |
Expand Down
3 changes: 2 additions & 1 deletion src/checks/behavioral/bad_args.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::check::Check;
use crate::project::Project;
use crate::runner::RunStatus;
use crate::types::{CheckGroup, CheckLayer, CheckResult, CheckStatus};
use crate::types::{CheckGroup, CheckLayer, CheckResult, CheckStatus, Confidence};

pub struct BadArgsCheck;

Expand Down Expand Up @@ -50,6 +50,7 @@ impl Check for BadArgsCheck {
group: CheckGroup::P4,
layer: CheckLayer::Behavioral,
status,
confidence: Confidence::High,
})
}
}
Expand Down
150 changes: 150 additions & 0 deletions src/checks/behavioral/env_hints.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
//! Check: `--help` advertises environment-variable bindings for flags.
//!
//! Covers: `p1-must-env-var`. Source-verified coverage already exists via
//! `p1-env-flags-source`; this check adds the behavioral layer by inspecting
//! the shipped `--help` surface. Heuristic (Medium confidence): it reads
//! clap-style `[env: FOO]` annotations, which are the canonical but not the
//! only way tools advertise env bindings.
//!
//! Skip when there are no flags at all — a tool with no flags has nothing
//! to bind to env vars. Warn when flags exist but no bindings are visible.

use crate::check::Check;
use crate::project::Project;
use crate::types::{CheckGroup, CheckLayer, CheckResult, CheckStatus, Confidence};

pub struct EnvHintsCheck;

impl Check for EnvHintsCheck {
fn id(&self) -> &str {
"p1-env-hints"
}

fn group(&self) -> CheckGroup {
CheckGroup::P1
}

fn layer(&self) -> CheckLayer {
CheckLayer::Behavioral
}

fn covers(&self) -> &'static [&'static str] {
&["p1-must-env-var"]
}

fn applicable(&self, project: &Project) -> bool {
project.runner.is_some()
}

fn run(&self, project: &Project) -> anyhow::Result<CheckResult> {
let status = match project.help_output() {
None => CheckStatus::Skip("could not probe --help".into()),
Some(help) => check_env_hints(help.flags().len(), help.env_hints().len()),
};

Ok(CheckResult {
id: self.id().to_string(),
label: "Flags advertise env-var bindings in --help".into(),
group: self.group(),
layer: self.layer(),
status,
confidence: Confidence::Medium,
})
}
}

/// Core unit. Takes parsed-flag count and parsed-env-hint count and returns
/// the `CheckStatus` that summarizes them.
fn check_env_hints(flag_count: usize, env_hint_count: usize) -> CheckStatus {
if flag_count == 0 {
return CheckStatus::Skip("target exposes no flags in --help".into());
}
if env_hint_count > 0 {
CheckStatus::Pass
} else {
CheckStatus::Warn(format!(
"{flag_count} flag(s) found in --help but no `[env: NAME]` bindings advertised"
))
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::runner::HelpOutput;

const HELP_WITH_ENV: &str = r#"Usage: foo [OPTIONS]

Options:
-q, --quiet Suppress output [env: FOO_QUIET=]
-h, --help Print help
"#;

const HELP_NO_ENV: &str = r#"Usage: foo [OPTIONS]

Options:
-q, --quiet Suppress output
-h, --help Print help
"#;

const HELP_NO_FLAGS: &str = r#"Usage: foo ARG
A tool that takes one positional argument.
"#;

// Non-English help: parser returns zero env hints and zero flags when
// English conventions don't appear. Per the coverage-matrix exception,
// this is documented English-only behavior.
const HELP_NON_ENGLISH: &str = r#"用法: outil URL

参数:
URL 目标
"#;

#[test]
fn happy_path_env_hint_present() {
let help = HelpOutput::from_raw(HELP_WITH_ENV);
let status = check_env_hints(help.flags().len(), help.env_hints().len());
assert_eq!(status, CheckStatus::Pass);
}

#[test]
fn skip_when_no_flags() {
let help = HelpOutput::from_raw(HELP_NO_FLAGS);
let status = check_env_hints(help.flags().len(), help.env_hints().len());
assert!(matches!(status, CheckStatus::Skip(_)));
}

#[test]
fn warn_when_flags_but_no_env_hints() {
let help = HelpOutput::from_raw(HELP_NO_ENV);
let status = check_env_hints(help.flags().len(), help.env_hints().len());
match status {
CheckStatus::Warn(msg) => {
assert!(msg.contains("env"));
assert!(msg.contains("flag"));
}
other => panic!("expected Warn, got {other:?}"),
}
}

#[test]
fn non_english_help_skipped_or_warned() {
// Localized help with no ASCII options block — parsers return empty
// flags + empty env_hints. Skip (no flags to bind).
let help = HelpOutput::from_raw(HELP_NON_ENGLISH);
let status = check_env_hints(help.flags().len(), help.env_hints().len());
assert!(matches!(status, CheckStatus::Skip(_)));
}

#[test]
fn unit_core_returns_pass_with_any_hint() {
assert_eq!(check_env_hints(3, 1), CheckStatus::Pass);
assert_eq!(check_env_hints(3, 10), CheckStatus::Pass);
}

#[test]
fn unit_core_warns_when_zero_hints() {
let status = check_env_hints(5, 0);
assert!(matches!(status, CheckStatus::Warn(_)));
}
}
Loading