← back to Cli Printing Press
fix(cli): probe nested path-param leaves so verify catches `args[]` index bugs (#1434)
5460c32aa10373023060fffeceb7f8ada1fcfd5f · 2026-05-15 17:32:20 -0700 · Trevin Chow
* fix(cli): probe nested path-param leaves so verify catches `args[]` index bugs
verify's matrix only invokes top-level commands. Nested leaves like
`tags contacts list-for-tag-id-using-get <tagId>` were never exercised
with positionals, so a generator codegen bug that mis-indexed
`args[]` for path-param substitution (issue #1199, fixed by #1211)
shipped silently with verify reporting 100% pass on three published
CLIs (keap, supabase, learndash).
Add a positional-binding probe that recursively walks the cobra tree
via --help, finds leaves whose Usage line carries `<placeholder>`
tokens, and runs each with `--dry-run <synthetic-positional>`. A leaf
is flagged only when the binary exits non-zero AND the output contains
the "is required" sentinel — unrelated failures (auth, transient,
intentional non-zero) are not treated as probe failures so
already-correct CLIs continue to pass. Top-level leaves (depth 1) are
skipped because the existing matrix already exercises them through
inferPositionalArgs; the probe starts at depth 2 to avoid
double-counting.
Failed probes are critical: they're surfaced as a new
PathParamProbes list on VerifyReport (omitempty) and folded into the
existing Total / Failed / Critical counts in finalizeVerifyReport, so
a single failing probe blocks the PASS verdict.
Closes #1198.
* docs(cli): scrub em-dashes and ticket numbers from path-param probe comments
AGENTS.md "Code & Comment Hygiene": no em-dashes or ticket numbers in
code comments. Drops the bare-ticket-reference shape in favor of
self-contained prose so the comments hold up if the referenced issue is
renumbered or migrated.
* fix(cli): probe only true leaves; replace Args with Positionals on probe result
Addresses Greptile P2 findings on #1434:
1. Parent/group commands whose Usage line carried a `<placeholder>`
token were being added to the probe set even when they had
children. The synthetic value would land as an "unknown command",
exit non-zero without matching the "is required" sentinel, and
silently pass — inflating Total / Passed without testing anything
meaningful. Probes now require the node to have no children
(`parseHelpCommands(helpOut)` returns empty). The walker still
recurses into every child regardless, so true leaves underneath a
placeholder-bearing parent are still discovered.
2. PathParamProbeResult.Args duplicated the command path that
Command already carries, leaving JSON consumers to strip the
leading words to recover the injected positionals. Replaced with
Positionals, which carries only the synthetic values one per
placeholder; Command + Positionals together describe the full
invocation without redundant prefix.
Files touched
M internal/cli/verify.goM internal/pipeline/runtime.goA internal/pipeline/runtime_pathparam.goA internal/pipeline/runtime_pathparam_test.goM internal/pipeline/runtime_report.goM internal/pipeline/runtime_structural.go
Diff
commit 5460c32aa10373023060fffeceb7f8ada1fcfd5f
Author: Trevin Chow <trevin@trevinchow.com>
Date: Fri May 15 17:32:20 2026 -0700
fix(cli): probe nested path-param leaves so verify catches `args[]` index bugs (#1434)
* fix(cli): probe nested path-param leaves so verify catches `args[]` index bugs
verify's matrix only invokes top-level commands. Nested leaves like
`tags contacts list-for-tag-id-using-get <tagId>` were never exercised
with positionals, so a generator codegen bug that mis-indexed
`args[]` for path-param substitution (issue #1199, fixed by #1211)
shipped silently with verify reporting 100% pass on three published
CLIs (keap, supabase, learndash).
Add a positional-binding probe that recursively walks the cobra tree
via --help, finds leaves whose Usage line carries `<placeholder>`
tokens, and runs each with `--dry-run <synthetic-positional>`. A leaf
is flagged only when the binary exits non-zero AND the output contains
the "is required" sentinel — unrelated failures (auth, transient,
intentional non-zero) are not treated as probe failures so
already-correct CLIs continue to pass. Top-level leaves (depth 1) are
skipped because the existing matrix already exercises them through
inferPositionalArgs; the probe starts at depth 2 to avoid
double-counting.
Failed probes are critical: they're surfaced as a new
PathParamProbes list on VerifyReport (omitempty) and folded into the
existing Total / Failed / Critical counts in finalizeVerifyReport, so
a single failing probe blocks the PASS verdict.
Closes #1198.
* docs(cli): scrub em-dashes and ticket numbers from path-param probe comments
AGENTS.md "Code & Comment Hygiene": no em-dashes or ticket numbers in
code comments. Drops the bare-ticket-reference shape in favor of
self-contained prose so the comments hold up if the referenced issue is
renumbered or migrated.
* fix(cli): probe only true leaves; replace Args with Positionals on probe result
Addresses Greptile P2 findings on #1434:
1. Parent/group commands whose Usage line carried a `<placeholder>`
token were being added to the probe set even when they had
children. The synthetic value would land as an "unknown command",
exit non-zero without matching the "is required" sentinel, and
silently pass — inflating Total / Passed without testing anything
meaningful. Probes now require the node to have no children
(`parseHelpCommands(helpOut)` returns empty). The walker still
recurses into every child regardless, so true leaves underneath a
placeholder-bearing parent are still discovered.
2. PathParamProbeResult.Args duplicated the command path that
Command already carries, leaving JSON consumers to strip the
leading words to recover the injected positionals. Replaced with
Positionals, which carries only the synthetic values one per
placeholder; Command + Positionals together describe the full
invocation without redundant prefix.
---
internal/cli/verify.go | 15 +
internal/pipeline/runtime.go | 34 ++-
internal/pipeline/runtime_pathparam.go | 179 ++++++++++++
internal/pipeline/runtime_pathparam_test.go | 438 ++++++++++++++++++++++++++++
internal/pipeline/runtime_report.go | 16 +
internal/pipeline/runtime_structural.go | 1 +
6 files changed, 667 insertions(+), 16 deletions(-)
diff --git a/internal/cli/verify.go b/internal/cli/verify.go
index ea185e90..861ab769 100644
--- a/internal/cli/verify.go
+++ b/internal/cli/verify.go
@@ -147,6 +147,21 @@ func printVerifyReport(report *pipeline.VerifyReport) {
r.Score)
}
+ if len(report.PathParamProbes) > 0 {
+ fmt.Println()
+ fmt.Println("Path-Param Probes (nested commands with <positional> args):")
+ for _, probe := range report.PathParamProbes {
+ status := "PASS"
+ if !probe.Passed {
+ status = "FAIL"
+ }
+ fmt.Printf(" %-4s %s\n", status, probe.Command)
+ if !probe.Passed && probe.Reason != "" {
+ fmt.Printf(" %s\n", probe.Reason)
+ }
+ }
+ }
+
fmt.Println()
if report.DataPipelineDetail != "" {
fmt.Printf("Data Pipeline: %s\n", report.DataPipelineDetail)
diff --git a/internal/pipeline/runtime.go b/internal/pipeline/runtime.go
index 44be836d..58f54375 100644
--- a/internal/pipeline/runtime.go
+++ b/internal/pipeline/runtime.go
@@ -28,22 +28,23 @@ type VerifyConfig struct {
// VerifyReport is the output of a runtime verification run.
type VerifyReport struct {
- Mode string `json:"mode"` // "live" or "mock"
- Total int `json:"total"`
- Passed int `json:"passed"`
- Failed int `json:"failed"`
- Critical int `json:"critical"`
- PassRate float64 `json:"pass_rate"`
- DataPipeline bool `json:"data_pipeline"`
- DataPipelineDetail string `json:"data_pipeline_detail,omitempty"` // PASS, WARN, SKIP, FAIL with context
- Freshness FreshnessResult `json:"freshness"`
- BrowserSessionRequired bool `json:"browser_session_required,omitempty"`
- BrowserSessionProof string `json:"browser_session_proof,omitempty"`
- BrowserSessionDetail string `json:"browser_session_detail,omitempty"`
- AuthEnvVars []AuthEnvVarStatus `json:"auth_env_vars,omitempty"`
- Verdict string `json:"verdict"` // PASS, WARN, FAIL
- Results []CommandResult `json:"results"`
- Binary string `json:"binary"`
+ Mode string `json:"mode"` // "live" or "mock"
+ Total int `json:"total"`
+ Passed int `json:"passed"`
+ Failed int `json:"failed"`
+ Critical int `json:"critical"`
+ PassRate float64 `json:"pass_rate"`
+ DataPipeline bool `json:"data_pipeline"`
+ DataPipelineDetail string `json:"data_pipeline_detail,omitempty"` // PASS, WARN, SKIP, FAIL with context
+ Freshness FreshnessResult `json:"freshness"`
+ BrowserSessionRequired bool `json:"browser_session_required,omitempty"`
+ BrowserSessionProof string `json:"browser_session_proof,omitempty"`
+ BrowserSessionDetail string `json:"browser_session_detail,omitempty"`
+ AuthEnvVars []AuthEnvVarStatus `json:"auth_env_vars,omitempty"`
+ Verdict string `json:"verdict"` // PASS, WARN, FAIL
+ Results []CommandResult `json:"results"`
+ PathParamProbes []PathParamProbeResult `json:"path_param_probes,omitempty"`
+ Binary string `json:"binary"`
}
type AuthEnvVarStatus struct {
@@ -293,6 +294,7 @@ func RunVerify(cfg VerifyConfig) (*VerifyReport, error) {
report.DataPipeline, report.DataPipelineDetail = runDataPipelineTest(binaryPath, report.Mode, buildEnv)
report.Freshness = runFreshnessContractTest(cfg.Dir)
+ report.PathParamProbes = runPathParamProbes(binaryPath, buildEnv(), paramDefaults)
if spec != nil && spec.Auth.RequiresBrowserSession {
report.BrowserSessionRequired = true
diff --git a/internal/pipeline/runtime_pathparam.go b/internal/pipeline/runtime_pathparam.go
new file mode 100644
index 00000000..d13fefac
--- /dev/null
+++ b/internal/pipeline/runtime_pathparam.go
@@ -0,0 +1,179 @@
+package pipeline
+
+import (
+ "context"
+ "errors"
+ "os/exec"
+ "regexp"
+ "strings"
+ "time"
+)
+
+// pathParamProbe identifies a cobra leaf command (by its full path from
+// root) whose Usage line carries one or more <placeholder> positional
+// tokens. The existing verify matrix only probes top-level commands, so
+// nested leaves never get their positionals exercised; a generator-
+// emitted args[]-indexing bug therefore ships silently with verify
+// reporting 100% pass.
+type pathParamProbe struct {
+ path []string
+ placeholders []string
+}
+
+// pathParamRequiredSentinelRe matches the usage-error sentinel that
+// generator-emitted positional validators print when their (potentially
+// mis-indexed) `len(args) < N` guard fires: e.g. `Error: tagId is
+// required`. Case-insensitive so generator wording drift does not blind
+// the probe.
+var pathParamRequiredSentinelRe = regexp.MustCompile(`(?i)\bis required\b`)
+
+// pathParamUsageRe captures the first content line under `Usage:`.
+// Tolerant of multi-level command paths: it does not anchor on the
+// number of leading words, so it works equally well on
+// `cli sub <id>` and `cli a b c <id>` shapes.
+var pathParamUsageRe = regexp.MustCompile(`(?m)^Usage:\s*\n\s+(.+)$`)
+
+// pathParamProbeMaxDepth caps recursion when walking the cobra tree.
+// Six levels covers every printed CLI in the library today by a wide
+// margin and prevents runaway walks if a buggy --help renderer ever
+// produces a cycle.
+const pathParamProbeMaxDepth = 6
+
+const pathParamProbeTimeout = 10 * time.Second
+
+// PathParamProbeResult records one positional-binding probe.
+// Positionals lists only the synthetic values injected for each
+// placeholder; Command already carries the human-readable command path,
+// so the two fields together describe the full invocation without
+// duplicating the path in both places.
+type PathParamProbeResult struct {
+ Command string `json:"command"`
+ Positionals []string `json:"positionals,omitempty"`
+ Passed bool `json:"passed"`
+ Reason string `json:"reason,omitempty"`
+}
+
+// discoverPathParamProbes walks the binary's cobra tree by recursively
+// reading --help output and returns nested leaves (depth >= 2) whose
+// Usage line has at least one <placeholder> token. Top-level (depth 1)
+// commands are already exercised by inferPositionalArgs through the
+// existing matrix, so the probe deliberately starts at depth 2 to avoid
+// double-counting them in Total / Critical. Nodes whose --help fails
+// are skipped rather than counted as failures (a non-responsive --help
+// is its own bug, surfaced elsewhere).
+//
+// Only leaves (nodes whose --help advertises no children) are probed.
+// A parent/group whose Usage happened to carry a <placeholder> token
+// (e.g. `cli tags <filter> [command]`) would otherwise be invoked with
+// the synthetic value as a subcommand, producing an "unknown command"
+// exit that silently passes the sentinel check and inflates Total /
+// Passed without testing anything meaningful.
+func discoverPathParamProbes(binary string) []pathParamProbe {
+ if binary == "" {
+ return nil
+ }
+ var probes []pathParamProbe
+ var walk func(path []string)
+ walk = func(path []string) {
+ if len(path) > pathParamProbeMaxDepth {
+ return
+ }
+
+ helpOut, ok := readCommandHelp(binary, path)
+ if !ok {
+ return
+ }
+
+ children := parseHelpCommands(helpOut)
+
+ if len(path) >= 2 && len(children) == 0 {
+ if placeholders := extractPositionalsFromUsage(helpOut); len(placeholders) > 0 {
+ probes = append(probes, pathParamProbe{
+ path: append([]string(nil), path...),
+ placeholders: placeholders,
+ })
+ }
+ }
+
+ for _, child := range children {
+ childPath := append(append([]string(nil), path...), child.Name)
+ walk(childPath)
+ }
+ }
+ walk(nil)
+ return probes
+}
+
+func readCommandHelp(binary string, path []string) (string, bool) {
+ args := append(append([]string{}, path...), "--help")
+ ctx, cancel := context.WithTimeout(context.Background(), pathParamProbeTimeout)
+ defer cancel()
+ cmd := exec.CommandContext(ctx, binary, args...)
+ applyDefaultSubprocessEnv(cmd)
+ out, err := cmd.CombinedOutput()
+ if err != nil {
+ return "", false
+ }
+ return string(out), true
+}
+
+// extractPositionalsFromUsage finds the Usage: line in a --help output
+// and returns the positional placeholder names. Returns nil when no
+// Usage line is found or no placeholders are present.
+func extractPositionalsFromUsage(helpOut string) []string {
+ m := pathParamUsageRe.FindStringSubmatch(helpOut)
+ if m == nil {
+ return nil
+ }
+ return extractPositionalPlaceholders(m[1])
+}
+
+// runPathParamProbes invokes each discovered placeholder-bearing command
+// with synthetic positionals + `--dry-run` and records whether the
+// binary rejected the call with an "is required" usage error. Only that
+// failure mode counts: unrelated transient errors (auth, mock-server
+// quirks, exit codes documented as success) are not failures so the
+// probe does not introduce false positives on already-correct CLIs.
+//
+// `paramDefaults` is the spec author's Param.Default map; when set, a
+// matching entry wins over the cross-domain canonicalargs registry and
+// the per-name fallback. The resolution chain matches inferPositionalArgs
+// so the probe uses the same synthetic value the top-level matrix uses.
+func runPathParamProbes(binary string, env []string, paramDefaults map[string]string) []PathParamProbeResult {
+ probes := discoverPathParamProbes(binary)
+ if len(probes) == 0 {
+ return nil
+ }
+ results := make([]PathParamProbeResult, 0, len(probes))
+ for _, probe := range probes {
+ positionals := make([]string, 0, len(probe.placeholders))
+ for _, name := range probe.placeholders {
+ positionals = append(positionals, resolvePositionalValue(name, paramDefaults))
+ }
+ invokeArgs := append([]string{}, probe.path...)
+ invokeArgs = append(invokeArgs, positionals...)
+ invokeArgs = append(invokeArgs, "--dry-run")
+
+ out, err := runCLIWithOutput(binary, invokeArgs, env, pathParamProbeTimeout)
+ result := PathParamProbeResult{
+ Command: strings.Join(probe.path, " "),
+ Positionals: positionals,
+ Passed: true,
+ }
+ // Only the "missing positional" sentinel counts as a probe failure.
+ // Other errors (auth, transient, intentional non-zero) belong to
+ // other checks. Timeouts are inconclusive: leave Passed=true so a
+ // hung --dry-run does not flip the verdict; the existing matrix's
+ // 10s timeout flags the same command independently.
+ if err != nil && pathParamRequiredSentinelRe.MatchString(string(out)) && !isContextDeadlineErr(err) {
+ result.Passed = false
+ result.Reason = `command rejected the call with "is required" despite the supplied positional args; regenerate with the current generator (path-param positional binding fix)`
+ }
+ results = append(results, result)
+ }
+ return results
+}
+
+func isContextDeadlineErr(err error) bool {
+ return errors.Is(err, context.DeadlineExceeded)
+}
diff --git a/internal/pipeline/runtime_pathparam_test.go b/internal/pipeline/runtime_pathparam_test.go
new file mode 100644
index 00000000..2722eda2
--- /dev/null
+++ b/internal/pipeline/runtime_pathparam_test.go
@@ -0,0 +1,438 @@
+package pipeline
+
+import (
+ "os/exec"
+ "path/filepath"
+ "strings"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+func TestExtractPositionalsFromUsage(t *testing.T) {
+ tests := []struct {
+ name string
+ helpOut string
+ expected []string
+ }{
+ {
+ name: "leaf with one placeholder",
+ helpOut: `Foo command
+
+Usage:
+ cli-name foo bar baz <id> [flags]
+
+Flags:
+ -h, --help help`,
+ expected: []string{"id"},
+ },
+ {
+ name: "leaf with two placeholders",
+ helpOut: `Compare two items.
+
+Usage:
+ cli-name items compare <owner> <repo> [flags]
+`,
+ expected: []string{"owner", "repo"},
+ },
+ {
+ name: "parent group (no placeholders)",
+ helpOut: `Group of foo commands
+
+Usage:
+ cli-name foo [command]
+
+Available Commands:
+ bar Bar things
+`,
+ expected: nil,
+ },
+ {
+ name: "leaf flag descriptors stripped",
+ helpOut: `Tag a thing.
+
+Usage:
+ cli-name save <url> [--tags=<csv>] [flags]
+`,
+ expected: []string{"url"},
+ },
+ {
+ name: "no usage block",
+ helpOut: "Just some text without a Usage section",
+ expected: nil,
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ got := extractPositionalsFromUsage(tt.helpOut)
+ assert.Equal(t, tt.expected, got)
+ })
+ }
+}
+
+// TestPathParamRequiredSentinel pins the regex that flags "is required"
+// usage errors in command output. Generator wording for path-param
+// validators is `Error: <name> is required`, so the regex must catch
+// that case-insensitively, and must NOT match Cobra's separate
+// `required flag(s) "X" not set` error (which is a flag bug, not the
+// positional-binding bug this probe targets).
+func TestPathParamRequiredSentinel(t *testing.T) {
+ matches := []string{
+ "Error: tagId is required",
+ "error: id is required\n",
+ "<region> is required",
+ "Error: COMPANY_ID is required",
+ }
+ for _, m := range matches {
+ assert.True(t, pathParamRequiredSentinelRe.MatchString(m), "should match: %q", m)
+ }
+
+ doesNotMatch := []string{
+ `Error: required flag(s) "owner" not set`,
+ "unknown command",
+ "connection refused",
+ }
+ for _, m := range doesNotMatch {
+ assert.False(t, pathParamRequiredSentinelRe.MatchString(m), "should not match: %q", m)
+ }
+}
+
+// buildPathParamProbeFixture builds a tiny CLI binary that simulates a
+// nested cobra tree with three leaf commands plus a top-level leaf:
+//
+// - `tags contacts list <tagId>`: broken. Prints `Error: tagId is
+// required` and exits 1 no matter what positionals the caller
+// supplies. This is the failure shape the probe targets.
+// - `tags contacts get <id>`: working. Accepts a positional and
+// prints a fake URL on `--dry-run`.
+// - `tags contacts ping <id>`: unrelated failure. Exits 1 with
+// `connection refused` regardless of positionals. The probe must
+// NOT flag this as a path-param failure (false-positive guard).
+// - `recipe <url>`: top-level placeholder-bearing leaf, deliberately
+// broken in the same shape as `list`. The probe must skip it
+// because depth-1 leaves are already exercised by the existing
+// matrix via inferPositionalArgs.
+func buildPathParamProbeFixture(t *testing.T) string {
+ t.Helper()
+ dir := t.TempDir()
+ mainFile := filepath.Join(dir, "main.go")
+ writeTestFile(t, mainFile, `package main
+
+import (
+ "fmt"
+ "os"
+ "strings"
+)
+
+func main() {
+ args := os.Args[1:]
+ if len(args) == 0 {
+ // root --help equivalent
+ printRootHelp()
+ return
+ }
+ joined := strings.Join(args, " ")
+
+ switch {
+ case joined == "--help":
+ printRootHelp()
+ case joined == "tags --help":
+ printTagsHelp()
+ case joined == "tags contacts --help":
+ printTagsContactsHelp()
+ case joined == "tags contacts list --help":
+ printListLeafHelp()
+ case joined == "tags contacts get --help":
+ printGetLeafHelp()
+ case joined == "tags contacts ping --help":
+ printPingLeafHelp()
+ case joined == "recipe --help":
+ printRecipeLeafHelp()
+ case strings.HasPrefix(joined, "tags contacts list "):
+ fmt.Fprintln(os.Stderr, "Error: tagId is required")
+ os.Exit(1)
+ case strings.HasPrefix(joined, "tags contacts get "):
+ fmt.Println("DRY-RUN: GET /v1/tags/contacts/" + lastArg(args, "--dry-run"))
+ case strings.HasPrefix(joined, "tags contacts ping "):
+ fmt.Fprintln(os.Stderr, "Error: connection refused")
+ os.Exit(1)
+ case strings.HasPrefix(joined, "recipe "):
+ fmt.Fprintln(os.Stderr, "Error: url is required")
+ os.Exit(1)
+ default:
+ fmt.Fprintln(os.Stderr, "unknown:", joined)
+ os.Exit(2)
+ }
+}
+
+func lastArg(args []string, ignore string) string {
+ for i := len(args) - 1; i >= 0; i-- {
+ if args[i] != ignore {
+ return args[i]
+ }
+ }
+ return ""
+}
+
+func printRootHelp() {
+ fmt.Println(`+"`"+`Test CLI
+
+Usage:
+ test-cli [command]
+
+Available Commands:
+ tags Manage tags
+ recipe Fetch a recipe
+`+"`"+`)
+}
+
+func printTagsHelp() {
+ fmt.Println(`+"`"+`Manage tags
+
+Usage:
+ test-cli tags [command]
+
+Available Commands:
+ contacts Tag-scoped contact operations
+`+"`"+`)
+}
+
+func printTagsContactsHelp() {
+ fmt.Println(`+"`"+`Tag-scoped contact operations
+
+Usage:
+ test-cli tags contacts [command]
+
+Available Commands:
+ list List contacts for a tag
+ get Get a single contact
+ ping Ping (simulates an unrelated failure)
+`+"`"+`)
+}
+
+func printListLeafHelp() {
+ fmt.Println(`+"`"+`List contacts for a tag
+
+Usage:
+ test-cli tags contacts list <tagId> [flags]
+
+Flags:
+ -h, --help help
+`+"`"+`)
+}
+
+func printGetLeafHelp() {
+ fmt.Println(`+"`"+`Get a single contact
+
+Usage:
+ test-cli tags contacts get <id> [flags]
+
+Flags:
+ -h, --help help
+`+"`"+`)
+}
+
+func printPingLeafHelp() {
+ fmt.Println(`+"`"+`Ping (simulates an unrelated failure)
+
+Usage:
+ test-cli tags contacts ping <id> [flags]
+
+Flags:
+ -h, --help help
+`+"`"+`)
+}
+
+func printRecipeLeafHelp() {
+ fmt.Println(`+"`"+`Fetch a recipe (top-level leaf at depth 1)
+
+Usage:
+ test-cli recipe <url> [flags]
+
+Flags:
+ -h, --help help
+`+"`"+`)
+}
+`)
+ binaryPath := filepath.Join(dir, "test-cli")
+ out, err := exec.Command("go", "build", "-o", binaryPath, mainFile).CombinedOutput()
+ require.NoError(t, err, "building probe fixture: %s", string(out))
+ return binaryPath
+}
+
+func TestDiscoverPathParamProbes_WalksNestedTree(t *testing.T) {
+ binary := buildPathParamProbeFixture(t)
+
+ probes := discoverPathParamProbes(binary)
+ require.Len(t, probes, 3, "should find the 3 nested leaves at depth 3 and skip the depth-1 'recipe' leaf")
+
+ paths := make([]string, 0, len(probes))
+ for _, p := range probes {
+ paths = append(paths, strings.Join(p.path, " "))
+ }
+ assert.Contains(t, paths, "tags contacts list")
+ assert.Contains(t, paths, "tags contacts get")
+ assert.Contains(t, paths, "tags contacts ping")
+ assert.NotContains(t, paths, "recipe",
+ "depth-1 leaves are exercised by the existing matrix via inferPositionalArgs; the probe must not double-count them")
+}
+
+func TestDiscoverPathParamProbes_EmptyBinaryReturnsNil(t *testing.T) {
+ assert.Nil(t, discoverPathParamProbes(""))
+}
+
+// TestDiscoverPathParamProbes_SkipsParentGroupsWithPlaceholders pins the
+// leaf-only contract: a parent/group command whose Usage line happens to
+// carry a <placeholder> token (e.g. `cli zones <zone-id> [command]`)
+// must NOT be probed. Probing it would invoke the synthetic value as a
+// subcommand, producing an "unknown command" exit that silently passes
+// the sentinel check and inflates Total / Passed without testing
+// anything meaningful.
+func TestDiscoverPathParamProbes_SkipsParentGroupsWithPlaceholders(t *testing.T) {
+ binary := buildParentGroupWithPlaceholderFixture(t)
+ probes := discoverPathParamProbes(binary)
+ paths := make([]string, 0, len(probes))
+ for _, p := range probes {
+ paths = append(paths, strings.Join(p.path, " "))
+ }
+ assert.NotContains(t, paths, "zones",
+ "depth-1 leaves are skipped, but this would have caught a depth-1 'parent' bug regardless")
+ assert.NotContains(t, paths, "zones records",
+ "parent group with <zone-id> in Usage must not be probed; only true leaves qualify")
+ assert.Contains(t, paths, "zones records list",
+ "the true leaf at depth 3 should still be probed")
+}
+
+func buildParentGroupWithPlaceholderFixture(t *testing.T) string {
+ t.Helper()
+ dir := t.TempDir()
+ mainFile := filepath.Join(dir, "main.go")
+ writeTestFile(t, mainFile, `package main
+
+import (
+ "fmt"
+ "os"
+ "strings"
+)
+
+func main() {
+ args := os.Args[1:]
+ joined := strings.Join(args, " ")
+ switch {
+ case joined == "" || joined == "--help":
+ fmt.Println(`+"`"+`Test
+
+Usage:
+ cli [command]
+
+Available Commands:
+ zones Manage zones (this group carries a placeholder in Usage)
+`+"`"+`)
+ case joined == "zones --help":
+ // Parent group whose Usage carries a placeholder. Listing
+ // children means it is NOT a leaf — the probe must skip it.
+ fmt.Println(`+"`"+`Manage zones
+
+Usage:
+ cli zones <zone-id> [command]
+
+Available Commands:
+ records Manage records under a zone
+`+"`"+`)
+ case joined == "zones records --help":
+ // Same: parent group, listed children, has placeholder.
+ fmt.Println(`+"`"+`Manage records
+
+Usage:
+ cli zones records <zone-id> [command]
+
+Available Commands:
+ list List records
+`+"`"+`)
+ case joined == "zones records list --help":
+ // True leaf at depth 3 — should be probed.
+ fmt.Println(`+"`"+`List records
+
+Usage:
+ cli zones records list <zone-id> [flags]
+
+Flags:
+ -h, --help help
+`+"`"+`)
+ }
+}
+`)
+ binaryPath := filepath.Join(dir, "cli")
+ out, err := exec.Command("go", "build", "-o", binaryPath, mainFile).CombinedOutput()
+ require.NoError(t, err, "build fixture: %s", string(out))
+ return binaryPath
+}
+
+func TestRunPathParamProbes_DistinguishesFailureModes(t *testing.T) {
+ binary := buildPathParamProbeFixture(t)
+
+ results := runPathParamProbes(binary, subprocessEnv(), nil)
+ require.Len(t, results, 3)
+
+ byCommand := map[string]PathParamProbeResult{}
+ for _, r := range results {
+ byCommand[r.Command] = r
+ }
+
+ broken := byCommand["tags contacts list"]
+ assert.False(t, broken.Passed, "leaf that returns 'is required' is the bug shape; must be flagged")
+ assert.NotEmpty(t, broken.Reason)
+ assert.NotEmpty(t, broken.Positionals, "Positionals should capture the synthetic values injected for the placeholder")
+
+ working := byCommand["tags contacts get"]
+ assert.True(t, working.Passed, "leaf that accepts the positional must pass")
+ assert.Empty(t, working.Reason)
+
+ unrelated := byCommand["tags contacts ping"]
+ assert.True(t, unrelated.Passed,
+ "leaf that fails for an unrelated reason ('connection refused') must NOT be flagged; that's a different check's bug")
+ assert.Empty(t, unrelated.Reason)
+}
+
+// TestFinalizeVerifyReport_PathParamProbeFailureFlipsVerdict pins the
+// scoring rule: a failed path-param probe is a critical failure, which
+// blocks the PASS verdict per finalizeVerifyReport's `Critical == 0`
+// gate.
+func TestFinalizeVerifyReport_PathParamProbeFailureFlipsVerdict(t *testing.T) {
+ report := &VerifyReport{
+ Results: []CommandResult{
+ {Command: "version", Score: 3, Help: true, DryRun: true, Execute: true},
+ },
+ PathParamProbes: []PathParamProbeResult{
+ {Command: "tags contacts list", Passed: false, Reason: "is required"},
+ {Command: "tags contacts get", Passed: true},
+ },
+ DataPipeline: true,
+ }
+ finalizeVerifyReport(report, 80, true)
+
+ assert.Equal(t, 3, report.Total)
+ assert.Equal(t, 2, report.Passed)
+ assert.Equal(t, 1, report.Failed)
+ assert.Equal(t, 1, report.Critical)
+ assert.NotEqual(t, "PASS", report.Verdict, "critical probe failure should block PASS")
+}
+
+func TestFinalizeVerifyReport_AllPathParamProbesPassingKeepsVerdict(t *testing.T) {
+ report := &VerifyReport{
+ Results: []CommandResult{
+ {Command: "version", Score: 3, Help: true, DryRun: true, Execute: true},
+ },
+ PathParamProbes: []PathParamProbeResult{
+ {Command: "tags contacts get", Passed: true},
+ },
+ DataPipeline: true,
+ }
+ finalizeVerifyReport(report, 80, true)
+
+ assert.Equal(t, 2, report.Total)
+ assert.Equal(t, 2, report.Passed)
+ assert.Equal(t, 0, report.Critical)
+ assert.Equal(t, "PASS", report.Verdict)
+}
diff --git a/internal/pipeline/runtime_report.go b/internal/pipeline/runtime_report.go
index 8f3dae97..dfd95e3e 100644
--- a/internal/pipeline/runtime_report.go
+++ b/internal/pipeline/runtime_report.go
@@ -12,6 +12,22 @@ func finalizeVerifyReport(report *VerifyReport, threshold int, requireDataPipeli
report.Critical++
}
}
+ // Path-param probes catch the failure mode where a nested leaf command
+ // exits with a "<positional> is required" usage error even though the
+ // caller supplied the positionals. The existing per-command matrix
+ // only probes top-level commands, so this gap let a generator codegen
+ // bug (mis-indexed args[] in path-param emit) ship silently with
+ // verify reporting 100% pass. Each failing probe is a critical
+ // failure: the command is unusable as shipped.
+ for _, probe := range report.PathParamProbes {
+ report.Total++
+ if probe.Passed {
+ report.Passed++
+ continue
+ }
+ report.Failed++
+ report.Critical++
+ }
if report.Total > 0 {
report.PassRate = float64(report.Passed) / float64(report.Total) * 100
}
diff --git a/internal/pipeline/runtime_structural.go b/internal/pipeline/runtime_structural.go
index b92ed061..e7ca3cc7 100644
--- a/internal/pipeline/runtime_structural.go
+++ b/internal/pipeline/runtime_structural.go
@@ -48,6 +48,7 @@ func runStructuralVerify(cfg VerifyConfig) (*VerifyReport, error) {
report.DataPipelineDetail = "FAIL (version command)"
}
report.Freshness = runFreshnessContractTest(cfg.Dir)
+ report.PathParamProbes = runPathParamProbes(binaryPath, subprocessEnv(), nil)
finalizeVerifyReport(report, cfg.Threshold, false)
← d7ccd850 fix(cli): use Chrome User-Agent for synthetic/cookie/compose
·
back to Cli Printing Press
·
fix(ci): switch queue update_method to merge so fork PRs can 2d0ad5ae →