From a0f2bb9df389f74a30a953767f4a45453a52a898 Mon Sep 17 00:00:00 2001 From: Matt Leaverton Date: Fri, 24 Apr 2026 17:34:24 -0500 Subject: [PATCH] fix: align --help output with actual flag parser (#6) --- cmd/kilroy/attractor_runs.go | 2 +- cmd/kilroy/help_usage_drift_test.go | 140 ++++++++++++++++++++++++++++ cmd/kilroy/main.go | 8 +- 3 files changed, 146 insertions(+), 4 deletions(-) create mode 100644 cmd/kilroy/help_usage_drift_test.go diff --git a/cmd/kilroy/attractor_runs.go b/cmd/kilroy/attractor_runs.go index fa172889..ad6145db 100644 --- a/cmd/kilroy/attractor_runs.go +++ b/cmd/kilroy/attractor_runs.go @@ -38,7 +38,7 @@ func runsUsage() { fmt.Fprintln(os.Stderr, " kilroy attractor runs list [--json] [--label KEY=VALUE] [--status STATUS] [--graph PATTERN] [--limit N]") fmt.Fprintln(os.Stderr, " kilroy attractor runs show ( | --latest [--label KEY=VALUE]) [--json] [--outputs] [--print ]") fmt.Fprintln(os.Stderr, " kilroy attractor runs wait ( | --latest [--label KEY=VALUE]) [--timeout ] [--interval ] [--json]") - fmt.Fprintln(os.Stderr, " kilroy attractor runs prune [--before YYYY-MM-DD] [--graph PATTERN] [--label KEY=VALUE] [--orphans] [--dry-run | --yes]") + fmt.Fprintln(os.Stderr, " kilroy attractor runs prune [--before YYYY-MM-DD] [--older-than ] [--graph PATTERN] [--label KEY=VALUE] [--orphans] [--dry-run | --yes]") } // runManifest is the subset of manifest.json fields we care about for list/prune. diff --git a/cmd/kilroy/help_usage_drift_test.go b/cmd/kilroy/help_usage_drift_test.go new file mode 100644 index 00000000..b820356b --- /dev/null +++ b/cmd/kilroy/help_usage_drift_test.go @@ -0,0 +1,140 @@ +package main + +import ( + "os" + "regexp" + "sort" + "strings" + "testing" +) + +// internalFlags are flags that are intentionally absent from user-facing +// help text (e.g. for inter-process communication between kilroy processes). +var internalFlags = map[string]bool{ + "--skip-cli-headless-warning": true, +} + +// extractFuncBody returns the source text of the named function by tracking +// brace depth from the opening "func funcName(" declaration. +func extractFuncBody(src, funcName string) (string, bool) { + needle := "func " + funcName + "(" + idx := strings.Index(src, needle) + if idx == -1 { + return "", false + } + depth := 0 + started := false + for i := idx; i < len(src); i++ { + switch src[i] { + case '{': + depth++ + started = true + case '}': + depth-- + if started && depth == 0 { + return src[idx : i+1], true + } + } + } + return src[idx:], true +} + +// caseFlagLineRe matches a `case "--foo"[, "--bar"]*:` arm. +// It deliberately does NOT match `case someVariable:` so internal flags +// represented by named constants are excluded automatically. +var caseFlagLineRe = regexp.MustCompile(`(?m)^\s+case\s+((?:"--[^"]+",?\s*)+):`) + +// quotedFlagRe extracts `--flag` values from a string of quoted tokens. +var quotedFlagRe = regexp.MustCompile(`"(--[^"]+)"`) + +// parseCaseFlags returns the sorted set of --flag names found in case arms +// within the given function body. +func parseCaseFlags(body string) []string { + seen := map[string]bool{} + for _, m := range caseFlagLineRe.FindAllStringSubmatch(body, -1) { + for _, fm := range quotedFlagRe.FindAllStringSubmatch(m[1], -1) { + seen[fm[1]] = true + } + } + var out []string + for f := range seen { + out = append(out, f) + } + sort.Strings(out) + return out +} + +// usageLineRe matches fmt.Fprintln lines that emit " kilroy ..." usage text. +var usageLineRe = regexp.MustCompile(`(?m)fmt\.Fprintln\(os\.Stderr,\s+" kilroy[^"]*"\)`) + +// dashFlagRe extracts --flag-name tokens (including hyphens in the name). +var dashFlagRe = regexp.MustCompile(`--([\w-]+)`) + +// parseUsageFlags returns the set of --flag names mentioned in usage Fprintln +// lines within the given function body. +func parseUsageFlags(body string) map[string]bool { + seen := map[string]bool{} + for _, line := range usageLineRe.FindAllString(body, -1) { + for _, m := range dashFlagRe.FindAllStringSubmatch(line, -1) { + seen["--"+m[1]] = true + } + } + return seen +} + +// checkDrift asserts that every --flag handled by parserFunc is also mentioned +// in the usage text emitted by usageFunc, within the given source file. +// Both functions must reside in the same file. +func checkDrift(t *testing.T, file, parserFunc, usageFunc string) { + t.Helper() + data, err := os.ReadFile(file) + if err != nil { + t.Fatalf("read %s: %v", file, err) + } + src := string(data) + + parserBody, ok := extractFuncBody(src, parserFunc) + if !ok { + t.Fatalf("func %s not found in %s", parserFunc, file) + } + usageBody, ok := extractFuncBody(src, usageFunc) + if !ok { + t.Fatalf("func %s not found in %s", usageFunc, file) + } + + parserFlags := parseCaseFlags(parserBody) + usageFlags := parseUsageFlags(usageBody) + + for _, flag := range parserFlags { + if internalFlags[flag] { + continue + } + if !usageFlags[flag] { + t.Errorf("%s: %s handles %q but %s does not mention it — add it to the help text", + file, parserFunc, flag, usageFunc) + } + } +} + +// TestHelpUsageDrift ensures that every --flag handled by the parser is also +// documented in the corresponding usage/help function. +// +// When you add a new case "--foo": arm to a parser, you MUST also add --foo to +// the usage function — otherwise this test will fail and remind you. +func TestHelpUsageDrift(t *testing.T) { + t.Run("attractorRun", func(t *testing.T) { + checkDrift(t, "main.go", "attractorRun", "usage") + }) + t.Run("attractorRunsList", func(t *testing.T) { + checkDrift(t, "attractor_runs.go", "attractorRunsList", "runsUsage") + }) + t.Run("attractorRunsShow", func(t *testing.T) { + checkDrift(t, "attractor_runs.go", "attractorRunsShow", "runsUsage") + }) + t.Run("attractorRunsWait", func(t *testing.T) { + checkDrift(t, "attractor_runs.go", "attractorRunsWait", "runsUsage") + }) + t.Run("attractorRunsPrune", func(t *testing.T) { + checkDrift(t, "attractor_runs.go", "attractorRunsPrune", "runsUsage") + }) +} diff --git a/cmd/kilroy/main.go b/cmd/kilroy/main.go index a0528b6c..ac243290 100644 --- a/cmd/kilroy/main.go +++ b/cmd/kilroy/main.go @@ -163,7 +163,7 @@ func graphDeclaredInputs(dotSource []byte) bool { func usage() { fmt.Fprintln(os.Stderr, "usage:") fmt.Fprintln(os.Stderr, " kilroy --version") - fmt.Fprintln(os.Stderr, " kilroy [--env-file ] attractor run [--detach] [--validate|--preflight|--test-run] [--allow-test-shim] [--confirm-stale-build] [--no-cxdb] [--force-model ] --graph [--config ] [--run-id ] [--logs-root ]") + fmt.Fprintln(os.Stderr, " kilroy [--env-file ] attractor run (--graph | --package ) [--tmux] [--detach] [--validate|--preflight|--test-run] [--skip-preflight] [--allow-test-shim] [--confirm-stale-build] [--no-cxdb] [--force-model ] [--config ] [--run-id ] [--logs-root ] [--input ] [--prompt-file ] [--workspace ] [--label KEY=VALUE ...]") fmt.Fprintln(os.Stderr, " kilroy attractor resume --logs-root ") fmt.Fprintln(os.Stderr, " kilroy attractor resume --cxdb --context-id ") fmt.Fprintln(os.Stderr, " kilroy attractor resume --run-branch [--repo ]") @@ -175,8 +175,10 @@ func usage() { fmt.Fprintln(os.Stderr, " kilroy attractor serve [--addr ]") fmt.Fprintln(os.Stderr, " kilroy attractor modeldb suggest [--refresh] [--ttl ] [--provider ]") fmt.Fprintln(os.Stderr, " kilroy attractor review --graph [--output ] [--json] [--max-turns ]") - fmt.Fprintln(os.Stderr, " kilroy attractor runs list [--json]") - fmt.Fprintln(os.Stderr, " kilroy attractor runs prune [--before YYYY-MM-DD] [--graph PATTERN] [--label KEY=VALUE] [--orphans] [--dry-run | --yes]") + fmt.Fprintln(os.Stderr, " kilroy attractor runs list [--json] [--label KEY=VALUE] [--status STATUS] [--graph PATTERN] [--limit N]") + fmt.Fprintln(os.Stderr, " kilroy attractor runs show ( | --latest [--label KEY=VALUE]) [--json] [--outputs] [--print ]") + fmt.Fprintln(os.Stderr, " kilroy attractor runs wait ( | --latest [--label KEY=VALUE]) [--timeout ] [--interval ] [--json]") + fmt.Fprintln(os.Stderr, " kilroy attractor runs prune [--before YYYY-MM-DD] [--older-than ] [--graph PATTERN] [--label KEY=VALUE] [--orphans] [--dry-run | --yes]") } func attractor(args []string) {