[object Object]

← back to Cli Printing Press

fix(cli): mcp-sync preserves manifest brand-casing and hand-edited descriptions (#373)

9f57c41d693463110979cfa8d05abbe95600f050 · 2026-04-28 20:02:06 -0700 · Trevin Chow

* fix(cli): mcp-sync preserves manifest brand-casing and hand-edited descriptions

Pilot sweep on the public library surfaced two regen-only regressions
in the manifest preservation chain. Both hit every CLI in the library;
the bugs ship now in v2.4.0 mcp-sync but only manifest during regen
against an existing manifest.json.

1. display_name regressed for single-word brand names. The
   "is this a derived default?" check rejected any existing display_name
   that matched titleCaseFirstRune(slug) — but for single-word brands
   ("Wikipedia", "Stripe", "Discord", "Pinterest") the title-cased slug
   IS the correct brand name. Library CLIs with these names fell
   through to the bare lowercase slug ("wikipedia") on regen.
   Drop the titleCased check; preserve any non-slug display_name. The
   only form rejected now is the bare lowercase slug we'd otherwise
   emit as last resort. titleCaseFirstRune (and the duplicate
   titleCaseFromSlug in mcpsync/sync.go) were dead-code-removed.

2. Hand-curated manifest descriptions were silently overwritten by the
   canonical .printing-press.json description on regen. recipe-goat's
   60-word hand-edited description (with detailed feature breakdown)
   got replaced by a different value sourced from research.json.
   Reorder manifestDescription's chain so existing-manifest hand-edits
   win over canonical. readExistingManifestDescription already filters
   out the derived "<displayName> API surface as MCP tools." default,
   so canonical still wins when the existing was itself a default.
   Trade-off: spec.description updates won't auto-propagate after
   first generation (user must clear manifest.json's description to
   re-derive). Documented in the now-divergent buildMCPBManifest
   comment.

Tests: TestWriteMCPBManifestPreservesExistingDisplayName pins the
brand-name preservation contract for Wikipedia/Stripe/ESPN/Cal.com/
Company GOAT plus the lowercase-slug rejection. New
TestWriteMCPBManifestPreservesExistingDescription covers both the
hand-edit-wins and derived-default-refreshes paths.

Verified end-to-end: re-running mcp-sync against
~/Code/printing-press-library/library/{media-and-entertainment/wikipedia,
food-and-dining/recipe-goat} now preserves display_name "Wikipedia"
and the recipe-goat hand-curated description verbatim.

Full suite: 2227/2227. Golden verify: 9/9. Lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* simplify: address /simplify findings on PR #373

- Consolidate manifest.json reads. buildMCPBManifest used to read +
  unmarshal manifest.json twice per call (once in
  readExistingManifestDisplayName, once in
  readExistingManifestDescription). Now loads once via
  loadExistingMCPBManifest and shares the snapshot. Each consumer still
  applies its own field-specific "is this a derived default?" check.
- Trim narrating comments per CLAUDE.md "default to no comments".
  Removed bug-history paragraphs ("The previous version of this
  function also rejected title-cased slugs…", "Pre-fix m.Description
  silently overwrote hand-edits…", "regressed library CLIs from 'ESPN'
  → 'espn'") from godocs and test preambles. That archaeology lives in
  the commit message.
- Replace "manifest.json" string literal in mcpsync/sync.go with the
  exported pipeline.MCPBManifestFilename constant — closes a small
  naming-leak that defeats the "promoted from string literals" rationale
  on the constants block.
- Add writeMCPBManifest test helper to match the existing writeManifest
  + readMCPBManifest helper trio. Test seed blocks collapse from 3
  lines (json.Marshal + os.WriteFile + require) to one helper call.

Net -77 lines. Tests 2227/2227. Golden 9/9. Lint clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Files touched

Diff

commit 9f57c41d693463110979cfa8d05abbe95600f050
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Tue Apr 28 20:02:06 2026 -0700

    fix(cli): mcp-sync preserves manifest brand-casing and hand-edited descriptions (#373)
    
    * fix(cli): mcp-sync preserves manifest brand-casing and hand-edited descriptions
    
    Pilot sweep on the public library surfaced two regen-only regressions
    in the manifest preservation chain. Both hit every CLI in the library;
    the bugs ship now in v2.4.0 mcp-sync but only manifest during regen
    against an existing manifest.json.
    
    1. display_name regressed for single-word brand names. The
       "is this a derived default?" check rejected any existing display_name
       that matched titleCaseFirstRune(slug) — but for single-word brands
       ("Wikipedia", "Stripe", "Discord", "Pinterest") the title-cased slug
       IS the correct brand name. Library CLIs with these names fell
       through to the bare lowercase slug ("wikipedia") on regen.
       Drop the titleCased check; preserve any non-slug display_name. The
       only form rejected now is the bare lowercase slug we'd otherwise
       emit as last resort. titleCaseFirstRune (and the duplicate
       titleCaseFromSlug in mcpsync/sync.go) were dead-code-removed.
    
    2. Hand-curated manifest descriptions were silently overwritten by the
       canonical .printing-press.json description on regen. recipe-goat's
       60-word hand-edited description (with detailed feature breakdown)
       got replaced by a different value sourced from research.json.
       Reorder manifestDescription's chain so existing-manifest hand-edits
       win over canonical. readExistingManifestDescription already filters
       out the derived "<displayName> API surface as MCP tools." default,
       so canonical still wins when the existing was itself a default.
       Trade-off: spec.description updates won't auto-propagate after
       first generation (user must clear manifest.json's description to
       re-derive). Documented in the now-divergent buildMCPBManifest
       comment.
    
    Tests: TestWriteMCPBManifestPreservesExistingDisplayName pins the
    brand-name preservation contract for Wikipedia/Stripe/ESPN/Cal.com/
    Company GOAT plus the lowercase-slug rejection. New
    TestWriteMCPBManifestPreservesExistingDescription covers both the
    hand-edit-wins and derived-default-refreshes paths.
    
    Verified end-to-end: re-running mcp-sync against
    ~/Code/printing-press-library/library/{media-and-entertainment/wikipedia,
    food-and-dining/recipe-goat} now preserves display_name "Wikipedia"
    and the recipe-goat hand-curated description verbatim.
    
    Full suite: 2227/2227. Golden verify: 9/9. Lint clean.
    
    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
    
    * simplify: address /simplify findings on PR #373
    
    - Consolidate manifest.json reads. buildMCPBManifest used to read +
      unmarshal manifest.json twice per call (once in
      readExistingManifestDisplayName, once in
      readExistingManifestDescription). Now loads once via
      loadExistingMCPBManifest and shares the snapshot. Each consumer still
      applies its own field-specific "is this a derived default?" check.
    - Trim narrating comments per CLAUDE.md "default to no comments".
      Removed bug-history paragraphs ("The previous version of this
      function also rejected title-cased slugs…", "Pre-fix m.Description
      silently overwrote hand-edits…", "regressed library CLIs from 'ESPN'
      → 'espn'") from godocs and test preambles. That archaeology lives in
      the commit message.
    - Replace "manifest.json" string literal in mcpsync/sync.go with the
      exported pipeline.MCPBManifestFilename constant — closes a small
      naming-leak that defeats the "promoted from string literals" rationale
      on the constants block.
    - Add writeMCPBManifest test helper to match the existing writeManifest
      + readMCPBManifest helper trio. Test seed blocks collapse from 3
      lines (json.Marshal + os.WriteFile + require) to one helper call.
    
    Net -77 lines. Tests 2227/2227. Golden 9/9. Lint clean.
    
    Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
    
    ---------
    
    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 internal/pipeline/climanifest_test.go |  87 +++++++++++++++++++++++++
 internal/pipeline/mcpb_manifest.go    | 118 ++++++++++------------------------
 internal/pipeline/mcpsync/sync.go     |  38 +++--------
 3 files changed, 130 insertions(+), 113 deletions(-)

diff --git a/internal/pipeline/climanifest_test.go b/internal/pipeline/climanifest_test.go
index 9e8da30a..25f8ef5e 100644
--- a/internal/pipeline/climanifest_test.go
+++ b/internal/pipeline/climanifest_test.go
@@ -701,6 +701,86 @@ func TestWriteMCPBManifest(t *testing.T) {
 	})
 }
 
+func TestWriteMCPBManifestPreservesExistingDisplayName(t *testing.T) {
+	cases := []struct {
+		name     string
+		apiSlug  string
+		existing string
+		want     string
+	}{
+		{"single-word title-case brand (Wikipedia)", "wikipedia", "Wikipedia", "Wikipedia"},
+		{"single-word title-case brand (Stripe)", "stripe", "Stripe", "Stripe"},
+		{"all-caps brand (ESPN)", "espn", "ESPN", "ESPN"},
+		{"branded with punctuation (Cal.com)", "cal-com", "Cal.com", "Cal.com"},
+		{"multi-word brand (Company GOAT)", "company-goat", "Company GOAT", "Company GOAT"},
+		{"lowercase slug treated as derived fallback", "espn", "espn", "espn"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			dir := t.TempDir()
+			writeMCPBManifest(t, dir, MCPBManifest{
+				ManifestVersion: MCPBManifestVersion,
+				Name:            tc.apiSlug + "-pp-mcp",
+				DisplayName:     tc.existing,
+				Description:     "stale description",
+			})
+			// CLIManifest WITHOUT DisplayName forces the chain to fall
+			// through to the existing manifest.
+			writeManifest(t, dir, CLIManifest{
+				APIName:   tc.apiSlug,
+				MCPBinary: tc.apiSlug + "-pp-mcp",
+				MCPReady:  "full",
+			})
+
+			require.NoError(t, WriteMCPBManifest(dir))
+			assert.Equal(t, tc.want, readMCPBManifest(t, dir).DisplayName)
+		})
+	}
+}
+
+func TestWriteMCPBManifestPreservesExistingDescription(t *testing.T) {
+	t.Run("hand-edited description preserved over canonical", func(t *testing.T) {
+		dir := t.TempDir()
+		handEdit := "Find the best version of any recipe across 37 trusted sites — trust-aware ranking weights real reader signal."
+		writeMCPBManifest(t, dir, MCPBManifest{
+			ManifestVersion: MCPBManifestVersion,
+			Name:            "recipe-goat-pp-mcp",
+			DisplayName:     "Recipe Goat",
+			Description:     handEdit,
+		})
+		writeManifest(t, dir, CLIManifest{
+			APIName:     "recipe-goat",
+			DisplayName: "Recipe Goat",
+			MCPBinary:   "recipe-goat-pp-mcp",
+			MCPReady:    "full",
+			Description: "Recipe GOAT aggregates recipes from canonical-description-source.json",
+		})
+
+		require.NoError(t, WriteMCPBManifest(dir))
+		assert.Equal(t, handEdit, readMCPBManifest(t, dir).Description)
+	})
+
+	t.Run("derived-default existing description refreshed from canonical", func(t *testing.T) {
+		dir := t.TempDir()
+		writeMCPBManifest(t, dir, MCPBManifest{
+			ManifestVersion: MCPBManifestVersion,
+			Name:            "wikipedia-pp-mcp",
+			DisplayName:     "Wikipedia",
+			Description:     "Wikipedia API surface as MCP tools.",
+		})
+		writeManifest(t, dir, CLIManifest{
+			APIName:     "wikipedia",
+			DisplayName: "Wikipedia",
+			MCPBinary:   "wikipedia-pp-mcp",
+			MCPReady:    "full",
+			Description: "Wikipedia REST API. Article summaries, search, related topics.",
+		})
+
+		require.NoError(t, WriteMCPBManifest(dir))
+		assert.Equal(t, "Wikipedia REST API. Article summaries, search, related topics.", readMCPBManifest(t, dir).Description)
+	})
+}
+
 func writeManifest(t *testing.T, dir string, m CLIManifest) {
 	t.Helper()
 	data, err := json.Marshal(m)
@@ -708,6 +788,13 @@ func writeManifest(t *testing.T, dir string, m CLIManifest) {
 	require.NoError(t, os.WriteFile(filepath.Join(dir, CLIManifestFilename), data, 0o644))
 }
 
+func writeMCPBManifest(t *testing.T, dir string, m MCPBManifest) {
+	t.Helper()
+	data, err := json.Marshal(m)
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(filepath.Join(dir, MCPBManifestFilename), data, 0o644))
+}
+
 func readMCPBManifest(t *testing.T, dir string) MCPBManifest {
 	t.Helper()
 	data, err := os.ReadFile(filepath.Join(dir, MCPBManifestFilename))
diff --git a/internal/pipeline/mcpb_manifest.go b/internal/pipeline/mcpb_manifest.go
index be361d1c..7fc8e2b1 100644
--- a/internal/pipeline/mcpb_manifest.go
+++ b/internal/pipeline/mcpb_manifest.go
@@ -161,15 +161,15 @@ func WriteMCPBManifestFromStruct(dir string, m CLIManifest) error {
 }
 
 func buildMCPBManifest(dir string, m CLIManifest) MCPBManifest {
-	// Resolution order mirrors manifestDescription: canonical source first
-	// (.printing-press.json populated by spec.display_name on modern
-	// prints), then existing manifest.json for older library CLIs whose
-	// codemod-baked display_name carries the right brand casing, then
-	// derived from the API slug. Without the existing-manifest fallback,
-	// mcp-sync regressed library CLIs from "ESPN" → "espn".
+	// display_name and description use opposite preservation rules:
+	// canonical wins for display_name (so spec updates flow through),
+	// existing wins for description (so hand-edits survive regen). Both
+	// consult the existing manifest.json — load once, share the snapshot.
+	existing := loadExistingMCPBManifest(dir)
+
 	displayName := m.DisplayName
-	if displayName == "" {
-		displayName = readExistingManifestDisplayName(dir, m.APIName)
+	if displayName == "" && existing != nil && existing.DisplayName != "" && existing.DisplayName != m.APIName {
+		displayName = existing.DisplayName
 	}
 	if displayName == "" {
 		displayName = m.APIName
@@ -184,7 +184,7 @@ func buildMCPBManifest(dir string, m CLIManifest) MCPBManifest {
 		// regeneration. A hardcoded "1.0.0" would defeat the host's
 		// "newer bundle available" prompt.
 		Version:     bundleVersion(m),
-		Description: manifestDescription(dir, m, displayName),
+		Description: manifestDescription(existing, m, displayName),
 		Author:      MCPBAuthor{Name: "CLI Printing Press"},
 		License:     "Apache-2.0",
 		Server: MCPBServer{
@@ -218,92 +218,44 @@ func bundleVersion(m CLIManifest) string {
 	return "0.0.0"
 }
 
-// manifestDescription prefers the catalog/spec description verbatim and
-// only falls back to a derived sentence when nothing better is available.
-// We deliberately keep this single-line — long_description is reserved for
-// multi-paragraph context, which we don't synthesize from spec data today.
-//
-// Resolution order: the .printing-press.json description (canonical source
-// when the printing-press version that produced the CLI populates it) →
-// the existing manifest.json's description (preserves rich descriptions
-// baked by older codemods or hand-edits when .printing-press.json lacks
-// the field — e.g., library CLIs printed under v1.x predate the field) →
-// derived from displayName.
-func manifestDescription(dir string, m CLIManifest, displayName string) string {
+// manifestDescription returns the existing hand-edited description over
+// the canonical one from .printing-press.json. The existing snapshot's
+// description is only treated as "hand-edited" when it differs from the
+// derived default — so a prior derived default still loses to canonical.
+func manifestDescription(existing *existingMCPBManifest, m CLIManifest, displayName string) string {
+	derivedDefault := displayName + " API surface as MCP tools."
+	if existing != nil && existing.Description != "" && existing.Description != derivedDefault {
+		return existing.Description
+	}
 	if m.Description != "" {
 		return m.Description
 	}
-	if existing := readExistingManifestDescription(dir, displayName); existing != "" {
-		return existing
-	}
-	return displayName + " API surface as MCP tools."
+	return derivedDefault
 }
 
-// readExistingManifestDescription returns the description from an existing
-// manifest.json on disk, but only if it's a real description rather than
-// the derived "<displayName> API surface as MCP tools." default we'd
-// otherwise re-emit. Returning the derived form would defeat the
-// fallback chain by treating an old derived-default as "preserved
-// content" and never advancing past it.
-func readExistingManifestDescription(dir, displayName string) string {
-	data, err := os.ReadFile(filepath.Join(dir, MCPBManifestFilename))
-	if err != nil {
-		return ""
-	}
-	var existing struct {
-		Description string `json:"description"`
-	}
-	if err := json.Unmarshal(data, &existing); err != nil {
-		return ""
-	}
-	derivedDefault := displayName + " API surface as MCP tools."
-	if existing.Description == "" || existing.Description == derivedDefault {
-		return ""
-	}
-	return existing.Description
+// existingMCPBManifest is the subset of manifest.json the manifest writer
+// reads when refreshing a published CLI's bundle. Single load site so the
+// display_name and description preservation rules don't each re-read the
+// file on every WriteMCPBManifest call.
+type existingMCPBManifest struct {
+	DisplayName string `json:"display_name"`
+	Description string `json:"description"`
 }
 
-// readExistingManifestDisplayName returns the display_name from an
-// existing manifest.json if it's a real brand name rather than a derived
-// fallback. The two derived forms we explicitly reject: the bare API slug
-// (lowercase, e.g. "espn") that buildMCPBManifest emits when nothing
-// better is known, and the title-cased slug ("Espn") that the older
-// spec.EffectiveDisplayName fallback used to produce. Anything else —
-// "ESPN", "Cal.com", "Company GOAT", "PokéAPI" — is real brand content
-// from a hand-edit or a codemod and worth preserving across regen.
-func readExistingManifestDisplayName(dir, apiSlug string) string {
+// loadExistingMCPBManifest returns nil when the file is missing or
+// unparseable. Callers branch on nil; non-nil means the snapshot is
+// usable but each field still needs its own "is this a derived default?"
+// check (see buildMCPBManifest and manifestDescription).
+func loadExistingMCPBManifest(dir string) *existingMCPBManifest {
 	data, err := os.ReadFile(filepath.Join(dir, MCPBManifestFilename))
 	if err != nil {
-		return ""
-	}
-	var existing struct {
-		DisplayName string `json:"display_name"`
+		return nil
 	}
+	var existing existingMCPBManifest
 	if err := json.Unmarshal(data, &existing); err != nil {
-		return ""
-	}
-	if existing.DisplayName == "" || existing.DisplayName == apiSlug {
-		return ""
-	}
-	titleCased := titleCaseFirstRune(apiSlug)
-	if existing.DisplayName == titleCased {
-		return ""
-	}
-	return existing.DisplayName
-}
-
-// titleCaseFirstRune capitalizes the first ASCII letter of slug. Mirrors
-// the older spec.EffectiveDisplayName fallback so we can detect and
-// reject preserved title-cased slugs that masquerade as real brand names.
-func titleCaseFirstRune(slug string) string {
-	if slug == "" {
-		return ""
-	}
-	runes := []rune(slug)
-	if runes[0] >= 'a' && runes[0] <= 'z' {
-		runes[0] -= 'a' - 'A'
+		return nil
 	}
-	return string(runes)
+	return &existing
 }
 
 // buildMCPBEnv maps each declared auth env var into the launch spec's env
diff --git a/internal/pipeline/mcpsync/sync.go b/internal/pipeline/mcpsync/sync.go
index dc8244d0..a9b60a75 100644
--- a/internal/pipeline/mcpsync/sync.go
+++ b/internal/pipeline/mcpsync/sync.go
@@ -347,12 +347,13 @@ func newRootCmd(flags *rootFlags) *cobra.Command {
 	return writeFileAtomic(path, []byte(src))
 }
 
-// readExistingManifestDisplayName returns the display_name from an existing
-// manifest.json on disk if it's a real brand name rather than the
-// title-cased slug fallback. Used by Sync to preserve PR #145 codemod
-// brand-casing for library CLIs printed before spec.display_name existed.
+// readExistingManifestDisplayName returns the display_name from an
+// existing manifest.json if it's a real brand name. The only form
+// rejected is the bare lowercase slug we'd otherwise emit as last
+// resort; everything else (ESPN, Wikipedia, Cal.com, Company GOAT,
+// PokéAPI) is preserved.
 func readExistingManifestDisplayName(cliDir string) string {
-	manifestData, err := os.ReadFile(filepath.Join(cliDir, "manifest.json"))
+	manifestData, err := os.ReadFile(filepath.Join(cliDir, pipeline.MCPBManifestFilename))
 	if err != nil {
 		return ""
 	}
@@ -363,36 +364,13 @@ func readExistingManifestDisplayName(cliDir string) string {
 	if err := json.Unmarshal(manifestData, &existing); err != nil {
 		return ""
 	}
-	if existing.DisplayName == "" {
-		return ""
-	}
-	// The derived form for old prints is the title-cased mcp-binary slug
-	// minus the "-pp-mcp" suffix (e.g., "espn-pp-mcp" → "Espn"). If the
-	// existing display_name matches that derived shape, treat it as no
-	// brand info and fall through.
-	derived := titleCaseFromSlug(strings.TrimSuffix(existing.Name, "-pp-mcp"))
-	if existing.DisplayName == derived {
+	apiSlug := strings.TrimSuffix(existing.Name, "-pp-mcp")
+	if existing.DisplayName == "" || existing.DisplayName == apiSlug {
 		return ""
 	}
 	return existing.DisplayName
 }
 
-// titleCaseFromSlug capitalizes the first rune of a slug. Approximates
-// the spec.EffectiveDisplayName fallback for slugs without an explicit
-// display_name (e.g., "espn" → "Espn"). Mirrors the case-detection logic
-// readExistingManifestDisplayName uses to decide whether the existing
-// manifest carries real brand information.
-func titleCaseFromSlug(slug string) string {
-	if slug == "" {
-		return ""
-	}
-	runes := []rune(slug)
-	if runes[0] >= 'a' && runes[0] <= 'z' {
-		runes[0] -= 'a' - 'A'
-	}
-	return string(runes)
-}
-
 // validateSpecNameMatchesDir refuses to migrate when spec.yaml.name
 // diverges from the CLI directory's basename. This catches the
 // weather-goat / open-meteo class of drift where an old emboss/rename

← 283c9923 fix(cli): transliterate spec strings to ASCII via Unidecode  ·  back to Cli Printing Press  ·  fix(cli): wire auth.optional through CLIManifest to MCPB use a158a629 →