← back to Cli Printing Press
fix(cli): address remaining ESPN retro findings — verify hints, FTS5, doctor (#179)
9da1cabde05d593c628fda65613a92980dd292cb · 2026-04-12 00:01:54 -0700 · Trevin Chow
Closes out the three remaining work units from the ESPN retro (#153):
- **WU-3 (verify hints):** Replace the hardcoded workflowTestFlags switch with
inferRequiredFlags, which probes each command with no args, parses cobra's
"required flag(s) ... not set" error, and supplies synthetic values keyed by
flag name. Generalizes the mechanism across APIs — previously only GitHub-
shaped commands passed this gate. Kept the positional-args path for
`changelog` since cobra doesn't surface positional requirements through the
required-flags error.
- **WU-4 (FTS5 keywords):** Add tag, tags, label, labels, category, categories,
metadata, notes to the text-field keyword set in collectTextFieldNames. ESPN
surfaced this when "Super Bowl" was unsearchable until a notes column was
added manually.
- **WU-5 (doctor no-auth OK):** Special-case "not required" in the doctor
template's status classifier so public APIs show OK instead of a misleading
WARN.
Adds 22 new verify-hint tests and 5 new FTS5 keyword tests.
Files touched
M internal/generator/schema_builder.goM internal/generator/schema_builder_test.goM internal/generator/templates/doctor.go.tmplM internal/pipeline/runtime.goM internal/pipeline/runtime_test.go
Diff
commit 9da1cabde05d593c628fda65613a92980dd292cb
Author: Trevin Chow <trevin@trevinchow.com>
Date: Sun Apr 12 00:01:54 2026 -0700
fix(cli): address remaining ESPN retro findings — verify hints, FTS5, doctor (#179)
Closes out the three remaining work units from the ESPN retro (#153):
- **WU-3 (verify hints):** Replace the hardcoded workflowTestFlags switch with
inferRequiredFlags, which probes each command with no args, parses cobra's
"required flag(s) ... not set" error, and supplies synthetic values keyed by
flag name. Generalizes the mechanism across APIs — previously only GitHub-
shaped commands passed this gate. Kept the positional-args path for
`changelog` since cobra doesn't surface positional requirements through the
required-flags error.
- **WU-4 (FTS5 keywords):** Add tag, tags, label, labels, category, categories,
metadata, notes to the text-field keyword set in collectTextFieldNames. ESPN
surfaced this when "Super Bowl" was unsearchable until a notes column was
added manually.
- **WU-5 (doctor no-auth OK):** Special-case "not required" in the doctor
template's status classifier so public APIs show OK instead of a misleading
WARN.
Adds 22 new verify-hint tests and 5 new FTS5 keyword tests.
---
internal/generator/schema_builder.go | 2 +
internal/generator/schema_builder_test.go | 63 +++++++++++++++++
internal/generator/templates/doctor.go.tmpl | 3 +
internal/pipeline/runtime.go | 106 ++++++++++++++++++++++++++--
internal/pipeline/runtime_test.go | 85 ++++++++++++++++++++++
5 files changed, 255 insertions(+), 4 deletions(-)
diff --git a/internal/generator/schema_builder.go b/internal/generator/schema_builder.go
index 7d525a73..16040434 100644
--- a/internal/generator/schema_builder.go
+++ b/internal/generator/schema_builder.go
@@ -260,6 +260,8 @@ func collectTextFieldNames(r spec.Resource) []string {
"title": true, "name": true, "description": true,
"body": true, "content": true, "summary": true, "subject": true,
"text": true, "message": true, "comment": true, "note": true,
+ "notes": true, "tag": true, "tags": true, "label": true, "labels": true,
+ "category": true, "categories": true, "metadata": true,
}
seen := make(map[string]bool)
diff --git a/internal/generator/schema_builder_test.go b/internal/generator/schema_builder_test.go
index 615109da..53d2acf7 100644
--- a/internal/generator/schema_builder_test.go
+++ b/internal/generator/schema_builder_test.go
@@ -3,6 +3,7 @@ package generator
import (
"testing"
+ "github.com/mvanhorn/cli-printing-press/internal/spec"
"github.com/stretchr/testify/assert"
)
@@ -35,3 +36,65 @@ func TestToSnakeCase(t *testing.T) {
})
}
}
+
+func TestCollectTextFieldNames(t *testing.T) {
+ // Fields like tag/label/category/metadata should be picked up for FTS5
+ // alongside the core text fields. Motivated by the ESPN retro where
+ // "notes" (event tags) were unsearchable until manually added.
+ mkResource := func(paramNames ...string) spec.Resource {
+ params := make([]spec.Param, 0, len(paramNames))
+ for _, n := range paramNames {
+ params = append(params, spec.Param{Name: n, Type: "string"})
+ }
+ return spec.Resource{
+ Endpoints: map[string]spec.Endpoint{
+ "get": {Params: params},
+ },
+ }
+ }
+
+ tests := []struct {
+ name string
+ params []string
+ wantIncl []string
+ wantExcl []string
+ }{
+ {
+ name: "picks up core text fields",
+ params: []string{"title", "description", "body"},
+ wantIncl: []string{"title", "description", "body"},
+ },
+ {
+ name: "picks up tag-family fields",
+ params: []string{"name", "tag", "tags", "label", "labels"},
+ wantIncl: []string{"name", "tag", "tags", "label", "labels"},
+ },
+ {
+ name: "picks up category and metadata fields",
+ params: []string{"title", "category", "categories", "metadata"},
+ wantIncl: []string{"title", "category", "categories", "metadata"},
+ },
+ {
+ name: "picks up notes and note",
+ params: []string{"note", "notes"},
+ wantIncl: []string{"note", "notes"},
+ },
+ {
+ name: "ignores non-text fields",
+ params: []string{"id", "created_at", "price"},
+ wantExcl: []string{"id", "created_at", "price"},
+ },
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ got := collectTextFieldNames(mkResource(tt.params...))
+ for _, want := range tt.wantIncl {
+ assert.Contains(t, got, want)
+ }
+ for _, exc := range tt.wantExcl {
+ assert.NotContains(t, got, exc)
+ }
+ })
+ }
+}
diff --git a/internal/generator/templates/doctor.go.tmpl b/internal/generator/templates/doctor.go.tmpl
index 29edc738..a24baa05 100644
--- a/internal/generator/templates/doctor.go.tmpl
+++ b/internal/generator/templates/doctor.go.tmpl
@@ -211,6 +211,9 @@ func newDoctorCmd(flags *rootFlags) *cobra.Command {
indicator := green("OK")
if strings.Contains(s, "error") || strings.Contains(s, "not configured") || strings.Contains(s, "unreachable") || strings.Contains(s, "invalid") {
indicator = red("FAIL")
+ } else if s == "not required" {
+ // Public APIs: no auth needed is a healthy state, not a warning.
+ indicator = green("OK")
} else if strings.Contains(s, "not ") || strings.Contains(s, "skipped") || strings.Contains(s, "inferred") {
indicator = yellow("WARN")
}
diff --git a/internal/pipeline/runtime.go b/internal/pipeline/runtime.go
index 82adb864..16953fde 100644
--- a/internal/pipeline/runtime.go
+++ b/internal/pipeline/runtime.go
@@ -625,10 +625,11 @@ func classifyCommandKind(cmd *discoveredCommand, spec *openAPISpec) {
}
// workflowTestFlags returns flags needed for workflow commands that require --org or --repo.
+// Retained for explicit positional-arg patterns (e.g., changelog takes two positional
+// args, not flags — cobra won't surface them through the "required flag(s) not set"
+// error). Flag-shaped requirements are now discovered dynamically via inferRequiredFlags.
func workflowTestFlags(cmdName string) []string {
switch cmdName {
- case "pr-triage", "stale", "actions-health", "security", "contributors":
- return []string{"--repo", "mock-owner/mock-repo"}
case "changelog":
return []string{"mock-owner", "mock-repo", "--since", "v0.0.1"}
default:
@@ -636,6 +637,98 @@ func workflowTestFlags(cmdName string) []string {
}
}
+// requiredFlagsRe matches cobra's standard "required flag(s) ... not set" error.
+// Cobra emits the flag names quoted, comma-separated: required flag(s) "event", "year" not set
+var requiredFlagsRe = regexp.MustCompile(`required flag\(s\) ((?:"[^"]+"(?:, )?)+) not set`)
+
+// flagNameRe extracts quoted flag names from the required-flags error payload.
+var flagNameRe = regexp.MustCompile(`"([^"]+)"`)
+
+// inferRequiredFlags probes a command by running it with no args, parses cobra's
+// "required flag(s) ... not set" error if present, and returns synthetic --flag value
+// pairs the verifier can use to exercise the command. Returns nil when the command
+// has no required flags (or when probing fails — the caller falls back gracefully).
+func inferRequiredFlags(binary, cmdName string) []string {
+ ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ defer cancel()
+
+ probe := exec.CommandContext(ctx, binary, cmdName)
+ out, _ := probe.CombinedOutput() // error expected when flags are missing
+
+ m := requiredFlagsRe.FindSubmatch(out)
+ if m == nil {
+ return nil
+ }
+
+ nameMatches := flagNameRe.FindAllSubmatch(m[1], -1)
+ if len(nameMatches) == 0 {
+ return nil
+ }
+
+ var args []string
+ for _, nm := range nameMatches {
+ flag := string(nm[1])
+ args = append(args, "--"+flag, syntheticFlagValue(flag))
+ }
+ return args
+}
+
+// syntheticFlagValue maps a required flag name to a synthetic test value. Shares
+// its philosophy with syntheticArgValue but keyed on flag names that appear in
+// "required flag(s)" errors. The mock server doesn't validate values, so any
+// non-empty string of the right shape works.
+func syntheticFlagValue(name string) string {
+ n := strings.ToLower(name)
+ switch n {
+ case "org", "organization", "owner":
+ return "mock-owner"
+ case "repo", "repository":
+ return "mock-owner/mock-repo"
+ case "team", "workspace", "project", "workspace-id", "project-id":
+ return "mock-project"
+ case "user", "username", "user-id", "account", "account-id":
+ return "mock-user"
+ case "event", "event-id", "game", "game-id", "match", "match-id":
+ return "mock-event-123"
+ case "season", "year":
+ return "2026"
+ case "sport", "league", "competition":
+ return "mock-league"
+ case "id", "uid", "uuid":
+ return "mock-id-123"
+ case "ticker", "symbol":
+ return "MOCK"
+ case "region", "location", "city":
+ return "mock-city"
+ case "date", "day":
+ return "2026-04-11"
+ case "since", "from", "start", "start-date":
+ return "2026-01-01"
+ case "until", "to", "end", "end-date":
+ return "2026-12-31"
+ case "query", "q", "search", "term":
+ return "mock-query"
+ case "name", "slug", "key":
+ return "mock-name"
+ case "type", "kind", "category":
+ return "mock-type"
+ case "status", "state":
+ return "active"
+ case "limit", "count", "size":
+ return "10"
+ case "format", "output":
+ return "json"
+ case "url", "endpoint", "base-url":
+ return "https://mock.example.com"
+ case "path", "file", "output-file":
+ return "/tmp/mock-file"
+ case "token", "api-key", "key-id", "secret":
+ return "mock-secret"
+ default:
+ return "mock-value"
+ }
+}
+
// runCommandTests executes the test suite for a single command.
func runCommandTests(binary string, cmd discoveredCommand, mode string, env []string) CommandResult {
result := CommandResult{
@@ -646,8 +739,13 @@ func runCommandTests(binary string, cmd discoveredCommand, mode string, env []st
// Test 1: --help
result.Help = runCLI(binary, []string{cmd.Name, "--help"}, env, 10*time.Second) == nil
- // Get any required flags/args for this command
- extraFlags := workflowTestFlags(cmd.Name)
+ // Get any required flags/args for this command.
+ // First, probe the binary for cobra-declared required flags (generic, spec-agnostic).
+ // Then fall back to the positional-arg map for commands that take bare positionals.
+ extraFlags := inferRequiredFlags(binary, cmd.Name)
+ if extraFlags == nil {
+ extraFlags = workflowTestFlags(cmd.Name)
+ }
// Build positional args + flags for test invocations
buildTestArgs := func(cmdName string, positionalArgs, flags []string, extra ...string) []string {
diff --git a/internal/pipeline/runtime_test.go b/internal/pipeline/runtime_test.go
index ebef0415..1f4ede58 100644
--- a/internal/pipeline/runtime_test.go
+++ b/internal/pipeline/runtime_test.go
@@ -245,6 +245,91 @@ func TestSyntheticArgValue(t *testing.T) {
}
}
+func TestSyntheticFlagValue(t *testing.T) {
+ tests := []struct {
+ name string
+ expected string
+ }{
+ {"org", "mock-owner"},
+ {"repo", "mock-owner/mock-repo"},
+ {"event", "mock-event-123"},
+ {"game-id", "mock-event-123"},
+ {"season", "2026"},
+ {"sport", "mock-league"},
+ {"ticker", "MOCK"},
+ {"date", "2026-04-11"},
+ {"since", "2026-01-01"},
+ {"limit", "10"},
+ {"status", "active"},
+ // Case insensitive
+ {"Event", "mock-event-123"},
+ {"ORG", "mock-owner"},
+ // Unknown falls back to mock-value
+ {"unknown-flag", "mock-value"},
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ assert.Equal(t, tt.expected, syntheticFlagValue(tt.name))
+ })
+ }
+}
+
+func TestRequiredFlagsRegex(t *testing.T) {
+ tests := []struct {
+ name string
+ output string
+ expected []string
+ }{
+ {
+ name: "single flag",
+ output: `Error: required flag(s) "event" not set`,
+ expected: []string{"event"},
+ },
+ {
+ name: "multiple flags",
+ output: `Error: required flag(s) "event", "year" not set`,
+ expected: []string{"event", "year"},
+ },
+ {
+ name: "three flags",
+ output: `Error: required flag(s) "org", "repo", "branch" not set`,
+ expected: []string{"org", "repo", "branch"},
+ },
+ {
+ name: "no required-flags error",
+ output: `Error: unknown command "foo"`,
+ expected: nil,
+ },
+ {
+ name: "surrounded by other output",
+ output: "Usage: cli foo [flags]\n\nError: required flag(s) \"event\" not set\nRun 'cli foo --help'",
+ expected: []string{"event"},
+ },
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ m := requiredFlagsRe.FindStringSubmatch(tt.output)
+ if tt.expected == nil {
+ assert.Nil(t, m)
+ return
+ }
+ require.NotNil(t, m)
+ nameMatches := flagNameRe.FindAllStringSubmatch(m[1], -1)
+ got := make([]string, 0, len(nameMatches))
+ for _, nm := range nameMatches {
+ got = append(got, nm[1])
+ }
+ assert.Equal(t, tt.expected, got)
+ })
+ }
+}
+
+func TestInferRequiredFlags_UnknownBinaryReturnsNil(t *testing.T) {
+ // Probing a non-existent binary should return nil cleanly, not panic or hang.
+ result := inferRequiredFlags("/nonexistent/binary/path", "somecmd")
+ assert.Nil(t, result)
+}
+
func TestParseSQLOutput(t *testing.T) {
tests := []struct {
name string
← 096950fd feat(cli): wrapper-only catalog entries for reverse-engineer
·
back to Cli Printing Press
·
chore(main): release 2.0.0 (#170) 575a3f4f →