From 063f9d9eb03e55e60482dd22d6905cd549ebc7b7 Mon Sep 17 00:00:00 2001 From: vadimsolovey Date: Thu, 1 Oct 2026 16:09:01 +0300 Subject: [PATCH 1/3] chore: gofmt help_context_test.go Co-Authored-By: Claude Fable 5.1 --- help_context_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/help_context_test.go b/help_context_test.go index d000045..5a78bbc 100644 --- a/help_context_test.go +++ b/help_context_test.go @@ -199,4 +199,3 @@ func TestDeprecatedCommandHelpShowsNotice(t *testing.T) { t.Fatalf("expected deprecation notice in --help output, got:\n%s", got) } } - From 28fed306446ef82130f98e324ed8067d19f60044 Mon Sep 17 00:00:00 2001 From: vadimsolovey Date: Thu, 1 Oct 2026 16:09:01 +0300 Subject: [PATCH 2/3] fix: help lists only applicable flags, groups question commands, states exit codes Three help-surface fixes found by an unauthenticated agent driving the binary with no docs: Per-command --help no longer appends every persistent presentation flag. help_flags.go classifies the dci persistent flags by what they act on, using the signals the pipeline already keys on: report/query result shape (--chart, --pivot, --flat, --rollup, --max-rows, --rows, --include-empty-rows, --drop-unlabeled-rows, --heatmap), the file-shaped exports (--for-reimport, --output-file inline), page-token paging (--all), list responses (--search), the list-insights view (--include-dismissed), and a resolvable positional argument (--id/--name). The agent-contract flags stay inline everywhere; the table flags that apply to every command fold into one "Output flags" pointer line; --help-full lists all of them. Flags are hidden only while help renders and restored after, so `dci commands --json` is unchanged and every flag is still accepted everywhere. A spec-gated test keeps the report-shaped and file-shaped sets in step with the live description. anomalies-recent and budgets-at-risk now list under the group of the operation they wrap, right after list-anomalies/list-budgets, instead of under "Additional Commands". Root help carries the exit-code and error-envelope contract from error_contract.go in a four-line paragraph, and `dci docs` points at it and the Help Center table. Co-Authored-By: Claude Fable 5.1 --- ai_tui_e2e_test.go | 13 +- docs_command.go | 1 + docs_command_test.go | 2 + help_flags.go | 288 ++++++++++++++++++++++++ help_flags_test.go | 471 ++++++++++++++++++++++++++++++++++++++++ main.go | 42 +++- main_test.go | 195 ++++++++++++++++- question_commands.go | 60 +++++ skills/dci-cli/SKILL.md | 4 +- 9 files changed, 1061 insertions(+), 15 deletions(-) create mode 100644 help_flags.go create mode 100644 help_flags_test.go diff --git a/ai_tui_e2e_test.go b/ai_tui_e2e_test.go index 09fe7b0..ae5c37e 100644 --- a/ai_tui_e2e_test.go +++ b/ai_tui_e2e_test.go @@ -821,6 +821,9 @@ func TestE2EDefaultHelpRoutesBareInvocation(t *testing.T) { help := startTUISession(t, tuiSessionConfig{configDir: configDir, args: []string{}, rawFrame: true}) help.waitFor("(interactive AI session)") // the help screen's own example line + // The exit-code contract is part of the help screen, so a human (or an + // agent reading the same screen) learns it before the first failure. + help.waitFor("Exit codes: 0 success") help.expectCleanExit() } @@ -873,10 +876,18 @@ func TestE2EAPICommandHelpRendersWithoutCredentials(t *testing.T) { }, }) session.waitFor("Returns every widget the account can see.") + // The command's help carries only the persistent flags that act on a + // list (help_flags.go): --search inline, the table flags folded into + // the pointer line, the report flags gone. + session.waitFor("Output flags (apply to every command; add --help-full to list them):") session.expectCleanExit() - if text := session.snapshot(); strings.Contains(text, "Open your browser") || strings.Contains(text, "credentials") { + text := session.snapshot() + if strings.Contains(text, "Open your browser") || strings.Contains(text, "credentials") { t.Fatalf("help started a login or blamed credentials:\n%s", text) } + if !helpListsFlag(text, "search") || helpListsFlag(text, "chart") || helpListsFlag(text, "table-mode") { + t.Fatalf("list-widgets help lists the wrong persistent flags:\n%s", text) + } if atomic.LoadInt32(&authorized) != 0 || atomic.LoadInt32(&authorizeHits) != 0 { t.Fatalf("help authenticated: %d authorized requests, %d OAuth endpoint hits", authorized, authorizeHits) } diff --git a/docs_command.go b/docs_command.go index d1c2d85..106b592 100644 --- a/docs_command.go +++ b/docs_command.go @@ -17,6 +17,7 @@ var docsEntryPoints = [][2]string{ {"Embedded agent guidance", "dci skill "}, {"Find a command for a task", "dci commands --search \"\""}, {"Machine-readable commands", "dci commands --json"}, + {"Exit codes and errors", "dci --help (Exit codes paragraph); full table: https://help.doit.com/docs/cli#exit-codes"}, } func newDocsCommand() *cobra.Command { diff --git a/docs_command_test.go b/docs_command_test.go index 0b81a23..db0e308 100644 --- a/docs_command_test.go +++ b/docs_command_test.go @@ -26,6 +26,8 @@ func TestDocsCommandListsHumanAndAgentEntryPoints(t *testing.T) { "dci skill", "dci commands --search", "dci commands --json", + "Exit codes and errors", + "https://help.doit.com/docs/cli#exit-codes", } { if !strings.Contains(output.String(), expected) { t.Errorf("docs output missing %q", expected) diff --git a/help_flags.go b/help_flags.go new file mode 100644 index 0000000..db74f83 --- /dev/null +++ b/help_flags.go @@ -0,0 +1,288 @@ +// Chapter: per-command flag visibility in help. Every presentation flag is +// a persistent flag of the hidden dci API command, so each command's --help +// used to append all ~30 of them — most inert for that command — and bury +// the command's own flags under --chart, --pivot, --for-reimport and +// friends. This chapter keeps every flag accepted everywhere (no script +// breaks) and hides from a command's help the ones that cannot act on it, +// deciding by the same signals the pipeline uses when it acts: the +// report/query result shape (charts, pivot, rollup, row caps), the +// file-shaped exports (export_pagination.go's fileExportOperations), the +// page-token paging of list commands (pagination.go), the list-insights +// view (response_transform.go), and a resolvable positional argument +// (name_resolution.go). The table-presentation flags that apply to every +// command fold into one pointer line; --help-full lists everything. `dci +// commands --json` is unaffected: flags are hidden only while help renders +// and restored right after. +package main + +import ( + "sort" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// helpFlagScope names what a dci persistent flag acts on. Every flag +// addOutputFlag registers must appear in helpFlagScopes (help_flags_test.go +// enforces it) so a new flag is classified deliberately rather than +// defaulting to inline everywhere. +type helpFlagScope int + +const ( + // helpFlagAlways is the agent contract plus the flags that act on every + // API call: inline in every command's help. + helpFlagAlways helpFlagScope = iota + // helpFlagPresentation shapes table/TOON rendering of any response: + // applies everywhere, so it folds into the one-line pointer instead of + // repeating under every command. + helpFlagPresentation + // helpFlagReport acts on report/query result rows (schema + rows). + helpFlagReport + // helpFlagExport acts on a file-shaped export body (CSV/NDJSON). + helpFlagExport + // helpFlagAllPages is --all: follows page tokens on paged lists and + // walks the time windows of a file export. + helpFlagAllPages + // helpFlagListSearch is --search: filters the items of a list response. + helpFlagListSearch + // helpFlagInsights is the list-insights view's own switch. + helpFlagInsights + // helpFlagResolution governs name resolution of a positional argument. + helpFlagResolution +) + +var helpFlagScopes = map[string]helpFlagScope{ + "output": helpFlagAlways, + "fields": helpFlagAlways, + "exclude": helpFlagAlways, + "full": helpFlagAlways, + "dry-run": helpFlagAlways, + "yes": helpFlagAlways, + "customer-context": helpFlagAlways, + + "table-mode": helpFlagPresentation, + "table-columns": helpFlagPresentation, + "table-width": helpFlagPresentation, + "table-max-col-width": helpFlagPresentation, + "no-truncate": helpFlagPresentation, + "raw-numbers": helpFlagPresentation, + "utc": helpFlagPresentation, + "output-order": helpFlagPresentation, + // --output-file writes any command's response to a file, but it is the + // primary way to keep a file-shaped export: inline there, folded elsewhere. + "output-file": helpFlagPresentation, + + "chart": helpFlagReport, + "pivot": helpFlagReport, + "flat": helpFlagReport, + "rollup": helpFlagReport, + "max-rows": helpFlagReport, + "rows": helpFlagReport, + "include-empty-rows": helpFlagReport, + "drop-unlabeled-rows": helpFlagReport, + "heatmap": helpFlagReport, + + "for-reimport": helpFlagExport, + + "all": helpFlagAllPages, + "search": helpFlagListSearch, + + "include-dismissed": helpFlagInsights, + + "id": helpFlagResolution, + "name": helpFlagResolution, +} + +// reportResultOperations are the operations whose response carries report +// rows (`result.rows`/`results.rows` beside a `schema`) — the shape +// nestedReportRows detects at render time and every report flag acts on. +// Help needs the answer before any response exists, so the set is spelled +// out here; help_flags_test.go checks it against the live spec's response +// schemas (DCI_COMMAND_DOCS_SPEC, fetched in CI). The beta +// get-async-operation-results shares its GA namesake's shape. +var reportResultOperations = map[string]bool{ + "query": true, + "get-report": true, + "get-async-operation-results": true, +} + +// helpFlagPlacement is where a dci persistent flag lands in one command's +// help: inline in the Flags block, folded into the output-flags pointer +// line, or hidden because it cannot act on that command. +type helpFlagPlacement int + +const ( + helpFlagInline helpFlagPlacement = iota + helpFlagCollapsed + helpFlagHidden +) + +func helpFlagPlacementFor(flagName string, cmd *cobra.Command) helpFlagPlacement { + shown := func(applies bool) helpFlagPlacement { + if applies { + return helpFlagInline + } + return helpFlagHidden + } + scope, known := helpFlagScopes[flagName] + if !known { + return helpFlagInline + } + switch scope { + case helpFlagPresentation: + if flagName == "output-file" && helpCommandIsFileExport(cmd) { + return helpFlagInline + } + return helpFlagCollapsed + case helpFlagReport: + return shown(helpCommandIsReportShaped(cmd)) + case helpFlagExport: + return shown(helpCommandIsFileExport(cmd)) + case helpFlagAllPages: + return shown(helpCommandPages(cmd) || helpCommandIsFileExport(cmd)) + case helpFlagListSearch: + // A file body has no items to filter; the preflight rejects --search + // on exports even though they page. + return shown(helpCommandIsList(cmd) && !helpCommandIsFileExport(cmd)) + case helpFlagInsights: + return shown(cmd.Name() == "list-insights") + case helpFlagResolution: + return shown(helpCommandResolvesNames(cmd)) + } + return helpFlagInline +} + +// helpCommandUnderAPI reports whether cmd is a command under the hidden dci +// API command — a GA operation, a question command, or the beta subtree — +// i.e. one that inherits the dci persistent flags. The dci command itself +// and the local root commands (status, login, docs …) render unchanged. +func helpCommandUnderAPI(cmd, dciCmd *cobra.Command) bool { + if cmd == nil || dciCmd == nil || cmd == dciCmd { + return false + } + for parent := cmd.Parent(); parent != nil; parent = parent.Parent() { + if parent == dciCmd { + return true + } + } + return false +} + +func helpCommandIsReportShaped(cmd *cobra.Command) bool { + return reportResultOperations[cmd.Name()] +} + +func helpCommandIsFileExport(cmd *cobra.Command) bool { + _, ok := fileExportOperations[cmd.Name()] + return ok +} + +// helpCommandPages mirrors operationHasQueryParam(op, "pageToken"): restish +// registers every query parameter as a local flag named after it, and the +// question commands register --page-token by hand. +func helpCommandPages(cmd *cobra.Command) bool { + return cmd.LocalFlags().Lookup("page-token") != nil +} + +// helpCommandIsList reports whether the command answers with a collection +// --search can filter: anything that pages, the list-* and search-* +// operations (some return their whole collection in one response), the +// curated list views, and the question commands wrapping a list. +func helpCommandIsList(cmd *cobra.Command) bool { + name := cmd.Name() + if helpCommandPages(cmd) || questionCommandNames[name] { + return true + } + if _, ok := listViews[name]; ok { + return true + } + return strings.HasPrefix(name, "list-") || strings.HasPrefix(name, "search-") +} + +// helpCommandResolvesNames reports whether --id/--name have a positional +// argument to govern. The resolution index is authoritative when loaded; +// help usually renders before any operation metadata is read, so the +// fallback is the command's own usage line: a path parameter after the +// name ("get-budget id") is what resolution acts on. +func helpCommandResolvesNames(cmd *cobra.Command) bool { + if len(resolutionIndex) > 0 { + _, ok := resolutionIndex[cmd.Name()] + return ok + } + return len(strings.Fields(cmd.Use)) > 1 +} + +// applyHelpFlagVisibility hides, for the duration of one help render, the +// dci persistent flags that cannot act on cmd (and the presentation flags +// the pointer line stands in for). It returns the restore function; the +// flags are shared with every other command and with the catalog, so the +// caller defers it. --help-full lists every flag and so does nothing here. +func applyHelpFlagVisibility(cmd *cobra.Command) func() { + dciCmd := findDCICommand() + if helpFullRequested || !helpCommandUnderAPI(cmd, dciCmd) { + return func() {} + } + var hidden []*pflag.Flag + dciCmd.PersistentFlags().VisitAll(func(flag *pflag.Flag) { + if flag.Hidden || helpFlagPlacementFor(flag.Name, cmd) == helpFlagInline { + return + } + flag.Hidden = true + hidden = append(hidden, flag) + }) + return func() { + for _, flag := range hidden { + flag.Hidden = false + } + } +} + +// helpCollapsedFlagsNote renders the pointer line for the presentation +// flags applyHelpFlagVisibility folded away: the usage template appends it +// after the Flags block. Empty for commands outside the API subtree and +// under --help-full, where nothing was folded. +func helpCollapsedFlagsNote(cmd *cobra.Command) string { + dciCmd := findDCICommand() + if helpFullRequested || !helpCommandUnderAPI(cmd, dciCmd) { + return "" + } + labels := make([]string, 0) + dciCmd.PersistentFlags().VisitAll(func(flag *pflag.Flag) { + if helpFlagPlacementFor(flag.Name, cmd) != helpFlagCollapsed { + return + } + label := "--" + flag.Name + if flag.Shorthand != "" { + label = "-" + flag.Shorthand + "/" + label + } + labels = append(labels, label) + }) + if len(labels) == 0 { + return "" + } + sort.Strings(labels) + return "Output flags (apply to every command; add --help-full to list them):\n" + + wrapHelpItems(labels, " ", 100) +} + +// wrapHelpItems joins items with ", " into lines no wider than width, each +// prefixed with indent. +func wrapHelpItems(items []string, indent string, width int) string { + var lines []string + current := indent + for _, item := range items { + if current != indent && len(current)+len(", ")+len(item) > width { + lines = append(lines, current) + current = indent + } + if current != indent { + current += ", " + } + current += item + } + if current != indent { + lines = append(lines, current) + } + return strings.Join(lines, "\n") +} diff --git a/help_flags_test.go b/help_flags_test.go new file mode 100644 index 0000000..711d2df --- /dev/null +++ b/help_flags_test.go @@ -0,0 +1,471 @@ +package main + +import ( + "bytes" + "regexp" + "sort" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// newHelpFlagTestTree builds the offline command tree help renders against: +// the local root commands, the hidden dci API command carrying the +// persistent flags, the question commands, and stand-ins for GA operations +// of every shape the visibility predicates key on. +func newHelpFlagTestTree(t *testing.T) (root, dciCmd *cobra.Command) { + t.Helper() + root = newLocalCommandTree(t) + dciCmd = findDCICommand() + add := func(use, group string, queryFlags ...string) { + command := &cobra.Command{Use: use, GroupID: group, Short: use, Run: func(*cobra.Command, []string) {}} + for _, flag := range queryFlags { + command.Flags().String(flag, "", flag) + } + if group != "" && !dciCmd.ContainsGroup(group) { + dciCmd.AddGroup(&cobra.Group{ID: group, Title: group + " Commands:"}) + } + dciCmd.AddCommand(command) + } + add("invite-user", "Users") + add("query", "Reports") + add("get-budget id", "Budgets") + add("list-budgets", "Budgets", "page-token", "max-results") + add("list-anomalies", "Anomalies", "page-token", "max-results") + add("list-roles", "Roles") // returns its whole collection in one response + add("list-insights", "Insights", "page-token") + add("export-datahub-dataset-records name", "DataHub", "page-token", "start-time", "end-time") + customizeDCIUsage() + + previousIndex := resolutionIndex + resolutionIndex = map[string]resolutionListTarget{} + previousFull := helpFullRequested + helpFullRequested = false + t.Cleanup(func() { + resolutionIndex = previousIndex + helpFullRequested = previousFull + }) + return root, dciCmd +} + +func findAPICommand(t *testing.T, dciCmd *cobra.Command, name string) *cobra.Command { + t.Helper() + for _, command := range dciCmd.Commands() { + if command.Name() == name { + return command + } + } + t.Fatalf("command %q not registered under dci", name) + return nil +} + +// Every persistent flag addOutputFlag registers must be classified, and +// every classification must name a real flag: a new flag gets a deliberate +// scope instead of silently landing inline under every command. +func TestHelpFlagScopesCoverEveryPersistentFlag(t *testing.T) { + _, dciCmd := newHelpFlagTestTree(t) + registered := map[string]bool{} + dciCmd.PersistentFlags().VisitAll(func(flag *pflag.Flag) { + registered[flag.Name] = true + if _, ok := helpFlagScopes[flag.Name]; !ok { + t.Errorf("persistent flag --%s has no help scope in helpFlagScopes", flag.Name) + } + }) + for name := range helpFlagScopes { + if !registered[name] { + t.Errorf("helpFlagScopes names --%s, which addOutputFlag does not register", name) + } + } +} + +func TestHelpFlagPlacementFollowsCommandShape(t *testing.T) { + _, dciCmd := newHelpFlagTestTree(t) + cases := []struct { + command string + inline []string + collapsed []string + hidden []string + }{ + { + command: "invite-user", + inline: []string{"output", "fields", "exclude", "full", "dry-run", "yes", "customer-context"}, + collapsed: []string{"table-mode", "table-columns", "table-width", "table-max-col-width", "no-truncate", "raw-numbers", "utc", "output-order", "output-file"}, + hidden: []string{"chart", "pivot", "flat", "rollup", "max-rows", "rows", "include-empty-rows", "drop-unlabeled-rows", "heatmap", "for-reimport", "all", "search", "include-dismissed", "id", "name"}, + }, + { + command: "query", + inline: []string{"chart", "pivot", "flat", "rollup", "max-rows", "rows", "include-empty-rows", "drop-unlabeled-rows", "heatmap", "output"}, + hidden: []string{"all", "search", "for-reimport", "include-dismissed", "id", "name"}, + }, + { + command: "list-budgets", + inline: []string{"all", "search"}, + hidden: []string{"chart", "max-rows", "for-reimport", "include-dismissed", "id", "name"}, + }, + { + command: "list-roles", + inline: []string{"search"}, + hidden: []string{"all", "chart"}, + }, + { + command: "list-insights", + inline: []string{"include-dismissed", "all", "search"}, + hidden: []string{"chart"}, + }, + { + command: "export-datahub-dataset-records", + inline: []string{"all", "for-reimport", "output-file", "id", "name"}, + hidden: []string{"search", "chart", "max-rows"}, + }, + { + command: "get-budget", + inline: []string{"id", "name"}, + collapsed: []string{"output-file"}, + hidden: []string{"all", "search", "chart"}, + }, + { + command: "budgets-at-risk", + inline: []string{"all", "search", "output"}, + hidden: []string{"chart", "for-reimport", "id"}, + }, + { + command: "anomalies-recent", + inline: []string{"all", "search"}, + hidden: []string{"chart", "max-rows"}, + }, + } + for _, testCase := range cases { + t.Run(testCase.command, func(t *testing.T) { + command := findAPICommand(t, dciCmd, testCase.command) + check := func(names []string, want helpFlagPlacement) { + for _, name := range names { + if got := helpFlagPlacementFor(name, command); got != want { + t.Errorf("--%s on %s: placement %d, want %d", name, testCase.command, got, want) + } + } + } + check(testCase.inline, helpFlagInline) + check(testCase.collapsed, helpFlagCollapsed) + check(testCase.hidden, helpFlagHidden) + }) + } +} + +// The resolution index, once loaded, is authoritative over the usage-line +// heuristic: a command with a positional argument the resolver skips (a +// numeric id) hides --id/--name, and one it covers shows them. +func TestHelpResolutionFlagsFollowTheIndexWhenLoaded(t *testing.T) { + _, dciCmd := newHelpFlagTestTree(t) + getBudget := findAPICommand(t, dciCmd, "get-budget") + if !helpCommandResolvesNames(getBudget) { + t.Fatal("get-budget id: positional argument not recognized without an index") + } + resolutionIndex = map[string]resolutionListTarget{"list-anomalies": {}} + if helpCommandResolvesNames(getBudget) { + t.Fatal("get-budget: the loaded index does not cover it, yet --id/--name apply") + } + resolutionIndex["get-budget"] = resolutionListTarget{} + if !helpCommandResolvesNames(getBudget) { + t.Fatal("get-budget: covered by the loaded index, yet --id/--name hidden") + } +} + +// Hiding is scoped to one render: the flags are shared with every other +// command and with `dci commands --json`, which must keep listing them. +func TestApplyHelpFlagVisibilityRestoresSharedFlags(t *testing.T) { + _, dciCmd := newHelpFlagTestTree(t) + before := catalogFlagsFromFlagSet(dciCmd.PersistentFlags()) + command := findAPICommand(t, dciCmd, "invite-user") + + restore := applyHelpFlagVisibility(command) + if !dciCmd.PersistentFlags().Lookup("chart").Hidden || !dciCmd.PersistentFlags().Lookup("table-mode").Hidden { + t.Fatal("--chart/--table-mode stayed visible on invite-user help") + } + if dciCmd.PersistentFlags().Lookup("output").Hidden { + t.Fatal("--output was hidden on invite-user help") + } + restore() + + dciCmd.PersistentFlags().VisitAll(func(flag *pflag.Flag) { + if flag.Hidden { + t.Errorf("--%s still hidden after the help render", flag.Name) + } + }) + after := catalogFlagsFromFlagSet(dciCmd.PersistentFlags()) + if len(after) != len(before) { + t.Fatalf("catalog flags after help = %d, want %d", len(after), len(before)) + } +} + +func TestHelpFullListsEveryFlag(t *testing.T) { + _, dciCmd := newHelpFlagTestTree(t) + command := findAPICommand(t, dciCmd, "invite-user") + helpFullRequested = true + restore := applyHelpFlagVisibility(command) + defer restore() + dciCmd.PersistentFlags().VisitAll(func(flag *pflag.Flag) { + if flag.Hidden { + t.Errorf("--help-full hid --%s", flag.Name) + } + }) + if note := helpCollapsedFlagsNote(command); note != "" { + t.Fatalf("--help-full rendered the folded-flags note: %q", note) + } +} + +func TestHelpCollapsedFlagsNote(t *testing.T) { + root, dciCmd := newHelpFlagTestTree(t) + + note := helpCollapsedFlagsNote(findAPICommand(t, dciCmd, "invite-user")) + for _, expected := range []string{"Output flags (apply to every command; add --help-full to list them):", "-M/--table-mode", "-C/--table-columns", "-O/--output-file", "--utc", "--output-order"} { + if !strings.Contains(note, expected) { + t.Errorf("invite-user note missing %q:\n%s", expected, note) + } + } + for _, line := range strings.Split(note, "\n")[1:] { + if !strings.HasPrefix(line, " ") || len(line) > 100 { + t.Errorf("note line not indented or too wide: %q", line) + } + } + + if note := helpCollapsedFlagsNote(findAPICommand(t, dciCmd, "export-datahub-dataset-records")); strings.Contains(note, "output-file") { + t.Fatalf("--output-file folded on a file export, where it is inline:\n%s", note) + } + + status, _, err := root.Find([]string{"status"}) + if err != nil { + t.Fatal(err) + } + if note := helpCollapsedFlagsNote(status); note != "" { + t.Fatalf("local command status rendered the API note: %q", note) + } + if note := helpCollapsedFlagsNote(dciCmd); note != "" { + t.Fatalf("the dci command itself rendered the note: %q", note) + } +} + +func TestWrapHelpItems(t *testing.T) { + got := wrapHelpItems([]string{"--aaaa", "--bbbb", "--cccc", "--dddd"}, " ", 20) + want := " --aaaa, --bbbb\n --cccc, --dddd" + if got != want { + t.Fatalf("wrapHelpItems = %q, want %q", got, want) + } + if got := wrapHelpItems(nil, " ", 20); got != "" { + t.Fatalf("wrapHelpItems(nil) = %q, want empty", got) + } +} + +// Rendered help, end to end through the usage template: the command's own +// flags and the agent contract are listed, the inert presentation flags are +// not, and the pointer line stands in for the folded ones. +func TestRenderedAPICommandHelpShowsOnlyApplicableFlags(t *testing.T) { + _, dciCmd := newHelpFlagTestTree(t) + render := func(name string) string { + command := findAPICommand(t, dciCmd, name) + restore := applyHelpFlagVisibility(command) + defer restore() + var output bytes.Buffer + command.SetOut(&output) + if err := command.Help(); err != nil { + t.Fatalf("help %s: %v", name, err) + } + return output.String() + } + + inviteUser := render("invite-user") + for _, flag := range []string{"output", "fields", "exclude", "dry-run", "yes", "customer-context"} { + if !helpListsFlag(inviteUser, flag) { + t.Errorf("invite-user help does not list --%s:\n%s", flag, inviteUser) + } + } + for _, flag := range []string{"chart", "pivot", "rollup", "max-rows", "for-reimport", "all", "search", "include-dismissed", "table-mode", "output-file", "utc"} { + if helpListsFlag(inviteUser, flag) { + t.Errorf("invite-user help lists --%s, which cannot act on it:\n%s", flag, inviteUser) + } + } + if !strings.Contains(inviteUser, "Output flags (apply to every command; add --help-full to list them):") { + t.Fatalf("invite-user help lacks the folded-flags pointer:\n%s", inviteUser) + } + + query := render("query") + for _, flag := range []string{"chart", "pivot", "max-rows", "rollup"} { + if !helpListsFlag(query, flag) { + t.Errorf("query help does not list --%s:\n%s", flag, query) + } + } + if helpListsFlag(query, "all") || helpListsFlag(query, "search") { + t.Fatalf("query help lists the paging flags:\n%s", query) + } +} + +// helpListsFlag reports whether help output has a Flags-block entry for +// --name: a line of its own, optionally with a shorthand. Mentions inside +// another flag's usage text or the folded-flags pointer line do not count. +func helpListsFlag(output, name string) bool { + return regexp.MustCompile(`(?m)^\s+(-[A-Za-z], )?--` + regexp.QuoteMeta(name) + `(\s|$)`).MatchString(output) +} + +func TestAdoptQuestionCommandGroups(t *testing.T) { + _, dciCmd := newHelpFlagTestTree(t) + for _, name := range []string{"budgets-at-risk", "anomalies-recent"} { + if group := findAPICommand(t, dciCmd, name).GroupID; group != "" { + t.Fatalf("%s grouped (%q) before adoption", name, group) + } + } + adoptQuestionCommandGroups(dciCmd) + if got := findAPICommand(t, dciCmd, "budgets-at-risk").GroupID; got != "Budgets" { + t.Fatalf("budgets-at-risk group = %q, want Budgets", got) + } + if got := findAPICommand(t, dciCmd, "anomalies-recent").GroupID; got != "Anomalies" { + t.Fatalf("anomalies-recent group = %q, want Anomalies", got) + } + + // A wrapped operation missing from the loaded spec leaves its question + // command ungrouped rather than pointing at a group cobra would reject. + parent := &cobra.Command{Use: "dci"} + parent.AddGroup(&cobra.Group{ID: "Budgets", Title: "Budgets Commands:"}) + parent.AddCommand(&cobra.Command{Use: "list-budgets", GroupID: "Budgets", Run: func(*cobra.Command, []string) {}}) + parent.AddCommand(newBudgetsAtRiskCommand()) + parent.AddCommand(newAnomaliesRecentCommand()) + adoptQuestionCommandGroups(parent) + if got := findAPICommand(t, parent, "budgets-at-risk").GroupID; got != "Budgets" { + t.Fatalf("budgets-at-risk group = %q, want Budgets", got) + } + if got := findAPICommand(t, parent, "anomalies-recent").GroupID; got != "" { + t.Fatalf("anomalies-recent grouped as %q with list-anomalies absent", got) + } + adoptQuestionCommandGroups(nil) // tolerated +} + +func TestOrderedGroupCommandsPlacesQuestionAfterWrappedOperation(t *testing.T) { + command := func(name, group string) *cobra.Command { + return &cobra.Command{Use: name, GroupID: group, Run: func(*cobra.Command, []string) {}} + } + names := func(commands []*cobra.Command) []string { + result := make([]string, len(commands)) + for index, command := range commands { + result[index] = command.Name() + } + return result + } + // cobra hands the template its commands sorted by name. + commands := []*cobra.Command{ + command("anomalies-recent", "Anomalies"), + command("budgets-at-risk", "Budgets"), + command("get-anomaly", "Anomalies"), + command("list-anomalies", "Anomalies"), + command("list-budgets", "Budgets"), + command("patch-anomaly", "Anomalies"), + } + hidden := command("hidden-anomaly", "Anomalies") + hidden.Hidden = true + commands = append(commands, hidden) + sort.Slice(commands, func(i, j int) bool { return commands[i].Name() < commands[j].Name() }) + + got := names(orderedGroupCommands(commands, "Anomalies")) + want := []string{"get-anomaly", "list-anomalies", "anomalies-recent", "patch-anomaly"} + if strings.Join(got, ",") != strings.Join(want, ",") { + t.Fatalf("Anomalies order = %v, want %v", got, want) + } + if got := names(orderedGroupCommands(commands, "Budgets")); strings.Join(got, ",") != "list-budgets,budgets-at-risk" { + t.Fatalf("Budgets order = %v", got) + } + + // Wrapped operation absent from the group: the question command keeps + // its alphabetical slot instead of vanishing. + without := []*cobra.Command{command("anomalies-recent", "Anomalies"), command("get-anomaly", "Anomalies")} + if got := names(orderedGroupCommands(without, "Anomalies")); strings.Join(got, ",") != "anomalies-recent,get-anomaly" { + t.Fatalf("order without list-anomalies = %v", got) + } + if got := orderedGroupCommands(commands, "Nothing"); len(got) != 0 { + t.Fatalf("empty group returned %v", names(got)) + } +} + +// The report and file-export sets help decides by must match the shapes +// the live spec declares: an operation that gains `result.rows` (or a +// CSV/NDJSON body) must be added here, or its help hides the flags that +// act on it. Spec-gated like the command docs checks (DCI_COMMAND_DOCS_SPEC; +// CI fetches the production description). +func TestHelpFlagShapeSetsMatchSpec(t *testing.T) { + spec := loadCommandDocsSpec(t) + reportShaped := map[string]bool{} + fileShaped := map[string]bool{} + for _, operation := range spec.api.Operations { + raw := spec.rawOperation(operation) + if raw == nil { + continue + } + responses, _ := raw["responses"].(map[string]any) + for status, response := range responses { + if status != "200" && status != "201" { + continue + } + content, _ := spec.resolve(response)["content"].(map[string]any) + for mediaType, body := range content { + if rawPassthroughContentType(mediaType) { + fileShaped[operation.Name] = true + } + bodyMap, _ := body.(map[string]any) + if specSchemaCarriesReportRows(spec, bodyMap["schema"], 0) { + reportShaped[operation.Name] = true + } + } + } + } + if len(reportShaped) == 0 { + t.Fatal("no report-shaped operation found in the spec; the walker is wrong") + } + assertSameNameSet(t, "report-shaped operations (reportResultOperations)", reportShaped, reportResultOperations) + exports := map[string]bool{} + for name := range fileExportOperations { + exports[name] = true + } + assertSameNameSet(t, "file-shaped exports (fileExportOperations)", fileShaped, exports) +} + +// specSchemaCarriesReportRows mirrors nestedReportRows: a `result` or +// `results` object with a `rows` property. +func specSchemaCarriesReportRows(spec commandDocsSpec, node any, depth int) bool { + schema := spec.resolve(node) + if schema == nil || depth > 8 { + return false + } + properties, _ := schema["properties"].(map[string]any) + for _, key := range []string{"result", "results"} { + container := spec.resolve(properties[key]) + if container == nil { + continue + } + if innerProperties, ok := container["properties"].(map[string]any); ok { + if _, hasRows := innerProperties["rows"]; hasRows { + return true + } + } + } + for _, combinator := range []string{"allOf", "oneOf", "anyOf"} { + members, _ := schema[combinator].([]any) + for _, member := range members { + if specSchemaCarriesReportRows(spec, member, depth+1) { + return true + } + } + } + return false +} + +func assertSameNameSet(t *testing.T, what string, fromSpec, inCode map[string]bool) { + t.Helper() + for name := range fromSpec { + if !inCode[name] { + t.Errorf("%s: the spec declares %s but the CLI set omits it", what, name) + } + } + for name := range inCode { + if !fromSpec[name] { + t.Errorf("%s: the CLI set names %s but the spec does not declare that shape", what, name) + } + } +} diff --git a/main.go b/main.go index 1df44b8..ab29166 100644 --- a/main.go +++ b/main.go @@ -1436,15 +1436,17 @@ Examples: Available Commands:{{range $cmds}}{{if (or .IsAvailableCommand (eq .Name "help"))}} {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{else}}{{range $group := .Groups}}{{if hasVisibleCommandsInGroup $cmds $group.ID}} -{{.Title}}{{range $cmds}}{{if (and (eq .GroupID $group.ID) (or .IsAvailableCommand (eq .Name "help")))}} - {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{if not .AllChildCommandsHaveGroup}} +{{.Title}}{{range commandsInGroup $cmds $group.ID}} + {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{if not .AllChildCommandsHaveGroup}} Additional Commands:{{range $cmds}}{{if (and (eq .GroupID "") (or .IsAvailableCommand (eq .Name "help")))}} {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{end}}{{if or .HasAvailableLocalFlags .HasAvailableInheritedFlags}} Flags: {{.LocalFlags.FlagUsages | trimTrailingWhitespaces}}{{if .HasAvailableInheritedFlags}} -{{.InheritedFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{end}}{{if .HasHelpSubCommands}} +{{.InheritedFlags.FlagUsages | trimTrailingWhitespaces}}{{end}}{{end}}{{with helpCollapsedFlagsNote .}} + +{{.}}{{end}}{{if .HasHelpSubCommands}} Additional help topics:{{range .Commands}}{{if .IsAdditionalHelpTopicCommand}} {{rpad .CommandPath .CommandPathPadding}} {{.Short}}{{end}}{{end}}{{end}} @@ -1456,7 +1458,18 @@ const dciLongDescription = "Command-line interface for the Cloud Intelligence™ "In pipes, scripts, and CI, bare `dci` prints this help.\n\n" + "Documentation: https://help.doit.com/docs/cli or run `dci docs` for every entry point.\n" + "AI agents: `dci commands --search \"\"` finds the command for a task described in plain words;\n" + - "`dci skill ` installs usage guidance; `dci commands --json` prints the machine-readable catalog." + "`dci skill ` installs usage guidance; `dci commands --json` prints the machine-readable catalog.\n\n" + + dciExitCodesParagraph + +// dciExitCodesParagraph states the exit-code and error contract from +// error_contract.go in the root help, so an agent reading --help once +// knows what a failure looks like before hitting one. The full table lives +// in the embedded skill and the Help Center; keep this to a few lines and +// keep the three in step. +const dciExitCodesParagraph = "Exit codes: 0 success, 1 failure, 2 usage, 10 authentication, 11 permission, 20 not found,\n" + + "21 conflict, 30 validation (or an unconfirmed destructive command), 40 server, 41 network, 50 rate limited.\n" + + "Errors go to stderr; in agent mode as one JSON line: {\"error\":{\"code\",\"message\",\"hint\",\"retryable\"}}.\n" + + "Full table: https://help.doit.com/docs/cli#exit-codes" var rootExamples = []string{ " dci (interactive AI session)", @@ -1485,13 +1498,14 @@ func customizeDCIUsage() { // Arguments: block from the curated command docs (command_docs.go). cobra.AddTemplateFunc("commandDocArguments", renderCommandDocArguments) cobra.AddTemplateFunc("hasVisibleCommandsInGroup", func(cmds []*cobra.Command, groupID string) bool { - for _, cmd := range cmds { - if cmd.GroupID == groupID && (cmd.IsAvailableCommand() || cmd.Name() == "help") { - return true - } - } - return false + return len(orderedGroupCommands(cmds, groupID)) > 0 }) + // A group's commands in help order: alphabetical, with each question + // command right after the operation it wraps (question_commands.go). + cobra.AddTemplateFunc("commandsInGroup", orderedGroupCommands) + // The pointer line standing in for the folded presentation flags + // (help_flags.go). + cobra.AddTemplateFunc("helpCollapsedFlagsNote", helpCollapsedFlagsNote) dciCmd := findDCICommand() if dciCmd == nil { @@ -1665,6 +1679,10 @@ func setupCompletion() { if cmd == cli.Root { loadAPI() hasAPICommands = len(dciCmd.Commands()) > 0 + // A question command lists with the resource it answers about + // (anomalies-recent beside list-anomalies), not under + // "Additional Commands". + adoptQuestionCommandGroups(dciCmd) // Copy command groups from the API subcommand to root so the // usage template can render grouped commands. for _, g := range dciCmd.Groups() { @@ -1693,6 +1711,10 @@ func setupCompletion() { // Curated examples (command_docs.go) replace restish's schema- // synthesized one; a missing or draft doc leaves it in place. defer applyCommandDocHelp(cmd)() + // Only the dci persistent flags that can act on this command stay + // in its Flags block (help_flags.go); restored right after the + // render, since the flags are shared with every other command. + defer applyHelpFlagVisibility(cmd)() defaultHelp(cmd, args) if cmd == cli.Root && !hasAPICommands { hint := "\n! To get started, authenticate with: dci login (or set DCI_API_KEY)\n\n" diff --git a/main_test.go b/main_test.go index 1f18ecd..bbeb993 100644 --- a/main_test.go +++ b/main_test.go @@ -1796,6 +1796,10 @@ func seedExpiredOAuthSession(t *testing.T, cacheDir string, staleSpec bool) { } } +// helpWithoutCredentialsSpec is the hermetic description behind the help +// tests: one operation of every shape the help surface treats differently +// — a paged list (and the question commands wrapping two of them), a +// create with a body, the report-shaped query, and the file-shaped export. const helpWithoutCredentialsSpec = `{ "openapi": "3.0.0", "info": {"title": "DCI test", "version": "1.0.0"}, @@ -1805,10 +1809,64 @@ const helpWithoutCredentialsSpec = `{ "operationId": "list-budgets", "summary": "List budgets", "description": "Returns the budgets your account can see.", - "tags": ["budgets"], - "parameters": [{"name": "maxResults", "in": "query", "schema": {"type": "integer"}}], + "tags": ["Budgets"], + "parameters": [ + {"name": "maxResults", "in": "query", "schema": {"type": "integer"}}, + {"name": "pageToken", "in": "query", "schema": {"type": "string"}} + ], "responses": {"200": {"description": "OK"}} } + }, + "/anomalies": { + "get": { + "operationId": "list-anomalies", + "summary": "List anomalies", + "tags": ["Anomalies"], + "parameters": [ + {"name": "maxResults", "in": "query", "schema": {"type": "integer"}}, + {"name": "pageToken", "in": "query", "schema": {"type": "string"}} + ], + "responses": {"200": {"description": "OK"}} + } + }, + "/anomalies/{id}": { + "get": { + "operationId": "get-anomaly", + "summary": "Retrieve an anomaly", + "tags": ["Anomalies"], + "parameters": [{"name": "id", "in": "path", "required": true, "schema": {"type": "string"}}], + "responses": {"200": {"description": "OK"}} + } + }, + "/users/invite": { + "post": { + "operationId": "invite-user", + "summary": "Invite a user", + "tags": ["Users"], + "requestBody": {"content": {"application/json": {"schema": {"type": "object", "properties": {"email": {"type": "string"}}}}}}, + "responses": {"200": {"description": "OK"}} + } + }, + "/query": { + "post": { + "operationId": "query", + "summary": "Run a query", + "tags": ["Reports"], + "requestBody": {"content": {"application/json": {"schema": {"type": "object"}}}}, + "responses": {"200": {"description": "OK"}} + } + }, + "/datahub/datasets/{name}/records": { + "get": { + "operationId": "export-datahub-dataset-records", + "summary": "Export dataset records", + "tags": ["DataHub"], + "parameters": [ + {"name": "name", "in": "path", "required": true, "schema": {"type": "string"}}, + {"name": "pageToken", "in": "query", "schema": {"type": "string"}} + ], + "responses": {"200": {"description": "OK", "content": {"text/csv": {"schema": {"type": "string"}}}}} + } } } }` @@ -1882,6 +1940,122 @@ func TestHelpRendersWithoutCredentials(t *testing.T) { assertHelpRendered(t, res, server) }) + // Per-command help lists only the dci persistent flags that can act on + // the command (help_flags.go); the ones that apply everywhere fold into + // one pointer line, and --help-full lists all of them. + t.Run("per-command help hides the flags that cannot act on the command", func(t *testing.T) { + environment, server := freshInstall(t, "CLAUDECODE=1") + cases := []struct { + command string + listed []string + absent []string + }{ + { + command: "invite-user", + listed: []string{"output", "fields", "exclude", "dry-run", "yes", "agent", "no-agent", "customer-context"}, + absent: []string{"chart", "pivot", "flat", "rollup", "max-rows", "rows", "include-empty-rows", "for-reimport", "output-file", "all", "search", "include-dismissed", "id", "name", "table-mode", "table-columns", "utc"}, + }, + { + command: "anomalies-recent", + listed: []string{"window", "severity", "max-results", "page-token", "all", "search", "output"}, + absent: []string{"chart", "pivot", "max-rows", "for-reimport", "output-file", "include-dismissed", "id", "table-mode"}, + }, + { + command: "query", + listed: []string{"chart", "pivot", "flat", "rollup", "max-rows", "rows", "output"}, + absent: []string{"all", "search", "for-reimport", "include-dismissed", "id", "table-mode"}, + }, + { + command: "export-datahub-dataset-records", + listed: []string{"all", "for-reimport", "output-file", "id", "name", "page-token"}, + absent: []string{"search", "chart", "max-rows", "include-dismissed"}, + }, + } + for _, testCase := range cases { + res := run(t, environment, testCase.command, "--help") + assertHelpRendered(t, res, server) + for _, flag := range testCase.listed { + if !helpListsFlag(res.output, flag) { + t.Errorf("%s --help does not list --%s:\n%s", testCase.command, flag, res.output) + } + } + for _, flag := range testCase.absent { + if helpListsFlag(res.output, flag) { + t.Errorf("%s --help lists --%s, which cannot act on it:\n%s", testCase.command, flag, res.output) + } + } + if !strings.Contains(res.output, "Output flags (apply to every command; add --help-full to list them):") { + t.Errorf("%s --help lacks the folded-flags pointer:\n%s", testCase.command, res.output) + } + } + + full := run(t, environment, "invite-user", "--help-full") + assertHelpRendered(t, full, server) + for _, flag := range []string{"chart", "for-reimport", "all", "table-mode", "output-file"} { + if !helpListsFlag(full.output, flag) { + t.Errorf("invite-user --help-full does not list --%s:\n%s", flag, full.output) + } + } + if strings.Contains(full.output, "Output flags (apply to every command") { + t.Fatalf("--help-full still folds flags:\n%s", full.output) + } + }) + + // Question commands list under the resource they answer about, right + // after the operation they wrap, not in the ungrouped tail. + t.Run("root help groups question commands with their resource", func(t *testing.T) { + // Root help lists API commands only once a spec cache exists (a + // fresh install stays offline), so seed the post-login state. The + // hermetic server stays the API base through apis.json alone: a + // DCI_API_BASE_URL override redirects the CLI to an isolated cache + // dir that never holds a description, and root help would stay + // offline. + environment, server := freshInstall(t, "CLAUDECODE=1") + seedExpiredOAuthSession(t, filepath.Join(strings.TrimPrefix(environment[0], "HOME="), "cache"), true) + environment = slices.DeleteFunc(environment, func(entry string) bool { return strings.HasPrefix(entry, "DCI_API_BASE_URL=") }) + res := run(t, environment, "--help") + assertHelpRendered(t, res, server) + lines := strings.Split(res.output, "\n") + follows := func(operation, question string) { + t.Helper() + for index, line := range lines { + if strings.HasPrefix(line, " "+operation+" ") && index+1 < len(lines) { + if !strings.HasPrefix(lines[index+1], " "+question+" ") { + t.Errorf("%s is not listed right after %s:\n%s\n%s", question, operation, line, lines[index+1]) + } + return + } + } + t.Errorf("%s not listed in root help:\n%s", operation, res.output) + } + follows("list-budgets", "budgets-at-risk") + follows("list-anomalies", "anomalies-recent") + if tail := rootHelpSection(res.output, "Additional Commands:"); strings.Contains(tail, "anomalies-recent") || strings.Contains(tail, "budgets-at-risk") { + t.Fatalf("question commands still listed under Additional Commands:\n%s", tail) + } + if anomalies := rootHelpSection(res.output, "Anomalies Commands"); !strings.Contains(anomalies, "anomalies-recent") { + t.Fatalf("anomalies-recent missing from the Anomalies group:\n%s", anomalies) + } + }) + + // The exit-code and error-envelope contract is readable from --help + // alone, before the first failure. + t.Run("root help states the exit-code contract", func(t *testing.T) { + environment, server := freshInstall(t, "CLAUDECODE=1") + res := run(t, environment, "--help") + assertHelpRendered(t, res, server) + for _, expected := range []string{ + "Exit codes: 0 success, 1 failure, 2 usage, 10 authentication, 11 permission, 20 not found,", + "21 conflict, 30 validation (or an unconfirmed destructive command), 40 server, 41 network, 50 rate limited.", + `{"error":{"code","message","hint","retryable"}}`, + "https://help.doit.com/docs/cli#exit-codes", + } { + if !strings.Contains(res.output, expected) { + t.Errorf("root help missing %q:\n%s", expected, res.output) + } + } + }) + t.Run("API command help with an expired session, agent mode", func(t *testing.T) { // The reported repro: a login that lapsed weeks ago, refresh token // included, and a spec cache past its stamp. Help used to die on @@ -2011,6 +2185,23 @@ func assertRootHelpBranded(t *testing.T, out string) { } } +// rootHelpSection returns the block of root help that starts at the line +// containing header and runs to the next blank line; empty when absent. +func rootHelpSection(output, header string) string { + lines := strings.Split(output, "\n") + for index, line := range lines { + if !strings.Contains(line, header) { + continue + } + end := index + 1 + for end < len(lines) && strings.TrimSpace(lines[end]) != "" { + end++ + } + return strings.Join(lines[index:end], "\n") + } + return "" +} + func assertPrivateFilePerms(t *testing.T, path string) { t.Helper() diff --git a/question_commands.go b/question_commands.go index 42ccfaa..e1e5a7f 100644 --- a/question_commands.go +++ b/question_commands.go @@ -49,6 +49,66 @@ func registerQuestionCommands() { dciCommand.AddCommand(newAnomaliesRecentCommand()) } +// adoptQuestionCommandGroups gives each question command under parent the +// cobra group of the operation it wraps, so root help lists anomalies-recent +// under "Anomalies Commands" beside list-anomalies rather than in the +// ungrouped "Additional Commands" tail. Restish assigns the groups from the +// spec's tags while hydrating the API subtree, so this runs once that has +// happened (the root help path); a wrapped operation missing from the +// loaded spec leaves its question command ungrouped, as before. +func adoptQuestionCommandGroups(parent *cobra.Command) { + if parent == nil { + return + } + byName := make(map[string]*cobra.Command, len(parent.Commands())) + for _, command := range parent.Commands() { + byName[command.Name()] = command + } + for name, wrapped := range questionCommandWrappedOperation { + question, operation := byName[name], byName[wrapped] + if question == nil || operation == nil || operation.GroupID == "" || !parent.ContainsGroup(operation.GroupID) { + continue + } + question.GroupID = operation.GroupID + } +} + +// orderedGroupCommands returns the help-visible commands of one group in +// the order the usage template lists them: cobra's alphabetical order, +// except that a question command follows the operation it wraps +// (budgets-at-risk right after list-budgets) instead of sorting to the top +// of the group by name. A question command whose wrapped operation is not +// in the group keeps its alphabetical slot. +func orderedGroupCommands(commands []*cobra.Command, groupID string) []*cobra.Command { + visible := func(command *cobra.Command) bool { + return command.GroupID == groupID && (command.IsAvailableCommand() || command.Name() == "help") + } + visibleNames := map[string]bool{} + for _, command := range commands { + if visible(command) { + visibleNames[command.Name()] = true + } + } + followers := map[string][]*cobra.Command{} + for _, command := range commands { + if wrapped, ok := questionCommandWrappedOperation[command.Name()]; ok && visible(command) && visibleNames[wrapped] { + followers[wrapped] = append(followers[wrapped], command) + } + } + ordered := make([]*cobra.Command, 0, len(commands)) + for _, command := range commands { + if !visible(command) { + continue + } + if wrapped, ok := questionCommandWrappedOperation[command.Name()]; ok && visibleNames[wrapped] { + continue // emitted right after the operation it wraps + } + ordered = append(ordered, command) + ordered = append(ordered, followers[command.Name()]...) + } + return ordered +} + func newBudgetsAtRiskCommand() *cobra.Command { command := &cobra.Command{ Use: "budgets-at-risk", diff --git a/skills/dci-cli/SKILL.md b/skills/dci-cli/SKILL.md index 76c1adf..2805fae 100644 --- a/skills/dci-cli/SKILL.md +++ b/skills/dci-cli/SKILL.md @@ -72,7 +72,7 @@ Use `--fields id,name` to project list or detail responses before output, and us 1. Confirm the CLI exists and is runnable: `dci --version` 2. Check session and active context: `dci status`; confirm identity and permissions with `dci validate` -3. Discover command shape before drafting or running commands: `dci --help` and `dci --help` (terse; add `--help-full` when you need the request/response schemas) +3. Discover command shape before drafting or running commands: `dci --help` and `dci --help` (terse: a command lists only the flags that can act on it, with the table flags folded into one line; add `--help-full` for every flag and the request/response schemas) 4. Prefer `list-*`, `get-*`, `get-report`, and `query` before `create-*`, `update-*`, or `delete-*` Use `dci skill list` to inspect the files embedded in the installed CLI. Use `dci skill update ` to refresh one installed copy, or omit the agent to update every detected installation; locally edited managed files require an explicit `--force` overwrite and are saved in a uniquely named sibling backup directory first. @@ -115,7 +115,7 @@ anatomy, and the flow archetypes. - CLI guide: https://help.doit.com/docs/cli (append `.md` to any Help Center URL for plain Markdown, e.g. https://help.doit.com/docs/cli.md) - Machine-readable Help Center index: https://help.doit.com/llms.txt (full corpus: https://help.doit.com/llms-full.txt) - API reference: https://developer.doit.com/ -- From the terminal: `dci docs` prints these entry points; `dci commands --search ""` finds the command for a task; `dci --help` is terse by default (`--help-full` adds the complete request/response schemas); `dci commands --json` is the machine-readable catalog. +- From the terminal: `dci docs` prints these entry points; `dci commands --search ""` finds the command for a task; `dci --help` is terse by default — only the flags that act on that command (`--help-full` lists every flag and the complete request/response schemas); `dci commands --json` is the machine-readable catalog. ## Reference Map From dba9e0f1fa97c6dcb19f99c9fb6a6ac156f4d7e0 Mon Sep 17 00:00:00 2001 From: vadimsolovey Date: Thu, 1 Oct 2026 16:40:42 +0300 Subject: [PATCH 3/3] fix: help flag visibility covers the beta results command, flag errors, and multi-argument commands Review follow-ups on the per-command flag visibility: - The beta subtree names the async results operation get-report-results (x-cli-name), not get-async-operation-results, so its help hid the report flags that act on it. reportResultOperations now carries both spellings, and the spec-gated test unions the embedded beta spec's response shapes (with an offline beta-only check). - The "Output flags" pointer line renders only while the flags it stands in for are actually folded. The usage template is shared with cobra's usage dump on a flag-parse error, where nothing is hidden. - The --id/--name fallback requires exactly one positional argument, the only arity name resolution ever covers. Co-Authored-By: Claude Fable 5.1 --- help_flags.go | 33 +++++++++---- help_flags_test.go | 119 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 128 insertions(+), 24 deletions(-) diff --git a/help_flags.go b/help_flags.go index db74f83..4f7f266 100644 --- a/help_flags.go +++ b/help_flags.go @@ -98,13 +98,16 @@ var helpFlagScopes = map[string]helpFlagScope{ // rows (`result.rows`/`results.rows` beside a `schema`) — the shape // nestedReportRows detects at render time and every report flag acts on. // Help needs the answer before any response exists, so the set is spelled -// out here; help_flags_test.go checks it against the live spec's response -// schemas (DCI_COMMAND_DOCS_SPEC, fetched in CI). The beta -// get-async-operation-results shares its GA namesake's shape. +// out here, keyed by command name — the beta subtree names its commands by +// x-cli-name, so the async results operation appears twice (GA spelling and +// beta spelling). help_flags_test.go checks the set against the response +// schemas of the live GA spec (DCI_COMMAND_DOCS_SPEC, fetched in CI) and +// the embedded beta spec. var reportResultOperations = map[string]bool{ "query": true, "get-report": true, "get-async-operation-results": true, + "get-report-results": true, // beta: getAsyncOperationResults } // helpFlagPlacement is where a dci persistent flag lands in one command's @@ -203,16 +206,25 @@ func helpCommandIsList(cmd *cobra.Command) bool { // helpCommandResolvesNames reports whether --id/--name have a positional // argument to govern. The resolution index is authoritative when loaded; // help usually renders before any operation metadata is read, so the -// fallback is the command's own usage line: a path parameter after the -// name ("get-budget id") is what resolution acts on. +// fallback is the command's own usage line: exactly one path parameter +// after the name ("get-budget id"), the only arity buildResolutionIndex +// ever resolves — a two-parameter command (create-ticket-comment +// customerid ticketid) is never resolvable. func helpCommandResolvesNames(cmd *cobra.Command) bool { if len(resolutionIndex) > 0 { _, ok := resolutionIndex[cmd.Name()] return ok } - return len(strings.Fields(cmd.Use)) > 1 + return len(strings.Fields(cmd.Use)) == 2 } +// helpFlagsFolded is true between applyHelpFlagVisibility and its restore: +// the pointer line renders only when the flags it stands in for were +// actually hidden. The usage template is shared with cobra's own usage dump +// on a flag-parse error, where nothing was folded and the full inherited +// list follows — a pointer line there would contradict it. +var helpFlagsFolded bool + // applyHelpFlagVisibility hides, for the duration of one help render, the // dci persistent flags that cannot act on cmd (and the presentation flags // the pointer line stands in for). It returns the restore function; the @@ -231,7 +243,9 @@ func applyHelpFlagVisibility(cmd *cobra.Command) func() { flag.Hidden = true hidden = append(hidden, flag) }) + helpFlagsFolded = true return func() { + helpFlagsFolded = false for _, flag := range hidden { flag.Hidden = false } @@ -240,11 +254,12 @@ func applyHelpFlagVisibility(cmd *cobra.Command) func() { // helpCollapsedFlagsNote renders the pointer line for the presentation // flags applyHelpFlagVisibility folded away: the usage template appends it -// after the Flags block. Empty for commands outside the API subtree and -// under --help-full, where nothing was folded. +// after the Flags block. Empty whenever nothing was folded — commands +// outside the API subtree, --help-full, and the usage dump cobra prints on +// a flag-parse error. func helpCollapsedFlagsNote(cmd *cobra.Command) string { dciCmd := findDCICommand() - if helpFullRequested || !helpCommandUnderAPI(cmd, dciCmd) { + if !helpFlagsFolded || helpFullRequested || !helpCommandUnderAPI(cmd, dciCmd) { return "" } labels := make([]string, 0) diff --git a/help_flags_test.go b/help_flags_test.go index 711d2df..356755a 100644 --- a/help_flags_test.go +++ b/help_flags_test.go @@ -2,13 +2,18 @@ package main import ( "bytes" + "io" + "net/http" + "net/url" "regexp" "sort" "strings" "testing" + "github.com/rest-sh/restish/openapi" "github.com/spf13/cobra" "github.com/spf13/pflag" + "gopkg.in/yaml.v3" ) // newHelpFlagTestTree builds the offline command tree help renders against: @@ -37,6 +42,12 @@ func newHelpFlagTestTree(t *testing.T) (root, dciCmd *cobra.Command) { add("list-roles", "Roles") // returns its whole collection in one response add("list-insights", "Insights", "page-token") add("export-datahub-dataset-records name", "DataHub", "page-token", "start-time", "end-time") + add("create-ticket-comment customerid ticketid", "Support Requests") + // The beta subtree names its commands by x-cli-name; its report-results + // command inherits the dci flags through the beta parent. + beta := &cobra.Command{Use: "beta", Run: func(*cobra.Command, []string) {}} + beta.AddCommand(&cobra.Command{Use: "get-report-results operationid", Run: func(*cobra.Command, []string) {}}) + dciCmd.AddCommand(beta) customizeDCIUsage() previousIndex := resolutionIndex @@ -56,6 +67,13 @@ func findAPICommand(t *testing.T, dciCmd *cobra.Command, name string) *cobra.Com if command.Name() == name { return command } + if command.Name() == "beta" { + for _, child := range command.Commands() { + if child.Name() == name { + return child + } + } + } } t.Fatalf("command %q not registered under dci", name) return nil @@ -135,6 +153,18 @@ func TestHelpFlagPlacementFollowsCommandShape(t *testing.T) { inline: []string{"all", "search"}, hidden: []string{"chart", "max-rows"}, }, + { + // Two path parameters: never resolvable, so --id/--name are inert. + command: "create-ticket-comment", + inline: []string{"output", "yes"}, + hidden: []string{"id", "name", "all", "search", "chart"}, + }, + { + // Beta spelling of the async results operation: report-shaped. + command: "get-report-results", + inline: []string{"chart", "pivot", "max-rows", "rollup", "id", "name"}, + hidden: []string{"all", "search", "for-reimport"}, + }, } for _, testCase := range cases { t.Run(testCase.command, func(t *testing.T) { @@ -217,8 +247,19 @@ func TestHelpFullListsEveryFlag(t *testing.T) { func TestHelpCollapsedFlagsNote(t *testing.T) { root, dciCmd := newHelpFlagTestTree(t) + noteFor := func(command *cobra.Command) string { + restore := applyHelpFlagVisibility(command) + defer restore() + return helpCollapsedFlagsNote(command) + } + + // Cobra prints the same usage template on a flag-parse error, with + // nothing folded and the full inherited list below: no pointer line then. + if note := helpCollapsedFlagsNote(findAPICommand(t, dciCmd, "invite-user")); note != "" { + t.Fatalf("pointer line rendered without the flags being folded: %q", note) + } - note := helpCollapsedFlagsNote(findAPICommand(t, dciCmd, "invite-user")) + note := noteFor(findAPICommand(t, dciCmd, "invite-user")) for _, expected := range []string{"Output flags (apply to every command; add --help-full to list them):", "-M/--table-mode", "-C/--table-columns", "-O/--output-file", "--utc", "--output-order"} { if !strings.Contains(note, expected) { t.Errorf("invite-user note missing %q:\n%s", expected, note) @@ -230,7 +271,7 @@ func TestHelpCollapsedFlagsNote(t *testing.T) { } } - if note := helpCollapsedFlagsNote(findAPICommand(t, dciCmd, "export-datahub-dataset-records")); strings.Contains(note, "output-file") { + if note := noteFor(findAPICommand(t, dciCmd, "export-datahub-dataset-records")); strings.Contains(note, "output-file") { t.Fatalf("--output-file folded on a file export, where it is inline:\n%s", note) } @@ -238,10 +279,10 @@ func TestHelpCollapsedFlagsNote(t *testing.T) { if err != nil { t.Fatal(err) } - if note := helpCollapsedFlagsNote(status); note != "" { + if note := noteFor(status); note != "" { t.Fatalf("local command status rendered the API note: %q", note) } - if note := helpCollapsedFlagsNote(dciCmd); note != "" { + if note := noteFor(dciCmd); note != "" { t.Fatalf("the dci command itself rendered the note: %q", note) } } @@ -391,8 +432,64 @@ func TestOrderedGroupCommandsPlacesQuestionAfterWrappedOperation(t *testing.T) { // CI fetches the production description). func TestHelpFlagShapeSetsMatchSpec(t *testing.T) { spec := loadCommandDocsSpec(t) - reportShaped := map[string]bool{} - fileShaped := map[string]bool{} + reportShaped, fileShaped := specResponseShapes(spec) + if len(reportShaped) == 0 { + t.Fatal("no report-shaped operation found in the spec; the walker is wrong") + } + // The beta subtree's commands carry their own names (x-cli-name) and + // draw from the embedded spec. + betaReport, betaFile := specResponseShapes(loadEmbeddedBetaSpec(t)) + for name := range betaReport { + reportShaped[name] = true + } + for name := range betaFile { + fileShaped[name] = true + } + assertSameNameSet(t, "report-shaped operations (reportResultOperations)", reportShaped, reportResultOperations) + exports := map[string]bool{} + for name := range fileExportOperations { + exports[name] = true + } + assertSameNameSet(t, "file-shaped exports (fileExportOperations)", fileShaped, exports) +} + +// The beta half of the check above runs without the production spec, so a +// renamed or newly report-shaped beta command fails offline too. +func TestHelpFlagShapeSetsCoverBetaSpec(t *testing.T) { + reportShaped, _ := specResponseShapes(loadEmbeddedBetaSpec(t)) + if !reportShaped["get-report-results"] { + t.Fatalf("beta spec walk found %v, want get-report-results among them", reportShaped) + } + for name := range reportShaped { + if !reportResultOperations[name] { + t.Errorf("beta command %s answers with report rows but reportResultOperations omits it", name) + } + } +} + +func loadEmbeddedBetaSpec(t *testing.T) commandDocsSpec { + t.Helper() + entrypoint, _ := url.Parse(defaultAPIBase + "/") + specURL, _ := url.Parse(defaultAPIBase + "/openapi.beta.yaml") + api, err := openapi.New().Load(*entrypoint, *specURL, &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(bytes.NewReader(embeddedBetaSpec)), + }) + if err != nil { + t.Fatalf("load beta spec: %v", err) + } + var raw map[string]any + if err := yaml.Unmarshal(embeddedBetaSpec, &raw); err != nil { + t.Fatalf("parse beta spec: %v", err) + } + return commandDocsSpec{api: api, raw: raw} +} + +// specResponseShapes walks every operation's success responses and reports, +// by command name, which answer with report rows and which with a file body. +func specResponseShapes(spec commandDocsSpec) (reportShaped, fileShaped map[string]bool) { + reportShaped = map[string]bool{} + fileShaped = map[string]bool{} for _, operation := range spec.api.Operations { raw := spec.rawOperation(operation) if raw == nil { @@ -415,15 +512,7 @@ func TestHelpFlagShapeSetsMatchSpec(t *testing.T) { } } } - if len(reportShaped) == 0 { - t.Fatal("no report-shaped operation found in the spec; the walker is wrong") - } - assertSameNameSet(t, "report-shaped operations (reportResultOperations)", reportShaped, reportResultOperations) - exports := map[string]bool{} - for name := range fileExportOperations { - exports[name] = true - } - assertSameNameSet(t, "file-shaped exports (fileExportOperations)", fileShaped, exports) + return reportShaped, fileShaped } // specSchemaCarriesReportRows mirrors nestedReportRows: a `result` or