# Tools Polish — MCP Tool Quality Playbook

**Your goal:** ensure every MCP tool exposed by this CLI carries a description an agent can use to decide whether to call the tool and how to call it correctly. Tool descriptions are how agents discover and choose tools — thin descriptions and missing classifications directly degrade agent UX.

**MCP descriptions ≠ Cobra Shorts.** They're different artifacts with different jobs:

- **Cobra Short** is `--help` text. Reader is a human who can scroll, experiment, and uses surrounding context. Terse-but-precise often works ("Print version", "Check CLI health").
- **MCP tool description** is what an agent reads while picking from a catalog of dozens of tools. The agent needs enough to decide *whether* to call this tool and *how* to call it correctly without trying it first. Usually richer than `--help` — typically 1-3 sentences naming the action, the key parameters, what comes back, and when to prefer this tool over alternatives.

This playbook has two passes. Both run on every CLI; do not skip either.

**The audit is a tripwire, not the assessment.** The audit's length-and-word-count thresholds catch the obviously inadequate cases — the 12-char "Create a tag" entries that any sensible reading would flag. But the real question — "does this description give an agent enough to use this tool correctly?" — requires reading the description against the operation it describes, which is judgment work. A 70-char description can be perfect or terrible; the binary can't tell. **Pass 2 is where the quality assessment actually happens.** Pass 1 is the floor.

## Pass 1: Address mechanical findings

Run the audit:

```bash
cli-printing-press tools-audit <cli-dir> --json
```

The audit emits findings on two surfaces:

- **Cobra source** (`internal/cli/*.go`) — `empty-short`, `thin-short`, `missing-read-only`. These check shell-out tools the runtime walker registers from Cobra metadata.
- **Tools manifest** (`tools-manifest.json`) — `empty-mcp-description`, `thin-mcp-description`. These check the descriptions agents actually see for typed endpoint tools, where the source is the OpenAPI spec rather than the Cobra Short.

The audit exempts `pp:endpoint`-annotated commands, parent groupers (no `RunE`, or the generated `parentNoSubcommandRunE(flags)` sentinel), and framework-skipped commands (`auth`, `doctor`, `version`, …) from the Cobra-side checks — those are not actionable leaf tool descriptions for the Cobra Short audit.

A finding here means the description is mechanically thin enough that there's no chance it's adequate. Fix it. But absence of a finding does NOT mean the description is good — the threshold is a floor, not a ceiling. That's what Pass 2 is for.

#### DO-NOT-EDIT files

Before fixing any finding, check the file header. Generated files carry a `// Generated by CLI Printing Press ... DO NOT EDIT.` comment. Hand-edits to those files are wiped on the next regeneration — fixing a thin Short in `bookings_create.go` (generated) only to have `mcp-sync` or a fresh generation overwrite it solves nothing.

For findings in DO-NOT-EDIT files:

1. **Mark the finding accepted in the ledger** with `note: "DO-NOT-EDIT generated file; needs generator-template fix"`.
2. **Surface a retro candidate** in your final summary. The accepted-with-rationale pattern is correct here, but the rationale is a system-level ask: the generator template that emitted this file should produce better defaults. Note the pattern in the polish summary so it can be addressed in a `/printing-press-retro` pass.

Files that are NOT DO-NOT-EDIT and ARE safe to edit:

- Hand-written novel commands (no generator header)
- The polish skill's own targets: README.md, SKILL.md, manifest.json
- `internal/cli/root.go` (the generator emits the scaffold but the skill mutates it during polish)
- `mcp-descriptions.json` — the sanctioned override file for MCP tool descriptions (see `thin-mcp-description` below)

`tools-manifest.json` and `internal/mcp/tools.go` are both DO-NOT-EDIT generated files. Don't edit them directly for description fixes — the override file is the path that survives regen.

### `empty-short`

A `cobra.Command{}` shell-out tool with no `Short:` field. The runtime walker falls back to `"Run \`<cmd>\` through the companion CLI binary."` — meaningless to agents.

**Fix:** write a verb-led, action-specific Short. Read the command's `RunE` body and `Flags()` block to ground the description in actual behavior. See **Description criteria** below.

### `thin-short`

Cobra Short text under 30 characters AND fewer than 4 words on a shell-out tool. Examples from real CLIs: `"Advertiser Search"`, `"Search Ads"`, `"Subreddit Posts"`, `"Product Reviews"`.

**This is a suspect, not a verdict.** Some short Shorts are accurate (`"Print version"`, `"Check CLI health"`). Decide per finding:

- If the Short doesn't tell an agent what action runs, what parameters it takes, or what comes back → rewrite per **Description criteria**.
- If the Short is brief but precise → mark accepted in the ledger with a one-line rationale.

### `missing-read-only`

Shell-out tool whose name matches a read-shaped pattern (`list`, `get`, `show`, `view`, `find`, `describe`, `context`, `stats`, `trending`, `trust`, `health`, `stale`, `orphans`, `reconcile`, `analytics`) but has no `mcp:read-only` annotation.

**Fix:** add `Annotations: map[string]string{"mcp:read-only": "true"},` to the command literal. The walker then emits `readOnlyHint: true` so MCP hosts skip the per-call permission prompt.

**Don't blindly accept.** If the name matches the heuristic but the body actually mutates state (writes to the local store, opens a browser, sends a notification, modifies config), do NOT add the annotation. The heuristic is a starting point; the body is the truth.

### `empty-mcp-description`

A typed endpoint tool whose `description` field in `tools-manifest.json` is empty.

**Fix:** write an MCP-grade description (see criteria below) to the override file (see "Override path" below). Do NOT edit `tools-manifest.json` or `internal/mcp/tools.go` directly — both are generated and edits get wiped on the next `mcp-sync`.

### `thin-mcp-description`

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).

**Acceptance is rare here.** A 30-char description is almost never enough for a typed MCP tool. Only accept if the underlying operation is genuinely so simple that more words add no information (e.g., a synthetic ping/heartbeat tool).

### Override path: `mcp-descriptions.json`

Both `tools-manifest.json` and `internal/mcp/tools.go` are generated artifacts (each carries the generator's "DO NOT EDIT" header) and get rewritten on every `mcp-sync` run. The sanctioned path for replacing thin spec-derived descriptions is the `mcp-descriptions.json` sidecar at the cli-dir root. Format:

```json
{
  "descriptions": {
    "tags_create": "Create a new tag in the workspace. Required: name. Optional: color (hex) and parentTagId. Returns the tag's id and slug. Tags must exist before they can be assigned to links.",
    "tags_update": "..."
  }
}
```

Keys are the MCP tool names exactly as they appear in `tools-manifest.json`'s `name` field (snake_case, e.g. `tags_create`, `bookings_attendees_booking-add`). Values are the new descriptions written per the MCP criteria below.

After writing one or more overrides, **run `cli-printing-press mcp-sync <cli-dir>`** to regenerate the manifest and runtime registration with the override applied. Then re-run the audit; the finding disappears because the manifest now reflects the override.

The override file is hand-editable, persists across regenerations (it lives outside the generator's emit set), and survives `mcp-sync` runs. It's the only place an agent should write MCP descriptions that have to last.

## Pass 2: Evaluate every command (the load-bearing pass)

This is where the actual quality assessment happens. Walk every user-facing command in `internal/cli/` and every entry in `tools-manifest.json`, including ones the audit didn't flag. The mechanical check in Pass 1 misses two real classes:

1. **Read-only commands whose names don't match the heuristic.** Platform-named shortcuts (`tiktok`, `instagram`, `google`), one-word reads (`search`, `analytics`, `export`), commands renamed away from their `list-*`/`get-*` form. Real reads that need `mcp:read-only` but won't trip the audit.
2. **Descriptions that clear the length threshold but still leave an agent guessing.** A 65-character description that says `"Manage your saved cookbook (list, search, tag, match)"` is category-shaped, not action-shaped. A 70-char MCP description that names the action but no parameters or return shape is technically over the threshold and still inadequate. Length is a proxy for content; only reading the description against the operation tells you whether it earns its bytes.

For each command:

- Read its `RunE` body (or for typed endpoint tools, the underlying spec method/path). Does it call an HTTP method (`client.Get`, `resolveRead`, `c.Post`, …)? Does it write to the local store, the filesystem, or config?
  - **HTTP GET only, no local writes** → mark `mcp:read-only` (even if the audit didn't flag).
  - **Any local write or external mutation** → do NOT mark; the default destructive hint is correct.
  - **Side effect** (browser open, notification, audio) → do NOT mark; agents should be prompted.
- Read the description (Cobra `Short` for shell-out, manifest entry for typed endpoint). Test against the criteria below — not just "is it long enough" but "does it actually answer the four questions an agent needs answered before calling this tool?" If the answer is "not really," rewrite.

## Description criteria

### Cobra Short (shell-out tools)

Agent-grade Shorts share four properties:

1. **Verb-led.** Open with the action: `"Search ..."`, `"Fetch ..."`, `"List ..."`, `"Estimate ..."`, `"Save ..."`. Not `"Cookbook commands"` or `"Manage ..."`.
2. **Action-specific, not category-shaped.** `"Search the LinkedIn Ad Library by company, keyword, country, and date range"` — not `"Search Ads"`. The reader should know the *one specific operation* this command performs.
3. **Parameter-aware.** Name the meaningful filters/options inline so the agent knows what to pass: `"... by max cook time, recency, and dietary tags"`, not just `"Pick dinner"`.
4. **Scope-explicit.** When the command operates on a subset, say so: `"Search recipe titles across curated sites without fetching the full recipes (returns metadata only)"` — tells the agent the cost/output trade-off vs. a richer search.

### MCP descriptions (typed endpoint tools)

The same four properties, plus a higher bar on length and content. Agent looking at a tool catalog needs roughly 1-3 sentences answering:

- **What action does this tool perform?** (the verb-led part)
- **What are the key parameters?** Required ones explicitly; the most useful optional ones too. Agents call tools blind — without the description, the only signal is the parameter schema, which doesn't tell them which ones matter.
- **What comes back?** The shape of the result. "Returns the new link's id and slug" beats "Creates a link."
- **When to prefer this tool?** When two or three tools could plausibly handle the request, what distinguishes this one.

Worked example:

```
Before: "Create a tag"
After:  "Create a new tag in the workspace. Required: name. Optional: color (hex)
        and parentTagId for nested tag groups. Returns the tag's id and slug.
        Tags must exist before they can be assigned to links — call this first
        if you're tagging a new category."
```

The before is what OpenAPI auto-generators emit. The after is what an agent picking between `tags_create`, `tags_update`, `links_create` (which can also accept tags) actually needs.

#### 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.

**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.

**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

- **Dev-state leakage.** `"(wip)"`, `"(planned)"`, `"(ranking integration wip)"`, `"(coming soon)"` — useless to agents, leaks dev backlog. Strip during polish.
- **Self-referential Long.** `Long: "Shortcut for 'feed get-on-this-day'. Events on this day"` — repeats the Short and tells the reader nothing new. Rewrite Long to add genuine context (parameters, output shape, edge cases) or delete it.
- **Bare verb fragments.** `"Show cooking history"`, `"List saved recipes"`, `"Print version"` — fine for humans browsing `--help`, weak for agents deciding which of many similar tools to invoke. Add the qualifier (`"... most recent first, with rating and notes"`, `"... optionally filtered by tag, site, or author"`).
- **Empty Long with non-empty Short.** Long should add genuine context or be omitted. A Long that just restates Short adds noise to the MCP tool catalog.

## Worked examples

Pre/post pairs from real polish passes.

### Cobra Shorts (shell-out tools)

```
Before: "Lightweight cross-site recipe search (metadata only, no fetch)"
After:  "Search recipe titles across curated sites without fetching the full recipes (returns metadata only)"
```

```
Before: "Show cooking history"
After:  "List past cooking sessions with ratings and notes, most recent first"
```

```
Before: "Advertiser Search"
After:  "Search the Google Ads Transparency Center for advertisers by name or domain"
```

```
Before: "Subreddit Posts"
After:  "Fetch posts from a subreddit, with sort (hot/new/top) and timeframe filters"
```

### MCP descriptions (typed endpoint tools)

```
Before: "Create a tag"
After:  "Create a new tag in the workspace. Required: name. Optional: color (hex) and parentTagId.
        Returns the tag's id and slug. Tags must exist before they can be assigned to links."
```

```
Before: "List all customers"
After:  "List customers in the workspace. Optional filters: email, externalId, country.
        Returns paginated array with id, email, name, createdAt. Use customers_get-id when
        you already have an id; use this when you need to search by attribute."
```

```
Before: "Retrieve analytics for a link, a domain, or the authenticated workspace."
After:  (already agent-grade — this is the upper bound of what the OpenAPI summary field
        produces. No rewrite needed; mark accepted with a brief note.)
```

## Per-finding response templates

### Empty Short

```go
// Add the Short field; choose verb-led action description.
Short:       "<verb> <object> <key qualifier>",
Annotations: map[string]string{"mcp:read-only": "true"}, // only if read
```

### Thin Short

```go
// Replace the existing Short with a parameter-aware rewrite.
// Read RunE body and Flags() block to ground the description in real behavior.
Short: "<verb> <specific scope> by <main filter>, <secondary filter>",
```

### Missing read-only

```go
// Add or extend the Annotations map. Preserve any existing keys.
Annotations: map[string]string{"mcp:read-only": "true"},
```

### Empty / thin MCP description

Write to `<cli-dir>/mcp-descriptions.json`, then run `mcp-sync` to apply.

```json
{
  "descriptions": {
    "tags_create": "<MCP-grade description per criteria above>",
    "tags_update": "<another override>"
  }
}
```

```bash
cli-printing-press mcp-sync <cli-dir>
```

The sync regenerates `tools-manifest.json` and `internal/mcp/tools.go` with the overrides applied. Both generated files now carry the richer text; both are wiped and re-emitted from the override file on every regen, so edits persist. Re-run `tools-audit` to confirm the finding is gone.

## Ledger and resumability

`tools-audit` writes `<cli-dir>/.printing-press-tools-polish.json` after every run. It contains the timestamp, cli-dir, one entry per finding, a `scorecard_before` snapshot captured on the first run, and an optional `progress` checkpoint.

1. **Delta computation.** On a second run within 24 hours, the audit prints `since last run: N resolved, M new`. Stale ledgers (>24h) are deleted automatically.
2. **Resumability.** If your context window flushes mid-polish, re-run `tools-audit <cli-dir>`. Findings you've fixed have disappeared from the new scan; findings you accepted are still recorded with status. The render's `next:` line at the bottom names the next finding you owe a decision on. Update `progress.last_processed_finding_id` to the finding key you've just decided so a future re-run resumes after it instead of re-scanning from the head.
3. **Audit trail of accept decisions.** When you decide a finding is fine as-is, you mark the entry `accepted` with a rationale. For `thin-mcp-description` and `empty-mcp-description` you must also fill the three pre-decision fields (see below). The next run filters the accepted entry out of the pending table only when the gate fields are populated; otherwise the binary surfaces it as an incomplete accept.

### Marking a finding accepted

#### Cobra-side findings (`empty-short`, `thin-short`, `missing-read-only`)

When the table shows a finding that's actually fine, edit the ledger entry directly:

```json
{
  "kind": "thin-short",
  "command": "version",
  "file": "version.go",
  "line": 12,
  "evidence": "Print version",
  "status": "accepted",
  "note": "Standard 'version' command; agents understand this without elaboration"
}
```

The pending table excludes the accepted entry on the next run; the header shows `(N accepted)` to confirm.

**Don't accept `empty-short` or `missing-read-only`.** They have no judgment call: empty descriptions always need text, and a read command always wants the annotation.

#### MCP-description findings (`thin-mcp-description`, `empty-mcp-description`)

These findings carry a stricter accept contract. The historical failure mode is bulk-accept: walking 200+ thin-description findings and stamping all of them with one rationale ("systemic to OpenAPI specs", "doesn't compound") instead of deliberating per item. The binary now refuses to count an accept as complete unless three pre-decision fields are populated:

```json
{
  "kind": "thin-mcp-description",
  "command": "tags_create",
  "file": "tools-manifest.json",
  "line": 0,
  "evidence": "Create a new tag",
  "status": "accepted",
  "note": "Spec summary is the only source; an override would inflate every CLI without compounding",

  "spec_source_material": "summary='Create a new tag'; one required body param 'name' (string); response is the tag object with id, name, slug",
  "target_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.",
  "gap_analysis": "Generator could compose target from the spec's request and response schemas; today it only emits the bare summary. Override is the right path until #384 lifts the baseline."
}
```

Each field has a job:

- **`spec_source_material`** — the honest answer to "what does the spec actually give us for this endpoint?" Quote from the source. Don't generalize ("specs are thin") — name the specific summary, parameters, request body, and response schema you read in `<cli-dir>/spec.json` or `spec.yaml`.
- **`target_description`** — what a 10/10 description for this tool would say given the source material. Even when accepting the current state, naming the target makes the gap visible.
- **`gap_analysis`** — why the generator can't produce `target_description` from `spec_source_material` today. This is the load-bearing field: it forces you to reason about whether the gap is genuinely systemic (file as a generator improvement, e.g., #384) or per-finding (write an override).

If you can't fill all three honestly, you don't yet understand the finding well enough to accept it. Write the override instead, or do the spec reading required to fill the fields.

**Forbidden accept patterns:**

- "Systemic to OpenAPI specs", "doesn't compound", "out of scope for polish", or any rationale you'd stamp identically across many findings. The binary clusters accepts by normalized note text and refuses runs where any cluster exceeds 5 entries.
- Accept-without-reading-spec. The pre-decision fields require the spec source material — guess and the gap analysis will read like generic prose.
- Accept-and-move-on without writing the corresponding generator-improvement task. If you're filing the same `gap_analysis` repeatedly, that's evidence the generator should produce the target instead; surface it in the polish summary as a retro candidate.

**Acceptance should be rare here.** The expected steady state is "fix it via override" or "fix it via generator improvement," not "accept it." See the override path under `thin-mcp-description` above.

### Don't manually mark findings as fixed

If you fixed a finding via code change or override, do nothing in the ledger. The next audit re-scans the source and the manifest; finding gone → finding gone from the table → delta line shows `1 resolved`. Never set `status: "fixed"` by hand.

### Completion gates the binary enforces

The audit's "complete" signal is no longer just "zero pending." Four gates must pass:

1. **Pre-decision fields gate.** Every accepted `thin-mcp-description` / `empty-mcp-description` entry has `spec_source_material`, `target_description`, and `gap_analysis` populated. Missing fields surface as `incomplete: N accepted finding(s) missing pre-decision fields` with each entry's `kind`/`command`/`file:line`.
2. **Duplicate-rationale gate.** No more than 5 accepted findings share the same normalized `note` text. The audit clusters notes by lowercase + collapsed whitespace, so paraphrasing without changing meaning doesn't work — differentiate per item or rewrite both. Surfaces as `incomplete: N accepted finding(s) share rationale "..."`.
3. **Scorecard-delta gate.** When `MCPDescriptionQuality` does not move from `scorecard_before` to current AND the run accepted any thin-mcp-description findings, the run is incomplete. Either the accepts were unwarranted (you owed an override that would have lifted the score) or the dimension is mis-scored (rare, surface to retro). Surfaces as `incomplete: MCPDescriptionQuality unchanged (X/10 → X/10) but N thin-mcp-description finding(s) accepted`.
4. **Resume hint.** The render's `next:` line at the bottom names the first pending finding (status unset, or accepted-without-fields). Update `progress.last_processed_finding_id` after each decision so a re-run after compaction picks up where you left off.

A run is complete only when the summary line reads `tools-audit: no pending findings (N accepted)` with no `incomplete:` block printed.

## Verification checklist

After applying fixes, before declaring the polish complete:

- [ ] `go build ./...` clean (annotations don't break compilation)
- [ ] `cli-printing-press tools-audit <cli-dir>` summary line reads `no pending findings`. No `incomplete:` block — every gate (pre-decision fields, duplicate rationale, scorecard delta) passes.
- [ ] `cli-printing-press dogfood --dir <cli-dir>` reports `MCP Surface: PASS`
- [ ] If `mcp-descriptions.json` was edited, `cli-printing-press mcp-sync <cli-dir>` was run to apply the overrides into `tools-manifest.json` and `internal/mcp/tools.go`. Re-run `tools-audit` after the sync to confirm thin-mcp-description findings disappeared.
- [ ] If commands were renamed or had their annotations restructured, smoke-test the binary by inspecting `--help` output for the affected commands

The ledger file persists until it ages out (24h). Once the polish PR merges and the CLI is rebuilt, the file is no longer load-bearing — the next `tools-audit` run can start fresh.

If you're polishing a CLI inside a clone of the public library repo (not the internal `$PRESS_LIBRARY/`), add `.printing-press-tools-polish.json` to that repo's root `.gitignore` before committing — the ledger is local working state, not part of the published CLI.
