From 0876cdde8c555e75556df301dcbe2f09fb488693 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Mon, 24 Aug 2026 20:53:46 +0530 Subject: [PATCH 1/4] feat(mcp): add parameterized 'onboard' prompt The server advertised prompts capability but registered none. Add the first: 'onboard', a multi-step workflow driving an agent end-to-end from an empty directory to a deployed, invocable Function. The eight steps map onto existing tools and resources: version (with a kubectl-context check), func://languages, create, run + invoke --target local + run_stop, registry elicitation, deploy + describe, invoke --target remote, and a summary read back out of tool output rather than out of what was requested. All four arguments (language, template, registry, cluster) are optional. Supplied values are rendered into the prompt and marked as decided; omitted ones render as an explicit instruction to ask the user at the relevant step, since neither a language nor a registry has a sane default. template defaults to http and cluster to local. Values are normalized (case, whitespace) and validated, with "cloudevent" resolving to the "cloudevents" template that actually exists on disk; the registry keeps its case, being an image reference prefix. Unrecognized values are rejected rather than rendered, so the agent is never sent off to run a command that cannot succeed. The prompt body is an embedded text/template, mirroring how the server's instructions are embedded. In read-only mode the registry, deploy and remote-invoke steps are omitted rather than handed to an agent that would be refused, and the user is told to restart with FUNC_ENABLE_MCP_WRITE=true. --- pkg/mcp/instructions.md | 19 ++ pkg/mcp/mcp.go | 5 + pkg/mcp/prompts.go | 61 ++++++ pkg/mcp/prompts_onboard.go | 131 ++++++++++++ pkg/mcp/prompts_onboard.md | 168 +++++++++++++++ pkg/mcp/prompts_onboard_test.go | 354 ++++++++++++++++++++++++++++++++ 6 files changed, 738 insertions(+) create mode 100644 pkg/mcp/prompts.go create mode 100644 pkg/mcp/prompts_onboard.go create mode 100644 pkg/mcp/prompts_onboard.md create mode 100644 pkg/mcp/prompts_onboard_test.go diff --git a/pkg/mcp/instructions.md b/pkg/mcp/instructions.md index 90ad89b37f..3f1dd7fe1f 100644 --- a/pkg/mcp/instructions.md +++ b/pkg/mcp/instructions.md @@ -51,6 +51,25 @@ This is essential because: - **OVERRIDE specific settings**: Call "deploy" tool with specific flags (e.g., --builder pack, --registry docker.io/user) - Example: "deploy with pack builder" → call deploy tool with --builder pack only +## Prompts + +This server also exposes prompts: named, parameterized workflows the client +invokes on the user's behalf. + +### onboard + +- Drives full end-to-end onboarding: prerequisites → language → scaffold → + local run and invoke → registry → deploy → remote invoke → summary +- All four arguments (`language`, `template`, `registry`, `cluster`) are + optional. Supplied values are treated as decided; omitted ones are gathered + from the user as the relevant step is reached +- `template` defaults to `http` and `cluster` defaults to `local` +- In read-only mode the deploy-dependent steps are omitted from the returned + prompt, since they would be refused +- If a user asks to "get started", "set up a Function from scratch", or + similar, suggest they invoke this prompt rather than improvising the + sequence yourself + ## Tool Usage Guide ### General Rules diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go index 5f439027c0..4581ead0b0 100644 --- a/pkg/mcp/mcp.go +++ b/pkg/mcp/mcp.go @@ -157,6 +157,11 @@ func New(options ...Option) *Server { mcp.AddTool(i, configGitSetTool, s.configGitSetHandler) mcp.AddTool(i, configGitRemoveTool, s.configGitRemoveHandler) + // Prompts + // ------- + // Multi-step, parameterized workflows the client can invoke by name + i.AddPrompt(onboardPrompt, s.onboardHandler) + // Resources // --------- // Current Function state diff --git a/pkg/mcp/prompts.go b/pkg/mcp/prompts.go new file mode 100644 index 0000000000..db9d8334b4 --- /dev/null +++ b/pkg/mcp/prompts.go @@ -0,0 +1,61 @@ +package mcp + +import ( + "fmt" + "strings" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// prompt helpers: + +// newUserPromptResult returns a GetPromptResult carrying a single user-role +// text message. Every func prompt is a single, self-contained instruction +// block for the agent, so this is the only shape currently needed. +func newUserPromptResult(description, text string) *mcp.GetPromptResult { + return &mcp.GetPromptResult{ + Description: description, + Messages: []*mcp.PromptMessage{ + { + Role: "user", + Content: &mcp.TextContent{Text: text}, + }, + }, + } +} + +// rawPromptArg returns the named argument from the request verbatim, for +// values which are case-sensitive (such as a container registry). Returns "" +// when the argument was not provided. +func rawPromptArg(r *mcp.GetPromptRequest, name string) string { + if r == nil || r.Params == nil { + return "" + } + return r.Params.Arguments[name] +} + +// promptArg returns the named argument from the request, normalized by +// trimming surrounding whitespace and lowercasing. Prompt arguments arrive as +// free-form strings typed by a human (or filled in by an agent), so " Go " +// and "go" must be treated as the same value. Returns "" when the argument +// was not provided. +func promptArg(r *mcp.GetPromptRequest, name string) string { + return strings.ToLower(strings.TrimSpace(rawPromptArg(r, name))) +} + +// validateChoice ensures value is one of allowed, returning an error naming +// the offending argument and the valid set. An empty value is always +// accepted: prompt arguments are optional by design, and an omitted one means +// "the agent should ask the user", not "invalid". +func validateChoice(name, value string, allowed []string) error { + if value == "" { + return nil + } + for _, a := range allowed { + if value == a { + return nil + } + } + return fmt.Errorf("invalid %q argument %q: must be one of %s", + name, value, strings.Join(allowed, ", ")) +} diff --git a/pkg/mcp/prompts_onboard.go b/pkg/mcp/prompts_onboard.go new file mode 100644 index 0000000000..b9e298d86a --- /dev/null +++ b/pkg/mcp/prompts_onboard.go @@ -0,0 +1,131 @@ +package mcp + +import ( + "context" + _ "embed" + "fmt" + "strings" + "text/template" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +//go:embed prompts_onboard.md +var onboardPromptBody string + +// onboardPromptTemplate is parsed once at init; a parse failure is a +// programming error in the embedded markdown, so panicking is correct. +var onboardPromptTemplate = template.Must( + template.New("onboard").Parse(onboardPromptBody)) + +// Defaults applied when the caller omits the argument. Language and registry +// have no default on purpose: there is no sane guess, so the agent is +// instructed to ask the user. +const ( + defaultOnboardTemplate = "http" + defaultOnboardCluster = "local" +) + +// Accepted argument values. Languages mirror the runtimes shipped in +// templates/; the authoritative list is still the func://languages resource, +// which the prompt instructs the agent to read, so this is a guard against +// obvious typos rather than a second source of truth. +var ( + onboardLanguages = []string{"go", "node", "python", "typescript", "rust", "quarkus", "springboot"} + onboardTemplates = []string{"http", "cloudevent", "cloudevents"} + onboardClusters = []string{"local", "remote"} +) + +var onboardPrompt = &mcp.Prompt{ + Name: "onboard", + Title: "Onboard a Function", + Description: "Multi-step onboarding: check prerequisites, choose a language, " + + "scaffold, run and invoke locally, configure a registry, deploy, invoke " + + "the live instance, and summarize. All arguments are optional; omitted " + + "ones are gathered from the user as the steps run.", + Arguments: []*mcp.PromptArgument{ + { + Name: "language", + Title: "Language", + Description: "Target runtime: " + strings.Join(onboardLanguages, ", ") + " (asked for if omitted)", + }, + { + Name: "template", + Title: "Template", + Description: "Function template: http or cloudevents (default: " + defaultOnboardTemplate + ")", + }, + { + Name: "registry", + Title: "Registry", + Description: "Container registry prefix, e.g. docker.io/alice (asked for if omitted)", + }, + { + Name: "cluster", + Title: "Cluster", + Description: "Deployment target: local (e.g. kind) or remote (default: " + defaultOnboardCluster + ")", + }, + }, +} + +// onboardParams are the values rendered into the onboarding prompt. +// Language and Registry are empty when the caller did not supply them, which +// the template renders as an explicit "ask the user" instruction. +type onboardParams struct { + Language string + Template string + Registry string + Cluster string + Readonly bool +} + +func (s *Server) onboardHandler(_ context.Context, r *mcp.GetPromptRequest) (*mcp.GetPromptResult, error) { + p, err := newOnboardParams(r, s.readonly.Load()) + if err != nil { + return nil, err + } + + var buf strings.Builder + if err := onboardPromptTemplate.Execute(&buf, p); err != nil { + return nil, fmt.Errorf("error rendering onboarding prompt: %w", err) + } + + return newUserPromptResult(onboardPrompt.Description, buf.String()), nil +} + +// newOnboardParams validates the request's arguments and applies defaults. +func newOnboardParams(r *mcp.GetPromptRequest, readonly bool) (p onboardParams, err error) { + p = onboardParams{ + Language: promptArg(r, "language"), + Template: promptArg(r, "template"), + // The registry is a case-sensitive image reference prefix, so unlike + // the other arguments it must not be lowercased. + Registry: strings.TrimSpace(rawPromptArg(r, "registry")), + Cluster: promptArg(r, "cluster"), + Readonly: readonly, + } + + if err = validateChoice("language", p.Language, onboardLanguages); err != nil { + return + } + if err = validateChoice("template", p.Template, onboardTemplates); err != nil { + return + } + if err = validateChoice("cluster", p.Cluster, onboardClusters); err != nil { + return + } + + if p.Template == "" { + p.Template = defaultOnboardTemplate + } + // "cloudevent" is accepted as an alias because that is how the format is + // spelled elsewhere (e.g. invoke's --format), but the template shipped in + // templates// is named "cloudevents"; normalize so the value + // handed to the create tool is one that actually exists. + if p.Template == "cloudevent" { + p.Template = "cloudevents" + } + if p.Cluster == "" { + p.Cluster = defaultOnboardCluster + } + return +} diff --git a/pkg/mcp/prompts_onboard.md b/pkg/mcp/prompts_onboard.md new file mode 100644 index 0000000000..c0756dc5a4 --- /dev/null +++ b/pkg/mcp/prompts_onboard.md @@ -0,0 +1,168 @@ +# Full Function Onboarding + +Take the user end-to-end: from nothing, to a scaffolded Function, to a +Function running locally, to a Function deployed and answering on a live URL. + +Work through the steps below **in order**. Complete each one and report its +outcome to the user before starting the next. If a step fails, stop, explain +the failure in plain terms, and help the user resolve it rather than skipping +ahead — every later step depends on the ones before it. + +## Session parameters + +| Parameter | Value | +|-----------|-------| +| language | {{if .Language}}`{{.Language}}`{{else}}**not provided** — ask the user in step 2{{end}} | +| template | `{{.Template}}` | +| registry | {{if .Registry}}`{{.Registry}}`{{else}}**not provided** — ask the user in step 5{{end}} | +| cluster | `{{.Cluster}}`{{if eq .Cluster "local"}} (a local cluster, e.g. kind){{else}} (a remote/shared cluster){{end}} | + +Treat provided values as decided: do not re-ask for them. Ask only for the +values marked "not provided". +{{if .Readonly}} +> **This server is in read-only mode.** The registry, deploy and remote-invoke +> steps mutate cluster state and would be refused, so they are omitted below. +> Run steps 1 through 4, give the step 8 summary for what was accomplished +> locally, and tell the user that finishing onboarding — deploying and +> invoking a live instance — requires restarting the MCP server with +> `FUNC_ENABLE_MCP_WRITE=true`. +{{end}} + +## Step 1 — Prerequisites + +Call the `version` tool. It reports the version of the `func` binary this +server drives. + +- If it succeeds, tell the user which version they are on and continue. +- If it fails because `func` is not installed or not on `PATH`, stop and guide + the user through installing it (https://knative.dev/docs/functions/install-func/), + then re-run this step. Do not attempt any later step until `version` + succeeds. +{{if eq .Cluster "local"}} +Because `cluster` is `local`, also confirm with the user that a local cluster +(e.g. kind) is running and that `kubectl` points at it. `func` deploys to +whatever context `kubectl` is currently using, so a wrong context is the most +common cause of a surprising deployment target. +{{else}} +Because `cluster` is `remote`, confirm with the user which cluster and +namespace `kubectl` is currently pointed at, and that it is the intended +deployment target. Do this now, before anything is built. +{{end}} +## Step 2 — Language selection + +{{if .Language}}The language is already chosen: `{{.Language}}`. Read the +`func://languages` resource anyway and verify `{{.Language}}` is listed. If it +is not, tell the user and ask them to choose from the languages actually +available.{{else}}Read the `func://languages` resource and present the user +with the runtimes it actually reports. Do not offer a guessed or remembered +list — the available runtimes depend on the installed binary and any +configured template repositories. Ask the user to choose one and wait for +their answer.{{end}} + +## Step 3 — Scaffold + +Ask the user where the Function should live, and what it should be called if +the directory name is not the name they want. Then call the `create` tool +with: + +- `language`: the runtime from step 2 +- `template`: `{{.Template}}` +- `path`: the **absolute** path to the Function directory + +Then `cd` into that directory yourself and stay there for the rest of this +session — every later tool call takes the same absolute path. Briefly show +the user what was generated (the handler file and `func.yaml`) so they know +where their code lives. + +## Step 4 — Local run and invoke + +Prove the Function works before involving a registry or a cluster. + +1. Call `run` with the Function's absolute `path`. It builds if needed, starts + the Function, and returns a `pid` and a `url`. +2. Call `invoke` with the same `path` and `target: "local"`. With no `data`, a + real `{"message":"Hello World"}` payload is sent — the handler actually + runs. +3. Show the user the response body verbatim. This is the moment the Function + becomes real to them; do not paraphrase it. +4. Call `run_stop` with the same `path`. Always do this, including when the + invoke failed, so the port and process are released. + +Warn the user that the first local build{{if .Language}} for +`{{.Language}}`{{end}} can be slow: builder images may need to be +downloaded, and Podman or Docker must be available. +{{if not .Readonly}} +## Step 5 — Registry configuration + +{{if .Registry}}Use the registry `{{.Registry}}`. Confirm it is well-formed +before continuing: a registry value is a **domain** plus a +**user/organization** joined by a slash, such as `docker.io/alice`. If it is +domain-only, acknowledge that edge case explicitly with the user. Also +confirm they are logged in to it (`docker login` / `podman login`), since the +deploy in step 6 pushes there.{{else}}Ask the user for their container registry. This is +the single most common place first-time onboarding goes wrong, so guide them +concretely rather than just asking: + +- A registry value is a **domain** plus a **user/organization**, joined by a + slash: `docker.io/alice`, `ghcr.io/alice`, `quay.io/alice`. +- If they give only a domain (`docker.io`), ask them to supply the user part, + or to explicitly confirm they mean the domain-only form. +- The image name is derived automatically as `/:latest`; + they do not need to supply an image name. +- They must be logged in to that registry (`docker login` / `podman login`), + because the deploy in step 6 pushes to it. +{{if eq .Cluster "local"}}- On a local cluster, an in-cluster registry such as + `registry.localtest.me/func` avoids pushing over the network entirely, if + their setup provides one. +{{end}} +Wait for the user's answer, then echo the final value back and confirm it +before continuing.{{end}} + +## Step 6 — Deploy + +Call the `deploy` tool with: + +- `path`: the Function's absolute path +- `registry`: the value from step 5{{if eq .Language "go"}} +- `builder`: `host` (the fastest builder for Go){{else if eq .Language "python"}} +- `builder`: `host` (the fastest builder for Python){{else if not .Language}} +- `builder`: `host` for Go and Python; omit it for other runtimes so the + default (`pack`) is used{{end}} + +This builds the image, pushes it to the registry, and creates the Function on +the cluster. It is the slowest step; tell the user what is happening rather +than going quiet. + +On success, call `describe` with the same `path` and read `url` out of the +result. That URL — not anything parsed out of the deploy log — is the +authoritative live address. Also note `namespace` and `ready` from the same +result. + +If `ready` is not `true`, do not declare success: report what `describe` +returned and help the user diagnose it. + +## Step 7 — Remote invoke + +Call `invoke` with the Function's `path` and `target: "remote"`. This hits the +deployed instance and runs its handler for real, so if the handler has been +modified to do anything with side effects, confirm with the user first. + +Show the response verbatim. A successful call with no error is the proof that +onboarding worked end-to-end: the Function is deployed, routable, and +answering. +{{end}} +## Step 8 — Summary + +Finish by printing a short, plain summary of what now exists: + +- **Function name** +- **Live URL** {{if .Readonly}}(not applicable — no deploy was performed in read-only mode){{else}}(from `describe` in step 6){{end}} +- **Runtime** (language and template) +- **Registry** +- **Namespace** {{if .Readonly}}(not applicable){{else}}(from `describe` in step 6){{end}} + +Take every value from actual tool output, not from what was requested — the +point of the summary is to tell the user what is true, not what was intended. +Then tell them the two things they will do next most often: edit the handler +and re-run `deploy` (which reuses the settings now stored in `func.yaml`, so +no arguments beyond `path` are needed). diff --git a/pkg/mcp/prompts_onboard_test.go b/pkg/mcp/prompts_onboard_test.go new file mode 100644 index 0000000000..4aa77890a7 --- /dev/null +++ b/pkg/mcp/prompts_onboard_test.go @@ -0,0 +1,354 @@ +package mcp + +import ( + "strings" + "testing" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// getPromptText invokes the named prompt and returns the text of its single +// message, failing the test if the prompt does not have exactly one text +// message. +func getPromptText(t *testing.T, client *mcp.ClientSession, name string, args map[string]string) string { + t.Helper() + + result, err := client.GetPrompt(t.Context(), &mcp.GetPromptParams{ + Name: name, + Arguments: args, + }) + if err != nil { + t.Fatalf("GetPrompt(%q) failed: %v", name, err) + } + if len(result.Messages) != 1 { + t.Fatalf("expected 1 message, got %d", len(result.Messages)) + } + m := result.Messages[0] + if m.Role != "user" { + t.Errorf("expected role 'user', got %q", m.Role) + } + content, ok := m.Content.(*mcp.TextContent) + if !ok { + t.Fatalf("expected TextContent, got %T", m.Content) + } + if strings.TrimSpace(content.Text) == "" { + t.Fatal("prompt text is empty") + } + return content.Text +} + +// TestPrompt_OnboardListed ensures the onboarding prompt is advertised by the +// server, with all four of its documented (and optional) arguments. +func TestPrompt_OnboardListed(t *testing.T) { + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + + result, err := client.ListPrompts(t.Context(), nil) + if err != nil { + t.Fatal(err) + } + + var ( + onboard mcp.Prompt + found bool + ) + for _, p := range result.Prompts { + if p.Name == "onboard" { + onboard, found = *p, true + break + } + } + if !found { + t.Fatal("prompt 'onboard' not found") + } + if onboard.Description == "" { + t.Error("prompt 'onboard' has no description") + } + + want := map[string]bool{"language": false, "template": false, "registry": false, "cluster": false} + for _, a := range onboard.Arguments { + if _, ok := want[a.Name]; !ok { + t.Errorf("unexpected argument %q", a.Name) + continue + } + want[a.Name] = true + if a.Description == "" { + t.Errorf("argument %q has no description", a.Name) + } + // All arguments are optional: any omitted value is gathered from + // the user by the agent as the steps run. + if a.Required { + t.Errorf("argument %q should be optional", a.Name) + } + } + for name, found := range want { + if !found { + t.Errorf("missing argument %q", name) + } + } +} + +// TestPrompt_OnboardSteps ensures every documented step of the onboarding +// workflow is present, and that each names the tool or resource it depends +// on. The prompt is the contract with the agent, so a step silently +// disappearing from the markdown must fail the build. +func TestPrompt_OnboardSteps(t *testing.T) { + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + + text := getPromptText(t, client, "onboard", nil) + + for _, want := range []string{ + "Step 1 — Prerequisites", + "Step 2 — Language selection", + "Step 3 — Scaffold", + "Step 4 — Local run and invoke", + "Step 5 — Registry configuration", + "Step 6 — Deploy", + "Step 7 — Remote invoke", + "Step 8 — Summary", + } { + if !strings.Contains(text, want) { + t.Errorf("prompt missing %q", want) + } + } + + // Each step must reference the tool or resource that implements it. + for _, want := range []string{ + "`version`", // step 1 + "func://languages", // step 2 + "`create`", // step 3 + "`run`", // step 4 + "`invoke`", // steps 4 and 7 + "`run_stop`", // step 4 + "`deploy`", // step 6 + "`describe`", // step 6 + `target: "local"`, // step 4 + `target: "remote"`, // step 7 + } { + if !strings.Contains(text, want) { + t.Errorf("prompt missing reference to %q", want) + } + } +} + +// TestPrompt_OnboardDefaults ensures omitted arguments produce their +// documented defaults, and that the arguments with no default instruct the +// agent to ask the user rather than guessing. +func TestPrompt_OnboardDefaults(t *testing.T) { + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + + text := getPromptText(t, client, "onboard", nil) + + if !strings.Contains(text, "| template | `http` |") { + t.Error("template did not default to 'http'") + } + if !strings.Contains(text, "| cluster | `local`") { + t.Error("cluster did not default to 'local'") + } + // Language and registry have no sane default; the agent must ask. + if strings.Count(text, "**not provided**") != 2 { + t.Errorf("expected language and registry to be marked 'not provided', got:\n%s", text) + } + if !strings.Contains(text, "ask the user in step 2") { + t.Error("prompt does not instruct the agent to ask for the language") + } + if !strings.Contains(text, "ask the user in step 5") { + t.Error("prompt does not instruct the agent to ask for the registry") + } +} + +// TestPrompt_OnboardParameterized ensures supplied arguments are rendered +// into the prompt and are not re-asked for. +func TestPrompt_OnboardParameterized(t *testing.T) { + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + + text := getPromptText(t, client, "onboard", map[string]string{ + "language": "node", + "template": "cloudevents", + "registry": "ghcr.io/alice", + "cluster": "remote", + }) + + for _, want := range []string{ + "| language | `node` |", + "| template | `cloudevents` |", + "| registry | `ghcr.io/alice` |", + "| cluster | `remote`", + } { + if !strings.Contains(text, want) { + t.Errorf("prompt missing %q", want) + } + } + if strings.Contains(text, "**not provided**") { + t.Error("prompt asks for a parameter which was supplied") + } + // A remote cluster changes the step 1 guidance. + if !strings.Contains(text, "which cluster and\nnamespace") { + t.Error("remote cluster guidance missing from step 1") + } +} + +// TestPrompt_OnboardNormalization ensures argument values are normalized: +// case and surrounding whitespace are insignificant for the enumerated +// arguments, the "cloudevent" alias resolves to the template name which +// actually exists, and the registry (an image reference prefix) keeps its +// case. +func TestPrompt_OnboardNormalization(t *testing.T) { + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + + text := getPromptText(t, client, "onboard", map[string]string{ + "language": " GO ", + "template": "CloudEvent", + "registry": " docker.io/Alice ", + "cluster": "Local", + }) + + if !strings.Contains(text, "| language | `go` |") { + t.Error("language was not normalized to 'go'") + } + if !strings.Contains(text, "| template | `cloudevents` |") { + t.Error("'cloudevent' was not normalized to the 'cloudevents' template name") + } + if !strings.Contains(text, "| cluster | `local`") { + t.Error("cluster was not normalized to 'local'") + } + if !strings.Contains(text, "| registry | `docker.io/Alice` |") { + t.Error("registry case was not preserved (image references are case-sensitive)") + } +} + +// TestPrompt_OnboardBuilder ensures the deploy step recommends the host +// builder for the runtimes which default to it, and stays neutral otherwise. +func TestPrompt_OnboardBuilder(t *testing.T) { + tests := []struct { + language string + want bool + }{ + {"go", true}, + {"python", true}, + {"node", false}, + {"rust", false}, + } + + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + + for _, tt := range tests { + t.Run(tt.language, func(t *testing.T) { + text := getPromptText(t, client, "onboard", map[string]string{ + "language": tt.language, + }) + got := strings.Contains(text, "`builder`: `host`") + if got != tt.want { + t.Errorf("host builder recommended = %v, want %v for %s", got, tt.want, tt.language) + } + }) + } +} + +// TestPrompt_OnboardInvalidArguments ensures unrecognized values are rejected +// with an actionable error rather than being rendered into the prompt, where +// they would send the agent off to run a command that cannot succeed. +func TestPrompt_OnboardInvalidArguments(t *testing.T) { + tests := []struct { + name string + args map[string]string + }{ + {"language", map[string]string{"language": "cobol"}}, + {"template", map[string]string{"template": "grpc"}}, + {"cluster", map[string]string{"cluster": "kind"}}, + } + + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := client.GetPrompt(t.Context(), &mcp.GetPromptParams{ + Name: "onboard", + Arguments: tt.args, + }) + if err == nil { + t.Fatalf("expected an error for invalid %s", tt.name) + } + if !strings.Contains(err.Error(), tt.name) { + t.Errorf("error does not name the offending argument %q: %v", tt.name, err) + } + if !strings.Contains(err.Error(), "must be one of") { + t.Errorf("error does not list the valid values: %v", err) + } + }) + } +} + +// TestPrompt_OnboardReadonly ensures the prompt adapts to read-only mode: the +// steps which mutate cluster state are omitted rather than sent to an agent +// that would be refused, and the user is told how to enable them. +func TestPrompt_OnboardReadonly(t *testing.T) { + client, _, err := newTestPairWithReadonly(t, true) + if err != nil { + t.Fatal(err) + } + + text := getPromptText(t, client, "onboard", nil) + + if !strings.Contains(text, EnvMCPWrite) { + t.Errorf("readonly prompt does not mention %s", EnvMCPWrite) + } + for _, unwanted := range []string{ + "Step 5 — Registry configuration", + "Step 6 — Deploy", + "Step 7 — Remote invoke", + } { + if strings.Contains(text, unwanted) { + t.Errorf("readonly prompt should not include %q", unwanted) + } + } + // The local half of onboarding still applies, and so does the summary. + for _, want := range []string{ + "Step 1 — Prerequisites", + "Step 4 — Local run and invoke", + "Step 8 — Summary", + } { + if !strings.Contains(text, want) { + t.Errorf("readonly prompt missing %q", want) + } + } +} + +// TestPrompt_OnboardNilParams ensures the argument helpers tolerate a request +// carrying no arguments at all, which is how the prompt is invoked by a +// client that offers no argument entry. +func TestPrompt_OnboardNilParams(t *testing.T) { + p, err := newOnboardParams(nil, false) + if err != nil { + t.Fatal(err) + } + if p.Template != defaultOnboardTemplate { + t.Errorf("expected template %q, got %q", defaultOnboardTemplate, p.Template) + } + if p.Cluster != defaultOnboardCluster { + t.Errorf("expected cluster %q, got %q", defaultOnboardCluster, p.Cluster) + } + if p.Language != "" || p.Registry != "" { + t.Errorf("expected empty language and registry, got %q and %q", p.Language, p.Registry) + } +} From 208cf7f9c528a7d843567fc0d04ea833f87b05fb Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Mon, 24 Aug 2026 21:05:02 +0530 Subject: [PATCH 2/4] chore: re-trigger CI The lint job failed fetching the golangci-lint JSON schema from golangci-lint.run (network timeout), not on anything in this change. From 51ddac5fc2ea554a61ad45afae6521ad0069e0f6 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Tue, 25 Aug 2026 01:44:50 +0530 Subject: [PATCH 3/4] fix(mcp): gather registry before the local run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The onboard prompt ran its local run and invoke before asking for a registry, on the theory that proving the Function works should not require one. It does: the default builder is pack, so a local run builds a container image, and naming that image needs a registry. Without one the build fails with "registry required" — for exactly the first-time user the prompt exists to serve, at a step the prompt forbids skipping past. Registry configuration is now step 4 and the local run step 5, which passes the gathered registry to run. Read-only mode omitted the registry step along with deploy and the remote invoke, yet still rendered "ask the user in step 5" in the parameters table, asked for a registry in the summary, and closed by telling the user to re-run deploy, which read-only refuses. The registry step is no longer omitted — a local build has to name an image whether or not it is ever pushed — and the remaining deploy-dependent text is now conditional. Language and template were validated against hardcoded lists. Which runtimes and templates exist depends on the installed binary and on any repositories added with the repository_add tool this same server exposes, so those lists rejected legitimate values outright. Only cluster is validated now, being prompt-internal and never passed to func; the rest are trimmed and passed through with their case intact, since a fold would corrupt the very repository-supplied names the lists used to reject. The agent is still told to check them against func://languages and func://templates. The deploy step named go and python as the host builder's runtimes. It now branches on oci.IsSupported, leaving pkg/oci the only place that list lives. Also: drop step 3's question about a Function name, there being no name argument to carry it (the name is the directory basename); drop the instruction to cd, which nothing depends on now that every tool call takes an absolute path; and assert table values in tests through a padding-insensitive match, so realigning the markdown cannot fail the build. Relates to #3737 --- pkg/mcp/instructions.md | 13 +- pkg/mcp/prompts.go | 8 +- pkg/mcp/prompts_onboard.go | 77 +++++---- pkg/mcp/prompts_onboard.md | 157 ++++++++++------- pkg/mcp/prompts_onboard_test.go | 297 +++++++++++++++++++++----------- 5 files changed, 345 insertions(+), 207 deletions(-) diff --git a/pkg/mcp/instructions.md b/pkg/mcp/instructions.md index 3f1dd7fe1f..7b68744ef5 100644 --- a/pkg/mcp/instructions.md +++ b/pkg/mcp/instructions.md @@ -59,13 +59,18 @@ invokes on the user's behalf. ### onboard - Drives full end-to-end onboarding: prerequisites → language → scaffold → - local run and invoke → registry → deploy → remote invoke → summary + registry → local run and invoke → deploy → remote invoke → summary +- The registry is gathered before the local run, because a containerized build + has to name an image and so fails without one - All four arguments (`language`, `template`, `registry`, `cluster`) are optional. Supplied values are treated as decided; omitted ones are gathered from the user as the relevant step is reached -- `template` defaults to `http` and `cluster` defaults to `local` -- In read-only mode the deploy-dependent steps are omitted from the returned - prompt, since they would be refused +- `template` defaults to `http` and `cluster` defaults to `local`. Only + `cluster` is validated against a fixed set; `language` and `template` are + passed through, since the valid values depend on the installed binary and on + any added template repositories +- In read-only mode the deploy and remote-invoke steps are omitted from the + returned prompt, since `deploy` would be refused - If a user asks to "get started", "set up a Function from scratch", or similar, suggest they invoke this prompt rather than improvising the sequence yourself diff --git a/pkg/mcp/prompts.go b/pkg/mcp/prompts.go index db9d8334b4..4618c29da5 100644 --- a/pkg/mcp/prompts.go +++ b/pkg/mcp/prompts.go @@ -36,9 +36,11 @@ func rawPromptArg(r *mcp.GetPromptRequest, name string) string { // promptArg returns the named argument from the request, normalized by // trimming surrounding whitespace and lowercasing. Prompt arguments arrive as -// free-form strings typed by a human (or filled in by an agent), so " Go " -// and "go" must be treated as the same value. Returns "" when the argument -// was not provided. +// free-form strings typed by a human (or filled in by an agent), so " Local " +// and "local" must be treated as the same value. Use this only for arguments +// whose valid values are enumerated by the prompt itself; values passed +// through to func (a runtime, a template, a registry) must keep their case +// and so use rawPromptArg. Returns "" when the argument was not provided. func promptArg(r *mcp.GetPromptRequest, name string) string { return strings.ToLower(strings.TrimSpace(rawPromptArg(r, name))) } diff --git a/pkg/mcp/prompts_onboard.go b/pkg/mcp/prompts_onboard.go index b9e298d86a..fde8068a55 100644 --- a/pkg/mcp/prompts_onboard.go +++ b/pkg/mcp/prompts_onboard.go @@ -8,6 +8,7 @@ import ( "text/template" "github.com/modelcontextprotocol/go-sdk/mcp" + "knative.dev/func/pkg/oci" ) //go:embed prompts_onboard.md @@ -26,33 +27,31 @@ const ( defaultOnboardCluster = "local" ) -// Accepted argument values. Languages mirror the runtimes shipped in -// templates/; the authoritative list is still the func://languages resource, -// which the prompt instructs the agent to read, so this is a guard against -// obvious typos rather than a second source of truth. -var ( - onboardLanguages = []string{"go", "node", "python", "typescript", "rust", "quarkus", "springboot"} - onboardTemplates = []string{"http", "cloudevent", "cloudevents"} - onboardClusters = []string{"local", "remote"} -) +// onboardClusters are the accepted values for the cluster argument. Unlike +// language, template and registry, cluster is not passed through to func: it +// only selects which guidance the prompt renders, so the set of valid values +// is fully known here and can be validated. +var onboardClusters = []string{"local", "remote"} var onboardPrompt = &mcp.Prompt{ Name: "onboard", Title: "Onboard a Function", Description: "Multi-step onboarding: check prerequisites, choose a language, " + - "scaffold, run and invoke locally, configure a registry, deploy, invoke " + + "scaffold, configure a registry, run and invoke locally, deploy, invoke " + "the live instance, and summarize. All arguments are optional; omitted " + "ones are gathered from the user as the steps run.", Arguments: []*mcp.PromptArgument{ { - Name: "language", - Title: "Language", - Description: "Target runtime: " + strings.Join(onboardLanguages, ", ") + " (asked for if omitted)", + Name: "language", + Title: "Language", + Description: "Target runtime, as reported by the func://languages resource, " + + "e.g. go, node, python (asked for if omitted)", }, { - Name: "template", - Title: "Template", - Description: "Function template: http or cloudevents (default: " + defaultOnboardTemplate + ")", + Name: "template", + Title: "Template", + Description: "Function template available for the chosen runtime, as reported " + + "by the func://templates resource (default: " + defaultOnboardTemplate + ")", }, { Name: "registry", @@ -76,6 +75,10 @@ type onboardParams struct { Registry string Cluster string Readonly bool + // HostBuilder reports whether the host builder supports Language, and is + // what the deploy step branches on rather than naming runtimes itself. + // False when no language was supplied. + HostBuilder bool } func (s *Server) onboardHandler(_ context.Context, r *mcp.GetPromptRequest) (*mcp.GetPromptResult, error) { @@ -93,39 +96,47 @@ func (s *Server) onboardHandler(_ context.Context, r *mcp.GetPromptRequest) (*mc } // newOnboardParams validates the request's arguments and applies defaults. +// +// Only cluster is validated against a fixed set. Language, template and +// registry are passed through: the runtimes and templates which actually +// exist depend on the installed binary and on any template repositories the +// user has added (see the repository_add tool), so a hardcoded list here +// would reject legitimate values. The prompt instructs the agent to verify +// them against the func://languages and func://templates resources instead, +// and create reports plainly when either is wrong. func newOnboardParams(r *mcp.GetPromptRequest, readonly bool) (p onboardParams, err error) { p = onboardParams{ - Language: promptArg(r, "language"), - Template: promptArg(r, "template"), - // The registry is a case-sensitive image reference prefix, so unlike - // the other arguments it must not be lowercased. + // Values handed to func are only trimmed, never case-folded: a + // registry is a case-sensitive image reference prefix, and a runtime + // or template name is matched against a directory name in a template + // repository. + Language: strings.TrimSpace(rawPromptArg(r, "language")), + Template: strings.TrimSpace(rawPromptArg(r, "template")), Registry: strings.TrimSpace(rawPromptArg(r, "registry")), Cluster: promptArg(r, "cluster"), Readonly: readonly, } - if err = validateChoice("language", p.Language, onboardLanguages); err != nil { - return - } - if err = validateChoice("template", p.Template, onboardTemplates); err != nil { - return - } if err = validateChoice("cluster", p.Cluster, onboardClusters); err != nil { return } - if p.Template == "" { + switch { + case p.Template == "": p.Template = defaultOnboardTemplate - } - // "cloudevent" is accepted as an alias because that is how the format is - // spelled elsewhere (e.g. invoke's --format), but the template shipped in - // templates// is named "cloudevents"; normalize so the value - // handed to the create tool is one that actually exists. - if p.Template == "cloudevent" { + case strings.EqualFold(p.Template, "cloudevent"): + // "cloudevent" is how the format is spelled elsewhere (e.g. invoke's + // --format), but no runtime ships a template by that name: the one on + // disk is "cloudevents". Correcting it is safe precisely because the + // value given cannot be valid as-is. p.Template = "cloudevents" } + if p.Cluster == "" { p.Cluster = defaultOnboardCluster } + + // Single source of truth for which runtimes the host builder supports. + p.HostBuilder = oci.IsSupported(strings.ToLower(p.Language)) return } diff --git a/pkg/mcp/prompts_onboard.md b/pkg/mcp/prompts_onboard.md index c0756dc5a4..3bee3af9c6 100644 --- a/pkg/mcp/prompts_onboard.md +++ b/pkg/mcp/prompts_onboard.md @@ -14,20 +14,21 @@ ahead — every later step depends on the ones before it. |-----------|-------| | language | {{if .Language}}`{{.Language}}`{{else}}**not provided** — ask the user in step 2{{end}} | | template | `{{.Template}}` | -| registry | {{if .Registry}}`{{.Registry}}`{{else}}**not provided** — ask the user in step 5{{end}} | +| registry | {{if .Registry}}`{{.Registry}}`{{else}}**not provided** — ask the user in step 4{{end}} | | cluster | `{{.Cluster}}`{{if eq .Cluster "local"}} (a local cluster, e.g. kind){{else}} (a remote/shared cluster){{end}} | Treat provided values as decided: do not re-ask for them. Ask only for the values marked "not provided". {{if .Readonly}} -> **This server is in read-only mode.** The registry, deploy and remote-invoke -> steps mutate cluster state and would be refused, so they are omitted below. -> Run steps 1 through 4, give the step 8 summary for what was accomplished -> locally, and tell the user that finishing onboarding — deploying and -> invoking a live instance — requires restarting the MCP server with -> `FUNC_ENABLE_MCP_WRITE=true`. +> **This server is in read-only mode.** The `deploy` tool is refused, so the +> deploy (step 6) and the remote invoke which depends on it (step 7) are +> omitted below. Run steps 1 through 5, give the step 8 summary for what was +> accomplished locally, and tell the user that finishing onboarding — +> deploying and invoking a live instance — requires restarting the MCP server +> with `FUNC_ENABLE_MCP_WRITE=true`. The registry in step 4 is still needed: +> the local build in step 5 has to name an image, even though read-only mode +> means nothing is pushed. {{end}} - ## Step 1 — Prerequisites Call the `version` tool. It reports the version of the `func` binary this @@ -49,59 +50,56 @@ namespace `kubectl` is currently pointed at, and that it is the intended deployment target. Do this now, before anything is built. {{end}} ## Step 2 — Language selection - -{{if .Language}}The language is already chosen: `{{.Language}}`. Read the -`func://languages` resource anyway and verify `{{.Language}}` is listed. If it -is not, tell the user and ask them to choose from the languages actually -available.{{else}}Read the `func://languages` resource and present the user -with the runtimes it actually reports. Do not offer a guessed or remembered -list — the available runtimes depend on the installed binary and any -configured template repositories. Ask the user to choose one and wait for -their answer.{{end}} - +{{if .Language}} +The language is already chosen. Read the `func://languages` resource anyway +and verify the chosen runtime is listed. If it is not, tell the user and ask +them to choose from the languages actually available — the runtimes on offer +depend on the installed binary and any configured template repositories. +{{else}} +Read the `func://languages` resource and present the user with the runtimes it +actually reports. Do not offer a guessed or remembered list — the available +runtimes depend on the installed binary and any configured template +repositories. Ask the user to choose one and wait for their answer. +{{end}} ## Step 3 — Scaffold -Ask the user where the Function should live, and what it should be called if -the directory name is not the name they want. Then call the `create` tool -with: +Ask the user where the Function should live. The Function's **name is the +basename of that directory** — there is no separate name argument — so if they +want a particular name, the directory has to carry it. Then call the `create` +tool with: - `language`: the runtime from step 2 - `template`: `{{.Template}}` - `path`: the **absolute** path to the Function directory -Then `cd` into that directory yourself and stay there for the rest of this -session — every later tool call takes the same absolute path. Briefly show -the user what was generated (the handler file and `func.yaml`) so they know -where their code lives. +Every later tool call takes that same absolute path: no tool here has a +working-directory default, and the MCP server's own working directory is +unrelated to yours. If `create` rejects the template, read the +`func://templates` resource to see what the chosen runtime actually ships and +agree on one with the user. -## Step 4 — Local run and invoke - -Prove the Function works before involving a registry or a cluster. - -1. Call `run` with the Function's absolute `path`. It builds if needed, starts - the Function, and returns a `pid` and a `url`. -2. Call `invoke` with the same `path` and `target: "local"`. With no `data`, a - real `{"message":"Hello World"}` payload is sent — the handler actually - runs. -3. Show the user the response body verbatim. This is the moment the Function - becomes real to them; do not paraphrase it. -4. Call `run_stop` with the same `path`. Always do this, including when the - invoke failed, so the port and process are released. +Briefly show the user what was generated (the handler file and `func.yaml`) so +they know where their code lives. -Warn the user that the first local build{{if .Language}} for -`{{.Language}}`{{end}} can be slow: builder images may need to be -downloaded, and Podman or Docker must be available. -{{if not .Readonly}} -## Step 5 — Registry configuration +## Step 4 — Registry configuration -{{if .Registry}}Use the registry `{{.Registry}}`. Confirm it is well-formed -before continuing: a registry value is a **domain** plus a -**user/organization** joined by a slash, such as `docker.io/alice`. If it is -domain-only, acknowledge that edge case explicitly with the user. Also -confirm they are logged in to it (`docker login` / `podman login`), since the -deploy in step 6 pushes there.{{else}}Ask the user for their container registry. This is -the single most common place first-time onboarding goes wrong, so guide them -concretely rather than just asking: +This comes before the local run on purpose. The default builder (`pack`) +builds a container image, and naming that image requires a registry, so a +local run fails with "registry required" without one. +{{if .Readonly}} +Nothing is pushed to it in read-only mode: it is needed only to name the image +the local build produces. +{{else}} +Nothing is pushed to it until the deploy in step 6. +{{end}}{{if .Registry}} +Use the registry from the session parameters. Confirm it is well-formed before +continuing: a registry value is a **domain** plus a **user/organization** +joined by a slash, such as `docker.io/alice`. If it is domain-only, +acknowledge that edge case explicitly with the user. +{{else}} +Ask the user for their container registry. This is the single most common place +first-time onboarding goes wrong, so guide them concretely rather than just +asking: - A registry value is a **domain** plus a **user/organization**, joined by a slash: `docker.io/alice`, `ghcr.io/alice`, `quay.io/alice`. @@ -109,26 +107,45 @@ concretely rather than just asking: or to explicitly confirm they mean the domain-only form. - The image name is derived automatically as `/:latest`; they do not need to supply an image name. -- They must be logged in to that registry (`docker login` / `podman login`), - because the deploy in step 6 pushes to it. -{{if eq .Cluster "local"}}- On a local cluster, an in-cluster registry such as - `registry.localtest.me/func` avoids pushing over the network entirely, if - their setup provides one. +{{if eq .Cluster "local"}}- On a local cluster, an in-cluster registry like `registry.localtest.me/func` + avoids pushing over the network entirely, if their setup provides one. {{end}} Wait for the user's answer, then echo the final value back and confirm it -before continuing.{{end}} +before continuing. +{{end}}{{if not .Readonly}} +Also confirm they are logged in to that registry (`docker login` / +`podman login`), since the deploy in step 6 pushes there. +{{end}} +## Step 5 — Local run and invoke + +Prove the Function works locally before involving a cluster. +1. Call `run` with the Function's absolute `path` and the `registry` from step + 4. It builds if needed, starts the Function, and returns a `pid` and a + `url`. +2. Call `invoke` with the same `path` and `target: "local"`. With no `data`, a + real `{"message":"Hello World"}` payload is sent — the handler actually + runs. +3. Show the user the response body verbatim. This is the moment the Function + becomes real to them; do not paraphrase it. +4. Call `run_stop` with the same `path`. Always do this, including when the + invoke failed, so the port and process are released. + +Warn the user that the first local build can be slow: builder images may need +to be downloaded, and Podman or Docker must be available. +{{if not .Readonly}} ## Step 6 — Deploy Call the `deploy` tool with: - `path`: the Function's absolute path -- `registry`: the value from step 5{{if eq .Language "go"}} -- `builder`: `host` (the fastest builder for Go){{else if eq .Language "python"}} -- `builder`: `host` (the fastest builder for Python){{else if not .Language}} -- `builder`: `host` for Go and Python; omit it for other runtimes so the - default (`pack`) is used{{end}} - +- `registry`: the value from step 4 +{{if .HostBuilder}}- `builder`: `host` (faster than the default `pack` builder for this runtime) +{{else if not .Language}}- `builder`: omit it unless the runtime chosen in step 2 is supported by the + `host` builder, which is faster where it applies. The default (`pack`) works + for every runtime, and `deploy` reports plainly when `host` does not support + the runtime +{{end}} This builds the image, pushes it to the registry, and creates the Function on the cluster. It is the slowest step; tell the user what is happening rather than going quiet. @@ -156,13 +173,21 @@ answering. Finish by printing a short, plain summary of what now exists: - **Function name** -- **Live URL** {{if .Readonly}}(not applicable — no deploy was performed in read-only mode){{else}}(from `describe` in step 6){{end}} +- **Live URL** {{if .Readonly}}(not applicable — read-only mode performed no deploy){{else}}(from `describe` in step 6){{end}} - **Runtime** (language and template) -- **Registry** -- **Namespace** {{if .Readonly}}(not applicable){{else}}(from `describe` in step 6){{end}} +- **Registry** (the value from step 4) +- **Namespace** {{if .Readonly}}(not applicable — read-only mode performed no deploy){{else}}(from `describe` in step 6){{end}} Take every value from actual tool output, not from what was requested — the point of the summary is to tell the user what is true, not what was intended. +{{if .Readonly}} +Then tell them the two things they will do next most often: edit the handler +and re-run the local `run` and `invoke` from step 5. Deploying requires +restarting this server with `FUNC_ENABLE_MCP_WRITE=true`, after which the +settings now stored in `func.yaml` are reused and `deploy` needs no arguments +beyond `path`. +{{else}} Then tell them the two things they will do next most often: edit the handler and re-run `deploy` (which reuses the settings now stored in `func.yaml`, so no arguments beyond `path` are needed). +{{end}} diff --git a/pkg/mcp/prompts_onboard_test.go b/pkg/mcp/prompts_onboard_test.go index 4aa77890a7..cd0ae93757 100644 --- a/pkg/mcp/prompts_onboard_test.go +++ b/pkg/mcp/prompts_onboard_test.go @@ -1,6 +1,8 @@ package mcp import ( + "fmt" + "regexp" "strings" "testing" @@ -37,6 +39,31 @@ func getPromptText(t *testing.T, client *mcp.ClientSession, name string, args ma return content.Text } +// paramValue returns the value the prompt's session-parameter table reports +// for the named parameter, or "" when the parameter has no row. Matching +// ignores the table's column padding, so realigning the markdown does not +// break these tests: only the rendered value is asserted on. +func paramValue(t *testing.T, text, name string) string { + t.Helper() + + re := regexp.MustCompile(fmt.Sprintf(`(?m)^\|\s*%s\s*\|\s*(.*?)\s*\|\s*$`, regexp.QuoteMeta(name))) + m := re.FindStringSubmatch(text) + if m == nil { + return "" + } + return m[1] +} + +// assertParam fails unless the session-parameter table reports want for the +// named parameter. +func assertParam(t *testing.T, text, name, want string) { + t.Helper() + + if got := paramValue(t, text, name); got != want { + t.Errorf("parameter %q = %q, want %q", name, got, want) + } +} + // TestPrompt_OnboardListed ensures the onboarding prompt is advertised by the // server, with all four of its documented (and optional) arguments. func TestPrompt_OnboardListed(t *testing.T) { @@ -91,8 +118,8 @@ func TestPrompt_OnboardListed(t *testing.T) { } // TestPrompt_OnboardSteps ensures every documented step of the onboarding -// workflow is present, and that each names the tool or resource it depends -// on. The prompt is the contract with the agent, so a step silently +// workflow is present, in order, and that each names the tool or resource it +// depends on. The prompt is the contract with the agent, so a step silently // disappearing from the markdown must fail the build. func TestPrompt_OnboardSteps(t *testing.T) { client, _, err := newTestPair(t) @@ -102,33 +129,45 @@ func TestPrompt_OnboardSteps(t *testing.T) { text := getPromptText(t, client, "onboard", nil) - for _, want := range []string{ + // The registry precedes the local run deliberately: a containerized + // build has to name an image, so `run` fails without a registry. + steps := []string{ "Step 1 — Prerequisites", "Step 2 — Language selection", "Step 3 — Scaffold", - "Step 4 — Local run and invoke", - "Step 5 — Registry configuration", + "Step 4 — Registry configuration", + "Step 5 — Local run and invoke", "Step 6 — Deploy", "Step 7 — Remote invoke", "Step 8 — Summary", - } { - if !strings.Contains(text, want) { + } + at := -1 + for _, want := range steps { + i := strings.Index(text, want) + if i < 0 { t.Errorf("prompt missing %q", want) + continue } + if i < at { + t.Errorf("step %q is out of order", want) + } + at = i } // Each step must reference the tool or resource that implements it. for _, want := range []string{ - "`version`", // step 1 - "func://languages", // step 2 - "`create`", // step 3 - "`run`", // step 4 - "`invoke`", // steps 4 and 7 - "`run_stop`", // step 4 - "`deploy`", // step 6 - "`describe`", // step 6 - `target: "local"`, // step 4 - `target: "remote"`, // step 7 + "`version`", // step 1 + "func://languages", // step 2 + "`create`", // step 3 + "func://templates", // step 3 + "`run`", // step 5 + "`invoke`", // steps 5 and 7 + "`run_stop`", // step 5 + "`deploy`", // step 6 + "`describe`", // step 6 + `target: "local"`, // step 5 + `target: "remote"`, // step 7 + "registry required", // step 4, on why it precedes the run } { if !strings.Contains(text, want) { t.Errorf("prompt missing reference to %q", want) @@ -147,21 +186,24 @@ func TestPrompt_OnboardDefaults(t *testing.T) { text := getPromptText(t, client, "onboard", nil) - if !strings.Contains(text, "| template | `http` |") { - t.Error("template did not default to 'http'") - } - if !strings.Contains(text, "| cluster | `local`") { - t.Error("cluster did not default to 'local'") + assertParam(t, text, "template", "`"+defaultOnboardTemplate+"`") + if got := paramValue(t, text, "cluster"); !strings.HasPrefix(got, "`"+defaultOnboardCluster+"`") { + t.Errorf("cluster = %q, want it to start with %q", got, "`"+defaultOnboardCluster+"`") } - // Language and registry have no sane default; the agent must ask. - if strings.Count(text, "**not provided**") != 2 { - t.Errorf("expected language and registry to be marked 'not provided', got:\n%s", text) - } - if !strings.Contains(text, "ask the user in step 2") { - t.Error("prompt does not instruct the agent to ask for the language") - } - if !strings.Contains(text, "ask the user in step 5") { - t.Error("prompt does not instruct the agent to ask for the registry") + + // Language and registry have no sane default; the agent must ask, and + // must be pointed at the step which actually gathers the value. + for _, tt := range []struct{ param, step string }{ + {"language", "step 2"}, + {"registry", "step 4"}, + } { + got := paramValue(t, text, tt.param) + if !strings.Contains(got, "**not provided**") { + t.Errorf("%s = %q, want it marked 'not provided'", tt.param, got) + } + if !strings.Contains(got, "ask the user in "+tt.step) { + t.Errorf("%s = %q, want it to point at %s", tt.param, got, tt.step) + } } } @@ -180,30 +222,26 @@ func TestPrompt_OnboardParameterized(t *testing.T) { "cluster": "remote", }) - for _, want := range []string{ - "| language | `node` |", - "| template | `cloudevents` |", - "| registry | `ghcr.io/alice` |", - "| cluster | `remote`", - } { - if !strings.Contains(text, want) { - t.Errorf("prompt missing %q", want) - } + assertParam(t, text, "language", "`node`") + assertParam(t, text, "template", "`cloudevents`") + assertParam(t, text, "registry", "`ghcr.io/alice`") + if got := paramValue(t, text, "cluster"); !strings.HasPrefix(got, "`remote`") { + t.Errorf("cluster = %q, want it to start with `remote`", got) } if strings.Contains(text, "**not provided**") { t.Error("prompt asks for a parameter which was supplied") } // A remote cluster changes the step 1 guidance. - if !strings.Contains(text, "which cluster and\nnamespace") { + if !strings.Contains(text, "Because `cluster` is `remote`") { t.Error("remote cluster guidance missing from step 1") } } -// TestPrompt_OnboardNormalization ensures argument values are normalized: -// case and surrounding whitespace are insignificant for the enumerated -// arguments, the "cloudevent" alias resolves to the template name which -// actually exists, and the registry (an image reference prefix) keeps its -// case. +// TestPrompt_OnboardNormalization ensures surrounding whitespace is +// insignificant, that values handed to func keep their case (a registry is a +// case-sensitive image reference prefix, and runtime and template names are +// matched against directory names in a template repository), and that the +// prompt-internal cluster argument is case-insensitive. func TestPrompt_OnboardNormalization(t *testing.T) { client, _, err := newTestPair(t) if err != nil { @@ -211,37 +249,78 @@ func TestPrompt_OnboardNormalization(t *testing.T) { } text := getPromptText(t, client, "onboard", map[string]string{ - "language": " GO ", - "template": "CloudEvent", + "language": " go ", + "template": " MyTemplate ", "registry": " docker.io/Alice ", "cluster": "Local", }) - if !strings.Contains(text, "| language | `go` |") { - t.Error("language was not normalized to 'go'") + assertParam(t, text, "language", "`go`") + assertParam(t, text, "template", "`MyTemplate`") + assertParam(t, text, "registry", "`docker.io/Alice`") + if got := paramValue(t, text, "cluster"); !strings.HasPrefix(got, "`local`") { + t.Errorf("cluster = %q, want it lowercased to `local`", got) } - if !strings.Contains(text, "| template | `cloudevents` |") { - t.Error("'cloudevent' was not normalized to the 'cloudevents' template name") +} + +// TestPrompt_OnboardCloudeventAlias ensures "cloudevent" — how the format is +// spelled by invoke's --format, but not the name of any template on disk — +// resolves to the "cloudevents" template which does exist, regardless of case. +func TestPrompt_OnboardCloudeventAlias(t *testing.T) { + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) } - if !strings.Contains(text, "| cluster | `local`") { - t.Error("cluster was not normalized to 'local'") + + for _, given := range []string{"cloudevent", "CloudEvent", " cloudevent "} { + t.Run(given, func(t *testing.T) { + text := getPromptText(t, client, "onboard", map[string]string{"template": given}) + assertParam(t, text, "template", "`cloudevents`") + }) } - if !strings.Contains(text, "| registry | `docker.io/Alice` |") { - t.Error("registry case was not preserved (image references are case-sensitive)") +} + +// TestPrompt_OnboardPassesThroughUnknownValues ensures runtimes and templates +// the prompt has never heard of are still rendered. Which ones exist depends +// on the installed binary and on any template repositories the user has added +// (see the repository_add tool), so rejecting them here would make the prompt +// unusable for exactly those users. +func TestPrompt_OnboardPassesThroughUnknownValues(t *testing.T) { + client, _, err := newTestPair(t) + if err != nil { + t.Fatal(err) + } + + text := getPromptText(t, client, "onboard", map[string]string{ + "language": "mylang", + "template": "mytemplate", + }) + + assertParam(t, text, "language", "`mylang`") + assertParam(t, text, "template", "`mytemplate`") + // The agent is told to check the value against the authoritative list + // rather than trusting it. + if !strings.Contains(text, "func://languages") { + t.Error("prompt does not point the agent at func://languages") } } // TestPrompt_OnboardBuilder ensures the deploy step recommends the host -// builder for the runtimes which default to it, and stays neutral otherwise. +// builder for the runtimes which support it, stays silent for those which do +// not, and gives generic guidance when the language is not yet known. The +// authority is pkg/oci; the prompt must not carry its own copy of the list. func TestPrompt_OnboardBuilder(t *testing.T) { tests := []struct { + name string language string want bool }{ - {"go", true}, - {"python", true}, - {"node", false}, - {"rust", false}, + {"go", "go", true}, + {"python", "python", true}, + {"node", "node", false}, + {"rust", "rust", false}, + {"mixed case", "Go", true}, + {"unset", "", false}, } client, _, err := newTestPair(t) @@ -250,58 +329,55 @@ func TestPrompt_OnboardBuilder(t *testing.T) { } for _, tt := range tests { - t.Run(tt.language, func(t *testing.T) { - text := getPromptText(t, client, "onboard", map[string]string{ - "language": tt.language, - }) + t.Run(tt.name, func(t *testing.T) { + args := map[string]string{} + if tt.language != "" { + args["language"] = tt.language + } + text := getPromptText(t, client, "onboard", args) + got := strings.Contains(text, "`builder`: `host`") if got != tt.want { - t.Errorf("host builder recommended = %v, want %v for %s", got, tt.want, tt.language) + t.Errorf("host builder recommended = %v, want %v for %q", got, tt.want, tt.language) + } + // With no language yet, the agent gets generic guidance instead + // of a naked recommendation it cannot evaluate. + if tt.language == "" && !strings.Contains(text, "`builder`: omit it unless") { + t.Error("expected generic builder guidance when no language is supplied") } }) } } -// TestPrompt_OnboardInvalidArguments ensures unrecognized values are rejected -// with an actionable error rather than being rendered into the prompt, where -// they would send the agent off to run a command that cannot succeed. -func TestPrompt_OnboardInvalidArguments(t *testing.T) { - tests := []struct { - name string - args map[string]string - }{ - {"language", map[string]string{"language": "cobol"}}, - {"template", map[string]string{"template": "grpc"}}, - {"cluster", map[string]string{"cluster": "kind"}}, - } - +// TestPrompt_OnboardInvalidCluster ensures an unrecognized cluster — the one +// argument whose valid values the prompt itself enumerates, because it only +// selects guidance and is never passed to func — is rejected with an +// actionable error rather than rendered. +func TestPrompt_OnboardInvalidCluster(t *testing.T) { client, _, err := newTestPair(t) if err != nil { t.Fatal(err) } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, err := client.GetPrompt(t.Context(), &mcp.GetPromptParams{ - Name: "onboard", - Arguments: tt.args, - }) - if err == nil { - t.Fatalf("expected an error for invalid %s", tt.name) - } - if !strings.Contains(err.Error(), tt.name) { - t.Errorf("error does not name the offending argument %q: %v", tt.name, err) - } - if !strings.Contains(err.Error(), "must be one of") { - t.Errorf("error does not list the valid values: %v", err) - } - }) + _, err = client.GetPrompt(t.Context(), &mcp.GetPromptParams{ + Name: "onboard", + Arguments: map[string]string{"cluster": "kind"}, + }) + if err == nil { + t.Fatal("expected an error for an invalid cluster") + } + if !strings.Contains(err.Error(), "cluster") { + t.Errorf("error does not name the offending argument: %v", err) + } + if !strings.Contains(err.Error(), "must be one of") { + t.Errorf("error does not list the valid values: %v", err) } } // TestPrompt_OnboardReadonly ensures the prompt adapts to read-only mode: the -// steps which mutate cluster state are omitted rather than sent to an agent -// that would be refused, and the user is told how to enable them. +// deploy and the remote invoke which depends on it are omitted rather than +// sent to an agent that would be refused, the user is told how to enable +// them, and nothing which survives refers to a step which no longer exists. func TestPrompt_OnboardReadonly(t *testing.T) { client, _, err := newTestPairWithReadonly(t, true) if err != nil { @@ -314,7 +390,6 @@ func TestPrompt_OnboardReadonly(t *testing.T) { t.Errorf("readonly prompt does not mention %s", EnvMCPWrite) } for _, unwanted := range []string{ - "Step 5 — Registry configuration", "Step 6 — Deploy", "Step 7 — Remote invoke", } { @@ -323,20 +398,37 @@ func TestPrompt_OnboardReadonly(t *testing.T) { } } // The local half of onboarding still applies, and so does the summary. + // The registry step in particular is NOT skipped: the local build in + // step 5 has to name an image, whether or not it is ever pushed. for _, want := range []string{ "Step 1 — Prerequisites", - "Step 4 — Local run and invoke", + "Step 4 — Registry configuration", + "Step 5 — Local run and invoke", "Step 8 — Summary", } { if !strings.Contains(text, want) { t.Errorf("readonly prompt missing %q", want) } } + // Every step the prompt still points the agent at must exist in it. + for _, ref := range regexp.MustCompile(`step (\d)`).FindAllStringSubmatch(text, -1) { + if strings.Contains(text, "(step "+ref[1]+")") { + continue // the read-only notice names the omitted steps on purpose + } + if !strings.Contains(text, "## Step "+ref[1]+" — ") { + t.Errorf("readonly prompt refers to %q, which it omits", ref[0]) + } + } + // Nor may it close by telling the user to do the thing that is refused. + if strings.Contains(text, "re-run `deploy`") { + t.Error("readonly prompt tells the user to re-run deploy, which is refused") + } } // TestPrompt_OnboardNilParams ensures the argument helpers tolerate a request -// carrying no arguments at all, which is how the prompt is invoked by a -// client that offers no argument entry. +// carrying no params at all. No client sends that — an argumentless GetPrompt +// arrives with an empty map — so this covers the defensive nil guards in the +// helpers rather than a reachable code path. func TestPrompt_OnboardNilParams(t *testing.T) { p, err := newOnboardParams(nil, false) if err != nil { @@ -351,4 +443,7 @@ func TestPrompt_OnboardNilParams(t *testing.T) { if p.Language != "" || p.Registry != "" { t.Errorf("expected empty language and registry, got %q and %q", p.Language, p.Registry) } + if p.HostBuilder { + t.Error("expected HostBuilder to be false with no language") + } } From afc1b72c345acfe256832ba4d1f525ec86f56e64 Mon Sep 17 00:00:00 2001 From: Ankitsinghsisodya Date: Tue, 25 Aug 2026 01:56:47 +0530 Subject: [PATCH 4/4] chore: re-trigger CI