[object Object]

← back to Cli Printing Press

fix(cli): wire auth.optional through CLIManifest to MCPB user_config Required (#374)

a158a62984b796eb90eb3e09554fe2f8785b88ac · 2026-04-28 21:27:50 -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>

* fix(cli): wire auth.optional through CLIManifest to MCPB user_config Required

spec.yaml's auth.optional field has existed for months ("true when the
key enhances a subset of features rather than gating core functionality")
but never reached the manifest layer. CLIManifest didn't carry an
AuthOptional field, so buildMCPBUserConfig derived Required from
authRequiresCredential(authType) alone — api_key always meant
Required: true regardless of spec intent.

Result: recipe-goat's USDA_FDC_API_KEY (only used by `recipe get
--nutrition`) shipped as Required in the MCPB Configure modal, hiding
the fact that every other tool works without the key. Same pattern
would hit any future CLI with an opt-in api_key.

Three small wires:
  - Add AuthOptional bool to CLIManifest (json: auth_optional)
  - Populate from parsed.Auth.Optional in populateMCPMetadata
  - In buildMCPBUserConfig: required = authRequiresCredential(type) && !AuthOptional

Description prefix already handles the optional case correctly (envVarDescription
prepends "Optional." when required is false), so opt-in keys now surface
as "Optional. Sets X for Y. Get a credential from <URL>." in the modal.

End-to-end verified against recipe-goat's spec (auth.optional: true,
auth.key_url set): manifest.json now omits "required" and the description
starts with "Optional." instead of being marked Required.

Note: mcp-sync writes manifest.json from .printing-press.json provenance,
not from the parsed spec directly. Existing library CLIs need their
.printing-press.json refreshed (one-line jq injection per CLI) for the
new field to surface in regen. Newly generated CLIs flow through cleanly
since populateMCPMetadata runs at generate time. Refreshing provenance
on mcp-sync is a separate concern worth its own PR.

Tests: new TestWriteMCPBManifest subtest pins the recipe-goat shape
(api_key + AuthOptional=true → Required false, "Optional." description
prefix, key_url surfaced). go test ./... 2228/2228. Golden 9/9.
golangci-lint clean.

---------

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

Files touched

Diff

commit a158a62984b796eb90eb3e09554fe2f8785b88ac
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Tue Apr 28 21:27:50 2026 -0700

    fix(cli): wire auth.optional through CLIManifest to MCPB user_config Required (#374)
    
    * 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>
    
    * fix(cli): wire auth.optional through CLIManifest to MCPB user_config Required
    
    spec.yaml's auth.optional field has existed for months ("true when the
    key enhances a subset of features rather than gating core functionality")
    but never reached the manifest layer. CLIManifest didn't carry an
    AuthOptional field, so buildMCPBUserConfig derived Required from
    authRequiresCredential(authType) alone — api_key always meant
    Required: true regardless of spec intent.
    
    Result: recipe-goat's USDA_FDC_API_KEY (only used by `recipe get
    --nutrition`) shipped as Required in the MCPB Configure modal, hiding
    the fact that every other tool works without the key. Same pattern
    would hit any future CLI with an opt-in api_key.
    
    Three small wires:
      - Add AuthOptional bool to CLIManifest (json: auth_optional)
      - Populate from parsed.Auth.Optional in populateMCPMetadata
      - In buildMCPBUserConfig: required = authRequiresCredential(type) && !AuthOptional
    
    Description prefix already handles the optional case correctly (envVarDescription
    prepends "Optional." when required is false), so opt-in keys now surface
    as "Optional. Sets X for Y. Get a credential from <URL>." in the modal.
    
    End-to-end verified against recipe-goat's spec (auth.optional: true,
    auth.key_url set): manifest.json now omits "required" and the description
    starts with "Optional." instead of being marked Required.
    
    Note: mcp-sync writes manifest.json from .printing-press.json provenance,
    not from the parsed spec directly. Existing library CLIs need their
    .printing-press.json refreshed (one-line jq injection per CLI) for the
    new field to surface in regen. Newly generated CLIs flow through cleanly
    since populateMCPMetadata runs at generate time. Refreshing provenance
    on mcp-sync is a separate concern worth its own PR.
    
    Tests: new TestWriteMCPBManifest subtest pins the recipe-goat shape
    (api_key + AuthOptional=true → Required false, "Optional." description
    prefix, key_url surfaced). go test ./... 2228/2228. Golden 9/9.
    golangci-lint clean.
    
    ---------
    
    Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 internal/pipeline/climanifest.go      |  8 +++++++-
 internal/pipeline/climanifest_test.go | 27 +++++++++++++++++++++++++++
 internal/pipeline/mcpb_manifest.go    |  7 ++++++-
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/internal/pipeline/climanifest.go b/internal/pipeline/climanifest.go
index da49636d..ce6aaa88 100644
--- a/internal/pipeline/climanifest.go
+++ b/internal/pipeline/climanifest.go
@@ -60,7 +60,12 @@ type CLIManifest struct {
 	// AuthKeyURL is the page where users register for an API key. Used by
 	// downstream emitters (MCPB manifest user_config descriptions, doctor
 	// hints) to point users at the right credential source.
-	AuthKeyURL    string                 `json:"auth_key_url,omitempty"`
+	AuthKeyURL string `json:"auth_key_url,omitempty"`
+	// AuthOptional is true when the credential gates a subset of features
+	// (e.g., USDA nutrition backfill on recipe-goat) rather than every
+	// API call. Drives the MCPB user_config Required field so opt-in
+	// keys don't surface as mandatory in install dialogs.
+	AuthOptional  bool                   `json:"auth_optional,omitempty"`
 	NovelFeatures []NovelFeatureManifest `json:"novel_features,omitempty"`
 }
 
@@ -203,6 +208,7 @@ func populateMCPMetadata(m *CLIManifest, parsed *spec.APISpec) {
 	m.AuthType = parsed.Auth.Type
 	m.AuthEnvVars = parsed.Auth.EnvVars
 	m.AuthKeyURL = parsed.Auth.KeyURL
+	m.AuthOptional = parsed.Auth.Optional
 	if m.DisplayName == "" {
 		m.DisplayName = parsed.EffectiveDisplayName()
 	}
diff --git a/internal/pipeline/climanifest_test.go b/internal/pipeline/climanifest_test.go
index 25f8ef5e..c1f7229e 100644
--- a/internal/pipeline/climanifest_test.go
+++ b/internal/pipeline/climanifest_test.go
@@ -660,6 +660,33 @@ func TestWriteMCPBManifest(t *testing.T) {
 		assert.Contains(t, key.Description, "Optional.")
 	})
 
+	t.Run("api_key auth with auth_optional=true flips Required to false", func(t *testing.T) {
+		// recipe-goat shape: USDA_FDC_API_KEY is api_key but only powers
+		// opt-in `recipe get --nutrition`. spec.yaml's `auth.optional: true`
+		// must override authRequiresCredential's per-type heuristic so the
+		// MCPB Configure modal doesn't mark it Required.
+		dir := t.TempDir()
+		writeManifest(t, dir, CLIManifest{
+			APIName:      "recipe-goat",
+			DisplayName:  "Recipe Goat",
+			MCPBinary:    "recipe-goat-pp-mcp",
+			MCPReady:     "full",
+			AuthType:     "api_key",
+			AuthEnvVars:  []string{"USDA_FDC_API_KEY"},
+			AuthKeyURL:   "https://fdc.nal.usda.gov/api-key-signup",
+			AuthOptional: true,
+		})
+
+		require.NoError(t, WriteMCPBManifest(dir))
+		got := readMCPBManifest(t, dir)
+
+		key, ok := got.UserConfig["usda_fdc_api_key"]
+		require.True(t, ok)
+		assert.False(t, key.Required, "auth_optional=true must flip Required to false even on api_key")
+		assert.Contains(t, key.Description, "Optional.", "description prefix should reflect optional state")
+		assert.Contains(t, key.Description, "https://fdc.nal.usda.gov/api-key-signup")
+	})
+
 	t.Run("multiple optional env vars (company-goat shape)", func(t *testing.T) {
 		dir := t.TempDir()
 		writeManifest(t, dir, CLIManifest{
diff --git a/internal/pipeline/mcpb_manifest.go b/internal/pipeline/mcpb_manifest.go
index 7fc8e2b1..78e980ab 100644
--- a/internal/pipeline/mcpb_manifest.go
+++ b/internal/pipeline/mcpb_manifest.go
@@ -282,7 +282,12 @@ func buildMCPBUserConfig(m CLIManifest) map[string]MCPBVar {
 	if len(m.AuthEnvVars) == 0 {
 		return nil
 	}
-	required := authRequiresCredential(m.AuthType)
+	// AuthOptional overrides the auth-type heuristic. api_key would otherwise
+	// be Required: true, but recipe-goat's USDA_FDC_API_KEY only powers an
+	// opt-in `--nutrition` flag — marking it required hides the fact that
+	// every other tool works without the key. The spec author's `optional:
+	// true` is the explicit signal.
+	required := authRequiresCredential(m.AuthType) && !m.AuthOptional
 	vars := make(map[string]MCPBVar, len(m.AuthEnvVars))
 	for _, name := range m.AuthEnvVars {
 		vars[userConfigKey(name)] = MCPBVar{

← 9f57c41d fix(cli): mcp-sync preserves manifest brand-casing and hand-  ·  back to Cli Printing Press  ·  fix(cli): mcp-sync refreshes .printing-press.json from spec. 40d1ffad →