← back to Cli Printing Press
fix(cli): drop shell line comments + skip happy_path on missing file fixtures (#1378)
295fd03ff06ba754dc0c8a1775497a31f729a7de · 2026-05-14 01:18:14 -0700 · Trevin Chow
* fix(cli): drop shell line comments + skip happy_path on missing file fixtures
Cobra Example fields routinely use trailing `# explanation` comments.
shellargs.Split treated `#` as a literal, so dogfood's happy_path ran the
binary with the comment text as positional args and reported FAIL. Examples
that reference a CSV / file flag whose target isn't on disk produced the
same FAIL noise.
Tighten shellargs.Split to honour `#` as a line comment at the start of a
word (quoted and mid-token `#` still pass through), and add a happy_path
guard that emits `file fixture required` skip for `--file` / `--csv` flags
whose values don't resolve relative to the dogfood CWD.
Closes #1125
* fix(cli): anchor flagNameSuggestsFile on a separator to dodge --profile
strings.Contains(n, "file") matched --profile because "profile" includes
the substring "file" at positions 3-6. That would silently skip happy_path
and json_fidelity for any command whose Example uses --profile <name>.
Switch to exact match on "file" / "csv" plus `-file` / `_file` / `-csv` /
`_csv` suffix anchors. Still catches --input-file, --output_file,
--import-csv shapes from real CLIs. --file-format (takes a format
identifier, not a path) and --profile no longer trip the guard.
Files touched
M internal/pipeline/live_dogfood.goM internal/pipeline/live_dogfood_test.goM internal/shellargs/shellargs.goM internal/shellargs/shellargs_test.go
Diff
commit 295fd03ff06ba754dc0c8a1775497a31f729a7de
Author: Trevin Chow <trevin@trevinchow.com>
Date: Thu May 14 01:18:14 2026 -0700
fix(cli): drop shell line comments + skip happy_path on missing file fixtures (#1378)
* fix(cli): drop shell line comments + skip happy_path on missing file fixtures
Cobra Example fields routinely use trailing `# explanation` comments.
shellargs.Split treated `#` as a literal, so dogfood's happy_path ran the
binary with the comment text as positional args and reported FAIL. Examples
that reference a CSV / file flag whose target isn't on disk produced the
same FAIL noise.
Tighten shellargs.Split to honour `#` as a line comment at the start of a
word (quoted and mid-token `#` still pass through), and add a happy_path
guard that emits `file fixture required` skip for `--file` / `--csv` flags
whose values don't resolve relative to the dogfood CWD.
Closes #1125
* fix(cli): anchor flagNameSuggestsFile on a separator to dodge --profile
strings.Contains(n, "file") matched --profile because "profile" includes
the substring "file" at positions 3-6. That would silently skip happy_path
and json_fidelity for any command whose Example uses --profile <name>.
Switch to exact match on "file" / "csv" plus `-file` / `_file` / `-csv` /
`_csv` suffix anchors. Still catches --input-file, --output_file,
--import-csv shapes from real CLIs. --file-format (takes a format
identifier, not a path) and --profile no longer trip the guard.
---
internal/pipeline/live_dogfood.go | 79 +++++++++-
internal/pipeline/live_dogfood_test.go | 266 +++++++++++++++++++++++++++++++++
internal/shellargs/shellargs.go | 10 ++
internal/shellargs/shellargs_test.go | 13 ++
4 files changed, 366 insertions(+), 2 deletions(-)
diff --git a/internal/pipeline/live_dogfood.go b/internal/pipeline/live_dogfood.go
index 8fc35106..20255f94 100644
--- a/internal/pipeline/live_dogfood.go
+++ b/internal/pipeline/live_dogfood.go
@@ -41,6 +41,7 @@ const reasonMutatingDryRunOnly = "mutating command dry-run only"
const reasonMutatingErrorPath = "mutating command; error_path would call live API without --dry-run"
const reasonNoLiveSignal = "no live happy/json pass; credential-unavailable skips cannot certify acceptance"
const reasonUnavailableRunnerCredentials = "unavailable for runner credentials"
+const reasonFileFixtureRequired = "file fixture required"
type LiveDogfoodOptions struct {
CLIDir string
@@ -674,13 +675,20 @@ func runLiveDogfoodCommand(command liveDogfoodCommand, ctx resolveCtx) []LiveDog
mutating := liveDogfoodCommandMutates(command)
useDryRun := mutating && commandSupportsDryRun(command.Help)
+ fixtureSkip := happyPathFileFixtureSkip(happyArgs, ctx.cliDir)
resolvedArgs, resolveSkipped, resolveReason := resolveCommandPositionals(command, happyArgs, ctx)
- if resolveSkipped {
+ switch {
+ case fixtureSkip != "":
+ results = append(results,
+ skippedLiveDogfoodResult(commandName, LiveDogfoodTestHappy, fixtureSkip),
+ skippedLiveDogfoodResult(commandName, LiveDogfoodTestJSON, fixtureSkip),
+ )
+ case resolveSkipped:
results = append(results,
skippedLiveDogfoodResult(commandName, LiveDogfoodTestHappy, resolveReason),
skippedLiveDogfoodResult(commandName, LiveDogfoodTestJSON, resolveReason),
)
- } else {
+ default:
happyArgs = resolvedArgs
runArgs := happyArgs
@@ -978,6 +986,73 @@ func endpointTargetsAuthResource(endpoint, path string) bool {
})
}
+// happyPathFileFixtureSkip returns a skip reason when the parsed Example
+// references a file-flag value that doesn't exist on disk relative to
+// cliDir. Flag names containing "file" or "csv" trigger the check; the
+// motivating cases are `--file accounts.csv` / `--csv prospects.csv` shapes
+// where the example would otherwise fail with `open <path>: no such file
+// or directory`, masking the signal that the command is callable.
+func happyPathFileFixtureSkip(args []string, cliDir string) string {
+ for i := 0; i < len(args); i++ {
+ a := args[i]
+ if !strings.HasPrefix(a, "--") {
+ continue
+ }
+ name := strings.TrimPrefix(a, "--")
+ var value string
+ if eq := strings.IndexByte(name, '='); eq >= 0 {
+ value = name[eq+1:]
+ name = name[:eq]
+ } else if i+1 < len(args) && !strings.HasPrefix(args[i+1], "-") {
+ value = args[i+1]
+ i++
+ }
+ if !flagNameSuggestsFile(name) {
+ continue
+ }
+ if value == "" || strings.Contains(value, "://") {
+ continue
+ }
+ if fileExistsRelativeTo(value, cliDir) {
+ continue
+ }
+ return fmt.Sprintf("%s: --%s %s", reasonFileFixtureRequired, name, value)
+ }
+ return ""
+}
+
+func flagNameSuggestsFile(name string) bool {
+ n := strings.ToLower(name)
+ if n == "file" || n == "csv" {
+ return true
+ }
+ // Anchor on a separator so `--profile` (contains "file") and similar
+ // non-file flags don't trigger spurious skips. Common shapes covered:
+ // `--input-file`, `--output_file`, `--import-csv`, `--config-csv`.
+ return strings.HasSuffix(n, "-file") || strings.HasSuffix(n, "_file") ||
+ strings.HasSuffix(n, "-csv") || strings.HasSuffix(n, "_csv")
+}
+
+func fileExistsRelativeTo(p, cliDir string) bool {
+ if p == "" {
+ return false
+ }
+ if filepath.IsAbs(p) {
+ _, err := os.Stat(p)
+ return err == nil
+ }
+ candidates := []string{p}
+ if cliDir != "" {
+ candidates = append(candidates, filepath.Join(cliDir, p))
+ }
+ for _, c := range candidates {
+ if _, err := os.Stat(c); err == nil {
+ return true
+ }
+ }
+ return false
+}
+
func liveDogfoodHappyArgs(command liveDogfoodCommand) ([]string, bool) {
examples := extractExamplesSection(command.Help)
for line := range strings.SplitSeq(examples, "\n") {
diff --git a/internal/pipeline/live_dogfood_test.go b/internal/pipeline/live_dogfood_test.go
index bfd91a44..8bb1808b 100644
--- a/internal/pipeline/live_dogfood_test.go
+++ b/internal/pipeline/live_dogfood_test.go
@@ -166,6 +166,272 @@ func TestLiveDogfoodCommandMutatesPrefersEndpointMethod(t *testing.T) {
}))
}
+func TestHappyPathFileFixtureSkip(t *testing.T) {
+ t.Parallel()
+
+ cliDir := t.TempDir()
+ existing := filepath.Join(cliDir, "fixture.csv")
+ require.NoError(t, os.WriteFile(existing, []byte("header\nvalue\n"), 0o600))
+
+ cases := []struct {
+ name string
+ args []string
+ wantSkip bool
+ wantPrefix string
+ }{
+ {
+ name: "no file flag",
+ args: []string{"sync", "--limit", "5"},
+ wantSkip: false,
+ },
+ {
+ name: "missing csv fixture",
+ args: []string{"vet", "--csv", "prospects.csv"},
+ wantSkip: true,
+ wantPrefix: "file fixture required: --csv prospects.csv",
+ },
+ {
+ name: "missing file fixture",
+ args: []string{"import-csv", "--file", "accounts.csv"},
+ wantSkip: true,
+ wantPrefix: "file fixture required: --file accounts.csv",
+ },
+ {
+ name: "missing fixture via --flag=value form",
+ args: []string{"import-csv", "--file=accounts.csv"},
+ wantSkip: true,
+ wantPrefix: "file fixture required: --file accounts.csv",
+ },
+ {
+ name: "existing fixture in cliDir",
+ args: []string{"vet", "--csv", "fixture.csv"},
+ wantSkip: false,
+ },
+ {
+ name: "URL value does not trigger skip",
+ args: []string{"upload", "--file", "https://example.com/data.csv"},
+ wantSkip: false,
+ },
+ {
+ name: "unrelated flag name ignored",
+ args: []string{"resolve", "--query", "anything.csv"},
+ wantSkip: false,
+ },
+ {
+ name: "case-insensitive flag match",
+ args: []string{"upload", "--CSV", "missing.csv"},
+ wantSkip: true,
+ },
+ {
+ // --profile contains "file" as a substring but is not a file
+ // flag; spurious skips here would silently drop test signal.
+ name: "profile flag does not trigger skip",
+ args: []string{"deploy", "--profile", "staging"},
+ wantSkip: false,
+ },
+ {
+ name: "hyphenated suffix matches",
+ args: []string{"upload", "--input-file", "missing.txt"},
+ wantSkip: true,
+ },
+ {
+ name: "underscore suffix matches",
+ args: []string{"upload", "--output_file", "missing.txt"},
+ wantSkip: true,
+ },
+ {
+ name: "csv suffix matches",
+ args: []string{"import", "--import-csv", "missing.csv"},
+ wantSkip: true,
+ },
+ {
+ // --file-format takes a format identifier (csv/json), not a path.
+ // Greptile's suggestion correctly excludes the prefix shape.
+ name: "file prefix without suffix anchor does not match",
+ args: []string{"export", "--file-format", "csv"},
+ wantSkip: false,
+ },
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ got := happyPathFileFixtureSkip(tc.args, cliDir)
+ if tc.wantSkip {
+ assert.NotEmpty(t, got, "expected skip reason")
+ if tc.wantPrefix != "" {
+ assert.Equal(t, tc.wantPrefix, got)
+ }
+ } else {
+ assert.Empty(t, got, "expected no skip")
+ }
+ })
+ }
+}
+
+func TestRunLiveDogfoodHappyPathHandlesShellCommentInExample(t *testing.T) {
+ if runtime.GOOS == "windows" {
+ t.Skip("test uses a shell script as the fake binary; skip on Windows")
+ }
+
+ dir, binaryName := writeLiveDogfoodShellCommentScript(t)
+ report, err := RunLiveDogfood(LiveDogfoodOptions{
+ CLIDir: dir,
+ BinaryName: binaryName,
+ Level: "full",
+ Timeout: 2 * time.Second,
+ })
+ require.NoError(t, err)
+
+ happy := findResultByCommandKind(report, "sync", LiveDogfoodTestHappy)
+ require.NotNil(t, happy, "expected sync happy_path result")
+ assert.Equal(t, LiveDogfoodStatusPass, happy.Status,
+ "trailing '# comment' in Cobra Example must not bleed into happy_path argv (reason=%q)", happy.Reason)
+ assert.Equal(t, []string{"sync"}, happy.Args,
+ "happy_path argv must contain only the subcommand path, not the comment text")
+}
+
+func writeLiveDogfoodShellCommentScript(t *testing.T) (dir string, binaryName string) {
+ t.Helper()
+
+ dir = t.TempDir()
+ binaryName = "fixture-pp-cli"
+ writeTestManifestForLiveDogfood(t, dir)
+
+ binPath := filepath.Join(dir, binaryName)
+ script := `#!/bin/sh
+set -u
+
+if [ "$1" = "agent-context" ]; then
+ cat <<'JSON'
+{
+ "commands": [
+ {"name":"sync"}
+ ]
+}
+JSON
+ exit 0
+fi
+
+if [ "$1" = "sync" ] && [ "${2:-}" = "--help" ]; then
+ cat <<'HELP'
+Refresh local cache.
+
+Usage:
+ fixture-pp-cli sync [flags]
+
+Examples:
+ fixture-pp-cli sync # full schema + records refresh
+
+Flags:
+ --json Output JSON
+HELP
+ exit 0
+fi
+
+if [ "$1" = "sync" ]; then
+ # Anything past 'sync' means the example's trailing comment leaked into
+ # argv — fail loudly so the test catches the regression.
+ if [ "$#" -gt 1 ] && [ "${2}" != "--json" ]; then
+ echo "unexpected sync args: $*" >&2
+ exit 4
+ fi
+ if [ "${2:-}" = "--json" ]; then
+ echo '{"synced":true}'
+ exit 0
+ fi
+ echo 'synced'
+ exit 0
+fi
+
+echo "unexpected args: $*" >&2
+exit 99
+`
+ require.NoError(t, os.WriteFile(binPath, []byte(script), 0o755))
+ return dir, binaryName
+}
+
+func TestRunLiveDogfoodSkipsHappyPathOnMissingFileFixture(t *testing.T) {
+ if runtime.GOOS == "windows" {
+ t.Skip("test uses a shell script as the fake binary; skip on Windows")
+ }
+
+ dir, binaryName := writeLiveDogfoodFileFixtureScript(t)
+ report, err := RunLiveDogfood(LiveDogfoodOptions{
+ CLIDir: dir,
+ BinaryName: binaryName,
+ Level: "full",
+ Timeout: 2 * time.Second,
+ })
+ require.NoError(t, err)
+
+ happy := findResultByCommandKind(report, "import-csv", LiveDogfoodTestHappy)
+ require.NotNil(t, happy, "expected import-csv happy_path result")
+ assert.Equal(t, LiveDogfoodStatusSkip, happy.Status)
+ assert.Contains(t, happy.Reason, reasonFileFixtureRequired)
+ assert.Contains(t, happy.Reason, "accounts.csv")
+
+ json := findResultByCommandKind(report, "import-csv", LiveDogfoodTestJSON)
+ require.NotNil(t, json, "expected import-csv json_fidelity result")
+ assert.Equal(t, LiveDogfoodStatusSkip, json.Status)
+ assert.Contains(t, json.Reason, reasonFileFixtureRequired)
+
+ help := findResultByCommandKind(report, "import-csv", LiveDogfoodTestHelp)
+ require.NotNil(t, help, "expected import-csv help result")
+ assert.Equal(t, LiveDogfoodStatusPass, help.Status, "help check must still pass when the only failure is a missing fixture")
+}
+
+func writeLiveDogfoodFileFixtureScript(t *testing.T) (dir string, binaryName string) {
+ t.Helper()
+
+ dir = t.TempDir()
+ binaryName = "fixture-pp-cli"
+ writeTestManifestForLiveDogfood(t, dir)
+
+ binPath := filepath.Join(dir, binaryName)
+ script := `#!/bin/sh
+set -u
+
+if [ "$1" = "agent-context" ]; then
+ cat <<'JSON'
+{
+ "commands": [
+ {"name":"import-csv"}
+ ]
+}
+JSON
+ exit 0
+fi
+
+if [ "$1" = "import-csv" ] && [ "${2:-}" = "--help" ]; then
+ cat <<'HELP'
+Import accounts from a CSV file.
+
+Usage:
+ fixture-pp-cli import-csv [flags]
+
+Examples:
+ fixture-pp-cli import-csv --file accounts.csv
+
+Flags:
+ --file string Path to the CSV file
+ --json Output JSON
+HELP
+ exit 0
+fi
+
+if [ "$1" = "import-csv" ]; then
+ # Without a real fixture, this would error out. The test exercises the
+ # skip path; the actual subprocess should never be invoked for happy_path.
+ echo 'open accounts.csv: no such file or directory' >&2
+ exit 1
+fi
+
+echo "unexpected args: $*" >&2
+exit 99
+`
+ require.NoError(t, os.WriteFile(binPath, []byte(script), 0o755))
+ return dir, binaryName
+}
+
func TestValidLiveDogfoodJSONOutputAcceptsNDJSON(t *testing.T) {
t.Parallel()
diff --git a/internal/shellargs/shellargs.go b/internal/shellargs/shellargs.go
index ea8bd9af..13d4e39e 100644
--- a/internal/shellargs/shellargs.go
+++ b/internal/shellargs/shellargs.go
@@ -49,6 +49,16 @@ func Split(s string) ([]string, error) {
case '"':
inQuote = true
tokenStarted = true
+ case '#':
+ if !tokenStarted {
+ // Shell line comment: '#' at the start of a word drops the
+ // rest of the input. Cobra Example fields routinely append
+ // trailing comments ("sync # full refresh"); without this
+ // branch a downstream consumer runs the binary with the
+ // comment text as positional args.
+ return tokens, nil
+ }
+ current.WriteRune(r)
case ' ', '\t', '\n', '\r':
if tokenStarted {
flush()
diff --git a/internal/shellargs/shellargs_test.go b/internal/shellargs/shellargs_test.go
index 37878aba..f186ba49 100644
--- a/internal/shellargs/shellargs_test.go
+++ b/internal/shellargs/shellargs_test.go
@@ -19,6 +19,19 @@ func TestSplit(t *testing.T) {
{"cli --name foo\\\nbar", []string{"cli", "--name", "foobar"}},
{"cli --name \"foo\\\nbar\"", []string{"cli", "--name", "foobar"}},
{`cli regex \\d+\\s+goat`, []string{"cli", "regex", `\d+\s+goat`}},
+ // Shell line comments: '#' at the start of a word drops the rest of
+ // the input. Cobra Example fields routinely use trailing comments.
+ {`cli sync # full schema + records refresh`, []string{"cli", "sync"}},
+ {`cli # whole-line comment`, []string{"cli"}},
+ {`cli foo --bar baz # explanation`, []string{"cli", "foo", "--bar", "baz"}},
+ // Quoted '#' is part of the value, not a comment.
+ {`cli query "# not a comment"`, []string{"cli", "query", "# not a comment"}},
+ // '#' embedded inside an unquoted token (no leading whitespace) is a
+ // literal character — bash semantics treat '#' as a comment only at
+ // the start of a word.
+ {`cli regex foo#bar`, []string{"cli", "regex", "foo#bar"}},
+ // Escaped '#' is a literal.
+ {`cli \#literal`, []string{"cli", "#literal"}},
}
for _, tc := range cases {
got, err := Split(tc.in)
← fb9125e8 fix(cli): emit structured JSON error from parents invoked wi
·
back to Cli Printing Press
·
fix(cli): descend --select into list-envelope responses (#13 3bcb9d90 →