diff --git a/docs/mcp-tools-reference.md b/docs/mcp-tools-reference.md index 1d95ba2c..9668f3e5 100644 --- a/docs/mcp-tools-reference.md +++ b/docs/mcp-tools-reference.md @@ -232,28 +232,33 @@ Render CI/CD workflow YAML from a workflow config. The CI workflow (`files`/`ci_ | `yaml_content` | string | yes | The YAML content of the workflow configuration | | `registry` | string | no | Container registry for the legacy CD/release output (default: `ghcr.io`) | | `platforms` | string | no | Build platforms for the legacy CD/release output (default: `linux/amd64,linux/arm64`) | +| `phase_config_yaml` | string | no | YAML of a prerequisite phase config (creates a two-phase plan) | +| `wfctl_version` | string | no | wfctl version to pin in the plan (default: latest) | --- #### `scaffold_environment` -Generate environment configuration (Docker Compose, kubernetes manifests). +Generate an `environments:` YAML section (per-environment provider, env vars, secrets provider, exposure method) for a workflow config. | Parameter | Type | Required | Description | |-----------|------|----------|-------------| -| `target` | string | yes | Target environment (`docker-compose`, `kubernetes`, `minikube`) | -| `config` | string | no | Existing workflow config to analyze | +| `provider` | string | yes | Deployment provider: `docker`, `kubernetes`, `aws-ecs`, `gcp-cloudrun`, `digitalocean` | +| `environments` | array | no | Environment names to generate (default: `['local', 'staging', 'production']`) | +| `secrets_provider` | string | no | Secrets provider: `env`, `aws-secrets-manager`, `gcp-secret-manager`, `vault` (default: `env`) | +| `exposure` | string | no | Exposure method for the `local` environment: `tailscale`, `cloudflare`, `port-forward` (default: `port-forward`) | --- #### `scaffold_infra` -Generate infrastructure-as-code for a workflow application. +Generate an `infra:` YAML section by analyzing the config's modules (e.g. `database.postgres` → RDS, `cache.redis` → ElastiCache). | Parameter | Type | Required | Description | |-----------|------|----------|-------------| -| `provider` | string | yes | IaC provider (`opentofu`, `terraform`, `pulumi`) | -| `config` | string | no | Existing workflow config | +| `yaml_content` | string | yes | Workflow YAML config content to analyze for infrastructure needs | +| `provider` | string | yes | Cloud provider for resources: `aws`, `gcp`, `azure`, `digitalocean` | +| `environment` | string | no | Environment name for resource sizing (default: `production`) | --- diff --git a/docs/plans/2026-06-04-mcp-tool-metadata-accuracy-design.md b/docs/plans/2026-06-04-mcp-tool-metadata-accuracy-design.md new file mode 100644 index 00000000..52e7185f --- /dev/null +++ b/docs/plans/2026-06-04-mcp-tool-metadata-accuracy-design.md @@ -0,0 +1,83 @@ +# MCP tool metadata accuracy (generate_github_actions params + scaffold ref-doc) — design + +**Date:** 2026-06-04 +**Status:** Design — adversarial review PASS (1 cycle, 2026-06-04) +**Repo:** `workflow` (`mcp/wfctl_tools.go` + `docs/mcp-tools-reference.md`) +**Guidance:** `docs/AGENT_GUIDE.md` + `CLAUDE.md` (no `docs/design-guidance.md`). Follow-on to #854. + +## 1. Problem + +Two MCP-tool-metadata inaccuracies logged as #854 follow-ups: + +- **F1 — undeclared params (schema gap).** `handleGenerateGithubActions` (`mcp/wfctl_tools.go:416`) reads `phase_config_yaml` and `wfctl_version` via `mcp.ParseString(req, ...)`, but the `generate_github_actions` tool **def** declares only `yaml_content`/`registry`/`platforms`. MCP clients see the tool's input schema and won't pass undeclared params, so the two-phase (`phase_config_yaml`) and version-pin (`wfctl_version`) capabilities are unreachable via MCP even though the handler honors them. The sibling `ci_plan` tool already declares both. +- **F2 — ref-doc drift.** `docs/mcp-tools-reference.md` documents fictional params for two scaffold tools: + - `scaffold_environment` — doc says `target` (docker-compose/kubernetes/minikube) + `config`; real def (`mcp/scaffold_tools.go:42-58`): `provider` (req: docker/kubernetes/aws-ecs/gcp-cloudrun/digitalocean), `environments` (array), `secrets_provider`, `exposure`. + - `scaffold_infra` — doc says `provider` (opentofu/terraform/pulumi) + `config`; real def (`:64-80`): `yaml_content` (req), `provider` (req: aws/gcp/azure/digitalocean), `environment`. + +## 2. Goal + +The `generate_github_actions` tool schema declares every param its handler reads; `docs/mcp-tools-reference.md` matches the real `scaffold_environment`/`scaffold_infra` defs. **No handler change** (the params are already read); F1 only makes them discoverable. Single PR, no release. + +## 3. Non-goals + +- No handler / behavior / CIPlan change. F1 adds **optional** param declarations only (backward-compatible — existing clients that omit them get the unchanged `""` default). +- No change to other tool defs or other ref-doc entries (the broader ref-doc param-naming drift, e.g. `config` vs `yaml_content` elsewhere, is out of scope — only the two scaffold entries flagged in #854). +- No new tool. + +## 4. Architecture / changes (2 surfaces, 1 PR) + +1. **`mcp/wfctl_tools.go`** — in the `generate_github_actions` tool def, add two `mcp.WithString` declarations (after `platforms`, before `WithReadOnlyHintAnnotation`), mirroring the `ci_plan` wording: + - `mcp.WithString("phase_config_yaml", mcp.Description("Optional YAML content of a prerequisite phase config (creates a 2-phase plan)"))` + - `mcp.WithString("wfctl_version", mcp.Description("wfctl version to pin in the plan (default: latest)"))` + Both optional (no `mcp.Required()`) — the handler already defaults them to `""`. + +2. **`docs/mcp-tools-reference.md`** — replace the `scaffold_environment` and `scaffold_infra` parameter tables with the real defs: + - `scaffold_environment`: `provider` (string, yes — docker/kubernetes/aws-ecs/gcp-cloudrun/digitalocean), `environments` (array, no — default `['local','staging','production']`), `secrets_provider` (string, no — env/aws-secrets-manager/gcp-secret-manager/vault, default env), `exposure` (string, no — exposure method **for local**: tailscale/cloudflare/port-forward, default port-forward; only applied to the `local` environment). + - `scaffold_infra`: `yaml_content` (string, yes), `provider` (string, yes — aws/gcp/azure/digitalocean), `environment` (string, no — default production). + Also refresh the one-line purpose of each to match the def description (environments section / infra section). + +## 5. Testing + +- **F1 (schema):** extend the existing `mcp/cigen_info_test.go` (or add a focused test) asserting the registered `generate_github_actions` tool's input schema declares `phase_config_yaml` and `wfctl_version`. Accessor: `srv.MCPServer().ListTools()["generate_github_actions"].Tool.InputSchema.Properties` is a `map[string]any` keyed by param name — assert both keys present. Run failing-first. +- **Runtime / multi-component:** launch the real `wfctl mcp` server; `tools/list` JSON-RPC response for `generate_github_actions` includes `phase_config_yaml`/`wfctl_version` in its `inputSchema.properties` (the client-visible schema). Capture transcript. +- **F2 (markdown):** the two tables render; param names match the def. No Go test (doc-only). +- `GOWORK=off go test ./mcp/...`, `GOWORK=off go build ./cmd/wfctl`, `GOWORK=off golangci-lint run --new-from-rev=origin/main ./mcp/...`. + +## Global Design Guidance + +Source: `docs/AGENT_GUIDE.md` + `CLAUDE.md`. + +| guidance | response | +|---|---| +| Update docs with behavior/CLI/config changes (CLAUDE.md) | F2 is the doc-accuracy fix; F1 updates the tool schema (the MCP "API") | +| Use `GOWORK=off` for Go commands | All verification uses `GOWORK=off` | +| Keep examples under `example/` | N/A | + +## Security Review + +None. F1 declares two optional string params already read by the handler (no new input surface — they were already parseable; this only advertises them). No auth/secrets/PII/trust-boundary change. F2 is markdown. + +## Infrastructure Impact + +None. Static tool-schema metadata + markdown. No release, no resources, no migration. + +## Multi-Component Validation + +Real boundary = MCP server ↔ client. Proof: launched `wfctl mcp`; the `tools/list` response for `generate_github_actions` carries `phase_config_yaml`/`wfctl_version` in `inputSchema.properties` (the schema a client actually receives). No mock. + +## Assumptions + +1. `handleGenerateGithubActions` reads `phase_config_yaml` + `wfctl_version` and passes them to `mcpAnalyzeFromYAML` → `cigen.Analyze` (two-phase + version pin). *(Verified: `mcp/wfctl_tools.go:416`.)* +2. Declaring an optional param in mcp-go is backward-compatible (clients omitting it are unaffected; the handler's `ParseString(..., "")` default is unchanged). *(mcp-go optional-param semantics; `ci_plan` already does this.)* +3. The real `scaffold_environment`/`scaffold_infra` param lists are as read from `mcp/scaffold_tools.go:42-80`. *(Verified.)* +4. `ListTools()["x"].Tool.InputSchema.Properties` exposes declared param names for the test. *(mcp.Tool.InputSchema.Properties; confirm exact type in impl.)* + +## Rollback + +Not a runtime-affecting change class (no build/deploy/version-pin/startup/migration/plugin-loading change). Rollback = revert the PR. No release, no state. + +## Self-challenge — top doubts + +1. **Does declaring `phase_config_yaml` change `generate_github_actions` behavior?** No — the handler already reads + honors it; declaring only makes it client-discoverable (and it genuinely produces a two-phase plan via `cigen.Analyze`). +2. **Should F2 match the handler or the def?** The **def** is the client contract (what `tools/list` advertises); the ref doc documents the client-facing params, so it matches the def. (Handlers reading those params is assumed; not re-audited here.) +3. **InputSchema accessor shape.** The test depends on `Tool.InputSchema.Properties` being a map of param names — confirm the exact field/type at implementation; fall back to asserting the raw marshaled schema contains the param names if the typed accessor differs. diff --git a/docs/plans/2026-06-04-mcp-tool-metadata-accuracy.md b/docs/plans/2026-06-04-mcp-tool-metadata-accuracy.md new file mode 100644 index 00000000..434ec529 --- /dev/null +++ b/docs/plans/2026-06-04-mcp-tool-metadata-accuracy.md @@ -0,0 +1,267 @@ +# MCP tool metadata accuracy Implementation Plan + +> **For the implementing agent:** REQUIRED SUB-SKILL: Use autodev:executing-plans to implement this plan task-by-task. + +**Goal:** Declare the two params `generate_github_actions`'s handler already reads (`phase_config_yaml`, `wfctl_version`) on its tool schema, and correct the drifted `scaffold_environment`/`scaffold_infra` entries in `docs/mcp-tools-reference.md`. + +**Architecture:** Add two `mcp.WithString` (optional) declarations to one tool def; rewrite two markdown param tables to match the real defs. No handler/behavior change. A `Contains`/schema-properties test locks the new declarations. + +**Tech Stack:** Go (`mcp` package), markdown. + +**Base branch:** main (worktree branch `feat/mcp-tool-metadata-accuracy`) + +--- + +## Scope Manifest + +**PR Count:** 1 +**Tasks:** 4 +**Estimated Lines of Change:** ~50 (informational; not enforced) + +**Out of scope:** +- No handler / behavior / CIPlan change (the two params are already read by the handler). +- No edit to other tool defs or other `docs/mcp-tools-reference.md` entries (the same `config`-vs-`yaml_content` drift exists for `api_extract`/`detect_project_features`/etc. — left as a separate concern; only the two #854-flagged scaffold entries are fixed here). +- No new tool, no release. + +**PR Grouping:** + +| PR # | Title | Tasks | Branch | +|------|-------|-------|--------| +| 1 | fix(mcp): declare generate_github_actions phase params + correct scaffold ref-doc | Task 1, Task 2, Task 3, Task 4 | feat/mcp-tool-metadata-accuracy | + +**Status:** Locked 2026-06-05T02:44:05Z + +--- + +### Task 1: Failing schema-declaration test + +**Change class:** Internal logic → unit test. + +**Files:** +- Modify: `mcp/cigen_info_test.go` (append a test — same `package mcp`) + +**Step 1: Write the failing test.** Append to `mcp/cigen_info_test.go`: + +```go +// TestGenerateGithubActionsDeclaresAllHandlerParams locks the tool's input +// schema to the params its handler reads (handleGenerateGithubActions reads +// yaml_content, registry, platforms, phase_config_yaml, wfctl_version). +func TestGenerateGithubActionsDeclaresAllHandlerParams(t *testing.T) { + srv := NewServer("") + tools := srv.MCPServer().ListTools() + gha, ok := tools["generate_github_actions"] + if !ok { + t.Fatal("generate_github_actions tool not registered") + } + props := gha.Tool.InputSchema.Properties + for _, p := range []string{"yaml_content", "registry", "platforms", "phase_config_yaml", "wfctl_version"} { + if _, ok := props[p]; !ok { + t.Errorf("generate_github_actions input schema missing declared param %q", p) + } + } +} +``` + +**Step 2: Run to verify it fails.** + +Run: `GOWORK=off go test ./mcp/ -run TestGenerateGithubActionsDeclaresAllHandlerParams -v` +Expected: FAIL — `missing declared param "phase_config_yaml"` and `"wfctl_version"` (not yet declared). + +> If `gha.Tool.InputSchema.Properties` is not `map[string]any` in this mcp-go version (the design verified it is), fall back to marshaling `gha.Tool.InputSchema` to JSON and asserting the param-name substrings. Decide by compiling; the typed accessor is expected to work. + +**Step 3: Commit** + +```bash +git add mcp/cigen_info_test.go +git commit -m "test(mcp): assert generate_github_actions declares all handler params (failing)" +``` + +--- + +### Task 2: Declare phase_config_yaml + wfctl_version (mcp/wfctl_tools.go) + +**Files:** +- Modify: `mcp/wfctl_tools.go` (`generate_github_actions` tool def — the `platforms` → `WithReadOnlyHintAnnotation` region, ~line 144-147) + +**Change class:** internal logic (tool schema) → covered by Task 1 test. + +**Step 1: Implement.** Find: + +```go + mcp.WithString("platforms", + mcp.Description("Platforms to build for (default: \"linux/amd64,linux/arm64\")"), + ), + mcp.WithReadOnlyHintAnnotation(true), + ), + s.handleGenerateGithubActions, +``` + +Replace with (insert the two optional params, mirroring the `ci_plan` wording): + +```go + mcp.WithString("platforms", + mcp.Description("Platforms to build for (default: \"linux/amd64,linux/arm64\")"), + ), + mcp.WithString("phase_config_yaml", + mcp.Description("Optional YAML content of a prerequisite phase config (creates a 2-phase plan)"), + ), + mcp.WithString("wfctl_version", + mcp.Description("wfctl version to pin in the plan (default: latest)"), + ), + mcp.WithReadOnlyHintAnnotation(true), + ), + s.handleGenerateGithubActions, +``` + +**Step 2: Verify.** + +Run: `GOWORK=off go test ./mcp/ -run TestGenerateGithubActionsDeclaresAllHandlerParams -v` +Expected: PASS. + +**Step 3: Commit** + +```bash +git add mcp/wfctl_tools.go +git commit -m "fix(mcp): declare phase_config_yaml + wfctl_version on generate_github_actions" +``` + +--- + +### Task 3: Correct scaffold_environment + scaffold_infra ref-doc (docs/mcp-tools-reference.md) + +**Files:** +- Modify: `docs/mcp-tools-reference.md` (`scaffold_environment` ~line 238-246, `scaffold_infra` ~line 249-257) + +**Change class:** Documentation → table renders; params match the def. + +**Step 1: Replace the `scaffold_environment` entry.** Find: + +```markdown +#### `scaffold_environment` + +Generate environment configuration (Docker Compose, kubernetes manifests). + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `target` | string | yes | Target environment (`docker-compose`, `kubernetes`, `minikube`) | +| `config` | string | no | Existing workflow config to analyze | +``` + +Replace with (real def — `mcp/scaffold_tools.go:42-61`): + +```markdown +#### `scaffold_environment` + +Generate an `environments:` YAML section (per-environment provider, region, env vars, secrets provider, exposure method) for a workflow config. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `provider` | string | yes | Deployment provider: `docker`, `kubernetes`, `aws-ecs`, `gcp-cloudrun`, `digitalocean` | +| `environments` | array | no | Environment names to generate (default: `['local', 'staging', 'production']`) | +| `secrets_provider` | string | no | Secrets provider: `env`, `aws-secrets-manager`, `gcp-secret-manager`, `vault` (default: `env`) | +| `exposure` | string | no | Exposure method for the `local` environment: `tailscale`, `cloudflare`, `port-forward` (default: `port-forward`) | +``` + +**Step 2: Replace the `scaffold_infra` entry.** Find: + +```markdown +#### `scaffold_infra` + +Generate infrastructure-as-code for a workflow application. + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `provider` | string | yes | IaC provider (`opentofu`, `terraform`, `pulumi`) | +| `config` | string | no | Existing workflow config | +``` + +Replace with (real def — `mcp/scaffold_tools.go:63-81`): + +```markdown +#### `scaffold_infra` + +Generate an `infra:` YAML section by analyzing the config's modules (e.g. `database.postgres` → RDS, `cache.redis` → ElastiCache). + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `yaml_content` | string | yes | Workflow YAML config content to analyze for infrastructure needs | +| `provider` | string | yes | Cloud provider for resources: `aws`, `gcp`, `azure`, `digitalocean` | +| `environment` | string | no | Environment name for resource sizing (default: `production`) | +``` + +**Step 3: Verify** — the two tables render; no stray pipe rows. + +Run: `grep -c "^| \`provider\` | string | yes | Deployment provider" docs/mcp-tools-reference.md` +Expected: `1` (the corrected scaffold_environment provider row exists). Eyeball both tables. + +**Step 4: Commit** + +```bash +git add docs/mcp-tools-reference.md +git commit -m "docs: correct scaffold_environment + scaffold_infra params in MCP tools reference" +``` + +--- + +### Task 4: Full gate + runtime tools/list check + +**Files:** none (verification). + +**Change class:** Go-repo code change + multi-component (MCP schema ↔ client). + +**Step 1: Full mcp package + build + lint.** + +Run: `GOWORK=off go test ./mcp/...` +Expected: `ok github.com/GoCodeAlone/workflow/mcp` + +Run: `GOWORK=off go build ./cmd/wfctl` +Expected: exit 0. + +Run: `GOWORK=off golangci-lint run --new-from-rev=origin/main ./mcp/...` +Expected: exit 0. + +**Step 2: Runtime — the live `tools/list` schema advertises the new params.** + +```bash +GOWORK=off go build -o /tmp/wfctl-mcp ./cmd/wfctl +printf '%s\n' \ + '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"c","version":"0"}}}' \ + '{"jsonrpc":"2.0","method":"notifications/initialized"}' \ + '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}' \ + | /tmp/wfctl-mcp mcp 2>/dev/null \ + | python3 -c "import sys,json +for line in sys.stdin: + line=line.strip() + if not line.startswith('{'): continue + try: m=json.loads(line) + except Exception: continue + if m.get('id')==2: + tools={t['name']:t for t in m['result']['tools']} + props=tools['generate_github_actions']['inputSchema']['properties'] + print('declares phase_config_yaml:', 'phase_config_yaml' in props) + print('declares wfctl_version:', 'wfctl_version' in props)" +``` +Expected: both print `True` (the client-visible `tools/list` schema now advertises the params). Capture the transcript. If the framing differs, the goal is a real launched-server `tools/list` showing the two params in `generate_github_actions.inputSchema.properties`. + +**Rollback:** revert the PR; the tool schema reverts to 3 declared params and the ref doc to its prior (wrong) entries. No release, no state. + +--- + +## Global Design Guidance + +Source: `docs/AGENT_GUIDE.md` + `CLAUDE.md`. + +| guidance | response | +|---|---| +| Update docs with behavior/CLI/config changes (CLAUDE.md) | F2 is the doc-accuracy fix; F1 updates the MCP tool schema | +| Use `GOWORK=off` | all verification commands use it | + +## Verification per change class + +- Task 1-2 (tool schema): Task 1 `Contains`/properties test + `go test ./mcp/`. +- Task 3 (markdown): tables render; provider row grep == 1. +- Task 4 (Go-repo + multi-component): full `go test ./mcp/...`, `go build ./cmd/wfctl`, `golangci-lint --new-from-rev`, and a **real launched `wfctl mcp` `tools/list`** showing the two params in the client-visible schema (MCP↔client boundary). + +## Rollback + +Not a runtime-affecting change class. Rollback = revert the PR. No release, no state. diff --git a/docs/plans/2026-06-04-mcp-tool-metadata-accuracy.md.scope-lock b/docs/plans/2026-06-04-mcp-tool-metadata-accuracy.md.scope-lock new file mode 100644 index 00000000..94eedb45 --- /dev/null +++ b/docs/plans/2026-06-04-mcp-tool-metadata-accuracy.md.scope-lock @@ -0,0 +1 @@ +7b586dcf872be55ac725fe4c6ee14772e47c71102db9dc6c481469b3959d38a3 diff --git a/mcp/cigen_info_test.go b/mcp/cigen_info_test.go index c195ca18..ed3d721d 100644 --- a/mcp/cigen_info_test.go +++ b/mcp/cigen_info_test.go @@ -58,3 +58,21 @@ func TestMCPOutputSurfacesCigen(t *testing.T) { t.Errorf("ci_plan description missing %q", "cigen") } } + +// TestGenerateGithubActionsDeclaresAllHandlerParams locks the tool's input +// schema to the params its handler reads (handleGenerateGithubActions reads +// yaml_content, registry, platforms, phase_config_yaml, wfctl_version). +func TestGenerateGithubActionsDeclaresAllHandlerParams(t *testing.T) { + srv := NewServer("") + tools := srv.MCPServer().ListTools() + gha, ok := tools["generate_github_actions"] + if !ok { + t.Fatal("generate_github_actions tool not registered") + } + props := gha.Tool.InputSchema.Properties + for _, p := range []string{"yaml_content", "registry", "platforms", "phase_config_yaml", "wfctl_version"} { + if _, ok := props[p]; !ok { + t.Errorf("generate_github_actions input schema missing declared param %q", p) + } + } +} diff --git a/mcp/wfctl_tools.go b/mcp/wfctl_tools.go index 3f6b9976..a968306a 100644 --- a/mcp/wfctl_tools.go +++ b/mcp/wfctl_tools.go @@ -144,6 +144,12 @@ func (s *Server) registerWfctlTools() { mcp.WithString("platforms", mcp.Description("Platforms to build for (default: \"linux/amd64,linux/arm64\")"), ), + mcp.WithString("phase_config_yaml", + mcp.Description("Optional YAML content of a prerequisite phase config (creates a 2-phase plan)"), + ), + mcp.WithString("wfctl_version", + mcp.Description("wfctl version to pin in the plan (default: latest)"), + ), mcp.WithReadOnlyHintAnnotation(true), ), s.handleGenerateGithubActions,