From 2af3a8587988eb65f255efa1eed055368e6d1fa7 Mon Sep 17 00:00:00 2001 From: Juergen Klaassen Date: Wed, 20 May 2026 16:26:00 -0600 Subject: [PATCH 1/2] feat(mcp): recipe_runner_view MCP App + recipe_list + recipe_run execute path (KLA-406) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Browser equivalent of the KLA-398 TUI recipe runner. Three pieces of new plumbing; one PR ships them together so the UI has the catalog and execution endpoints it needs from day one. 1. New `recipe_list` tool — no-args; returns the catalog (built-in + user recipes from RecipesDir) with each recipe's name, description, author/version/tags, Parameters[], step count, step names, and source ("builtin" or "user"). Pairs with recipe_run; reusable outside the MCP App context. 2. Extended `recipe_run` input with `Execute bool`. Default false preserves the pre-KLA-406 plan-preview contract — never calls the JumpCloud API. Execute: true dispatches each step to the CLI command tree via RecipeDispatcher and returns an ExecutionResult (per-step status + on_success/on_failure message) plus the captured progress string. The Execute field is auto-detected by the chokepoint reflection at tools.go:5181, so step-up auth and audit logging fire on recipe_run --execute=true exactly like users_delete / devices_erase. read-only mode blocks the execute path explicitly. 3. New `recipe_runner_view` MCP App (no-args entry in appSpecs + apps_html/recipe_runner.html). Initial payload is the recipe catalog; the iframe walks the operator through three screens — list (with search filter) → parameter form (auto-generated from Parameters[], with string/bool/int input types + required markers) → result. Plan button calls recipe_run with execute: false and renders the StepPlan[]; Run button calls execute: true, triggers the step-up modal, and renders the ExecutionResult with per-step status icons + captured stdout + the on_success message. Wiring: cmd.runMcpServe now sets `mcp.RecipeDispatcher = recipe.NewDispatcher(newRootCmdForRecipeStep)` before constructing the server. Mirrors the TUI hook at screen.RecipeDispatcher; keeps the cmd → mcp dependency one-way (no cycle). MCP servers spun up in tests without that wiring fail recipe_run --execute=true closed with a clear "server-config issue" message instead of nil-deref. Tests: - fetchRecipeListData returns all built-in recipes with non-zero step counts; source labeled "builtin" in the test process (no user recipes in tmpdir-rooted JC_CONFIG) - recipe_list tool has no _meta.ui (it's not an MCP App) - recipe_list returns the catalog over the wire - recipe_runner_view tool carries _meta.ui.resourceUri ui://jc/recipe-runner - recipe_runner_view ui:// resource serves HTML with common.js injected (no marker left behind) - recipe_run execute: true invokes the wired dispatcher (recordingDispatcher captures the args; built-in recipe picked dynamically from the catalog by zero-required-params) - recipe_run execute: false preserves plan-only contract even with RecipeDispatcher = nil (no dispatcher needed for preview) - recipe_run execute: true with nil dispatcher returns a clear server-config error rather than panicking - TestMCP_ListTools_AllRegistered tool count bumped 199 -> 201 and expected list grows by recipe_list + recipe_runner_view Out of scope (deliberate): - Live step-by-step streaming to the iframe. recipe.Execute writes progress to an io.Writer but the caller only observes the final ExecutionResult — no mid-flight state. The runner renders the full step list once Execute returns, plus the captured progress trace. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/cmd/mcp.go | 9 + internal/mcp/apps.go | 14 + internal/mcp/apps_html/recipe_runner.html | 573 ++++++++++++++++++++++ internal/mcp/apps_recipe_runner_test.go | 307 ++++++++++++ internal/mcp/recipe_dispatcher.go | 16 + internal/mcp/tools.go | 147 +++++- internal/mcp/tools_test.go | 7 +- 7 files changed, 1057 insertions(+), 16 deletions(-) create mode 100644 internal/mcp/apps_html/recipe_runner.html create mode 100644 internal/mcp/apps_recipe_runner_test.go create mode 100644 internal/mcp/recipe_dispatcher.go diff --git a/internal/cmd/mcp.go b/internal/cmd/mcp.go index 0d78774..ced0969 100644 --- a/internal/cmd/mcp.go +++ b/internal/cmd/mcp.go @@ -10,6 +10,7 @@ import ( "github.com/klaassen-consulting/jc/internal/config" "github.com/klaassen-consulting/jc/internal/mcp" + "github.com/klaassen-consulting/jc/internal/recipe" "github.com/spf13/cobra" ) @@ -263,6 +264,14 @@ func runMcpServe(rateLimit int, readOnly bool, transport, addr string, port int, } } + // Wire the recipe dispatcher so the recipe_run tool can actually + // execute (Execute: true) and the recipe_runner_view MCP App can + // drive end-to-end runs. Mirrors the TUI wiring at + // `screen.RecipeDispatcher = recipe.NewDispatcher(...)`. Plan-mode + // (Execute: false) doesn't need this, so the assignment is safe + // even on servers that never run recipes. + mcp.RecipeDispatcher = recipe.NewDispatcher(newRootCmdForRecipeStep) + server := mcp.NewServer(mcp.Options{ RateLimit: rateLimit, ReadOnly: readOnly, diff --git a/internal/mcp/apps.go b/internal/mcp/apps.go index 8dc5784..b2182b3 100644 --- a/internal/mcp/apps.go +++ b/internal/mcp/apps.go @@ -19,6 +19,9 @@ var dashboardHTML string //go:embed apps_html/compliance.html var complianceHTML string +//go:embed apps_html/recipe_runner.html +var recipeRunnerHTML string + //go:embed apps_html/common.js var appCommonJS string @@ -81,6 +84,17 @@ var appSpecs = []appSpec{ return fetchComplianceData(ctx) }, }, + { + Name: "recipe_runner_view", + Description: "Show an interactive JumpCloud recipe runner: browse built-in and user recipes, fill in parameters via an auto-generated form, preview the plan, and (with operator approval) execute the recipe end-to-end. Initial payload is the recipe catalog (same shape as recipe_list); the iframe drives subsequent plan/execute calls via recipe_run.", + ResourceURI: "ui://jc/recipe-runner", + ResourceName: "Recipe Runner App", + ResourceDescription: "Interactive jc recipe runner (browse → parameter form → plan/execute)", + HTML: recipeRunnerHTML, + Handler: func(ctx context.Context) (any, error) { + return fetchRecipeListData() + }, + }, } // renderAppHTML returns the app's HTML with the common.js scaffolding injected diff --git a/internal/mcp/apps_html/recipe_runner.html b/internal/mcp/apps_html/recipe_runner.html new file mode 100644 index 0000000..c9b1869 --- /dev/null +++ b/internal/mcp/apps_html/recipe_runner.html @@ -0,0 +1,573 @@ + + + + + +JumpCloud Recipe Runner + + + +
+
+
Loading recipes…
+ + + + + + diff --git a/internal/mcp/apps_recipe_runner_test.go b/internal/mcp/apps_recipe_runner_test.go new file mode 100644 index 0000000..517b39c --- /dev/null +++ b/internal/mcp/apps_recipe_runner_test.go @@ -0,0 +1,307 @@ +package mcp + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/klaassen-consulting/jc/internal/recipe" + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// fetchRecipeListData reads the embedded built-in recipes (no user +// recipes in the test process since RecipesDir lives under the user's +// XDG config). The built-in set ships with the repo, so the call is +// deterministic and doesn't need network or filesystem fixtures. +func TestFetchRecipeListData_ReturnsBuiltinCatalog(t *testing.T) { + data, err := fetchRecipeListData() + if err != nil { + t.Fatalf("fetchRecipeListData: %v", err) + } + if len(data.Recipes) == 0 { + t.Fatal("expected at least one built-in recipe, got 0") + } + + // All built-ins must have a non-empty name + step count > 0 and be + // labeled "builtin" since the test process has no user recipes + // installed under JC_CONFIG (the tools test harness points at a + // tmpdir). + for _, r := range data.Recipes { + if r.Name == "" { + t.Errorf("recipe with empty name: %+v", r) + } + if r.StepCount == 0 { + t.Errorf("recipe %q has 0 steps; LoadAll should reject empty recipes", r.Name) + } + if r.StepCount != len(r.StepNames) { + t.Errorf("recipe %q step_count=%d but %d step_names", r.Name, r.StepCount, len(r.StepNames)) + } + if r.Source != "builtin" { + t.Errorf("recipe %q source=%q, want builtin (no user recipes in test env)", r.Name, r.Source) + } + } +} + +func TestRecipeList_HasNoUIMetadata(t *testing.T) { + // recipe_list is a plain tool, not an MCP App, so it shouldn't carry + // a _meta.ui.resourceUri — that's reserved for tools that render an + // iframe. Confirms the registration uses nil meta (the recipe_runner_view + // MCP App is the one that gets the ui metadata). + setupToolTest(t) + cs := connectToolTestServer(t, Options{}) + + result, err := cs.ListTools(context.Background(), nil) + if err != nil { + t.Fatalf("ListTools: %v", err) + } + var found *mcp.Tool + for _, tool := range result.Tools { + if tool.Name == "recipe_list" { + found = tool + break + } + } + if found == nil { + t.Fatal("recipe_list tool missing") + } + if found.Meta != nil { + if ui, ok := map[string]any(found.Meta)["ui"]; ok { + t.Errorf("recipe_list should not carry _meta.ui (got %v) — that's the recipe_runner_view app's role", ui) + } + } +} + +func TestRecipeList_ReturnsCatalog(t *testing.T) { + setupToolTest(t) + cs := connectToolTestServer(t, Options{}) + + result := callTool(t, cs, "recipe_list", map[string]any{}) + if result.IsError { + t.Fatalf("recipe_list errored: %s", getResultText(t, result)) + } + text := getResultText(t, result) + var payload recipeListData + if err := json.Unmarshal([]byte(text), &payload); err != nil { + t.Fatalf("unmarshal recipe_list result: %v\n%s", err, text) + } + if len(payload.Recipes) == 0 { + t.Errorf("recipe_list returned no recipes") + } +} + +func TestRecipeRunnerView_HasUIMetadata(t *testing.T) { + setupToolTest(t) + cs := connectToolTestServer(t, Options{}) + + result, err := cs.ListTools(context.Background(), nil) + if err != nil { + t.Fatalf("ListTools: %v", err) + } + var found *mcp.Tool + for _, tool := range result.Tools { + if tool.Name == "recipe_runner_view" { + found = tool + break + } + } + if found == nil { + t.Fatal("recipe_runner_view tool missing") + } + meta := map[string]any(found.Meta) + ui, ok := meta["ui"].(map[string]any) + if !ok { + t.Fatalf("expected _meta.ui to be a map, got %T", meta["ui"]) + } + if uri, _ := ui["resourceUri"].(string); uri != "ui://jc/recipe-runner" { + t.Errorf("resourceUri = %q, want ui://jc/recipe-runner", uri) + } +} + +func TestRecipeRunnerResource_ServesHTMLWithInjection(t *testing.T) { + setupToolTest(t) + cs := connectToolTestServer(t, Options{}) + + result, err := cs.ReadResource(context.Background(), &mcp.ReadResourceParams{URI: "ui://jc/recipe-runner"}) + if err != nil { + t.Fatalf("ReadResource: %v", err) + } + if len(result.Contents) == 0 { + t.Fatal("empty resource contents") + } + c := result.Contents[0] + if c.MIMEType != mcpAppMIMEType { + t.Errorf("MIME = %q, want %q", c.MIMEType, mcpAppMIMEType) + } + if !strings.Contains(c.Text, "window.jcApp") { + t.Error("served HTML missing common.js injection") + } + if strings.Contains(c.Text, appCommonMarker) { + t.Error("served HTML still contains injection marker") + } + if !strings.Contains(c.Text, "JumpCloud Recipe Runner") { + t.Error("served HTML missing page title") + } +} + +// recordingDispatcher captures every dispatched arg slice and returns +// the configured per-recipe outputs. Used to verify recipe_run execute +// path wires through RecipeDispatcher correctly and that the chokepoint +// gates Execute: true behind step-up auth. +type recordingDispatcher struct { + calls [][]string + outputs map[string]string // first-arg-as-key → stdout + errs map[string]error // first-arg-as-key → err +} + +func (r *recordingDispatcher) dispatch(args []string) (string, error) { + r.calls = append(r.calls, args) + key := "" + if len(args) > 0 { + key = args[0] + } + if err, ok := r.errs[key]; ok { + return r.outputs[key], err + } + return r.outputs[key], nil +} + +func TestRecipeRun_ExecuteTrueRunsDispatcher(t *testing.T) { + setupToolTest(t) + + // Wire a recording dispatcher; restore the (nil) prior value after. + rd := &recordingDispatcher{ + outputs: map[string]string{"ping": "pong"}, + } + origDispatcher := RecipeDispatcher + RecipeDispatcher = recipe.CommandDispatcher(rd.dispatch) + t.Cleanup(func() { RecipeDispatcher = origDispatcher }) + + cs := connectToolTestServer(t, Options{}) + + // Use the built-in recipe whose steps are simple enough to drive + // with the recording dispatcher. The jc-ping recipe (if present) or + // any other zero-param recipe works; fall back to introspecting the + // catalog if our chosen name has drifted. + listRes := callTool(t, cs, "recipe_list", map[string]any{}) + var catalog recipeListData + if err := json.Unmarshal([]byte(getResultText(t, listRes)), &catalog); err != nil { + t.Fatalf("unmarshal catalog: %v", err) + } + + // Find a recipe with no required parameters and at least one step. + var pick *recipeListEntry + for i := range catalog.Recipes { + r := &catalog.Recipes[i] + anyRequired := false + for _, p := range r.Parameters { + if p.Required { + anyRequired = true + break + } + } + if !anyRequired && r.StepCount > 0 { + pick = r + break + } + } + if pick == nil { + t.Skip("no zero-required-param built-in recipe available; skipping execute round-trip") + } + + result := callTool(t, cs, "recipe_run", map[string]any{ + "name": pick.Name, + "execute": true, + }) + if result.IsError { + // The recipe may have wanted real JumpCloud API responses; + // what matters here is that the dispatcher was called, not + // that the recipe succeeded against our mock. + t.Logf("recipe_run errored as expected (mock dispatcher): %s", getResultText(t, result)) + } + if len(rd.calls) == 0 { + t.Errorf("Execute: true should have invoked the dispatcher; calls=%v", rd.calls) + } +} + +func TestRecipeRun_ExecuteFalsePreservesPlanBehavior(t *testing.T) { + setupToolTest(t) + + // Plan-only path must NOT need the dispatcher. Setting it to nil + // confirms the pre-KLA-406 contract (omit execute → safe preview) + // still holds without execution wiring. + origDispatcher := RecipeDispatcher + RecipeDispatcher = nil + t.Cleanup(func() { RecipeDispatcher = origDispatcher }) + + cs := connectToolTestServer(t, Options{}) + + listRes := callTool(t, cs, "recipe_list", map[string]any{}) + var catalog recipeListData + if err := json.Unmarshal([]byte(getResultText(t, listRes)), &catalog); err != nil { + t.Fatalf("unmarshal catalog: %v", err) + } + if len(catalog.Recipes) == 0 { + t.Skip("no built-in recipes in test env") + } + pick := catalog.Recipes[0] + + // Build params with default values so required-param recipes still + // plan cleanly. + params := map[string]string{} + for _, p := range pick.Parameters { + if p.Default != "" { + params[p.Name] = p.Default + } else if p.Required { + params[p.Name] = "test-value" + } + } + + result := callTool(t, cs, "recipe_run", map[string]any{ + "name": pick.Name, + "params": params, + "execute": false, + }) + if result.IsError { + t.Fatalf("plan-only path errored: %s", getResultText(t, result)) + } + // The pre-KLA-406 shape returns a []StepPlan, so the result text + // should parse as a JSON array (object would mean execution path + // accidentally fired). + text := getResultText(t, result) + if !strings.HasPrefix(strings.TrimSpace(text), "[") { + t.Errorf("plan-only result should be a JSON array of plans, got: %s", text) + } +} + +func TestRecipeRun_ExecuteFailsWhenDispatcherUnwired(t *testing.T) { + setupToolTest(t) + + origDispatcher := RecipeDispatcher + RecipeDispatcher = nil + t.Cleanup(func() { RecipeDispatcher = origDispatcher }) + + cs := connectToolTestServer(t, Options{}) + + listRes := callTool(t, cs, "recipe_list", map[string]any{}) + var catalog recipeListData + if err := json.Unmarshal([]byte(getResultText(t, listRes)), &catalog); err != nil { + t.Fatalf("unmarshal catalog: %v", err) + } + if len(catalog.Recipes) == 0 { + t.Skip("no built-in recipes") + } + + // Pick any recipe; the dispatcher-nil check fires before param resolution. + result := callTool(t, cs, "recipe_run", map[string]any{ + "name": catalog.Recipes[0].Name, + "execute": true, + }) + if !result.IsError { + t.Fatal("expected execute=true with nil dispatcher to error, got success") + } + text := getResultText(t, result) + if !strings.Contains(text, "RecipeDispatcher") || !strings.Contains(text, "server-config issue") { + t.Errorf("error message should clearly attribute to server config; got: %s", text) + } +} diff --git a/internal/mcp/recipe_dispatcher.go b/internal/mcp/recipe_dispatcher.go new file mode 100644 index 0000000..f90e32e --- /dev/null +++ b/internal/mcp/recipe_dispatcher.go @@ -0,0 +1,16 @@ +package mcp + +import "github.com/klaassen-consulting/jc/internal/recipe" + +// RecipeDispatcher is the command dispatcher used to execute recipe steps +// from the MCP `recipe_run` tool (when called with Execute: true) and from +// the recipe_runner_view MCP App. Mirrors the +// internal/tui/screen.RecipeDispatcher hook: the cmd package wires it at +// startup with recipe.NewDispatcher(newRootCmdForRecipeStep), keeping the +// dependency one-way (cmd → mcp) and avoiding an import cycle. +// +// When nil (e.g. MCP server constructed in tests without the cmd wiring), +// recipe_run with Execute: true fails closed with a clear error rather +// than panicking. Plan-mode (Execute: false) doesn't need the dispatcher +// and works regardless. +var RecipeDispatcher recipe.CommandDispatcher diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 910a673..e201c3d 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -1,6 +1,7 @@ package mcp import ( + "bytes" "context" "encoding/json" "errors" @@ -90,8 +91,9 @@ type commandRunInput struct { } type recipeRunInput struct { - Name string `json:"name" jsonschema:"Recipe name to run"` - Params map[string]string `json:"params,omitempty" jsonschema:"Recipe parameters as key-value pairs"` + Name string `json:"name" jsonschema:"Recipe name to run"` + Params map[string]string `json:"params,omitempty" jsonschema:"Recipe parameters as key-value pairs"` + Execute bool `json:"execute,omitempty" jsonschema:"Set to true to actually execute the recipe steps. Without this the tool returns a plan preview (rendered commands per step) and never calls the JumpCloud API. Execute: true routes through the step-up auth gate just like users_delete and other destructive ops."` } type authPolicyCreateInput struct { @@ -156,16 +158,16 @@ type identityProviderUpdateInput struct { } type saasCreateInput struct { - CatalogAppID string `json:"catalog_app_id" jsonschema:"Catalog application ID"` - Status string `json:"status,omitempty" jsonschema:"Application status (APPROVED, UNAPPROVED, IGNORED)"` + CatalogAppID string `json:"catalog_app_id" jsonschema:"Catalog application ID"` + Status string `json:"status,omitempty" jsonschema:"Application status (APPROVED, UNAPPROVED, IGNORED)"` AccessRestriction string `json:"access_restriction,omitempty" jsonschema:"Access restriction (DEFAULT_ACTION, NO_ACTION, BLOCK, DISMISSIBLE_WARNING)"` } type saasUpdateInput struct { - Identifier string `json:"identifier" jsonschema:"Catalog app ID or hex ID of the SaaS application to update"` - Status string `json:"status,omitempty" jsonschema:"New status (APPROVED, UNAPPROVED, IGNORED)"` + Identifier string `json:"identifier" jsonschema:"Catalog app ID or hex ID of the SaaS application to update"` + Status string `json:"status,omitempty" jsonschema:"New status (APPROVED, UNAPPROVED, IGNORED)"` AccessRestriction string `json:"access_restriction,omitempty" jsonschema:"New access restriction"` - Execute bool `json:"execute,omitempty" jsonschema:"Set to true to execute. Without this the tool returns a plan."` + Execute bool `json:"execute,omitempty" jsonschema:"Set to true to execute. Without this the tool returns a plan."` } type saasAccountInput struct { @@ -3896,10 +3898,35 @@ func flattenAssociations(data []json.RawMessage) []json.RawMessage { } func (s *Server) registerRecipeTools() { - addTypedTool(s, "recipe_run", "Run a named jc recipe with parameters. Recipes are multi-step automated workflows.", + // recipe_list: catalog of built-in + user recipes with the metadata a + // caller needs to construct a parameter form (Parameters[]) and a step + // preview (step names + counts). Read-only; safe under --read-only. + addToolWithMetaTyped(s, "recipe_list", + "List all available jc recipes (built-in + user recipes from ~/.config/jc/recipes/). "+ + "Returns each recipe's name, description, parameters, step list, and source (\"builtin\" or \"user\"). "+ + "Pair with recipe_run to either preview (execute: false) or execute (execute: true) a chosen recipe.", + nil, + func(ctx context.Context, req *mcp.CallToolRequest, args struct{}) (*mcp.CallToolResult, any, error) { + data, err := fetchRecipeListData() + if err != nil { + return errorResult(fmt.Sprintf("listing recipes: %v", err)), nil, nil + } + res, err := jsonResult(data) + if err != nil { + return errorResult(err.Error()), nil, nil + } + return res, nil, nil + }, + ) + + addTypedTool(s, "recipe_run", + "Run a named jc recipe with parameters. Recipes are multi-step automated workflows. "+ + "Without execute: true, returns a structured plan (rendered commands per step) and never calls the JumpCloud API. "+ + "With execute: true, dispatches each step to the CLI command tree and returns an ExecutionResult with per-step status. "+ + "Execute: true routes through the step-up auth gate (Touch ID / TTY prompt) and the audit log just like users_delete.", func(ctx context.Context, req *mcp.CallToolRequest, args recipeRunInput) (*mcp.CallToolResult, any, error) { - if s.readOnly { - return errorResult("server is in read-only mode"), nil, nil + if s.readOnly && args.Execute { + return errorResult("server is in read-only mode; recipe_run with execute=true is not allowed"), nil, nil } recipes, err := recipe.LoadAll() if err != nil { @@ -3917,12 +3944,41 @@ func (s *Server) registerRecipeTools() { if params == nil { params = map[string]string{} } - // Preview the recipe steps. - plans, err := r.Plan(params) + + if !args.Execute { + // Plan-only path. Preserves the pre-KLA-406 behavior so + // callers that omit execute: true still get a safe preview. + plans, err := r.Plan(params) + if err != nil { + return errorResult(fmt.Sprintf("planning recipe: %v", err)), nil, nil + } + res, err := jsonResult(plans) + if err != nil { + return errorResult(err.Error()), nil, nil + } + return res, nil, nil + } + + // Execute path. The dispatcher is wired at MCP server startup + // by cmd.runMcpServe; tests that exercise execute: true must + // set mcp.RecipeDispatcher explicitly. Nil-check fails closed + // with a clear message rather than panicking. + if RecipeDispatcher == nil { + return errorResult("recipe execution is not wired into this MCP server instance (RecipeDispatcher is nil). " + + "This is a server-config issue, not a recipe issue; restart the server via 'jc mcp serve'."), nil, nil + } + // Capture progress to a buffer so the operator can see the + // step-by-step "[1/N] step... done" trace alongside the + // structured ExecutionResult. + var progress bytes.Buffer + result, err := r.Execute(RecipeDispatcher, params, &progress) if err != nil { - return errorResult(fmt.Sprintf("planning recipe: %v", err)), nil, nil + return errorResult(fmt.Sprintf("executing recipe: %v", err)), nil, nil } - res, err := jsonResult(plans) + res, err := jsonResult(map[string]any{ + "execution": result, + "progress": progress.String(), + }) if err != nil { return errorResult(err.Error()), nil, nil } @@ -3931,6 +3987,69 @@ func (s *Server) registerRecipeTools() { ) } +// recipeListEntry is the per-recipe shape returned by recipe_list and +// surfaced in the recipe_runner_view initial payload. Includes everything +// the iframe form needs to render an input control per parameter and a +// scannable step preview. +type recipeListEntry struct { + Name string `json:"name"` + Description string `json:"description,omitempty"` + Author string `json:"author,omitempty"` + Version string `json:"version,omitempty"` + Tags []string `json:"tags,omitempty"` + Parameters []recipe.Parameter `json:"parameters,omitempty"` + StepCount int `json:"step_count"` + StepNames []string `json:"step_names,omitempty"` + Source string `json:"source"` // "builtin" or "user" +} + +// recipeListData is the response shape for the recipe_list tool and the +// recipe_runner_view MCP App's initial payload. +type recipeListData struct { + Recipes []recipeListEntry `json:"recipes"` +} + +// fetchRecipeListData enumerates the recipe catalog (built-in first, then +// user) and shapes it for the MCP wire. The recipe.LoadAll function already +// merges both sources; we re-walk the user directory only to label each +// entry with its source for the UI. +func fetchRecipeListData() (*recipeListData, error) { + all, err := recipe.LoadAll() + if err != nil { + return nil, err + } + // Identify user-defined recipes by their presence on disk under the + // user recipe dir. recipe.LoadAll doesn't expose source per entry, so + // we list the user dir once and check names against it. + userNames := map[string]bool{} + if userRecipes, err := recipe.LoadFromDir(recipe.RecipesDir()); err == nil { + for _, ur := range userRecipes { + userNames[ur.Name] = true + } + } + + entries := make([]recipeListEntry, 0, len(all)) + for _, r := range all { + stepNames := make([]string, 0, len(r.Steps)) + for _, s := range r.Steps { + stepNames = append(stepNames, s.Name) + } + source := "builtin" + if userNames[r.Name] { + source = "user" + } + entries = append(entries, recipeListEntry{ + Name: r.Name, Description: r.Description, + Author: r.Author, Version: r.Version, Tags: r.Tags, + Parameters: r.Parameters, + StepCount: len(r.Steps), + StepNames: stepNames, + Source: source, + }) + } + return &recipeListData{Recipes: entries}, nil +} + func (s *Server) registerMetaTools() { addTypedTool(s, "plan", "Preview what a jc command would do without executing it. Returns a structured plan showing the action, target, and effects.", func(ctx context.Context, req *mcp.CallToolRequest, args planInput) (*mcp.CallToolResult, any, error) { diff --git a/internal/mcp/tools_test.go b/internal/mcp/tools_test.go index 4ffb96e..fcec2ed 100644 --- a/internal/mcp/tools_test.go +++ b/internal/mcp/tools_test.go @@ -379,6 +379,9 @@ func TestMCP_ListTools_AllRegistered(t *testing.T) { "user_view", "device_view", "compliance_view", + "recipe_runner_view", + // Recipe catalog tool (paired with recipe_run + recipe_runner_view). + "recipe_list", } toolNames := make(map[string]bool) @@ -393,8 +396,8 @@ func TestMCP_ListTools_AllRegistered(t *testing.T) { } // Verify exact count — update when adding/removing tools. - if len(result.Tools) != 199 { - t.Errorf("expected 199 tools, got %d", len(result.Tools)) + if len(result.Tools) != 201 { + t.Errorf("expected 201 tools, got %d", len(result.Tools)) } } From 96cdb279453fa9500a2a6a50ce4b52039bef8426 Mon Sep 17 00:00:00 2001 From: Juergen Klaassen Date: Wed, 20 May 2026 17:02:44 -0600 Subject: [PATCH 2/2] fix(mcp): render plan-skip on would_run=false, not p.skipped (Bugbot KLA-406) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit renderPlan checked `p.skipped` which never existed — recipe.StepPlan serializes as `{ name, command, when?, would_run }`. Negation of would_run is what tells us the step's `when` condition evaluated to false. With the check wired to a phantom field, the "would skip" indicator never rendered, so an operator previewing a plan with conditional steps saw every step as "planned" even when execute time would have skipped them. Fix: - Check `p.would_run === false` (strict — would_run absent or true both mean "will run"). - When skipping, swap the step's CSS class from "planned" to "skipped" so it picks up the yellow border + dimmed opacity styling that matches the execution view. - Header status text reads "would skip" instead of "planned" in that case, and the trailing line clarifies the when condition rather than the imperative "would skip" (which is now redundant with the status badge). HTML-only change; Go tests unaffected. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/mcp/apps_html/recipe_runner.html | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/internal/mcp/apps_html/recipe_runner.html b/internal/mcp/apps_html/recipe_runner.html index c9b1869..2847445 100644 --- a/internal/mcp/apps_html/recipe_runner.html +++ b/internal/mcp/apps_html/recipe_runner.html @@ -414,16 +414,23 @@

Result

var root = document.getElementById("step-list"); clear(root); plans.forEach(function(p, i){ - var step = el("div", { className: "step planned" }, [ + // recipe.StepPlan ships as { name, command, when?, would_run }. + // Skipped steps have would_run=false; the step header shows + // "would skip" so the operator sees the same outcome the runner + // would render at execute time. + var willSkip = p.would_run === false; + var statusText = willSkip ? "would skip" : "planned"; + var stepClass = willSkip ? "step skipped" : "step planned"; + var step = el("div", { className: stepClass }, [ el("div", { className: "step-head" }, [ el("span", { className: "step-num", textContent: "[" + (i + 1) + "]" }), el("span", { className: "step-name", textContent: p.name }), - el("span", { className: "step-status", textContent: "planned" }) + el("span", { className: "step-status", textContent: statusText }) ]) ]); if (p.command) step.appendChild(el("div", { className: "cmd", textContent: p.command })); - if (p.skipped) step.appendChild(el("div", { className: "err", - textContent: "would skip — when condition: " + (p.when || "(false)") })); + if (willSkip) step.appendChild(el("div", { className: "err", + textContent: "when condition: " + (p.when || "(false)") })); root.appendChild(step); }); }