From 0f0f12ed9cb37bdcebca9d7b199bb9ad0a15cac8 Mon Sep 17 00:00:00 2001 From: Keaton Svoma Date: Thu, 24 Sep 2026 18:03:46 -0500 Subject: [PATCH 1/4] test(mcp): add a failing test for the list_commands catalog The test runs the list_commands handler against the real command tree. TestMain lets the test binary run as the jamf-cli child. The test requires valid JSON, every catalog command, the destructive mark on each command, and a result under maxChildOutputBytes. The test fails on this commit. The handler requests `commands -o json`, and capChildOutput cuts that output mid-string at 262144 bytes. Co-Authored-By: Claude Opus 5.5 --- internal/commands/mcp.go | 6 +- internal/commands/mcp_list_commands_test.go | 116 ++++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 internal/commands/mcp_list_commands_test.go diff --git a/internal/commands/mcp.go b/internal/commands/mcp.go index 26ca7d24..de61ad65 100644 --- a/internal/commands/mcp.go +++ b/internal/commands/mcp.go @@ -124,7 +124,7 @@ instead.`, "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 + return listCommands(ctx, executable, serverProfile), nil, nil }) mcp.AddTool(server, &mcp.Tool{ @@ -224,6 +224,10 @@ func childEnv() []string { return append(kept, "JAMF_CLI_MCP=1") } +func listCommands(ctx context.Context, executable, serverProfile string) *mcp.CallToolResult { + return runChild(ctx, executable, serverProfile, []string{"commands", "-o", "json"}) +} + // 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..c7f9a0a7 --- /dev/null +++ b/internal/commands/mcp_list_commands_test.go @@ -0,0 +1,116 @@ +// Copyright 2026, Jamf Software LLC + +package commands + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "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"` +} + +// decodeListCommands accepts one JSON array or a stream of JSON objects, and +// fails on anything that is not valid JSON. +func decodeListCommands(t *testing.T, text string) []listCommandsRow { + t.Helper() + var rows []listCommandsRow + dec := json.NewDecoder(strings.NewReader(text)) + for { + var raw json.RawMessage + err := dec.Decode(&raw) + if errors.Is(err, io.EOF) { + return rows + } + if err != nil { + tail := text[max(0, len(text)-200):] + t.Fatalf("list_commands returned %d bytes that are not valid JSON (%v); it ends with:\n%s", len(text), err, tail) + } + if raw[0] == '[' { + var batch []listCommandsRow + if err := json.Unmarshal(raw, &batch); err != nil { + t.Fatalf("list_commands array does not decode as catalog rows: %v", err) + } + rows = append(rows, batch...) + continue + } + var row listCommandsRow + if err := json.Unmarshal(raw, &row); err != nil { + t.Fatalf("list_commands value %s does not decode as a catalog row: %v", raw, err) + } + rows = append(rows, row) + } +} + +// TestListCommands_ReturnsTheWholeCatalogAsValidJSON drives the list_commands +// handler against the real command tree. The catalog is fixed per build, so a +// catalog that outgrows the tool-result ceiling fails here, not at runtime. +func TestListCommands_ReturnsTheWholeCatalogAsValidJSON(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, "") + text := mcpResultText(res) + if res.IsError { + t.Fatalf("list_commands failed: %s", text) + } + if len(text) > maxChildOutputBytes { + t.Errorf("list_commands returned %d bytes, over the %d-byte ceiling for one tool result; "+ + "narrow the projection the handler requests", len(text), maxChildOutputBytes) + } + + rows := decodeListCommands(t, text) + got := make(map[string]listCommandsRow, len(rows)) + for _, r := range rows { + got[r.Command] = r + } + + want := collectCommands(NewRootCmd("test", "abc123", "2024-01-01", "unknown"), "", "", "") + var missing []string + for _, e := range want { + r, ok := got[e.Command] + if !ok { + missing = append(missing, e.Command) + continue + } + if r.Destructive == nil || *r.Destructive != e.Destructive { + t.Errorf("%q: destructive must be %v in list_commands, got %v", e.Command, e.Destructive, r.Destructive) + } + } + if len(missing) > 0 { + t.Errorf("list_commands carries %d of the %d catalog commands; missing %d, first: %v", + len(want)-len(missing), len(want), len(missing), missing[:min(5, len(missing))]) + } + if len(rows) != len(want) { + t.Errorf("list_commands returned %d rows for a catalog of %d commands", len(rows), len(want)) + } +} From 73b9f721c3464ff1fb4c021ceb077fd03101c135 Mon Sep 17 00:00:00 2001 From: Keaton Svoma Date: Thu, 24 Sep 2026 18:17:52 -0500 Subject: [PATCH 2/4] fix(mcp): return the whole catalog from list_commands list_commands requested `commands -o json` and returned it through runChild. That output is larger than maxChildOutputBytes, so capChildOutput cut it mid-string. The model got invalid JSON that held only part of the catalog, and no Protect, School or Security Cloud commands. CombinedOutput also put the stderr list hint into the result. listCommands now requests only command, description and destructive, as NDJSON. It reads stdout alone, and it puts stderr into the result only when the child fails. It does not call capChildOutput, because a cut catalog is invalid JSON. The catalog is fixed per build, so the test keeps it under the cap. The tool description, the `mcp` help text and agent_context.md now say which fields list_commands returns. They tell the model to use run_command with ` --help` for flags, and with `commands --select command, -o ndjson` for other fields. Co-Authored-By: Claude Opus 5.5 --- internal/commands/agent_context.md | 6 ++- internal/commands/mcp.go | 44 ++++++++++++++++++--- internal/commands/mcp_list_commands_test.go | 18 +++++++++ 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/internal/commands/agent_context.md b/internal/commands/agent_context.md index e5151ff9..094ab0a4 100644 --- a/internal/commands/agent_context.md +++ b/internal/commands/agent_context.md @@ -84,7 +84,11 @@ not carry privilege data. `jamf-cli mcp serve` exposes the command tree to MCP clients over stdio via three tools: -- `list_commands` — the catalog. +- `list_commands` — every command, as one JSON object per line with only + `command`, `description` and `destructive`. The catalog with every field is + too large for one tool result. For one command's flags, call `run_command` + with ` --help`. For another catalog field, call `run_command` with + `commands --select command, -o ndjson`. - `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/mcp.go b/internal/commands/mcp.go index de61ad65..f66af307 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 : every command, with its description and destructive mark - 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,10 +119,15 @@ 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.", + Description: "List every available jamf-cli command as one JSON object per line, " + + "with its command, description and destructive fields. 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. For one " + + "command's flags and arguments, run it with --help through run_command, e.g. " + + "[\"pro\",\"computers\",\"list\",\"--help\"]. For another catalog field " + + "(flags, aliases, product, group, privileges, gatewayPermissions, scopes), use " + + "run_command [\"commands\",\"--select\",\"command,\",\"-o\",\"ndjson\"]; the " + + "catalog with every field is too large for one tool result.", }, func(ctx context.Context, _ *mcp.CallToolRequest, _ struct{}) (*mcp.CallToolResult, any, error) { return listCommands(ctx, executable, serverProfile), nil, nil }) @@ -224,8 +229,35 @@ func childEnv() []string { return append(kept, "JAMF_CLI_MCP=1") } +// listCommandsArgs requests only the fields an AI needs to choose a command, +// because the full catalog does not fit in one tool result. +var listCommandsArgs = []string{"commands", "--select", "command,description,destructive", "-o", "ndjson"} + +// listCommands returns the catalog child's stdout alone, since one stderr line +// in it makes the catalog invalid JSON. It skips capChildOutput: a cut catalog +// is invalid JSON too, and the catalog is fixed per build, so +// TestListCommands_ReturnsTheWholeCatalogAsValidJSON holds it under the cap. func listCommands(ctx context.Context, executable, serverProfile string) *mcp.CallToolResult { - return runChild(ctx, executable, serverProfile, []string{"commands", "-o", "json"}) + childArgs, err := buildChildArgs(serverProfile, listCommandsArgs) + 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) + } + return &mcp.CallToolResult{ + Content: []mcp.Content{&mcp.TextContent{Text: string(out)}}, + } } // runChild re-invokes this binary with the given args, injecting the server's diff --git a/internal/commands/mcp_list_commands_test.go b/internal/commands/mcp_list_commands_test.go index c7f9a0a7..bc36da0a 100644 --- a/internal/commands/mcp_list_commands_test.go +++ b/internal/commands/mcp_list_commands_test.go @@ -114,3 +114,21 @@ func TestListCommands_ReturnsTheWholeCatalogAsValidJSON(t *testing.T) { t.Errorf("list_commands returned %d rows for a catalog of %d commands", len(rows), len(want)) } } + +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), "")) + 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), "") + 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)) + } +} From a2a25db513051979e3a4bcc3fe15c70f130cc4d6 Mon Sep 17 00:00:00 2001 From: Keaton Svoma Date: Fri, 25 Sep 2026 11:56:41 -0500 Subject: [PATCH 3/4] feat(mcp): browse and search the catalog in list_commands The whole catalog did not fit in one tool result. As NDJSON with three fields it is 210125 bytes. Claude Code 2.1.274 saves a result above its default 25,000-token limit to a file, and gives the model only an error line with the path. `commands` takes three new flags: --prefix , --children and --search . With no flags it prints the whole catalog, as before. queryCatalog, catalogChildren and searchCatalog use the same entry builder as collectCommands, so a row is the same in every view. list_commands takes two optional arguments, prefix and query, and runs `commands` with those flags. capCatalogLines keeps each result under maxListCommandsBytes, at a line boundary. When it drops rows, it adds a JSON line that counts them. TestQueryCatalog_ChildrenReachEveryCommandOnceUnderTheCeiling walks every --children level from the top. It requires each level to fit under maxListCommandsBytes, and every catalog command to appear exactly once. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 25 ++ internal/commands/agent_context.md | 19 +- .../commands_catalog_projection_test.go | 2 + .../commands/commands_catalog_query_test.go | 179 ++++++++++ internal/commands/mcp.go | 98 ++++-- internal/commands/mcp_list_commands_test.go | 157 ++++++--- internal/commands/root.go | 326 ++++++++++++++---- skills/skills/jamf-investigate/SKILL.md | 2 +- 8 files changed, 650 insertions(+), 158 deletions(-) create mode 100644 internal/commands/commands_catalog_query_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 61b97f84..90e6685b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,31 @@ 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 64 KiB, below Claude Code's default 25,000-token +tool-result limit. 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 094ab0a4..eedaddbd 100644 --- a/internal/commands/agent_context.md +++ b/internal/commands/agent_context.md @@ -79,16 +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` — every command, as one JSON object per line with only - `command`, `description` and `destructive`. The catalog with every field is - too large for one tool result. For one command's flags, call `run_command` - with ` --help`. For another catalog field, call `run_command` with - `commands --select command, -o ndjson`. +- `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..9f29d7c2 --- /dev/null +++ b/internal/commands/commands_catalog_query_test.go @@ -0,0 +1,179 @@ +// 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) + } +} diff --git a/internal/commands/mcp.go b/internal/commands/mcp.go index f66af307..bf2b8740 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 : every command, with its description and destructive mark + - 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,17 +119,21 @@ instead.`, mcp.AddTool(server, &mcp.Tool{ Name: "list_commands", - Description: "List every available jamf-cli command as one JSON object per line, " + - "with its command, description and destructive fields. 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. For one " + - "command's flags and arguments, run it with --help through run_command, e.g. " + - "[\"pro\",\"computers\",\"list\",\"--help\"]. For another catalog field " + - "(flags, aliases, product, group, privileges, gatewayPermissions, scopes), use " + - "run_command [\"commands\",\"--select\",\"command,\",\"-o\",\"ndjson\"]; the " + - "catalog with every field is too large for one tool result.", - }, func(ctx context.Context, _ *mcp.CallToolRequest, _ struct{}) (*mcp.CallToolResult, any, error) { - return listCommands(ctx, executable, serverProfile), 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.\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.\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{ @@ -229,16 +233,39 @@ func childEnv() []string { return append(kept, "JAMF_CLI_MCP=1") } -// listCommandsArgs requests only the fields an AI needs to choose a command, -// because the full catalog does not fit in one tool result. -var listCommandsArgs = []string{"commands", "--select", "command,description,destructive", "-o", "ndjson"} +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 under Claude Code's + // default 25,000-token tool-result limit, above which the client saves the + // result to a file and hands the model only its path. + maxListCommandsBytes = 64 << 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. It skips capChildOutput: a cut catalog -// is invalid JSON too, and the catalog is fixed per build, so -// TestListCommands_ReturnsTheWholeCatalogAsValidJSON holds it under the cap. -func listCommands(ctx context.Context, executable, serverProfile string) *mcp.CallToolResult { - childArgs, err := buildChildArgs(serverProfile, listCommandsArgs) +// 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()) } @@ -255,9 +282,38 @@ func listCommands(ctx context.Context, executable, serverProfile string) *mcp.Ca } return errorResult(text) } + if len(bytes.TrimSpace(out)) == 0 { + 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: string(out)}}, + 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 diff --git a/internal/commands/mcp_list_commands_test.go b/internal/commands/mcp_list_commands_test.go index bc36da0a..038b1300 100644 --- a/internal/commands/mcp_list_commands_test.go +++ b/internal/commands/mcp_list_commands_test.go @@ -5,9 +5,7 @@ package commands import ( "context" "encoding/json" - "errors" "fmt" - "io" "os" "strings" "testing" @@ -31,87 +29,120 @@ func TestMain(m *testing.M) { } type listCommandsRow struct { - Command string `json:"command"` - Destructive *bool `json:"destructive"` + Command string `json:"command"` + Destructive *bool `json:"destructive"` + Subcommands int `json:"subcommands"` + Flags *string `json:"flags"` + Truncated bool `json:"truncated"` } -// decodeListCommands accepts one JSON array or a stream of JSON objects, and -// fails on anything that is not valid JSON. +// decodeListCommands requires one valid JSON object on every line. func decodeListCommands(t *testing.T, text string) []listCommandsRow { t.Helper() var rows []listCommandsRow - dec := json.NewDecoder(strings.NewReader(text)) - for { - var raw json.RawMessage - err := dec.Decode(&raw) - if errors.Is(err, io.EOF) { - return rows - } - if err != nil { - tail := text[max(0, len(text)-200):] - t.Fatalf("list_commands returned %d bytes that are not valid JSON (%v); it ends with:\n%s", len(text), err, tail) - } - if raw[0] == '[' { - var batch []listCommandsRow - if err := json.Unmarshal(raw, &batch); err != nil { - t.Fatalf("list_commands array does not decode as catalog rows: %v", err) - } - rows = append(rows, batch...) - continue - } + for i, line := range strings.Split(strings.TrimSuffix(text, "\n"), "\n") { var row listCommandsRow - if err := json.Unmarshal(raw, &row); err != nil { - t.Fatalf("list_commands value %s does not decode as a catalog row: %v", raw, err) + 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 } -// TestListCommands_ReturnsTheWholeCatalogAsValidJSON drives the list_commands -// handler against the real command tree. The catalog is fixed per build, so a -// catalog that outgrows the tool-result ceiling fails here, not at runtime. -func TestListCommands_ReturnsTheWholeCatalogAsValidJSON(t *testing.T) { +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, "") + res := listCommands(context.Background(), exe, "", in) text := mcpResultText(res) if res.IsError { - t.Fatalf("list_commands failed: %s", text) + t.Fatalf("list_commands %+v failed: %s", in, text) } - if len(text) > maxChildOutputBytes { - t.Errorf("list_commands returned %d bytes, over the %d-byte ceiling for one tool result; "+ - "narrow the projection the handler requests", len(text), maxChildOutputBytes) + 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) +} - rows := decodeListCommands(t, text) - got := make(map[string]listCommandsRow, len(rows)) - for _, r := range rows { - got[r.Command] = r +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) + } +} - want := collectCommands(NewRootCmd("test", "abc123", "2024-01-01", "unknown"), "", "", "") - var missing []string - for _, e := range want { - r, ok := got[e.Command] - if !ok { - missing = append(missing, e.Command) - continue +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.Destructive == nil || *r.Destructive != e.Destructive { - t.Errorf("%q: destructive must be %v in list_commands, got %v", e.Command, e.Destructive, r.Destructive) + 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 len(missing) > 0 { - t.Errorf("list_commands carries %d of the %d catalog commands; missing %d, first: %v", - len(want)-len(missing), len(want), len(missing), missing[:min(5, len(missing))]) + 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 len(rows) != len(want) { - t.Errorf("list_commands returned %d rows for a catalog of %d commands", len(rows), len(want)) + 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:]) } } @@ -119,12 +150,12 @@ 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), "")) + 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), "") + 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") } @@ -132,3 +163,15 @@ func TestListCommands_KeepsStderrOutOfTheCatalog(t *testing.T) { 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) + } +} diff --git a/internal/commands/root.go b/internal/commands/root.go index e4ba7db2..5740cd06 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 @@ -1271,93 +1289,165 @@ func newCommandsCmd(root *cobra.Command, cliCtx *registry.CLIContext) *cobra.Com return printRows(cliCtx, commandEntriesToMaps(entries, full)) }, } + 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 { - 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 - } +// 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 +} - fullPath := child.Name() - if prefix != "" { - fullPath = prefix + " " + child.Name() - } +func queryCatalog(root *cobra.Command, q catalogQuery) ([]commandEntry, error) { + node, path, product, group, err := resolveCatalogPrefix(root, q.Prefix) + if err != nil { + return nil, err + } - // 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() + var entries []commandEntry + 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)...) + } - // Determine group for this child's subtree. - childGroup := group - if child.GroupID != "" { - childGroup = groupTitle(child.GroupID) - } + if q.Search != "" { + entries = searchCatalog(entries, q.Search) + } + return entries, 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, ",") +// 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 } - 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"], + } + if next == nil { + return nil, "", "", "", exitcode.New(exitcode.Usage, + fmt.Sprintf("--prefix %q: %q has no subcommand %q", prefix, catalogPathOrTop(path), word)). + WithHint("run 'jamf-cli commands --children' to see the top level, then add one word at a time") + } + product, group = catalogScope(next, product, group) + path = joinCatalogPath(path, next.Name()) + node = next + } + return node, path, product, group, nil +} - Gateway: child.Annotations[annotationGateway], - GatewayBasis: child.Annotations[annotationGatewayBasis], - GatewayDetail: child.Annotations[annotationGatewayDetail], - GatewaySuccessor: gatewaySuccessorOf(child), +func catalogPathOrTop(path string) string { + if path == "" { + return "the top level" + } + return path +} - GatewayPrivileges: gatewayPrivilegesOf(child), - GatewayPermissions: gatewayPermissionsOf(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 +} - Scopes: scopesOf(child), - } +// searchCatalog keeps the entries whose path, description or aliases hold every +// word of search, each word matching the start of a word there. +func searchCatalog(entries []commandEntry, search string) []commandEntry { + words := catalogWords(search) + 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 +} - // 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 catalogMatchesAll(hay, words []string) bool { + for _, w := range words { + found := false + for _, h := range hay { + if strings.HasPrefix(h, w) { + found = true + break } + } + if !found { + return false + } + } + return true +} - // 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 - - entries = append(entries, entry) +// catalogWords lowercases s, splits it on anything but letters and digits, and +// folds a plural to its singular, so "policy" finds "policies". +func catalogWords(s string) []string { + words := strings.FieldsFunc(strings.ToLower(s), func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsDigit(r) + }) + for i, w := range words { + switch { + case len(w) > 4 && strings.HasSuffix(w, "ies"): + words[i] = w[:len(w)-3] + "y" + case len(w) > 3 && strings.HasSuffix(w, "s") && !strings.HasSuffix(w, "ss"): + words[i] = w[:len(w)-1] } + } + return words +} - // 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 +1455,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 +1622,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. From 9564c67cb1f9e8698ff3bb492d9c55e3270b92bf Mon Sep 17 00:00:00 2001 From: Keaton Svoma Date: Fri, 25 Sep 2026 21:40:27 -0500 Subject: [PATCH 4/4] fix(mcp): address the review of the list_commands redesign - catalogWordForms gives each search word its possible singular forms. The old fold made "patches" into "patche", so `--search patches` found 1 command where `--search patch` found 87. - A search with no letters or digits is refused. It matched every command. - A --children listing in table, CSV or plain output carries subcommands on every row, zero included. The first row decides the columns, and it is often a command with no count. - maxListCommandsBytes is 40 KiB. Claude Code 2.1.274 saves a text tool result longer than about 50,000 characters to a file, whatever its token count. A 64 KiB result was saved, and a 40 KiB result arrived inline. - The empty-result hint suits a browse or a search. The tool description says that some groups can run, and that search rows carry no flags. The unknown-prefix hint no longer names a CLI form. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +- .../commands/commands_catalog_query_test.go | 57 +++++++++++++ internal/commands/mcp.go | 19 +++-- internal/commands/mcp_list_commands_test.go | 26 ++++++ internal/commands/root.go | 82 +++++++++++++------ 5 files changed, 155 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90e6685b..cdffab73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,8 +28,9 @@ one JSON object per line: - `query` returns the commands whose path, description or aliases contain every word, for example `"delete policy"`. -Each result is kept under 64 KiB, below Claude Code's default 25,000-token -tool-result limit. An MCP client that parsed the old array gets NDJSON rows +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 `, diff --git a/internal/commands/commands_catalog_query_test.go b/internal/commands/commands_catalog_query_test.go index 9f29d7c2..2a1c26ca 100644 --- a/internal/commands/commands_catalog_query_test.go +++ b/internal/commands/commands_catalog_query_test.go @@ -177,3 +177,60 @@ func TestCommandsCmd_ChildrenFlagPrintsSubcommandCounts(t *testing.T) { 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 bf2b8740..66f8abf6 100644 --- a/internal/commands/mcp.go +++ b/internal/commands/mcp.go @@ -125,9 +125,11 @@ instead.`, "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.\n\n" + + "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.\n\n" + + "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 " + @@ -242,10 +244,10 @@ const ( listCommandsBrowseFields = "command,description,destructive,subcommands,flags" listCommandsSearchFields = "command,description,destructive" - // maxListCommandsBytes keeps one list_commands result under Claude Code's - // default 25,000-token tool-result limit, above which the client saves the - // result to a file and hands the model only its path. - maxListCommandsBytes = 64 << 10 + // 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. @@ -283,7 +285,10 @@ func listCommands(ctx context.Context, executable, serverProfile string, in list return errorResult(text) } if len(bytes.TrimSpace(out)) == 0 { - out = []byte(`{"matches":0,"hint":"no command matches; use fewer or shorter words, or browse with prefix"}` + "\n") + 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)}}, diff --git a/internal/commands/mcp_list_commands_test.go b/internal/commands/mcp_list_commands_test.go index 038b1300..2664ab5d 100644 --- a/internal/commands/mcp_list_commands_test.go +++ b/internal/commands/mcp_list_commands_test.go @@ -175,3 +175,29 @@ func TestListCommandsArgs_KeepModelTextAsFlagValues(t *testing.T) { 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 5740cd06..289ab687 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -1286,7 +1286,16 @@ With no flags, every command in the tree is listed. Three flags narrow the list: // 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\"") @@ -1323,7 +1332,7 @@ func queryCatalog(root *cobra.Command, q catalogQuery) ([]commandEntry, error) { } if q.Search != "" { - entries = searchCatalog(entries, q.Search) + return searchCatalog(entries, q.Search) } return entries, nil } @@ -1343,7 +1352,7 @@ func resolveCatalogPrefix(root *cobra.Command, prefix string) (node *cobra.Comma if next == nil { return nil, "", "", "", exitcode.New(exitcode.Usage, fmt.Sprintf("--prefix %q: %q has no subcommand %q", prefix, catalogPathOrTop(path), word)). - WithHint("run 'jamf-cli commands --children' to see the top level, then add one word at a time") + 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()) @@ -1389,9 +1398,15 @@ func catalogChildren(node *cobra.Command, path, product, group string) []command } // searchCatalog keeps the entries whose path, description or aliases hold every -// word of search, each word matching the start of a word there. -func searchCatalog(entries []commandEntry, search string) []commandEntry { +// 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, " ")) @@ -1399,40 +1414,57 @@ func searchCatalog(entries []commandEntry, search string) []commandEntry { kept = append(kept, e) } } - return kept + return kept, nil } +// 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 { - found := false - for _, h := range hay { - if strings.HasPrefix(h, w) { - found = true - break - } - } - if !found { + if !catalogMatchesOne(hay, w) { return false } } return true } -// catalogWords lowercases s, splits it on anything but letters and digits, and -// folds a plural to its singular, so "policy" finds "policies". +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 +} + +// catalogWords lowercases s and splits it on anything but letters and digits. func catalogWords(s string) []string { - words := strings.FieldsFunc(strings.ToLower(s), func(r rune) bool { + return strings.FieldsFunc(strings.ToLower(s), func(r rune) bool { return !unicode.IsLetter(r) && !unicode.IsDigit(r) }) - for i, w := range words { - switch { - case len(w) > 4 && strings.HasSuffix(w, "ies"): - words[i] = w[:len(w)-3] + "y" - case len(w) > 3 && strings.HasSuffix(w, "s") && !strings.HasSuffix(w, "ss"): - words[i] = w[:len(w)-1] - } +} + +// 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 words + return forms } // collectCommands recursively walks the command tree and returns leaf commands.