← back to Cli Printing Press
fix(cli): detect cross-cutting novel features in dogfood scorer (#1364)
a60103844391e29535b12d8a416d558944086192 · 2026-05-13 21:27:08 -0700 · Trevin Chow
* fix(cli): detect cross-cutting novel features in dogfood scorer
The dogfood novel-features detector walks per-command files via the
cobra command tree, so features whose Command field is a parenthetical
marker ("(any) --dry-run", "(internal client behavior)", "(any read
command, default behavior)") were always reported missing — even when
the feature shipped correctly as a global flag or an internal helper.
Real CLIs (e.g. dataforseo) were getting dogfood-FAIL with 6 of 8
"missing" features actually present in source, just not as command
files.
Add a cross-cutting fallback that runs when the regular path/leaf
matcher returns false:
- Features carrying --<flag> tokens are detected by a literal string
scan of internal/cli/*.go for the flag name. Undeclared flags still
report missing (negative test preserved).
- Parenthetical "(internal ...)" markers without a specific flag
resolve when any agent-authored package exists under internal/
(i.e., a subdirectory outside the generator-reserved cli/cliutil/mcp
set with at least one .go file).
- "(any ...)" / "(global ...)" descriptions without a flag are
trusted rather than reported missing, since they describe
behaviour with no anchor point.
Plain command names ("sql", "search") flow through the regular
matcher unchanged — the fallback never masks genuinely-unbuilt
commands.
Closes #1197
* fix(cli): guard cross-cutting fallback against command-plus-flag false positives
Greptile flagged that the cross-cutting fallback's early-exit only
covered the plain bare-command case (`sql`, `search`). A real command
verb followed by a globally-declared flag (`sql --format`) slipped
through the guard and reached the flag-scan branch, where a coincident
quoted occurrence of the flag name in `internal/cli/*.go` would absorb
the otherwise-unbuilt command as "found".
Add a second guard: when there is no paren marker but the command
string has a non-empty leading command path (a verb before any flags),
hand control back to the regular matcher. Pin the fixed behaviour with
a new sub-case in `TestCheckNovelFeatures_CrossCutting` ("sql --dry-run"
against a CLI that declares `dry-run` globally) and a row in the
`TestMatchCrossCuttingFeature` table.
Files touched
M internal/pipeline/dogfood.goM internal/pipeline/dogfood_test.go
Diff
commit a60103844391e29535b12d8a416d558944086192
Author: Trevin Chow <trevin@trevinchow.com>
Date: Wed May 13 21:27:08 2026 -0700
fix(cli): detect cross-cutting novel features in dogfood scorer (#1364)
* fix(cli): detect cross-cutting novel features in dogfood scorer
The dogfood novel-features detector walks per-command files via the
cobra command tree, so features whose Command field is a parenthetical
marker ("(any) --dry-run", "(internal client behavior)", "(any read
command, default behavior)") were always reported missing — even when
the feature shipped correctly as a global flag or an internal helper.
Real CLIs (e.g. dataforseo) were getting dogfood-FAIL with 6 of 8
"missing" features actually present in source, just not as command
files.
Add a cross-cutting fallback that runs when the regular path/leaf
matcher returns false:
- Features carrying --<flag> tokens are detected by a literal string
scan of internal/cli/*.go for the flag name. Undeclared flags still
report missing (negative test preserved).
- Parenthetical "(internal ...)" markers without a specific flag
resolve when any agent-authored package exists under internal/
(i.e., a subdirectory outside the generator-reserved cli/cliutil/mcp
set with at least one .go file).
- "(any ...)" / "(global ...)" descriptions without a flag are
trusted rather than reported missing, since they describe
behaviour with no anchor point.
Plain command names ("sql", "search") flow through the regular
matcher unchanged — the fallback never masks genuinely-unbuilt
commands.
Closes #1197
* fix(cli): guard cross-cutting fallback against command-plus-flag false positives
Greptile flagged that the cross-cutting fallback's early-exit only
covered the plain bare-command case (`sql`, `search`). A real command
verb followed by a globally-declared flag (`sql --format`) slipped
through the guard and reached the flag-scan branch, where a coincident
quoted occurrence of the flag name in `internal/cli/*.go` would absorb
the otherwise-unbuilt command as "found".
Add a second guard: when there is no paren marker but the command
string has a non-empty leading command path (a verb before any flags),
hand control back to the regular matcher. Pin the fixed behaviour with
a new sub-case in `TestCheckNovelFeatures_CrossCutting` ("sql --dry-run"
against a CLI that declares `dry-run` globally) and a row in the
`TestMatchCrossCuttingFeature` table.
---
internal/pipeline/dogfood.go | 140 ++++++++++++++++++++++++++++-
internal/pipeline/dogfood_test.go | 182 ++++++++++++++++++++++++++++++++++++++
2 files changed, 321 insertions(+), 1 deletion(-)
diff --git a/internal/pipeline/dogfood.go b/internal/pipeline/dogfood.go
index b5140953..99049456 100644
--- a/internal/pipeline/dogfood.go
+++ b/internal/pipeline/dogfood.go
@@ -327,7 +327,13 @@ func checkNovelFeatures(cliDir, researchDir string) NovelFeaturesCheckResult {
}
built := make([]NovelFeature, 0)
for _, nf := range research.NovelFeatures {
- if matchNovelFeature(nf, paths, leaves) {
+ matched := matchNovelFeature(nf, paths, leaves)
+ if !matched {
+ if cm, applied := matchCrossCuttingFeature(nf.Command, cliDir); applied && cm {
+ matched = true
+ }
+ }
+ if matched {
result.Found++
built = append(built, nf)
} else {
@@ -401,6 +407,138 @@ func matchNovelFeature(nf NovelFeature, paths, leaves map[string]bool) bool {
return false
}
+// matchCrossCuttingFeature handles novel features whose command string
+// describes cross-cutting code — global flags or internal helpers —
+// rather than naming a CLI command. The regular path/leaf matcher can't
+// see these because they don't appear in the cobra command tree.
+//
+// Returns (matched, applied):
+// - applied=false means the feature isn't cross-cutting (no flag token,
+// no recognized parenthetical marker); the caller should fall back
+// to the regular matcher.
+// - applied=true, matched=true means a positive signal was found.
+// - applied=true, matched=false means the feature is genuinely missing
+// (e.g., a `--<flag>` named explicitly that isn't declared anywhere
+// in CLI source).
+//
+// Detection strategy is deliberately a "cheap retrofit" (per issue
+// #1197): a literal-string scan of `internal/cli/*.go` for declared
+// flags, and a presence check for agent-authored packages under
+// `internal/` for `(internal ...)` markers. Parenthetical descriptors
+// without a flag to anchor on are trusted rather than reported missing.
+func matchCrossCuttingFeature(cmd, cliDir string) (matched, applied bool) {
+ lc := strings.ToLower(strings.TrimSpace(cmd))
+ flags := extractFlagNames(lc)
+ marker := parenLeadMarker(lc)
+ if marker == "" && len(flags) == 0 {
+ return false, false
+ }
+ // A plain command name followed by a flag ("sql --format") is not a
+ // cross-cutting feature: the verb names the command, and the flag is
+ // only an argument. Without a paren marker the regular path/leaf
+ // matcher is the authority; bailing here keeps the fallback from
+ // silently masking unbuilt commands when a flag name happens to
+ // appear elsewhere in internal/cli/*.go.
+ if marker == "" && commandPath(cmd) != "" {
+ return false, false
+ }
+
+ // Flag-bearing features take the strict path: a declared flag means
+ // found; a named flag with no declaration means genuinely missing.
+ // This preserves the negative-test case for "(any) --nonexistent".
+ if len(flags) > 0 {
+ for _, f := range flags {
+ if flagDeclaredInCLI(f, cliDir) {
+ return true, true
+ }
+ }
+ return false, true
+ }
+
+ // Parenthetical-only feature — no specific flag to anchor on.
+ switch marker {
+ case "internal":
+ return hasAgentInternalPackage(cliDir), true
+ case "global", "any":
+ // Behavioural descriptions like "(any read command, default
+ // behavior)" carry no specific signal. Trust the planner rather
+ // than emit a false-positive missing.
+ return true, true
+ }
+ return false, false
+}
+
+// parenLeadMarker returns the cross-cutting marker word at the start of
+// a feature command — "internal", "any", or "global" — or "" if the
+// feature doesn't begin with one of the known parenthetical markers.
+// Caller passes an already-lowercased and trimmed string.
+func parenLeadMarker(lc string) string {
+ if !strings.HasPrefix(lc, "(") {
+ return ""
+ }
+ inner := lc[1:]
+ end := len(inner)
+ for i, r := range inner {
+ if r == ' ' || r == ')' || r == ',' {
+ end = i
+ break
+ }
+ }
+ switch inner[:end] {
+ case "internal", "any", "global":
+ return inner[:end]
+ }
+ return ""
+}
+
+// flagDeclaredInCLI reports whether name appears as a quoted string
+// literal in any `internal/cli/*.go` file. Cobra registers long-flag
+// names via string literals on Flags()/PersistentFlags() calls, so a
+// `"<name>"` substring match is a reliable cheap signal without parsing
+// Go source.
+func flagDeclaredInCLI(name, cliDir string) bool {
+ if name == "" {
+ return false
+ }
+ needle := `"` + name + `"`
+ for _, f := range listGoFiles(filepath.Join(cliDir, "internal", "cli")) {
+ data, err := os.ReadFile(f)
+ if err != nil {
+ continue
+ }
+ if strings.Contains(string(data), needle) {
+ return true
+ }
+ }
+ return false
+}
+
+// hasAgentInternalPackage reports whether the CLI carries any
+// agent-authored package directory under internal/ — that is, a
+// subdirectory with at least one .go file that isn't the always-emitted
+// command package (cli) or a generator-reserved package (cliutil, mcp).
+// Used to corroborate "(internal ...)" novel-feature claims when no
+// specific keyword is available to pinpoint a package.
+func hasAgentInternalPackage(cliDir string) bool {
+ entries, err := os.ReadDir(filepath.Join(cliDir, "internal"))
+ if err != nil {
+ return false
+ }
+ for _, e := range entries {
+ if !e.IsDir() {
+ continue
+ }
+ switch e.Name() {
+ case "cli", "cliutil", "mcp":
+ continue
+ }
+ if len(listGoFiles(filepath.Join(cliDir, "internal", e.Name()))) > 0 {
+ return true
+ }
+ }
+ return false
+}
+
// matchPath matches a planned path against a set of built paths:
// exact match, or sibling hyphen-prefix (same parent, leaf ↔ leaf-foo).
func matchPath(plan string, paths map[string]bool) bool {
diff --git a/internal/pipeline/dogfood_test.go b/internal/pipeline/dogfood_test.go
index 473b0819..2ccebbfe 100644
--- a/internal/pipeline/dogfood_test.go
+++ b/internal/pipeline/dogfood_test.go
@@ -1164,6 +1164,188 @@ func TestCheckNovelFeatures_ZeroSurvivors(t *testing.T) {
assert.NotContains(t, string(rootData), "planned health")
}
+// TestCheckNovelFeatures_CrossCutting pins the cross-cutting-feature
+// fallback added in #1197: planned features whose Command is a
+// parenthetical marker ("(any) --dry-run", "(internal client behavior)",
+// "(any read command, default behavior)") are detected against
+// rootFlags / agent-authored internal packages rather than reported as
+// missing. Flag-named features still report missing when the flag isn't
+// declared anywhere in CLI source.
+func TestCheckNovelFeatures_CrossCutting(t *testing.T) {
+ // Helper: minimal CLI fixture with a single command file plus a
+ // root.go that declares a few persistent flags as string literals.
+ // Optional agentPkg adds an agent-authored internal/<name>/<name>.go.
+ setupCLI := func(t *testing.T, agentPkg string) string {
+ t.Helper()
+ cliDir := t.TempDir()
+ cliCodeDir := filepath.Join(cliDir, "internal", "cli")
+ require.NoError(t, os.MkdirAll(cliCodeDir, 0o755))
+ writeTestFile(t, filepath.Join(cliCodeDir, "health.go"),
+ `package cli
+func newHealthCmd() *cobra.Command {
+ return &cobra.Command{Use: "health"}
+}`)
+ writeTestFile(t, filepath.Join(cliCodeDir, "root.go"), strings.Join([]string{
+ `package cli`,
+ ``,
+ `func newRootCmd() *cobra.Command {`,
+ ` rootCmd := &cobra.Command{Use: "test-pp-cli"}`,
+ ` rootCmd.PersistentFlags().Bool("dry-run", false, "preview only")`,
+ ` rootCmd.PersistentFlags().String("tier", "", "service tier")`,
+ ` rootCmd.AddCommand(newHealthCmd())`,
+ ` return rootCmd`,
+ `}`,
+ ``,
+ }, "\n"))
+ if agentPkg != "" {
+ agentDir := filepath.Join(cliDir, "internal", agentPkg)
+ require.NoError(t, os.MkdirAll(agentDir, 0o755))
+ writeTestFile(t, filepath.Join(agentDir, "request.go"),
+ "package "+agentPkg+"\n\nfunc DoRequest() {}\n")
+ }
+ return cliDir
+ }
+
+ t.Run("global flag declared on rootCmd resolves (any) marker", func(t *testing.T) {
+ cliDir := setupCLI(t, "")
+ researchDir := t.TempDir()
+ require.NoError(t, writeResearchJSON(&ResearchResult{
+ APIName: "test",
+ NovelFeatures: []NovelFeature{
+ {Name: "Dry-run mode", Command: "(any) --dry-run"},
+ {Name: "Service tier", Command: "(any) --tier standard"},
+ },
+ }, researchDir))
+
+ result := checkNovelFeatures(cliDir, researchDir)
+ assert.Equal(t, 2, result.Planned)
+ assert.Equal(t, 2, result.Found)
+ assert.Empty(t, result.Missing)
+ })
+
+ t.Run("undeclared flag reports missing", func(t *testing.T) {
+ cliDir := setupCLI(t, "")
+ researchDir := t.TempDir()
+ require.NoError(t, writeResearchJSON(&ResearchResult{
+ APIName: "test",
+ NovelFeatures: []NovelFeature{
+ {Name: "Bogus flag", Command: "(any) --nonexistent-flag"},
+ },
+ }, researchDir))
+
+ result := checkNovelFeatures(cliDir, researchDir)
+ assert.Equal(t, 0, result.Found)
+ assert.Equal(t, []string{"(any) --nonexistent-flag"}, result.Missing)
+ })
+
+ t.Run("internal marker resolves when an agent-authored package exists", func(t *testing.T) {
+ cliDir := setupCLI(t, "dfs")
+ researchDir := t.TempDir()
+ require.NoError(t, writeResearchJSON(&ResearchResult{
+ APIName: "test",
+ NovelFeatures: []NovelFeature{
+ {Name: "Adaptive client", Command: "(internal client behavior)"},
+ {Name: "Config resolution", Command: "(internal config resolution)"},
+ },
+ }, researchDir))
+
+ result := checkNovelFeatures(cliDir, researchDir)
+ assert.Equal(t, 2, result.Found)
+ assert.Empty(t, result.Missing)
+ })
+
+ t.Run("internal marker reports missing when no agent package exists", func(t *testing.T) {
+ cliDir := setupCLI(t, "")
+ researchDir := t.TempDir()
+ require.NoError(t, writeResearchJSON(&ResearchResult{
+ APIName: "test",
+ NovelFeatures: []NovelFeature{
+ {Name: "Adaptive client", Command: "(internal client behavior)"},
+ },
+ }, researchDir))
+
+ result := checkNovelFeatures(cliDir, researchDir)
+ assert.Equal(t, 0, result.Found)
+ assert.Equal(t, []string{"(internal client behavior)"}, result.Missing)
+ })
+
+ t.Run("any-marker description without flag trusts the planner", func(t *testing.T) {
+ cliDir := setupCLI(t, "")
+ researchDir := t.TempDir()
+ require.NoError(t, writeResearchJSON(&ResearchResult{
+ APIName: "test",
+ NovelFeatures: []NovelFeature{
+ {Name: "Default behavior", Command: "(any read command, default behavior)"},
+ },
+ }, researchDir))
+
+ result := checkNovelFeatures(cliDir, researchDir)
+ assert.Equal(t, 1, result.Found)
+ assert.Empty(t, result.Missing)
+ })
+
+ t.Run("regular commands still reported missing when unbuilt", func(t *testing.T) {
+ // "sql" and "search" don't have command files and don't carry
+ // cross-cutting markers, so they must still appear as missing —
+ // the cross-cutting fallback must not mask genuinely-unbuilt
+ // commands. The "sql --dry-run" variant pins that a real command
+ // verb followed by a globally-declared flag is still reported
+ // missing: the fallback must defer to the regular matcher for
+ // these shapes, not absorb them just because the flag happens to
+ // be quoted in internal/cli/*.go.
+ cliDir := setupCLI(t, "dfs")
+ researchDir := t.TempDir()
+ require.NoError(t, writeResearchJSON(&ResearchResult{
+ APIName: "test",
+ NovelFeatures: []NovelFeature{
+ {Name: "Local SQL", Command: "sql"},
+ {Name: "Local search", Command: "search"},
+ {Name: "SQL dry-run", Command: "sql --dry-run"},
+ },
+ }, researchDir))
+
+ result := checkNovelFeatures(cliDir, researchDir)
+ assert.Equal(t, 0, result.Found)
+ assert.ElementsMatch(t, []string{"sql", "search", "sql --dry-run"}, result.Missing)
+ })
+}
+
+// TestMatchCrossCuttingFeature covers the helper's edge cases directly:
+// non-cross-cutting inputs return applied=false so the regular path
+// matcher gets to decide; flag detection extracts =value suffixes and
+// surrounding punctuation; case-folding is consistent.
+func TestMatchCrossCuttingFeature(t *testing.T) {
+ cliDir := t.TempDir()
+ cliCodeDir := filepath.Join(cliDir, "internal", "cli")
+ require.NoError(t, os.MkdirAll(cliCodeDir, 0o755))
+ writeTestFile(t, filepath.Join(cliCodeDir, "root.go"),
+ "package cli\n\nfunc init() {\n\t_ = \"dry-run\"\n\t_ = \"agent\"\n}\n")
+
+ cases := []struct {
+ name string
+ cmd string
+ matched bool
+ applied bool
+ }{
+ {"plain command name yields applied=false", "health", false, false},
+ {"space-separated path yields applied=false", "portfolio perf", false, false},
+ {"command followed by flag yields applied=false", "sql --dry-run", false, false},
+ {"bare flag matches declared name", "--dry-run", true, true},
+ {"flag with =value suffix matches", "--dry-run=true", true, true},
+ {"flag with trailing punctuation matches", "--dry-run,", true, true},
+ {"uppercase paren marker normalizes", "(ANY) --dry-run", true, true},
+ {"undeclared flag reports missing", "(any) --no-such-flag", false, true},
+ {"unknown paren marker yields applied=false", "(observation) something", false, false},
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ matched, applied := matchCrossCuttingFeature(tc.cmd, cliDir)
+ assert.Equal(t, tc.applied, applied, "applied")
+ assert.Equal(t, tc.matched, matched, "matched")
+ })
+ }
+}
+
func captureStderr[T any](t *testing.T, captured *string, fn func() T) T {
t.Helper()
← 77fce43f fix(cli): percent-encode path param values in replacePathPar
·
back to Cli Printing Press
·
fix(cli): coerce number-typed --limit param to int (#1370) 80f5bc6c →