docs(skills): polish skill — read spec.json directly for required/optional, not the manifest (#395)

e326adfd94782cb3d643bc7b2ffb68295574a264 · 2026-04-29 12:11:23 -0700 · Trevin Chow

Tighten the "Where to mine the content from" section so agents
walking findings know:

- The spec is authoritative for required-vs-optional and parameter
  location. tools-manifest.json's params field is generated from the
  spec and can drop or misclassify entries (the canonical example
  being enum-typed path params landing as location: "query" because
  the generator reclassifies them to CLI flags).
- A concrete per-tool walk: find the operation by path/method in
  spec.paths, then read description / parameters[] /
  requestBody.schema.required[] / responses[].content.schema. Path
  params are structurally required regardless of how the CLI renders
  them; body required-ness comes from the schema's required[] array,
  following $ref to components.schemas if needed.
- The manifest's params field is a sanity-check, not a source of
  truth — discrepancies should be surfaced in the polish summary so
  generator bugs get filed.
- The runtime tools.go exposes a subset of the spec's superset; the
  description should reflect the runtime, but the required/optional
  metadata still comes from the spec.

Closes the failure mode where an agent fills pre-decision fields
plausibly from manifest data without actually walking the spec
per-finding. The new prose makes the spec walk explicit and tells
the agent which spec fields to read.

Files touched

M skills/printing-press-polish/references/tools-polish.md

Diff

commit e326adfd94782cb3d643bc7b2ffb68295574a264
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Wed Apr 29 12:11:23 2026 -0700

    docs(skills): polish skill — read spec.json directly for required/optional, not the manifest (#395)
    
    Tighten the "Where to mine the content from" section so agents
    walking findings know:
    
    - The spec is authoritative for required-vs-optional and parameter
      location. tools-manifest.json's params field is generated from the
      spec and can drop or misclassify entries (the canonical example
      being enum-typed path params landing as location: "query" because
      the generator reclassifies them to CLI flags).
    - A concrete per-tool walk: find the operation by path/method in
      spec.paths, then read description / parameters[] /
      requestBody.schema.required[] / responses[].content.schema. Path
      params are structurally required regardless of how the CLI renders
      them; body required-ness comes from the schema's required[] array,
      following $ref to components.schemas if needed.
    - The manifest's params field is a sanity-check, not a source of
      truth — discrepancies should be surfaced in the polish summary so
      generator bugs get filed.
    - The runtime tools.go exposes a subset of the spec's superset; the
      description should reflect the runtime, but the required/optional
      metadata still comes from the spec.
    
    Closes the failure mode where an agent fills pre-decision fields
    plausibly from manifest data without actually walking the spec
    per-finding. The new prose makes the spec walk explicit and tells
    the agent which spec fields to read.
---
 .../references/tools-polish.md                     | 28 ++++++++++++++++------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/skills/printing-press-polish/references/tools-polish.md b/skills/printing-press-polish/references/tools-polish.md
index fc917d55..f8053b35 100644
--- a/skills/printing-press-polish/references/tools-polish.md
+++ b/skills/printing-press-polish/references/tools-polish.md
@@ -151,15 +151,29 @@ The before is what OpenAPI auto-generators emit. The after is what an agent pick
 
 #### Where to mine the content from
 
-Don't write MCP descriptions from general API knowledge — that produces plausible but inaccurate prose. The CLI ships with the source material; ground every rewrite in it:
+Don't write MCP descriptions from general API knowledge — that produces plausible but inaccurate prose. The CLI ships with the source material; ground every rewrite in it.
 
-1. **The OpenAPI spec** at `<cli-dir>/spec.json` (or `spec.yaml`). For each tool, find the matching operation by `path` and `method` (those are in `tools-manifest.json` next to the description). The operation object usually has:
-   - A richer `description` field, often several sentences, that the manifest's `summary` truncated. This is your primary source for the rewrite.
-   - `parameters[].description` — use these to name the meaningful parameters in your rewrite. Don't list every parameter; pick the 1-3 that drive how the agent calls the tool.
-   - `responses['200'].content[*].schema` — tells you what comes back. Translate the schema shape into a one-clause "Returns ..." rather than a field-by-field listing.
-2. **The runtime registration** at `<cli-dir>/internal/mcp/tools.go`. Find the matching `mcplib.NewTool(...)` block. The parameter list there is what the runtime actually exposes (sometimes a subset of what the spec defines). Your description should reflect that subset, not the spec's superset.
+**The spec is authoritative. `tools-manifest.json` is a derivative.** For required-vs-optional, parameter location (path / query / body), and the canonical parameter list, read `<cli-dir>/spec.json` (or `spec.yaml`) directly. The manifest's `params` field is generated from the spec; generator bugs can drop or misclassify entries. When the manifest disagrees with the spec, **the spec wins**. The manifest's `description` field is the spec's `summary` shaped for MCP — useful as orientation, not as the source of truth for parameter contracts.
 
-If the spec's operation `description` is itself empty or as thin as the `summary`, you're upstream of an incomplete spec — flag this in the accept rationale and write the best description you can from the parameters and response schema. Don't make up parameters or behaviors the spec doesn't document.
+**Per-tool spec walk** (do this for every finding, not a sample):
+
+1. **Find the operation.** `tools-manifest.json` gives you `path` and `method`. Look up `spec.paths[<path>][<method>]`.
+
+2. **Description prose** comes from the operation:
+   - `description` field — usually richer than `summary`. Primary source.
+   - `summary` — fallback when `description` is missing, or when `description` is just a deprecation banner with no action verb. Don't override the action verb with the deprecation warning alone.
+   - `responses['200'].content[*].schema` (or `'201'` for creates, `'204'` for deletes) — tells you what comes back. Translate the schema shape into a one-clause `Returns …` rather than a field-by-field listing.
+
+3. **Required-vs-optional** comes from the spec's parameter sources, NOT from the manifest:
+   - `parameters[]`: every entry with `in: path` is structurally required (must appear in the URL) regardless of how the CLI renders it as flag or positional arg. Every entry with `in: query` is required iff its own `required: true`.
+   - `requestBody.content.application/json.schema.required[]` — the canonical list of required body fields. Body fields not in this array are optional.
+   - When the body schema is a `$ref`, follow the ref to `components.schemas[<name>]` and read `required[]` there.
+
+4. **Sanity-check the manifest.** Glance at `tools-manifest.json`'s `params` for the same tool. If it disagrees with what you found in the spec, trust the spec for your description and surface the discrepancy in the polish summary so the generator bug can be filed and fixed.
+
+5. **The runtime registration** at `<cli-dir>/internal/mcp/tools.go` is what's actually exposed at runtime. If the runtime registers only a subset of the spec's parameter superset, your description should reflect what the runtime exposes — but the required/optional metadata for that subset still comes from the spec.
+
+If the spec's operation `description` is itself empty or as thin as the `summary`, you're upstream of an incomplete spec. Flag this in the accept rationale and write the best description you can from `parameters[]`, `requestBody.schema.required[]`, and the response schema. Don't make up parameters or behaviors the spec doesn't document.
 
 ### Anti-patterns to remove

← cdd9153b fix(cli): tools-manifest.json classifies reclassified path p · back to Cli Printing Press · feat(cli,skills): generator-time MCP description enrichment 2b7a51e1 →