diff --git a/CHANGELOG.md b/CHANGELOG.md index 61b97f84..cdffab73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,32 @@ commit types the repo already uses (`feat!`/`build!` for a breaking change). ## Unreleased +### Behaviour — the MCP `list_commands` tool browses and searches the catalog + +`list_commands` returned the whole catalog in one result. That result was +larger than the 256 KiB cap on a child's output, so the tool cut it +mid-string: the model got invalid JSON with no Protect, School or Security +Cloud commands. + +The tool now takes two optional arguments, `prefix` and `query`, and returns +one JSON object per line: + +- With no arguments, it lists the top level. A row with `"subcommands": N` + has N commands under it. +- `prefix` opens one command path, for example `"pro"` or `"pro computers"`. + A runnable command is listed with `description`, `destructive` and `flags`. +- `query` returns the commands whose path, description or aliases contain + every word, for example `"delete policy"`. + +Each result is kept under 40 KiB. Claude Code saves a text tool result +longer than 50,000 characters to a file, and gives the model only the file +path. An MCP client that parsed the old array gets NDJSON rows +now. The old result was always cut and invalid, so no client parsed it. + +`jamf-cli commands` takes the same selection as `--prefix `, +`--children` and `--search `. With no flags it prints the whole +catalog, as before. + ### Behaviour — computer group member counts come from the collection that carries one `pro group-tools` and `pro audit` read member counts from diff --git a/internal/commands/agent_context.md b/internal/commands/agent_context.md index e5151ff9..eedaddbd 100644 --- a/internal/commands/agent_context.md +++ b/internal/commands/agent_context.md @@ -79,12 +79,25 @@ privileges it requires (from the spec's `x-required-privileges`); the field is omitted when no privileges are declared. Classic, Protect, and School commands do not carry privilege data. +The whole catalog is large. To read part of it: + +- `jamf-cli commands --children` lists the top level. Each row with commands + under it carries `"subcommands": N`. +- `jamf-cli commands --prefix "pro computers" --children` lists one level down. + `--prefix` without `--children` lists everything under the path. +- `jamf-cli commands --search "delete policy"` lists the commands whose path, + description or aliases contain every word. + ## MCP `jamf-cli mcp serve` exposes the command tree to MCP clients over stdio via three tools: -- `list_commands` — the catalog. +- `list_commands` — browse or search the catalog, one JSON object per line. + With no arguments it lists the top level. A row with `"subcommands": N` has + N commands under it: pass its `command` as `prefix` to open it. Pass `query` + to find commands by words, with or without `prefix`. For one command's + arguments, call `run_command` with ` --help`. - `run_command` — execute one command and get its output back as text. - `generate_report` — write a self-contained HTML fleet report into the directory `jamf-cli config set-report-dir` designates, and return its path and diff --git a/internal/commands/commands_catalog_projection_test.go b/internal/commands/commands_catalog_projection_test.go index b1f4178d..d659b530 100644 --- a/internal/commands/commands_catalog_projection_test.go +++ b/internal/commands/commands_catalog_projection_test.go @@ -81,6 +81,8 @@ func populatedEntry(t *testing.T) commandEntry { f.SetString("x") case reflect.Bool: f.SetBool(true) + case reflect.Int: + f.SetInt(1) case reflect.Slice: if f.Type().Elem().Kind() != reflect.String { t.Fatalf("commandEntry.%s is a slice of %s, which this populator cannot fill — extend it", diff --git a/internal/commands/commands_catalog_query_test.go b/internal/commands/commands_catalog_query_test.go new file mode 100644 index 00000000..2a1c26ca --- /dev/null +++ b/internal/commands/commands_catalog_query_test.go @@ -0,0 +1,236 @@ +// Copyright 2026, Jamf Software LLC + +package commands + +import ( + "encoding/json" + "reflect" + "strings" + "testing" +) + +// renderCatalogNDJSON renders entries the way `commands -o ndjson --select +// ` prints them: one compact JSON object per row, projected to fields. +func renderCatalogNDJSON(t *testing.T, entries []commandEntry, fields string) string { + t.Helper() + keep := strings.Split(fields, ",") + var b strings.Builder + for _, row := range commandEntriesToMaps(entries, true) { + projected := map[string]any{} + for _, k := range keep { + if v, ok := row[k]; ok { + projected[k] = v + } + } + line, err := json.Marshal(projected) + if err != nil { + t.Fatal(err) + } + b.Write(line) + b.WriteByte('\n') + } + return b.String() +} + +// TestQueryCatalog_ChildrenReachEveryCommandOnceUnderTheCeiling walks the tree +// the way list_commands does, one --children level at a time from the top. +func TestQueryCatalog_ChildrenReachEveryCommandOnceUnderTheCeiling(t *testing.T) { + root := NewRootCmd("test", "abc123", "2024-01-01", "unknown") + full := map[string]commandEntry{} + for _, e := range collectCommands(root, "", "", "") { + full[e.Command] = e + } + + reached := map[string]int{} + queue := []string{""} + var levels, largest int + for len(queue) > 0 { + prefix := queue[0] + queue = queue[1:] + levels++ + + entries, err := queryCatalog(root, catalogQuery{Prefix: prefix, Children: true}) + if err != nil { + t.Fatalf("prefix %q: %v", prefix, err) + } + if size := len(renderCatalogNDJSON(t, entries, listCommandsBrowseFields)); size > maxListCommandsBytes { + t.Errorf("prefix %q renders %d bytes, over the %d-byte list_commands ceiling", prefix, size, maxListCommandsBytes) + } else if size > largest { + largest = size + } + + for _, e := range entries { + if e.Subcommands > 0 && e.Command != prefix { + queue = append(queue, e.Command) + } + want, ok := full[e.Command] + if !ok { + if e.Subcommands == 0 { + t.Errorf("prefix %q: %q is neither a catalog command nor a group", prefix, e.Command) + } + continue + } + reached[e.Command]++ + got := e + got.Subcommands = 0 + if !reflect.DeepEqual(got, want) { + t.Errorf("prefix %q: %q differs from its full-catalog entry:\n got %+v\nwant %+v", prefix, e.Command, got, want) + } + } + } + + for command := range full { + if reached[command] != 1 { + t.Errorf("%q reached %d times walking --children levels, want 1", command, reached[command]) + } + } + t.Logf("%d levels, largest %d bytes, %d commands", levels, largest, len(full)) +} + +func TestQueryCatalog_PrefixIsTheFullCatalogSubtreeAndTakesAliases(t *testing.T) { + root := NewRootCmd("test", "abc123", "2024-01-01", "unknown") + + got, err := queryCatalog(root, catalogQuery{Prefix: "pro computers"}) + if err != nil { + t.Fatal(err) + } + var want []commandEntry + for _, e := range collectCommands(root, "", "", "") { + if e.Command == "pro computer-inventory" || strings.HasPrefix(e.Command, "pro computer-inventory ") { + want = append(want, e) + } + } + if len(want) == 0 { + t.Fatal("the full catalog has no pro computer-inventory commands; pick another resource") + } + if !reflect.DeepEqual(got, want) { + t.Errorf("--prefix \"pro computers\" returned %d entries, want the %d pro computer-inventory entries of the full catalog", len(got), len(want)) + } +} + +func TestQueryCatalog_ChildrenOfACommandIsTheCommandItself(t *testing.T) { + root := NewRootCmd("test", "abc123", "2024-01-01", "unknown") + got, err := queryCatalog(root, catalogQuery{Prefix: "version", Children: true}) + if err != nil { + t.Fatal(err) + } + if len(got) != 1 || got[0].Command != "version" { + t.Errorf("--prefix version --children must return the version command alone, got %+v", got) + } +} + +func TestQueryCatalog_RefusesAnUnknownPrefix(t *testing.T) { + root := NewRootCmd("test", "abc123", "2024-01-01", "unknown") + _, err := queryCatalog(root, catalogQuery{Prefix: "pro no-such-resource"}) + if err == nil || !strings.Contains(err.Error(), "no-such-resource") { + t.Fatalf("an unknown prefix must be refused naming the unknown word, got %v", err) + } +} + +func TestQueryCatalog_SearchMatchesEveryWordAcrossPlurals(t *testing.T) { + root := NewRootCmd("test", "abc123", "2024-01-01", "unknown") + + got, err := queryCatalog(root, catalogQuery{Search: "delete policy"}) + if err != nil { + t.Fatal(err) + } + var found bool + for _, e := range got { + if e.Command == "pro classic-policies delete" { + found = true + } + hay := strings.ToLower(e.Command + " " + e.Description) + if !strings.Contains(hay, "delet") || !strings.Contains(hay, "polic") { + t.Errorf("%q matched \"delete policy\" without both words", e.Command) + } + } + if !found { + t.Errorf("\"delete policy\" must find \"pro classic-policies delete\", got %d rows", len(got)) + } + + scoped, err := queryCatalog(root, catalogQuery{Prefix: "protect", Search: "list"}) + if err != nil { + t.Fatal(err) + } + if len(scoped) == 0 { + t.Fatal("\"list\" under protect found nothing") + } + for _, e := range scoped { + if !strings.HasPrefix(e.Command, "protect ") { + t.Errorf("a search under --prefix protect returned %q", e.Command) + } + } +} + +func TestCommandsCmd_ChildrenFlagPrintsSubcommandCounts(t *testing.T) { + stdout, _, err := runRoot(t, "commands", "--children", "-o", "json") + if err != nil { + t.Fatalf("commands --children failed: %v", err) + } + counts := map[string]float64{} + for _, row := range commandRows(t, stdout) { + if n, ok := row["subcommands"].(float64); ok { + counts[row["command"].(string)] = n + } + } + if counts["pro"] == 0 { + t.Errorf("commands --children must count the commands under pro, got %v", counts) + } +} + +func TestQueryCatalog_SearchFoldsEveryPluralToItsSingular(t *testing.T) { + root := NewRootCmd("test", "abc123", "2024-01-01", "unknown") + for _, pair := range [][2]string{{"policy", "policies"}, {"patch", "patches"}, {"class", "classes"}, {"address", "addresses"}} { + singular, err := queryCatalog(root, catalogQuery{Search: pair[0]}) + if err != nil { + t.Fatal(err) + } + plural, err := queryCatalog(root, catalogQuery{Search: pair[1]}) + if err != nil { + t.Fatal(err) + } + if len(singular) == 0 || len(plural) != len(singular) { + t.Errorf("--search %q found %d commands and %q found %d; a plural must find what its singular finds", + pair[1], len(plural), pair[0], len(singular)) + } + } +} + +func TestQueryCatalog_RefusesASearchWithNoWords(t *testing.T) { + root := NewRootCmd("test", "abc123", "2024-01-01", "unknown") + for _, search := range []string{"-", "--", " / "} { + if got, err := queryCatalog(root, catalogQuery{Search: search}); err == nil { + t.Errorf("--search %q has no words and must be refused, got %d commands", search, len(got)) + } + } +} + +func TestCommandsCmd_ChildrenCountIsAColumnInEveryTableFormat(t *testing.T) { + for _, format := range []string{"table", "csv"} { + stdout, _, err := runRoot(t, "commands", "--children", "-o", format) + if err != nil { + t.Fatalf("commands --children -o %s failed: %v", format, err) + } + if !strings.Contains(strings.ToLower(stdout), "subcommands") { + t.Errorf("-o %s must carry a subcommands column, got:\n%s", format, stdout[:min(400, len(stdout))]) + } + } + + stdout, _, err := runRoot(t, "commands", "--children", "-o", "plain") + if err != nil { + t.Fatalf("commands --children -o plain failed: %v", err) + } + var sawZero, sawCount bool + for _, line := range strings.Split(stdout, "\n") { + fields := strings.Split(line, "\t") + switch last := fields[len(fields)-1]; { + case strings.HasPrefix(line, "agent-context\t"): + sawZero = last == "0" + case strings.HasPrefix(line, "pro\t"): + sawCount = last != "" && last != "0" + } + } + if !sawZero || !sawCount { + t.Errorf("-o plain must end each row with its count, 0 included, got:\n%s", stdout[:min(400, len(stdout))]) + } +} diff --git a/internal/commands/mcp.go b/internal/commands/mcp.go index 26ca7d24..66f8abf6 100644 --- a/internal/commands/mcp.go +++ b/internal/commands/mcp.go @@ -37,7 +37,7 @@ func newMCPCmd() *cobra.Command { Long: `Serve jamf-cli's command tree to MCP-capable AI clients over stdio. The connecting AI gets three tools: - - list_commands : the full command catalog (names, descriptions, flags) + - list_commands : browse the command catalog one level at a time, or search it - run_command : execute any jamf-cli command and get its output back - generate_report : write a shareable HTML fleet report and return its path @@ -119,12 +119,23 @@ instead.`, mcp.AddTool(server, &mcp.Tool{ Name: "list_commands", - Description: "List every available jamf-cli command with its description and " + - "flags. Commands that mutate or erase state are marked \"destructive\": true " + - "and require an explicit --yes. Call this first to discover what you can run, " + - "then use run_command.", - }, func(ctx context.Context, _ *mcp.CallToolRequest, _ struct{}) (*mcp.CallToolResult, any, error) { - return runChild(ctx, executable, serverProfile, []string{"commands", "-o", "json"}), nil, nil + Description: "Browse or search the jamf-cli command catalog. Call this first to find " + + "what you can run, then use run_command. Each result line is one JSON object.\n\n" + + "With no arguments, it lists the top level: the products (pro, protect, school, " + + "security, platform) and the core commands. A row with \"subcommands\": N stands " + + "for N commands under it. Pass its command as prefix to open it, for example " + + "prefix \"pro\", then prefix \"pro computers\". A row with no subcommands is a " + + "command you can run, listed with its flags. A few groups can also be run on " + + "their own.\n\n" + + "Pass query to find commands by words in their path or description, for " + + "example \"delete policy\". Add prefix to search one product or resource. " + + "Search rows carry no flags: pass a command as prefix to see its flags.\n\n" + + "Commands marked \"destructive\": true mutate or erase state and require an " + + "explicit --yes. For a command's arguments and flag details, run it with --help " + + "through run_command, e.g. [\"pro\",\"computers\",\"list\",\"--help\"]. A last line " + + "with \"truncated\": true means rows were left out: narrow the query or add a prefix.", + }, func(ctx context.Context, _ *mcp.CallToolRequest, in listCommandsInput) (*mcp.CallToolResult, any, error) { + return listCommands(ctx, executable, serverProfile, in), nil, nil }) mcp.AddTool(server, &mcp.Tool{ @@ -224,6 +235,92 @@ func childEnv() []string { return append(kept, "JAMF_CLI_MCP=1") } +type listCommandsInput struct { + Prefix string `json:"prefix,omitempty" jsonschema:"a command path to open, such as \"pro\" or \"pro computers\"; omit it for the top level"` + Query string `json:"query,omitempty" jsonschema:"words to find in command paths and descriptions, such as \"delete policy\"; searches under prefix when both are set"` +} + +const ( + listCommandsBrowseFields = "command,description,destructive,subcommands,flags" + listCommandsSearchFields = "command,description,destructive" + + // maxListCommandsBytes keeps one list_commands result inline in Claude Code. + // Claude Code 2.1.274 saves a text tool result longer than 50,000 characters + // to a file and hands the model only its path, whatever its token count. + maxListCommandsBytes = 40 << 10 +) + +// listCommandsArgs builds the `commands` invocation for one list_commands call. +// The model's text is joined to its flag with "=", so it reaches the child as a +// flag value and never as a flag of its own. +func listCommandsArgs(in listCommandsInput) []string { + args := []string{"commands", "-o", "ndjson"} + if prefix := strings.TrimSpace(in.Prefix); prefix != "" { + args = append(args, "--prefix="+prefix) + } + if query := strings.TrimSpace(in.Query); query != "" { + return append(args, "--search="+query, "--select="+listCommandsSearchFields) + } + return append(args, "--children", "--select="+listCommandsBrowseFields) +} + +// listCommands returns the catalog child's stdout alone, since one stderr line +// in it makes the catalog invalid JSON. +func listCommands(ctx context.Context, executable, serverProfile string, in listCommandsInput) *mcp.CallToolResult { + childArgs, err := buildChildArgs(serverProfile, listCommandsArgs(in)) + if err != nil { + return errorResult(err.Error()) + } + + var stderr bytes.Buffer + child := exec.CommandContext(ctx, executable, childArgs...) + child.Env = childEnv() + child.Stderr = &stderr + out, err := child.Output() + if err != nil { + text := fmt.Sprintf("listing commands failed: %v", err) + if warnings := strings.TrimSpace(tailWarnings(stderr.Bytes())); warnings != "" { + text += "\n\n" + warnings + } + return errorResult(text) + } + if len(bytes.TrimSpace(out)) == 0 { + out = []byte(`{"matches":0,"hint":"nothing is listed under this prefix"}` + "\n") + if strings.TrimSpace(in.Query) != "" { + out = []byte(`{"matches":0,"hint":"no command matches; use fewer or shorter words, or browse with prefix"}` + "\n") + } + } + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: capCatalogLines(out)}}, + } +} + +// listCommandsNoteBytes is room kept under maxListCommandsBytes for the +// truncation line. +const listCommandsNoteBytes = 256 + +// capCatalogLines keeps whole NDJSON lines up to maxListCommandsBytes. When it +// drops any, it ends with one JSON line counting them, so every line parses. +func capCatalogLines(out []byte) string { + lines := strings.SplitAfter(string(out), "\n") + var b strings.Builder + for i, line := range lines { + if b.Len()+len(line) <= maxListCommandsBytes-listCommandsNoteBytes { + b.WriteString(line) + continue + } + omitted := 0 + for _, rest := range lines[i:] { + if strings.TrimSpace(rest) != "" { + omitted++ + } + } + fmt.Fprintf(&b, `{"truncated":true,"omitted":%d,"hint":"narrow the query or add a prefix"}`+"\n", omitted) + break + } + return b.String() +} + // runChild re-invokes this binary with the given args, injecting the server's // profile and --no-input, and returns the combined output as an MCP tool // result. A non-zero exit is reported as an error result (IsError) with the diff --git a/internal/commands/mcp_list_commands_test.go b/internal/commands/mcp_list_commands_test.go new file mode 100644 index 00000000..2664ab5d --- /dev/null +++ b/internal/commands/mcp_list_commands_test.go @@ -0,0 +1,203 @@ +// Copyright 2026, Jamf Software LLC + +package commands + +import ( + "context" + "encoding/json" + "fmt" + "os" + "strings" + "testing" +) + +// runAsCLIEnv makes this test binary run the jamf-cli command tree instead of +// its tests, so os.Executable() can stand in for jamf-cli as an MCP child. +const runAsCLIEnv = "JAMF_CLI_TEST_RUN_AS_CLI" + +func TestMain(m *testing.M) { + if os.Getenv(runAsCLIEnv) == "1" { + root := NewRootCmd("test", "abc123", "2024-01-01", "unknown") + root.SetArgs(os.Args[1:]) + if err := root.Execute(); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) + } + os.Exit(m.Run()) +} + +type listCommandsRow struct { + Command string `json:"command"` + Destructive *bool `json:"destructive"` + Subcommands int `json:"subcommands"` + Flags *string `json:"flags"` + Truncated bool `json:"truncated"` +} + +// decodeListCommands requires one valid JSON object on every line. +func decodeListCommands(t *testing.T, text string) []listCommandsRow { + t.Helper() + var rows []listCommandsRow + for i, line := range strings.Split(strings.TrimSuffix(text, "\n"), "\n") { + var row listCommandsRow + if err := json.Unmarshal([]byte(line), &row); err != nil { + t.Fatalf("list_commands line %d is not a JSON object (%v): %q", i+1, err, line) + } + rows = append(rows, row) + } + return rows +} + +func runListCommands(t *testing.T, in listCommandsInput) []listCommandsRow { + t.Helper() + t.Setenv(runAsCLIEnv, "1") + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + res := listCommands(context.Background(), exe, "", in) + text := mcpResultText(res) + if res.IsError { + t.Fatalf("list_commands %+v failed: %s", in, text) + } + if len(text) > maxListCommandsBytes { + t.Errorf("list_commands %+v returned %d bytes, over the %d-byte ceiling", in, len(text), maxListCommandsBytes) + } + return decodeListCommands(t, text) +} + +func TestListCommands_OpensTheTopLevelThenAResource(t *testing.T) { + top := map[string]listCommandsRow{} + for _, r := range runListCommands(t, listCommandsInput{}) { + top[r.Command] = r + } + for _, product := range []string{"pro", "protect", "school", "security", "platform"} { + if top[product].Subcommands == 0 { + t.Errorf("the top level must list %q with a subcommand count, got %+v", product, top[product]) + } + } + + rows := runListCommands(t, listCommandsInput{Prefix: "pro computers"}) + var list *listCommandsRow + for i := range rows { + if rows[i].Command == "pro computer-inventory list" { + list = &rows[i] + } + } + if list == nil { + t.Fatalf("prefix \"pro computers\" must list pro computer-inventory list, got %+v", rows) + } + if list.Destructive == nil || list.Flags == nil { + t.Errorf("a command row must carry destructive and flags, got %+v", *list) + } +} + +func TestListCommands_SearchesUnderAPrefix(t *testing.T) { + rows := runListCommands(t, listCommandsInput{Prefix: "pro", Query: "delete policy"}) + var found bool + for _, r := range rows { + if !strings.HasPrefix(r.Command, "pro ") { + t.Errorf("a search under prefix pro returned %q", r.Command) + } + if r.Command == "pro classic-policies delete" { + found = true + if r.Destructive == nil || !*r.Destructive { + t.Errorf("pro classic-policies delete must be marked destructive, got %+v", r) + } + } + } + if !found { + t.Errorf("query \"delete policy\" must find pro classic-policies delete, got %+v", rows) + } +} + +func TestListCommands_RefusesAnUnknownPrefix(t *testing.T) { + t.Setenv(runAsCLIEnv, "1") + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + res := listCommands(context.Background(), exe, "", listCommandsInput{Prefix: "pro no-such-resource"}) + if !res.IsError || !strings.Contains(mcpResultText(res), "no-such-resource") { + t.Errorf("an unknown prefix must be an error naming it, got %+v: %s", res.IsError, mcpResultText(res)) + } +} + +func TestListCommands_CutsAtALineAndSaysHowManyItOmitted(t *testing.T) { + line := `{"command":"pro example","description":"` + strings.Repeat("x", 100) + `","destructive":false}` + "\n" + child := writeFakeReportChild(t, strings.Repeat(line, 2000), "", 0) + + res := listCommands(context.Background(), child, "", listCommandsInput{Query: "example"}) + text := mcpResultText(res) + if len(text) > maxListCommandsBytes { + t.Errorf("a cut result is %d bytes, over the %d-byte ceiling", len(text), maxListCommandsBytes) + } + rows := decodeListCommands(t, text) + last := rows[len(rows)-1] + if !last.Truncated { + t.Fatalf("a cut result must end with a truncated line, got %q", text[len(text)-200:]) + } + if !strings.Contains(text, fmt.Sprintf(`"omitted":%d`, 2000-(len(rows)-1))) { + t.Errorf("the truncated line must count the omitted rows, got %q", text[len(text)-200:]) + } +} + +func TestListCommands_KeepsStderrOutOfTheCatalog(t *testing.T) { + row := `{"command":"pro computers list","description":"List computers","destructive":false}` + "\n" + hint := "hint: 1766 results returned. Narrow with --select=\n" + + ok := mcpResultText(listCommands(context.Background(), writeFakeReportChild(t, row, hint, 0), "", listCommandsInput{})) + if ok != row { + t.Errorf("a successful catalog must be the child's stdout alone, got:\n%s", ok) + } + + res := listCommands(context.Background(), writeFakeReportChild(t, "", "Error: config unreadable\n", 1), "", listCommandsInput{}) + if !res.IsError { + t.Fatal("a failed catalog child must be an error result") + } + if !strings.Contains(mcpResultText(res), "config unreadable") { + t.Errorf("a failed catalog must carry the child's stderr, got:\n%s", mcpResultText(res)) + } +} + +func TestListCommandsArgs_KeepModelTextAsFlagValues(t *testing.T) { + args := listCommandsArgs(listCommandsInput{Prefix: "--profile other", Query: "-p prod"}) + for _, a := range args { + if a == "--profile" || a == "-p" { + t.Errorf("model text reached the child as its own argument: %q in %v", a, args) + } + } + if _, err := buildChildArgs("prod", args); err != nil { + t.Errorf("list_commands arguments must pass buildChildArgs, got %v", err) + } +} + +func TestListCommands_NamesTheEmptyResultByMode(t *testing.T) { + empty := writeFakeReportChild(t, "", "", 0) + + browse := mcpResultText(listCommands(context.Background(), empty, "", listCommandsInput{Prefix: "pro"})) + if !strings.Contains(browse, `"matches":0`) || strings.Contains(browse, "words") { + t.Errorf("an empty browse must say nothing is listed and mention no words, got %q", browse) + } + search := mcpResultText(listCommands(context.Background(), empty, "", listCommandsInput{Query: "zzz"})) + if !strings.Contains(search, `"matches":0`) || !strings.Contains(search, "words") { + t.Errorf("an empty search must suggest other words, got %q", search) + } +} + +func TestListCommands_RefusesAQueryWithNoWords(t *testing.T) { + t.Setenv(runAsCLIEnv, "1") + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + res := listCommands(context.Background(), exe, "", listCommandsInput{Query: "-"}) + if !res.IsError || !strings.Contains(mcpResultText(res), "no letters or digits") { + t.Errorf("a query with no words must be an error, got %v: %.300s", res.IsError, mcpResultText(res)) + } +} diff --git a/internal/commands/root.go b/internal/commands/root.go index e4ba7db2..289ab687 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -1237,6 +1237,9 @@ type commandEntry struct { // tenant credential still reaches at least platform-devices and // platform-device-groups. Nothing refuses on it. Scopes []string `json:"scopes,omitempty"` + // Subcommands counts the catalog commands under this one. Set only by a + // --children query, where a row stands for its whole subtree. + Subcommands int `json:"subcommands,omitempty"` } // isFullDetailFormat reports whether an output format carries the full @@ -1255,12 +1258,27 @@ func isFullDetailFormat(format string) bool { // newCommandsCmd creates the "commands" subcommand that outputs the full // command tree in a machine-readable format. func newCommandsCmd(root *cobra.Command, cliCtx *registry.CLIContext) *cobra.Command { - return &cobra.Command{ + var q catalogQuery + cmd := &cobra.Command{ Use: "commands", Short: "List all available commands", - Long: `List all available commands in a structured format for discovery by scripts and AI agents.`, + Long: `List all available commands in a structured format for discovery by scripts and AI agents. + +With no flags, every command in the tree is listed. Three flags narrow the list: + + --prefix only the commands under one command path, for example + --prefix "pro computers". Aliases are accepted. + --children only the direct children of --prefix, or of the top level + when there is no prefix. A row that has commands under it + carries "subcommands": the number of them. + --search only the commands whose path, description or aliases + contain every word. A word matches the start of a word, + and singular and plural forms match each other.`, RunE: func(cmd *cobra.Command, args []string) error { - entries := collectCommands(root, "", "", "") + entries, err := queryCatalog(root, q) + if err != nil { + return err + } // Structured formats always get full detail; table/plain // show only command+description unless --wide is set. // --select names its own fields, so the narrow row set must not @@ -1268,96 +1286,200 @@ func newCommandsCmd(root *cobra.Command, cliCtx *registry.CLIContext) *cobra.Com // because `api` is only in the wide rows and the projection then // matched no field in any row. full := wide || isFullDetailFormat(outputFmt) || len(selectFields) > 0 - return printRows(cliCtx, commandEntriesToMaps(entries, full)) + rows := commandEntriesToMaps(entries, full) + // A table's columns are its first row's keys, and the first row is + // often a command with no count, so a table, CSV or plain listing + // carries the count on every row, zero included. + if q.Children && !output.RendersStructureVerbatim(cliCtx.Output.Format()) { + for i, e := range entries { + rows[i]["subcommands"] = e.Subcommands + } + } + return printRows(cliCtx, rows) }, } + cmd.Flags().StringVar(&q.Prefix, "prefix", "", "list only the commands under this command path, e.g. \"pro computers\"") + cmd.Flags().BoolVar(&q.Children, "children", false, "list only the direct children of --prefix (or of the top level), with a subcommand count on each group") + cmd.Flags().StringVar(&q.Search, "search", "", "list only the commands whose path, description or aliases contain every word") + return cmd } -// collectCommands recursively walks the command tree and returns leaf commands. -// product and group are inherited from parent context and updated as we descend. -func collectCommands(cmd *cobra.Command, prefix, product, group string) []commandEntry { +// catalogQuery selects part of the command catalog. The zero value selects the +// whole catalog, which is what `commands` printed before it took flags. +type catalogQuery struct { + Prefix string + Children bool + Search string +} + +func queryCatalog(root *cobra.Command, q catalogQuery) ([]commandEntry, error) { + node, path, product, group, err := resolveCatalogPrefix(root, q.Prefix) + if err != nil { + return nil, err + } + var entries []commandEntry - for _, child := range cmd.Commands() { - // "commands" is skipped only at the root, where it is this catalog - // command itself. Matching the name at any depth is the same mistake - // chainSkip made with "version": it silently dropped - // `pro mdm-commands commands` — a real generated operation, and one the - // gateway refuses, so the catalog was missing exactly the entry a reader - // consults it for. "help" stays unconditional: cobra gives every command - // one. - if child.Hidden || child.Name() == "help" || (child.Name() == "commands" && prefix == "") { - continue + switch { + case q.Children: + entries = catalogChildren(node, path, product, group) + case node == root: + entries = collectCommands(root, "", "", "") + default: + if isCatalogCommand(node) { + entries = append(entries, newCommandEntry(node, path, product, group)) } + entries = append(entries, collectCommands(node, path, product, group)...) + } - fullPath := child.Name() - if prefix != "" { - fullPath = prefix + " " + child.Name() - } + if q.Search != "" { + return searchCatalog(entries, q.Search) + } + return entries, nil +} - // Determine product for this child's subtree. Only top-level namespaces - // set the product: product is empty only at the root, so gating on it - // prevents a nested command that happens to be named after a namespace - // (e.g. "pro report security") from being re-tagged. - childProduct := product - if product == "" && (child.Name() == "pro" || child.Name() == "protect" || child.Name() == "school" || child.Name() == "security" || child.Name() == "platform") { - childProduct = child.Name() +// resolveCatalogPrefix walks root to the command a space-separated path names, +// taking the same product and group collectCommands would assign on the way. +func resolveCatalogPrefix(root *cobra.Command, prefix string) (node *cobra.Command, path, product, group string, err error) { + node = root + for _, word := range strings.Fields(prefix) { + var next *cobra.Command + for _, child := range node.Commands() { + if !skipInCatalog(child, path) && (child.Name() == word || child.HasAlias(word)) { + next = child + break + } } - - // Determine group for this child's subtree. - childGroup := group - if child.GroupID != "" { - childGroup = groupTitle(child.GroupID) + if next == nil { + return nil, "", "", "", exitcode.New(exitcode.Usage, + fmt.Sprintf("--prefix %q: %q has no subcommand %q", prefix, catalogPathOrTop(path), word)). + WithHint("start from the top level, with no prefix, and add one word of a listed command path at a time") } + product, group = catalogScope(next, product, group) + path = joinCatalogPath(path, next.Name()) + node = next + } + return node, path, product, group, nil +} - // Leaf command: has RunE or Run - if child.RunE != nil || child.Run != nil { - var privileges []string - if p := child.Annotations["jamf:privileges"]; p != "" { - privileges = strings.Split(p, ",") - } - entry := commandEntry{ - Command: fullPath, - Description: child.Short, - Product: childProduct, - Group: childGroup, - Destructive: child.Annotations["jamf:destructive"] == "true", - Preview: child.Annotations["jamf:preview"] == "true", - Privileges: privileges, - API: child.Annotations["jamf:api"], +func catalogPathOrTop(path string) string { + if path == "" { + return "the top level" + } + return path +} - Gateway: child.Annotations[annotationGateway], - GatewayBasis: child.Annotations[annotationGatewayBasis], - GatewayDetail: child.Annotations[annotationGatewayDetail], - GatewaySuccessor: gatewaySuccessorOf(child), +// catalogChildren lists node's direct children, each counted by the catalog +// commands under it. A command with no children lists itself, so a path that +// reaches a runnable command still answers with that command's row. +func catalogChildren(node *cobra.Command, path, product, group string) []commandEntry { + var entries []commandEntry + for _, child := range node.Commands() { + if skipInCatalog(child, path) { + continue + } + childPath := joinCatalogPath(path, child.Name()) + childProduct, childGroup := catalogScope(child, product, group) + entry := commandEntry{Command: childPath, Description: child.Short, Product: childProduct, Group: childGroup} + if isCatalogCommand(child) { + entry = newCommandEntry(child, childPath, childProduct, childGroup) + } + if child.HasSubCommands() { + entry.Subcommands = len(collectCommands(child, childPath, childProduct, childGroup)) + } + if entry.Subcommands == 0 && !isCatalogCommand(child) { + continue + } + entries = append(entries, entry) + } + if len(entries) == 0 && isCatalogCommand(node) && path != "" { + entries = append(entries, newCommandEntry(node, path, product, group)) + } + return entries +} - GatewayPrivileges: gatewayPrivilegesOf(child), - GatewayPermissions: gatewayPermissionsOf(child), +// searchCatalog keeps the entries whose path, description or aliases hold every +// word of search, each word matching the start of a word there. A search with +// no letters or digits is refused, since it would match every command. +func searchCatalog(entries []commandEntry, search string) ([]commandEntry, error) { + words := catalogWords(search) + if len(words) == 0 { + return nil, exitcode.New(exitcode.Usage, + fmt.Sprintf("--search %q has no letters or digits to match", search)). + WithHint("search for one or more words, such as \"delete policy\"") + } + var kept []commandEntry + for _, e := range entries { + hay := catalogWords(e.Command + " " + e.Description + " " + strings.Join(e.Aliases, " ")) + if catalogMatchesAll(hay, words) { + kept = append(kept, e) + } + } + return kept, nil +} - Scopes: scopesOf(child), - } +// catalogMatchesAll reports whether every word of words has a form that starts +// some form of a word of hay. +func catalogMatchesAll(hay, words []string) bool { + for _, w := range words { + if !catalogMatchesOne(hay, w) { + return false + } + } + return true +} - // Collect aliases: for leaf commands under a top-level group - // (e.g., "computers list"), expose the group's aliases ("comp") - // so agents know "comp list" also works. - if len(child.Aliases) > 0 { - entry.Aliases = child.Aliases - } else if len(cmd.Aliases) > 0 { - entry.Aliases = cmd.Aliases +func catalogMatchesOne(hay []string, word string) bool { + for _, wf := range catalogWordForms(word) { + for _, h := range hay { + for _, hf := range catalogWordForms(h) { + if strings.HasPrefix(hf, wf) { + return true + } } + } + } + return false +} - // Collect non-hidden local flags - var flags []string - child.LocalFlags().VisitAll(func(f *pflag.Flag) { - if !f.Hidden { - flags = append(flags, "--"+f.Name) - } - }) - entry.Flags = flags +// catalogWords lowercases s and splits it on anything but letters and digits. +func catalogWords(s string) []string { + return strings.FieldsFunc(strings.ToLower(s), func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsDigit(r) + }) +} - entries = append(entries, entry) - } +// catalogWordForms returns w and each singular it can be the plural of, so +// "policies" yields "policy", "patches" yields "patch" and "caches" yields +// "cache". A wrong candidate costs nothing, because a match needs a real word +// to start with it. +func catalogWordForms(w string) []string { + forms := []string{w} + if len(w) <= 3 || !strings.HasSuffix(w, "s") || strings.HasSuffix(w, "ss") { + return forms + } + forms = append(forms, w[:len(w)-1]) + if strings.HasSuffix(w, "es") { + forms = append(forms, w[:len(w)-2]) + } + if len(w) > 4 && strings.HasSuffix(w, "ies") { + forms = append(forms, w[:len(w)-3]+"y") + } + return forms +} - // Recurse into subcommands +// collectCommands recursively walks the command tree and returns leaf commands. +// product and group are inherited from parent context and updated as we descend. +func collectCommands(cmd *cobra.Command, prefix, product, group string) []commandEntry { + var entries []commandEntry + for _, child := range cmd.Commands() { + if skipInCatalog(child, prefix) { + continue + } + fullPath := joinCatalogPath(prefix, child.Name()) + childProduct, childGroup := catalogScope(child, product, group) + if isCatalogCommand(child) { + entries = append(entries, newCommandEntry(child, fullPath, childProduct, childGroup)) + } if child.HasSubCommands() { entries = append(entries, collectCommands(child, fullPath, childProduct, childGroup)...) } @@ -1365,6 +1487,89 @@ func collectCommands(cmd *cobra.Command, prefix, product, group string) []comman return entries } +// skipInCatalog reports whether child stays out of the catalog. parentPath is +// the path of child's parent, empty at the root. +// +// "commands" is skipped only at the root, where it is this catalog command +// itself. Matching the name at any depth is the same mistake chainSkip made +// with "version": it silently dropped `pro mdm-commands commands` — a real +// generated operation, and one the gateway refuses, so the catalog was missing +// exactly the entry a reader consults it for. "help" stays unconditional: +// cobra gives every command one. +func skipInCatalog(child *cobra.Command, parentPath string) bool { + return child.Hidden || child.Name() == "help" || (child.Name() == "commands" && parentPath == "") +} + +func joinCatalogPath(parentPath, name string) string { + if parentPath == "" { + return name + } + return parentPath + " " + name +} + +// catalogScope returns the product and group child's subtree carries. Only +// top-level namespaces set the product: product is empty only at the root, so +// gating on it prevents a nested command that happens to be named after a +// namespace (e.g. "pro report security") from being re-tagged. +func catalogScope(child *cobra.Command, product, group string) (string, string) { + if product == "" { + switch child.Name() { + case "pro", "protect", "school", "security", "platform": + product = child.Name() + } + } + if child.GroupID != "" { + group = groupTitle(child.GroupID) + } + return product, group +} + +func isCatalogCommand(cmd *cobra.Command) bool { + return cmd.RunE != nil || cmd.Run != nil +} + +func newCommandEntry(cmd *cobra.Command, fullPath, product, group string) commandEntry { + var privileges []string + if p := cmd.Annotations["jamf:privileges"]; p != "" { + privileges = strings.Split(p, ",") + } + entry := commandEntry{ + Command: fullPath, + Description: cmd.Short, + Product: product, + Group: group, + Destructive: cmd.Annotations["jamf:destructive"] == "true", + Preview: cmd.Annotations["jamf:preview"] == "true", + Privileges: privileges, + API: cmd.Annotations["jamf:api"], + + Gateway: cmd.Annotations[annotationGateway], + GatewayBasis: cmd.Annotations[annotationGatewayBasis], + GatewayDetail: cmd.Annotations[annotationGatewayDetail], + GatewaySuccessor: gatewaySuccessorOf(cmd), + + GatewayPrivileges: gatewayPrivilegesOf(cmd), + GatewayPermissions: gatewayPermissionsOf(cmd), + + Scopes: scopesOf(cmd), + } + + // For leaf commands under a top-level group (e.g., "computers list"), + // expose the group's aliases ("comp") so agents know "comp list" also works. + if len(cmd.Aliases) > 0 { + entry.Aliases = cmd.Aliases + } else if parent := cmd.Parent(); parent != nil && len(parent.Aliases) > 0 { + entry.Aliases = parent.Aliases + } + + cmd.LocalFlags().VisitAll(func(f *pflag.Flag) { + if !f.Hidden { + entry.Flags = append(entry.Flags, "--"+f.Name) + } + }) + return entry +} + // commandEntriesToMaps converts command entries to the []map[string]interface{} // format expected by the output formatter. When full is true, aliases and flags // columns are included; otherwise only command and description are emitted @@ -1449,6 +1654,11 @@ func commandEntriesToMaps(entries []commandEntry, full bool) []map[string]any { if len(e.Scopes) > 0 { m["scopes"] = e.Scopes } + // Positive-only: zero is every row of a whole-catalog listing, + // where no row stands for a subtree. + if e.Subcommands > 0 { + m["subcommands"] = e.Subcommands + } } result[i] = m } diff --git a/skills/skills/jamf-investigate/SKILL.md b/skills/skills/jamf-investigate/SKILL.md index fdbe2dca..71e1d320 100644 --- a/skills/skills/jamf-investigate/SKILL.md +++ b/skills/skills/jamf-investigate/SKILL.md @@ -9,7 +9,7 @@ You are a Jamf Pro investigation assistant. The user will ask a natural language ## Rules 1. **Never call the Jamf API directly.** Always use `jamf-cli` commands via the Bash tool. -2. **Start with the broadest useful command.** `jamf-cli pro overview` gives a quick instance snapshot. `jamf-cli commands -o json` lists all available commands. +2. **Start with the broadest useful command.** `jamf-cli pro overview` gives a quick instance snapshot. To find a command, run `jamf-cli commands --search ""`, or `jamf-cli commands --children` to see the top level. `jamf-cli commands -o json` lists every command, and it is too large to read whole. 3. **Chain commands as needed.** If the first command's output reveals you need more detail, run follow-up commands. 4. **Use structured output for parsing.** Always pass `-o json` when you need to process results programmatically. Use `-o table` when showing results to the user. 5. **Use `--field` to extract specific values.** For example: `jamf-cli pro computers list -o json --field id` to get just IDs.