← back to Cli Printing Press
feat(cli,skills): generator-time MCP description enrichment from spec (#396)
2b7a51e1bec9e81dd3b340f8cfd680126e75af39 · 2026-04-29 13:37:12 -0700 · Trevin Chow
Lifts the generator's baseline MCP tool description from the bare
spec summary ("Create project") to a structured composition that
names parameters and return shape ("Create project. Required: name,
visibility. Optional: owner_email. Returns the new Project."). The
goal: cut the residual that polish has to hand-tune from ~80% of
findings to the 16-20% that genuinely need per-CLI judgment.
Composition rule (in new internal/mcpdesc package):
{action}. [Required: <list>.] [Optional: <list>.] [Returns clause.]
+ method-specific marker (Destructive for DELETE, Partial update
for PATCH when no other return clause)
- Required: every path param (regardless of CLI default — they're
structurally required) plus required body/query fields. Params
with declared defaults gain "(default: X)" annotation so agents
know they may omit and the runtime fills in.
- Optional: capped at 3 entries with "(plus N more)" indicator
when more exist.
- Returns clause: response-type + HTTP-method aware. Arrays say
"Returns array of X."; objects say "Returns the X." with method
qualifiers ("Returns the new X." for POST, "Returns the updated
X." for PATCH/PUT).
Pre-composed pass-through: when endpoint.Description already
contains structural markers (Required: / Optional: / Returns ...),
Compose treats it as authoritative and passes it through with only
auth-annotation and DELETE-marker enforcement applied. This is
critical: mcpoverrides.Apply writes hand-tuned overrides into
endpoint.Description before the generator runs, so without
pass-through the composer would double-stamp the override's
Required/Returns clauses.
Wiring:
- internal/pipeline/toolsmanifest.go switches to mcpdesc.Compose
for tools-manifest.json descriptions.
- internal/generator/templates/mcp_tools.go.tmpl uses a new
template helper composeMCPDesc that wraps mcpdesc.Compose. The
old mcpDescriptionRich helper is deleted (its response-shape and
method-context logic is now part of Compose, plus the new
param-list composition).
- The two paths now produce identical descriptions for the same
endpoint, fixing a pre-existing minor inconsistency where the
manifest got the bare summary while the runtime got
response-shape enrichment.
Polish skill prose updated: thin-mcp-description findings should
now be uncommon. When seen, they typically mean the spec is
genuinely thin (no body, no response, sparse description) or an
existing override is too short.
Goldens refreshed to reflect the lifted baseline. The
generate-golden-api fixture's tools.go now carries the composed
descriptions for every endpoint (verified: parameter-aware, return
shape correct, method markers applied).
Override sidecar (#382) still wins where present. Library sweep to
re-run mcp-sync on existing CLIs is post-merge work; this PR lifts
the baseline for every NEW generation.
20 unit tests in internal/mcpdesc cover happy paths, edge cases
(empty description, oversize defaults, override pass-through,
DELETE-marker idempotency, optional truncation), and the auth
annotation delegation back to naming.MCPDescription.
Closes #384
Files touched
M internal/generator/generator.goM internal/generator/templates/mcp_tools.go.tmplA internal/mcpdesc/compose.goA internal/mcpdesc/compose_test.goM internal/pipeline/toolsmanifest.goM skills/printing-press-polish/references/tools-polish.mdM testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
Diff
commit 2b7a51e1bec9e81dd3b340f8cfd680126e75af39
Author: Trevin Chow <trevin@trevinchow.com>
Date: Wed Apr 29 13:37:12 2026 -0700
feat(cli,skills): generator-time MCP description enrichment from spec (#396)
Lifts the generator's baseline MCP tool description from the bare
spec summary ("Create project") to a structured composition that
names parameters and return shape ("Create project. Required: name,
visibility. Optional: owner_email. Returns the new Project."). The
goal: cut the residual that polish has to hand-tune from ~80% of
findings to the 16-20% that genuinely need per-CLI judgment.
Composition rule (in new internal/mcpdesc package):
{action}. [Required: <list>.] [Optional: <list>.] [Returns clause.]
+ method-specific marker (Destructive for DELETE, Partial update
for PATCH when no other return clause)
- Required: every path param (regardless of CLI default — they're
structurally required) plus required body/query fields. Params
with declared defaults gain "(default: X)" annotation so agents
know they may omit and the runtime fills in.
- Optional: capped at 3 entries with "(plus N more)" indicator
when more exist.
- Returns clause: response-type + HTTP-method aware. Arrays say
"Returns array of X."; objects say "Returns the X." with method
qualifiers ("Returns the new X." for POST, "Returns the updated
X." for PATCH/PUT).
Pre-composed pass-through: when endpoint.Description already
contains structural markers (Required: / Optional: / Returns ...),
Compose treats it as authoritative and passes it through with only
auth-annotation and DELETE-marker enforcement applied. This is
critical: mcpoverrides.Apply writes hand-tuned overrides into
endpoint.Description before the generator runs, so without
pass-through the composer would double-stamp the override's
Required/Returns clauses.
Wiring:
- internal/pipeline/toolsmanifest.go switches to mcpdesc.Compose
for tools-manifest.json descriptions.
- internal/generator/templates/mcp_tools.go.tmpl uses a new
template helper composeMCPDesc that wraps mcpdesc.Compose. The
old mcpDescriptionRich helper is deleted (its response-shape and
method-context logic is now part of Compose, plus the new
param-list composition).
- The two paths now produce identical descriptions for the same
endpoint, fixing a pre-existing minor inconsistency where the
manifest got the bare summary while the runtime got
response-shape enrichment.
Polish skill prose updated: thin-mcp-description findings should
now be uncommon. When seen, they typically mean the spec is
genuinely thin (no body, no response, sparse description) or an
existing override is too short.
Goldens refreshed to reflect the lifted baseline. The
generate-golden-api fixture's tools.go now carries the composed
descriptions for every endpoint (verified: parameter-aware, return
shape correct, method markers applied).
Override sidecar (#382) still wins where present. Library sweep to
re-run mcp-sync on existing CLIs is post-merge work; this PR lifts
the baseline for every NEW generation.
20 unit tests in internal/mcpdesc cover happy paths, edge cases
(empty description, oversize defaults, override pass-through,
DELETE-marker idempotency, optional truncation), and the auth
annotation delegation back to naming.MCPDescription.
Closes #384
---
internal/generator/generator.go | 60 +---
internal/generator/templates/mcp_tools.go.tmpl | 4 +-
internal/mcpdesc/compose.go | 268 +++++++++++++++++
internal/mcpdesc/compose_test.go | 330 +++++++++++++++++++++
internal/pipeline/toolsmanifest.go | 17 +-
.../references/tools-polish.md | 7 +-
.../printing-press-golden/internal/mcp/tools.go | 14 +-
7 files changed, 644 insertions(+), 56 deletions(-)
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 4fae9c94..27394ca2 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -18,6 +18,7 @@ import (
"unicode"
"github.com/mvanhorn/cli-printing-press/v2/internal/browsersniff"
+ "github.com/mvanhorn/cli-printing-press/v2/internal/mcpdesc"
"github.com/mvanhorn/cli-printing-press/v2/internal/naming"
"github.com/mvanhorn/cli-printing-press/v2/internal/profiler"
"github.com/mvanhorn/cli-printing-press/v2/internal/spec"
@@ -212,8 +213,7 @@ func New(s *spec.APISpec, outputDir string) *Generator {
"resolveEnvVarField": resolveEnvVarField,
"add": func(a, b int) int { return a + b },
"oneline": naming.OneLine,
- "mcpDescription": naming.MCPDescription,
- "mcpDescriptionRich": mcpDescriptionRich,
+ "composeMCPDesc": composeMCPDesc,
"flagName": flagName,
"paramIdent": paramIdent,
"safeTypeName": safeTypeName,
@@ -2552,48 +2552,20 @@ func resolveEnvVarField(envVar string) string {
return envVarField(envVar)
}
-// mcpDescriptionRich builds an enriched MCP tool description that includes
-// the base description plus response shape hints and method context.
-// This gives agents enough information to choose the right tool without
-// trial-and-error. Total length is capped to prevent token bloat.
-func mcpDescriptionRich(desc string, noAuth bool, authType string, publicCount, totalCount int, method, respType, respItem string) string {
- base := naming.MCPDescription(desc, noAuth, authType, publicCount, totalCount)
-
- var suffix string
-
- // Add response shape hint
- if respType == "array" && respItem != "" {
- suffix = "Returns array of " + respItem + "."
- } else if respType == "array" {
- suffix = "Returns array."
- } else if respType == "object" && respItem != "" {
- suffix = "Returns " + respItem + "."
- }
-
- // Add method context for non-obvious cases
- switch method {
- case "DELETE":
- if suffix != "" {
- suffix += " Destructive."
- } else {
- suffix = "Destructive operation."
- }
- case "PATCH":
- if suffix == "" {
- suffix = "Partial update."
- }
- }
-
- if suffix == "" {
- return base
- }
-
- result := base + " " + suffix
- // Cap at 200 chars to prevent token bloat (PostHog learned this the hard way)
- if len(result) > 200 {
- result = result[:197] + "..."
- }
- return result
+// composeMCPDesc is the template helper that wraps mcpdesc.Compose so
+// the mcp_tools.go.tmpl template can build a full description from
+// the parsed endpoint plus auth context. The composer in
+// internal/mcpdesc shapes the action sentence + Required/Optional
+// parameter lines + Returns clause; this wrapper just packages the
+// arguments into the Input struct.
+func composeMCPDesc(endpoint spec.Endpoint, noAuth bool, authType string, publicCount, totalCount int) string {
+ return mcpdesc.Compose(mcpdesc.Input{
+ Endpoint: endpoint,
+ NoAuth: noAuth,
+ AuthType: authType,
+ PublicCount: publicCount,
+ TotalCount: totalCount,
+ })
}
func exampleValue(p spec.Param) string {
diff --git a/internal/generator/templates/mcp_tools.go.tmpl b/internal/generator/templates/mcp_tools.go.tmpl
index 3176e39a..0a62c05a 100644
--- a/internal/generator/templates/mcp_tools.go.tmpl
+++ b/internal/generator/templates/mcp_tools.go.tmpl
@@ -38,7 +38,7 @@ func RegisterTools(s *server.MCPServer) {
{{- range $eName, $endpoint := $resource.Endpoints}}
s.AddTool(
mcplib.NewTool({{printf "%q" (printf "%s_%s" (snake $name) (snake $eName))}},
- mcplib.WithDescription({{printf "%q" (mcpDescriptionRich $endpoint.Description $endpoint.NoAuth $.Auth.Type $.MCPPublicCount $.MCPTotalCount $endpoint.Method $endpoint.Response.Type $endpoint.Response.Item)}}),
+ mcplib.WithDescription({{printf "%q" (composeMCPDesc $endpoint $endpoint.NoAuth $.Auth.Type $.MCPPublicCount $.MCPTotalCount)}}),
{{- range $endpoint.Params}}
{{- if eq .Type "integer"}}
mcplib.WithNumber({{printf "%q" .Name}}{{if .Required}}, mcplib.Required(){{end}}, mcplib.Description({{printf "%q" (oneline .Description)}})),
@@ -69,7 +69,7 @@ func RegisterTools(s *server.MCPServer) {
{{- range $eName, $endpoint := $subResource.Endpoints}}
s.AddTool(
mcplib.NewTool({{printf "%q" (printf "%s_%s_%s" (snake $name) (snake $subName) (snake $eName))}},
- mcplib.WithDescription({{printf "%q" (mcpDescriptionRich $endpoint.Description $endpoint.NoAuth $.Auth.Type $.MCPPublicCount $.MCPTotalCount $endpoint.Method $endpoint.Response.Type $endpoint.Response.Item)}}),
+ mcplib.WithDescription({{printf "%q" (composeMCPDesc $endpoint $endpoint.NoAuth $.Auth.Type $.MCPPublicCount $.MCPTotalCount)}}),
{{- range $endpoint.Params}}
{{- if eq .Type "integer"}}
mcplib.WithNumber({{printf "%q" .Name}}{{if .Required}}, mcplib.Required(){{end}}, mcplib.Description({{printf "%q" (oneline .Description)}})),
diff --git a/internal/mcpdesc/compose.go b/internal/mcpdesc/compose.go
new file mode 100644
index 00000000..e2e3251f
--- /dev/null
+++ b/internal/mcpdesc/compose.go
@@ -0,0 +1,268 @@
+// Package mcpdesc composes MCP tool descriptions from a parsed spec
+// endpoint, producing a verb-led action sentence followed by Required
+// and Optional parameter lines and a Returns clause keyed off the HTTP
+// method and response shape. The goal is a baseline description that
+// gives an agent enough to choose and call a tool without trial-and-
+// error, leaving polish to handle only the residual that needs hand-
+// tuning.
+//
+// Used by the runtime tools.go template (per-tool description at MCP
+// registration time) and the tools-manifest.json writer (per-tool
+// description that audit, override agents, and MCP hosts read). The
+// override sidecar (mcp-descriptions.json) takes precedence wherever
+// present; this package supplies the baseline.
+package mcpdesc
+
+import (
+ "fmt"
+ "strings"
+
+ "github.com/mvanhorn/cli-printing-press/v2/internal/naming"
+ "github.com/mvanhorn/cli-printing-press/v2/internal/spec"
+)
+
+// optionalListMax caps how many optional params Compose lists inline
+// before truncating to "(plus N more)". Three is enough to convey the
+// common filters/options without bloating the description; agents that
+// need the full list read tools-manifest.json's params field.
+const optionalListMax = 3
+
+// defaultValueMaxLen caps an inline "(default: X)" annotation. Very
+// long defaults (large objects, multi-line strings) are skipped rather
+// than bloating the description; the agent can read the spec for full
+// detail.
+const defaultValueMaxLen = 30
+
+// HTTP method constants. Method strings are matched at multiple
+// composition sites; defining them once keeps spelling consistent and
+// gives the compiler a chance to catch typos in any future site.
+const (
+ methodPOST = "POST"
+ methodPATCH = "PATCH"
+ methodPUT = "PUT"
+ methodDELETE = "DELETE"
+)
+
+type Input struct {
+ Endpoint spec.Endpoint
+ NoAuth bool
+ AuthType string
+ PublicCount int
+ TotalCount int
+}
+
+// Compose returns the composed description ready for storage in
+// tools-manifest.json or registration in tools.go. Auth annotation is
+// delegated to naming.MCPDescription so the (public)/(requires auth)
+// suffix logic stays single-sourced.
+//
+// Two pre-composition signals shape the output:
+//
+// - "Required:" / "Optional:" with the colon (case-insensitive):
+// structural marker of a hand-tuned override (typically from
+// mcpoverrides.Apply). Full pass-through — Compose only applies
+// auth and method-marker normalization.
+//
+// - Informal "returns " mention in the spec description: keep
+// composing Required/Optional from spec, but skip the generated
+// Returns clause to avoid "Returns X. Returns the X." doubling.
+func Compose(in Input) string {
+ desc := in.Endpoint.Description
+
+ var composed string
+ if hasStructuralOverride(desc) {
+ composed = composeAction(desc)
+ } else {
+ var parts []string
+ if action := composeAction(desc); action != "" {
+ parts = append(parts, action)
+ }
+ if required := composeRequired(in.Endpoint); required != "" {
+ parts = append(parts, required)
+ }
+ if optional := composeOptional(in.Endpoint); optional != "" {
+ parts = append(parts, optional)
+ }
+ if !mentionsReturn(desc) {
+ if returns := composeReturns(in.Endpoint); returns != "" {
+ parts = append(parts, returns)
+ }
+ }
+ composed = strings.Join(parts, " ")
+ }
+
+ composed = appendMethodMarker(composed, in.Endpoint.Method)
+ return naming.MCPDescription(composed, in.NoAuth, in.AuthType, in.PublicCount, in.TotalCount)
+}
+
+// hasStructuralOverride detects the colon-terminated markers an
+// override author actually writes: "Required:" or "Optional:". The
+// colon is the structural cue. Substring "returns " is too noisy —
+// spec authors mention return shape in narrative prose constantly
+// ("Returns the deleted resource"), and treating those as overrides
+// would skip Required/Optional composition for valid spec content.
+func hasStructuralOverride(desc string) bool {
+ lower := strings.ToLower(desc)
+ return strings.Contains(lower, "required:") || strings.Contains(lower, "optional:")
+}
+
+// mentionsReturn detects an informal "returns" mention in the action
+// sentence. Used only to suppress the generated Returns clause —
+// Required/Optional composition still runs.
+func mentionsReturn(desc string) bool {
+ return strings.Contains(strings.ToLower(desc), "returns ")
+}
+
+func composeAction(desc string) string {
+ s := strings.TrimSpace(desc)
+ if s == "" {
+ return ""
+ }
+ last := s[len(s)-1]
+ if last != '.' && last != '!' && last != '?' {
+ s += "."
+ }
+ return s
+}
+
+// composeRequired renders the "Required:" line. Path params count as
+// required regardless of CLI default — they're structurally in the
+// URL and treating them as optional disagrees with the spec's
+// parameters[]. Defaulted params gain "(default: X)" annotations so
+// the agent knows what the runtime fills in when the param is omitted.
+func composeRequired(ep spec.Endpoint) string {
+ names := collectParams(ep, isRequiredParam)
+ if len(names) == 0 {
+ return ""
+ }
+ return "Required: " + strings.Join(names, ", ") + "."
+}
+
+// composeOptional renders the "Optional:" line, capped at
+// optionalListMax with "(plus N more)" when more are present. Path
+// params never appear here.
+func composeOptional(ep spec.Endpoint) string {
+ names := collectParams(ep, isOptionalParam)
+ if len(names) == 0 {
+ return ""
+ }
+ if len(names) > optionalListMax {
+ hidden := len(names) - optionalListMax
+ return fmt.Sprintf("Optional: %s (plus %d more).", strings.Join(names[:optionalListMax], ", "), hidden)
+ }
+ return "Optional: " + strings.Join(names, ", ") + "."
+}
+
+// collectParams walks ep.Params followed by ep.Body, returning the
+// formatted names of params for which include is true. Single iteration
+// site so the path-param-always-required rule and the body/query rules
+// stay aligned.
+func collectParams(ep spec.Endpoint, include func(spec.Param) bool) []string {
+ var names []string
+ for _, p := range ep.Params {
+ if include(p) {
+ names = append(names, formatParam(p))
+ }
+ }
+ for _, p := range ep.Body {
+ if include(p) {
+ names = append(names, formatParam(p))
+ }
+ }
+ return names
+}
+
+// isRequiredParam classifies path params as required regardless of
+// the spec's Required flag (they're URL slots) and trusts the flag
+// for everything else. ep.Body fields don't carry the path-param
+// annotation, so the path-param branch only fires for ep.Params.
+func isRequiredParam(p spec.Param) bool {
+ if p.Positional || p.PathParam {
+ return true
+ }
+ return p.Required
+}
+
+func isOptionalParam(p spec.Param) bool {
+ if p.Positional || p.PathParam {
+ return false
+ }
+ return !p.Required
+}
+
+// composeReturns derives a return-shape clause from the response type
+// and HTTP method. Method-specific markers (Destructive, Partial
+// update) are added later by appendMethodMarker so they apply
+// uniformly to both fresh-composed and override paths.
+func composeReturns(ep spec.Endpoint) string {
+ method := strings.ToUpper(ep.Method)
+ respType := ep.Response.Type
+ item := strings.TrimSpace(ep.Response.Item)
+
+ switch {
+ case respType == "array" && item != "":
+ return "Returns array of " + item + "."
+ case respType == "array":
+ return "Returns array."
+ case respType == "object" && item != "":
+ switch method {
+ case methodPOST:
+ return "Returns the new " + item + "."
+ case methodPATCH, methodPUT:
+ return "Returns the updated " + item + "."
+ default:
+ return "Returns the " + item + "."
+ }
+ }
+ return ""
+}
+
+// appendMethodMarker adds the method-specific safety/semantic marker
+// when it isn't already present. Applies uniformly to fresh-composed
+// and override paths so DELETE always carries "Destructive." even
+// when an override author left it off.
+//
+// PATCH only gets "Partial update." when no Returns clause is
+// present — "Returns the updated X." already conveys partial-update
+// semantics.
+func appendMethodMarker(desc, method string) string {
+ if desc == "" {
+ return desc
+ }
+ lower := strings.ToLower(desc)
+ switch strings.ToUpper(method) {
+ case methodDELETE:
+ if !strings.Contains(lower, "destructive") {
+ return desc + " Destructive."
+ }
+ case methodPATCH:
+ if !strings.Contains(lower, "partial update") && !strings.Contains(lower, "returns") {
+ return desc + " Partial update."
+ }
+ }
+ return desc
+}
+
+func formatParam(p spec.Param) string {
+ if p.Default == nil {
+ return p.Name
+ }
+ val, ok := formatDefault(p.Default)
+ if !ok {
+ return p.Name
+ }
+ return p.Name + " (default: " + val + ")"
+}
+
+// formatDefault returns the value and true when usable inline, or
+// false when empty or unsuitable for annotation (oversize, multi-line).
+func formatDefault(v any) (string, bool) {
+ s := strings.TrimSpace(fmt.Sprintf("%v", v))
+ if s == "" || len(s) > defaultValueMaxLen {
+ return "", false
+ }
+ if strings.ContainsAny(s, "\n\r") {
+ return "", false
+ }
+ return s, true
+}
diff --git a/internal/mcpdesc/compose_test.go b/internal/mcpdesc/compose_test.go
new file mode 100644
index 00000000..a0d97a3b
--- /dev/null
+++ b/internal/mcpdesc/compose_test.go
@@ -0,0 +1,330 @@
+package mcpdesc
+
+import (
+ "strings"
+ "testing"
+
+ "github.com/mvanhorn/cli-printing-press/v2/internal/spec"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestCompose_CreatePOST(t *testing.T) {
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "POST",
+ Path: "/projects",
+ Description: "Create project",
+ Body: []spec.Param{
+ {Name: "name", Required: true},
+ {Name: "visibility", Required: true},
+ {Name: "owner_email", Required: false},
+ },
+ Response: spec.ResponseDef{Type: "object", Item: "Project"},
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Equal(t, "Create project. Required: name, visibility. Optional: owner_email. Returns the new Project.", got)
+}
+
+func TestCompose_ListGETArrayResponse(t *testing.T) {
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "GET",
+ Path: "/projects",
+ Description: "List projects",
+ Params: []spec.Param{
+ {Name: "status", Required: false},
+ {Name: "limit", Required: false},
+ {Name: "cursor", Required: false},
+ },
+ Response: spec.ResponseDef{Type: "array", Item: "Project"},
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Equal(t, "List projects. Optional: status, limit, cursor. Returns array of Project.", got)
+}
+
+func TestCompose_PatchUPDATEWithPathParams(t *testing.T) {
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "PATCH",
+ Path: "/projects/{projectId}/tasks/{taskId}",
+ Description: "Update project task",
+ Params: []spec.Param{
+ {Name: "projectId", Required: true, Positional: true},
+ {Name: "taskId", Required: true, Positional: true},
+ },
+ Body: []spec.Param{
+ {Name: "title", Required: false},
+ {Name: "priority", Required: false},
+ {Name: "completed", Required: false},
+ },
+ // No response shape declared
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Equal(t, "Update project task. Required: projectId, taskId. Optional: title, priority, completed. Partial update.", got)
+}
+
+func TestCompose_DeleteAddsDestructive(t *testing.T) {
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "DELETE",
+ Path: "/items/{itemId}",
+ Description: "Delete item",
+ Params: []spec.Param{
+ {Name: "itemId", Required: true, Positional: true},
+ },
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Equal(t, "Delete item. Required: itemId. Destructive.", got)
+}
+
+func TestCompose_PathParamWithDefaultStaysRequired(t *testing.T) {
+ // API-contract view: enum-typed path param with default still
+ // shows as Required (it's structurally in the URL). Default
+ // annotation tells the agent "you may skip it; runtime fills in".
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "POST",
+ Path: "/calendars/{calendar}/disconnect",
+ Description: "Disconnect a calendar",
+ Params: []spec.Param{
+ {
+ Name: "calendar",
+ Default: "apple",
+ Positional: false,
+ PathParam: true, // reclassified by parser
+ Required: false,
+ },
+ },
+ Body: []spec.Param{
+ {Name: "id", Required: true},
+ },
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Contains(t, got, "Required: calendar (default: apple), id.", "path param must be Required regardless of default; default value annotated")
+ assert.NotContains(t, got, "Optional: calendar", "path param must not appear as Optional")
+}
+
+func TestCompose_OptionalTruncationHonored(t *testing.T) {
+ body := []spec.Param{
+ {Name: "a", Required: false},
+ {Name: "b", Required: false},
+ {Name: "c", Required: false},
+ {Name: "d", Required: false},
+ {Name: "e", Required: false},
+ }
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "POST",
+ Path: "/things",
+ Description: "Create thing",
+ Body: body,
+ Response: spec.ResponseDef{Type: "object", Item: "Thing"},
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Contains(t, got, "Optional: a, b, c (plus 2 more).")
+}
+
+func TestCompose_PassesThroughHandTunedOverride(t *testing.T) {
+ // mcpoverrides.Apply writes the override into endpoint.Description
+ // before Compose runs. If Compose blindly added Required/Optional/
+ // Returns on top, the override "Required: name" + composer
+ // "Required: name, X" would double-stamp. Pre-composed descriptions
+ // (any of "Required:" / "Optional:" / "Returns ") get passed
+ // through with only auth-suffix and period normalization applied.
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "POST",
+ Path: "/tags",
+ Description: "Create a new tag in the workspace. Required: name. Returns the tag's id and slug. Tags must exist before they can be assigned to links.",
+ Body: []spec.Param{
+ {Name: "name", Required: true},
+ {Name: "color", Required: false},
+ },
+ Response: spec.ResponseDef{Type: "object", Item: "Tag"},
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ expected := "Create a new tag in the workspace. Required: name. Returns the tag's id and slug. Tags must exist before they can be assigned to links."
+ assert.Equal(t, expected, got)
+}
+
+func TestCompose_PreComposedSpecDescriptionPassesThrough(t *testing.T) {
+ // Auto-generated specs sometimes describe what an endpoint
+ // returns in the description itself. Treat that as authoritative
+ // to avoid "Returns pet inventories by status. Returns array of
+ // integers." — keep what the spec author chose.
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "GET",
+ Path: "/store/inventory",
+ Description: "Returns pet inventories by status",
+ Response: spec.ResponseDef{Type: "object", Item: "Inventory"},
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Equal(t, "Returns pet inventories by status.", got)
+ assert.Equal(t, 1, strings.Count(strings.ToLower(got), "returns"), "must not double-up Returns clause")
+}
+
+func TestCompose_DeleteAlwaysGetsDestructiveEvenWithReturnsInAction(t *testing.T) {
+ // appendMethodMarker fires after composition so the Destructive
+ // marker is added to pre-composed (override) descriptions too,
+ // not just fresh-composed ones. Agents need to know the call
+ // removes data regardless of whether the override mentioned it.
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "DELETE",
+ Path: "/things/{id}",
+ Description: "Returns the deleted resource",
+ Params: []spec.Param{{Name: "id", Required: true, Positional: true}},
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Contains(t, got, "Destructive", "DELETE method must always carry the destructive marker")
+}
+
+func TestCompose_DeleteSkipsDestructiveWhenAlreadyPresent(t *testing.T) {
+ // If the override or spec description already mentions destructive,
+ // don't double up. Case-insensitive match.
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "DELETE",
+ Path: "/things/{id}",
+ Description: "Delete the resource. This is destructive.",
+ Params: []spec.Param{{Name: "id", Required: true, Positional: true}},
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Equal(t, 1, strings.Count(strings.ToLower(got), "destructive"), "destructive marker must not double up")
+}
+
+func TestCompose_SpecDescriptionWithReturnsStillGetsRequiredOptional(t *testing.T) {
+ // A spec description that mentions "returns" in narrative prose
+ // (e.g., "Permanently deletes a card. Returns the deleted card.")
+ // is NOT a structural override. Required/Optional composition must
+ // still run; only the explicit Returns clause is suppressed to
+ // avoid doubling. Without this, every endpoint whose description
+ // happens to use the word "returns" loses parameter context — a
+ // large fraction of real-world specs.
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "DELETE",
+ Path: "/cards/{cardId}",
+ Description: "Permanently deletes a card. Returns the deleted card.",
+ Params: []spec.Param{
+ {Name: "cardId", Required: true, Positional: true, Description: "Card ID"},
+ {Name: "force", Required: false, Description: "Skip confirmation"},
+ },
+ Response: spec.ResponseDef{Type: "object", Item: "Card"},
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Contains(t, got, "Required: cardId", "Required clause must be added even when description mentions returns")
+ assert.Contains(t, got, "Optional: force", "Optional clause must be added")
+ assert.Equal(t, 1, strings.Count(strings.ToLower(got), "returns"), "explicit Returns clause must be suppressed when description already mentions returns")
+ assert.Contains(t, got, "Destructive", "DELETE method must carry the destructive marker")
+}
+
+func TestCompose_NoParamsNoResponse(t *testing.T) {
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "GET",
+ Path: "/health",
+ Description: "Health check",
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Equal(t, "Health check.", got)
+}
+
+func TestCompose_AppendsAuthAnnotationViaMCPDescription(t *testing.T) {
+ // Compose delegates the (public)/(requires auth) suffix to
+ // naming.MCPDescription so the auth-annotation logic stays
+ // single-sourced. Mixed-auth: 1 public out of 5 → public side
+ // is the minority, gets "(public)" suffix.
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "GET",
+ Path: "/public/status",
+ Description: "Get status",
+ Response: spec.ResponseDef{Type: "object", Item: "Status"},
+ },
+ NoAuth: true,
+ AuthType: "bearer_token",
+ PublicCount: 1,
+ TotalCount: 5,
+ }
+ got := Compose(in)
+ assert.Contains(t, got, "(public)", "mixed-auth APIs annotate the minority side")
+}
+
+func TestCompose_DefaultAnnotationBoundedToScalars(t *testing.T) {
+ // Defaults that exceed defaultValueMaxLen or contain newlines
+ // should be silently dropped from the inline annotation; the
+ // param still appears, just without the (default: ...) suffix.
+ tests := []struct {
+ name string
+ dflt any
+ wantHas string
+ wantNoHas string
+ }{
+ {"scalar string", "apple", "(default: apple)", ""},
+ {"int default", 25, "(default: 25)", ""},
+ {"bool default", true, "(default: true)", ""},
+ {"oversize default skipped", strings.Repeat("x", 50), "name", "(default:"},
+ {"newline default skipped", "a\nb", "name", "(default:"},
+ }
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ ep := spec.Endpoint{
+ Method: "GET",
+ Path: "/x",
+ Description: "Get",
+ Params: []spec.Param{
+ {Name: "name", Required: false, Default: tc.dflt},
+ },
+ }
+ got := Compose(Input{Endpoint: ep, AuthType: "none"})
+ assert.Contains(t, got, tc.wantHas)
+ if tc.wantNoHas != "" {
+ assert.NotContains(t, got, tc.wantNoHas)
+ }
+ })
+ }
+}
+
+func TestCompose_EmptyDescriptionStillBuildsParts(t *testing.T) {
+ // Defensive: if Endpoint.Description is somehow empty, the
+ // composed string still contains Required/Optional/Returns so
+ // downstream consumers get something useful instead of "".
+ in := Input{
+ Endpoint: spec.Endpoint{
+ Method: "POST",
+ Path: "/x",
+ Description: "",
+ Body: []spec.Param{{Name: "name", Required: true}},
+ Response: spec.ResponseDef{Type: "object", Item: "X"},
+ },
+ AuthType: "none",
+ }
+ got := Compose(in)
+ assert.Equal(t, "Required: name. Returns the new X.", got)
+}
diff --git a/internal/pipeline/toolsmanifest.go b/internal/pipeline/toolsmanifest.go
index 4c052b00..97f039ee 100644
--- a/internal/pipeline/toolsmanifest.go
+++ b/internal/pipeline/toolsmanifest.go
@@ -10,6 +10,7 @@ import (
"sort"
"strings"
+ "github.com/mvanhorn/cli-printing-press/v2/internal/mcpdesc"
"github.com/mvanhorn/cli-printing-press/v2/internal/mcpoverrides"
"github.com/mvanhorn/cli-printing-press/v2/internal/naming"
"github.com/mvanhorn/cli-printing-press/v2/internal/spec"
@@ -151,7 +152,13 @@ func WriteToolsManifest(dir string, parsed *spec.APISpec) error {
continue
}
toolName := mcpoverrides.ToolName(rName, "", eName)
- desc := naming.MCPDescription(endpoint.Description, endpoint.NoAuth, parsed.Auth.Type, public, total)
+ desc := mcpdesc.Compose(mcpdesc.Input{
+ Endpoint: endpoint,
+ NoAuth: endpoint.NoAuth,
+ AuthType: parsed.Auth.Type,
+ PublicCount: public,
+ TotalCount: total,
+ })
tool := buildManifestTool(toolName, desc, endpoint)
manifest.Tools = append(manifest.Tools, tool)
}
@@ -167,7 +174,13 @@ func WriteToolsManifest(dir string, parsed *spec.APISpec) error {
continue
}
toolName := mcpoverrides.ToolName(rName, subName, eName)
- desc := naming.MCPDescription(endpoint.Description, endpoint.NoAuth, parsed.Auth.Type, public, total)
+ desc := mcpdesc.Compose(mcpdesc.Input{
+ Endpoint: endpoint,
+ NoAuth: endpoint.NoAuth,
+ AuthType: parsed.Auth.Type,
+ PublicCount: public,
+ TotalCount: total,
+ })
tool := buildManifestTool(toolName, desc, endpoint)
manifest.Tools = append(manifest.Tools, tool)
}
diff --git a/skills/printing-press-polish/references/tools-polish.md b/skills/printing-press-polish/references/tools-polish.md
index f8053b35..d552729a 100644
--- a/skills/printing-press-polish/references/tools-polish.md
+++ b/skills/printing-press-polish/references/tools-polish.md
@@ -77,7 +77,12 @@ A typed endpoint tool whose `description` field in `tools-manifest.json` is empt
### `thin-mcp-description`
-A typed endpoint tool whose description is both short (<60 chars) and low-word-count (<8 words). The dominant pattern: spec-derived summaries like `"Create a tag"`, `"List domains"`, `"Update a link"` — fine for OpenAPI doc rendering, inadequate for an agent picking from a tool catalog. The agent looking at `"Create a tag"` has to guess what fields to pass, what comes back, and when to prefer this over alternatives.
+A typed endpoint tool whose description is both short (<60 chars) and low-word-count (<8 words).
+
+The generator composes a baseline description from spec material at generation time (action + `Required:` + `Optional:` + `Returns` clause), so thin findings should be uncommon when the spec is reasonably complete. When you do see one, it usually means either:
+
+- **The spec is genuinely thin.** No body fields, no response schema, no useful description text — the operation is `GET /health` style with nothing to compose. Verify by reading the spec; if confirmed, accept with the pre-decision fields filled honestly.
+- **An existing override is too short.** Someone wrote a short override into `mcp-descriptions.json` that bypasses the composer. Replace it with a richer override following the criteria below.
**Fix:** rewrite per the MCP description criteria below and write the new text to the override file (see "Override path" below).
diff --git a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
index e3c079f0..b0b94daa 100644
--- a/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
+++ b/testdata/golden/expected/generate-golden-api/printing-press-golden/internal/mcp/tools.go
@@ -26,7 +26,7 @@ import (
func RegisterTools(s *server.MCPServer) {
s.AddTool(
mcplib.NewTool("projects_create",
- mcplib.WithDescription("Create project Returns Project."),
+ mcplib.WithDescription("Create project. Required: name, visibility. Optional: owner_email. Returns the new Project."),
mcplib.WithDestructiveHintAnnotation(false),
mcplib.WithOpenWorldHintAnnotation(true),
),
@@ -34,7 +34,7 @@ func RegisterTools(s *server.MCPServer) {
)
s.AddTool(
mcplib.NewTool("projects_get",
- mcplib.WithDescription("Get project"),
+ mcplib.WithDescription("Get project. Required: projectId."),
mcplib.WithString("projectId", mcplib.Required(), mcplib.Description("Project id")),
mcplib.WithReadOnlyHintAnnotation(true),
mcplib.WithDestructiveHintAnnotation(false),
@@ -44,7 +44,7 @@ func RegisterTools(s *server.MCPServer) {
)
s.AddTool(
mcplib.NewTool("projects_list",
- mcplib.WithDescription("List projects Returns array of Project."),
+ mcplib.WithDescription("List projects. Optional: status, limit (default: 25), cursor. Returns array of Project."),
mcplib.WithString("status", mcplib.Description("Status")),
mcplib.WithString("limit", mcplib.Description("Limit")),
mcplib.WithString("cursor", mcplib.Description("Cursor")),
@@ -56,7 +56,7 @@ func RegisterTools(s *server.MCPServer) {
)
s.AddTool(
mcplib.NewTool("projects_tasks_list-project",
- mcplib.WithDescription("List project tasks Returns array of Task."),
+ mcplib.WithDescription("List project tasks. Required: projectId. Optional: priority, limit (default: 50), cursor. Returns array of Task."),
mcplib.WithString("projectId", mcplib.Required(), mcplib.Description("Project id")),
mcplib.WithString("priority", mcplib.Description("Priority")),
mcplib.WithString("limit", mcplib.Description("Limit")),
@@ -69,7 +69,7 @@ func RegisterTools(s *server.MCPServer) {
)
s.AddTool(
mcplib.NewTool("projects_tasks_update-project",
- mcplib.WithDescription("Update project task Partial update."),
+ mcplib.WithDescription("Update project task. Required: projectId, taskId. Optional: completed, priority, title. Partial update."),
mcplib.WithString("projectId", mcplib.Required(), mcplib.Description("Project id")),
mcplib.WithString("taskId", mcplib.Required(), mcplib.Description("Task id")),
mcplib.WithOpenWorldHintAnnotation(true),
@@ -78,7 +78,7 @@ func RegisterTools(s *server.MCPServer) {
)
s.AddTool(
mcplib.NewTool("public_get-status",
- mcplib.WithDescription("Get public service status (public)"),
+ mcplib.WithDescription("Get public service status. (public)"),
mcplib.WithReadOnlyHintAnnotation(true),
mcplib.WithDestructiveHintAnnotation(false),
mcplib.WithOpenWorldHintAnnotation(true),
@@ -87,7 +87,7 @@ func RegisterTools(s *server.MCPServer) {
)
s.AddTool(
mcplib.NewTool("reports_summary_get-report-year",
- mcplib.WithDescription("Get a report summary for a year"),
+ mcplib.WithDescription("Get a report summary for a year. Required: year."),
mcplib.WithString("year", mcplib.Required(), mcplib.Description("Year")),
mcplib.WithReadOnlyHintAnnotation(true),
mcplib.WithDestructiveHintAnnotation(false),
← e326adfd docs(skills): polish skill — read spec.json directly for req
·
back to Cli Printing Press
·
fix(cli): manifest description no longer doubles 'API' when fae7e6e0 →