← back to Cli Printing Press
fix(cli): mcp-sync stops conflating API slug with binary name (#487)
2899613de8060e80778fe7ae00ea737d74c5db50 · 2026-05-01 21:29:43 -0700 · Trevin Chow
* fix(cli): mcp-sync stops conflating API slug with binary name
reconcileSpecNameWithDir compared filepath.Base(cliDir) directly
against spec.yaml.name. Working dirs (paths.WorkingCLIDir) and
public-library checkouts both use the suffixed binary form
`<slug>-pp-cli/`, while spec.yaml.name holds the bare slug — so the
comparison always mismatched and the auto-fix path overwrote the
slug-of-record with the binary name. The next regen then re-suffixed
that, emitting phantom `cmd/<slug>-pp-cli-pp-cli/` and
`cmd/<slug>-pp-cli-pp-mcp/` directories alongside the canonical ones
and registering a wrong-identity MCP server.
Add a slugFromDir helper that strips naming.CurrentCLISuffix from the
basename before comparing. Both validate and reconcile paths now
accept suffixed dirs whose stripped form matches the slug. Drift
cases (e.g. dir = weather-goat with spec.yaml.name = weather) still
fire because the stripped form differs from the spec name.
Three regression tests cover the previously-unexercised suffixed-dir
paths: validator accept, reconcile no-op, and drift-on-suffixed-dir
where the rewrite must reach the bare slug rather than the suffixed
binary name.
Fixes #481
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): use existing naming.TrimCLISuffix in mcp-sync slug derivation
Inline naming.TrimCLISuffix(filepath.Base(cliDir)) at the two call sites
instead of the bespoke slugFromDir helper. naming.TrimCLISuffix already
exists, is the established repo idiom (runtime.go:116, emboss.go:245),
and additionally handles the legacy -cli suffix and numeric rerun
suffixes — coverage the bespoke helper silently dropped, leaving older
library CLIs exposed to the same conflation bug.
Adds a regression test for the legacy -cli suffix to lock in the
broader coverage. Trims the now-redundant test doc comments to match
the file's existing 1-3 line style.
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
M internal/pipeline/mcpsync/sync.goM internal/pipeline/mcpsync/sync_test.go
Diff
commit 2899613de8060e80778fe7ae00ea737d74c5db50
Author: Trevin Chow <trevin@trevinchow.com>
Date: Fri May 1 21:29:43 2026 -0700
fix(cli): mcp-sync stops conflating API slug with binary name (#487)
* fix(cli): mcp-sync stops conflating API slug with binary name
reconcileSpecNameWithDir compared filepath.Base(cliDir) directly
against spec.yaml.name. Working dirs (paths.WorkingCLIDir) and
public-library checkouts both use the suffixed binary form
`<slug>-pp-cli/`, while spec.yaml.name holds the bare slug — so the
comparison always mismatched and the auto-fix path overwrote the
slug-of-record with the binary name. The next regen then re-suffixed
that, emitting phantom `cmd/<slug>-pp-cli-pp-cli/` and
`cmd/<slug>-pp-cli-pp-mcp/` directories alongside the canonical ones
and registering a wrong-identity MCP server.
Add a slugFromDir helper that strips naming.CurrentCLISuffix from the
basename before comparing. Both validate and reconcile paths now
accept suffixed dirs whose stripped form matches the slug. Drift
cases (e.g. dir = weather-goat with spec.yaml.name = weather) still
fire because the stripped form differs from the spec name.
Three regression tests cover the previously-unexercised suffixed-dir
paths: validator accept, reconcile no-op, and drift-on-suffixed-dir
where the rewrite must reach the bare slug rather than the suffixed
binary name.
Fixes #481
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): use existing naming.TrimCLISuffix in mcp-sync slug derivation
Inline naming.TrimCLISuffix(filepath.Base(cliDir)) at the two call sites
instead of the bespoke slugFromDir helper. naming.TrimCLISuffix already
exists, is the established repo idiom (runtime.go:116, emboss.go:245),
and additionally handles the legacy -cli suffix and numeric rerun
suffixes — coverage the bespoke helper silently dropped, leaving older
library CLIs exposed to the same conflation bug.
Adds a regression test for the legacy -cli suffix to lock in the
broader coverage. Trims the now-redundant test doc comments to match
the file's existing 1-3 line style.
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/mcpsync/sync.go | 48 ++++++++++++----------
internal/pipeline/mcpsync/sync_test.go | 74 +++++++++++++++++++++++++++++++++-
2 files changed, 99 insertions(+), 23 deletions(-)
diff --git a/internal/pipeline/mcpsync/sync.go b/internal/pipeline/mcpsync/sync.go
index 6021b157..9ddd5b8d 100644
--- a/internal/pipeline/mcpsync/sync.go
+++ b/internal/pipeline/mcpsync/sync.go
@@ -13,6 +13,7 @@ import (
"github.com/mvanhorn/cli-printing-press/v3/internal/generator"
"github.com/mvanhorn/cli-printing-press/v3/internal/graphql"
"github.com/mvanhorn/cli-printing-press/v3/internal/mcpoverrides"
+ "github.com/mvanhorn/cli-printing-press/v3/internal/naming"
"github.com/mvanhorn/cli-printing-press/v3/internal/openapi"
"github.com/mvanhorn/cli-printing-press/v3/internal/pipeline"
"github.com/mvanhorn/cli-printing-press/v3/internal/spec"
@@ -86,7 +87,7 @@ func Sync(cliDir string, opts Options) (Result, error) {
return Result{}, err
}
if renamedFrom != "" {
- fmt.Fprintf(os.Stderr, "mcp-sync: rewrote spec.yaml name from %q to %q to match directory basename\n", renamedFrom, parsed.Name)
+ fmt.Fprintf(os.Stderr, "mcp-sync: rewrote spec.yaml name from %q to %q to match directory-derived slug\n", renamedFrom, parsed.Name)
}
// Preserve the existing manifest.json's display_name onto the parsed
// spec when the spec itself doesn't carry one. Library CLIs printed
@@ -544,25 +545,27 @@ func readRegistryDisplayName(slug string) string {
}
// 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
-// updated the directory but left spec.yaml.name behind, producing
-// spurious cmd/<spec.name>-pp-{cli,mcp}/ directories on regen and a
-// wrong MCP server identity. Caller can pass --force to bypass when
-// they know the divergence is intentional (e.g., a deliberate alias).
+// diverges from the slug derived from the CLI directory's basename
+// (via naming.TrimCLISuffix, which strips both -pp-cli and legacy -cli
+// forms). This catches the weather-goat / open-meteo class of drift
+// where an old emboss/rename updated the directory but left
+// spec.yaml.name behind, producing spurious cmd/<spec.name>-pp-{cli,mcp}/
+// directories on regen and a wrong MCP server identity. Caller can pass
+// --force to bypass when they know the divergence is intentional
+// (e.g., a deliberate alias).
func validateSpecNameMatchesDir(cliDir string, parsed *spec.APISpec) error {
if parsed == nil || parsed.Name == "" {
return nil
}
- dirName := filepath.Base(cliDir)
- if dirName == parsed.Name {
+ expected := naming.TrimCLISuffix(filepath.Base(cliDir))
+ if expected == parsed.Name {
return nil
}
return fmt.Errorf(
- "spec.yaml name %q does not match directory basename %q. "+
+ "spec.yaml name %q does not match directory-derived slug %q. "+
"This produces spurious cmd/%s-pp-{cli,mcp}/ directories on regen and an incorrect MCP server identity. "+
- "Fix spec.yaml's `name:` field to match the directory, or pass --force to bypass",
- parsed.Name, dirName, parsed.Name,
+ "Fix spec.yaml's `name:` field to match the directory slug, or pass --force to bypass",
+ parsed.Name, expected, parsed.Name,
)
}
@@ -571,30 +574,31 @@ func validateSpecNameMatchesDir(cliDir string, parsed *spec.APISpec) error {
var internalSpecNameLine = regexp.MustCompile(`(?m)^name:[ \t]*.*$`)
// reconcileSpecNameWithDir auto-fixes the common drift case (internal
-// YAML spec whose top-level `name:` doesn't match the directory) by
-// rewriting the line in place and updating parsed.Name. Falls back to
-// validateSpecNameMatchesDir's --force-required error for OpenAPI and
-// GraphQL specs, where rewriting info.title or schema metadata is too
-// invasive to do silently. The non-empty renamedFrom return signals
-// the caller to log the rename.
+// YAML spec whose top-level `name:` doesn't match the slug implied by
+// the directory) by rewriting the line in place and updating
+// parsed.Name. Falls back to validateSpecNameMatchesDir's
+// --force-required error for OpenAPI and GraphQL specs, where
+// rewriting info.title or schema metadata is too invasive to do
+// silently. The non-empty renamedFrom return signals the caller to
+// log the rename.
func reconcileSpecNameWithDir(cliDir string, parsed *spec.APISpec) (renamedFrom string, err error) {
if parsed == nil || parsed.Name == "" {
return "", nil
}
- dirName := filepath.Base(cliDir)
- if dirName == parsed.Name {
+ expected := naming.TrimCLISuffix(filepath.Base(cliDir))
+ if expected == parsed.Name {
return "", nil
}
specPath, data, ok := findInternalYAMLSpec(cliDir)
if !ok || !internalSpecNameLine.Match(data) {
return "", validateSpecNameMatchesDir(cliDir, parsed)
}
- rewritten := internalSpecNameLine.ReplaceAll(data, []byte("name: "+dirName))
+ rewritten := internalSpecNameLine.ReplaceAll(data, []byte("name: "+expected))
if err := writeFileAtomic(specPath, rewritten); err != nil {
return "", fmt.Errorf("rewriting %s: %w", specPath, err)
}
oldName := parsed.Name
- parsed.Name = dirName
+ parsed.Name = expected
return oldName, nil
}
diff --git a/internal/pipeline/mcpsync/sync_test.go b/internal/pipeline/mcpsync/sync_test.go
index 5bfa7934..b066d084 100644
--- a/internal/pipeline/mcpsync/sync_test.go
+++ b/internal/pipeline/mcpsync/sync_test.go
@@ -303,7 +303,7 @@ func TestValidateSpecNameMatchesDirRejectsDrift(t *testing.T) {
err := validateSpecNameMatchesDir(cliDir, parsed)
require.Error(t, err)
assert.Contains(t, err.Error(), "spec.yaml name \"weather\"")
- assert.Contains(t, err.Error(), "directory basename \"weather-goat\"")
+ assert.Contains(t, err.Error(), "directory-derived slug \"weather-goat\"")
assert.Contains(t, err.Error(), "--force")
}
@@ -406,6 +406,78 @@ func TestReconcileSpecNameWithDirNoYAMLFallsThroughToValidator(t *testing.T) {
assert.Contains(t, err.Error(), "--force")
}
+// TestValidateSpecNameMatchesDirAcceptsSuffixedDir — working dirs and
+// public-library checkouts both use the suffixed form `<slug>-pp-cli/`,
+// while spec.yaml.name holds the bare slug. The validator must reduce
+// the basename to its slug form before comparing.
+func TestValidateSpecNameMatchesDirAcceptsSuffixedDir(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "producthunt-pp-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+ parsed := &spec.APISpec{Name: "producthunt"}
+ if err := validateSpecNameMatchesDir(cliDir, parsed); err != nil {
+ t.Errorf("suffixed dir matching its slug should accept; got %v", err)
+ }
+}
+
+// TestValidateSpecNameMatchesDirAcceptsLegacySuffixedDir — older library
+// CLIs predating -pp-cli use the bare -cli suffix. naming.TrimCLISuffix
+// handles both forms; this locks in coverage for the legacy shape.
+func TestValidateSpecNameMatchesDirAcceptsLegacySuffixedDir(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "weather-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+ parsed := &spec.APISpec{Name: "weather"}
+ if err := validateSpecNameMatchesDir(cliDir, parsed); err != nil {
+ t.Errorf("legacy -cli suffix should reduce to the slug and accept; got %v", err)
+ }
+}
+
+// TestReconcileSpecNameWithDirNoOpOnSuffixedDir — exercises the
+// reconcile path: spec.yaml must not be rewritten and parsed.Name must
+// not be mutated when the directory is the suffixed form of the slug.
+func TestReconcileSpecNameWithDirNoOpOnSuffixedDir(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "producthunt-pp-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+ specPath := filepath.Join(cliDir, "spec.yaml")
+ original := "name: producthunt\ndescription: \"Product Hunt API\"\nversion: \"1.0.0\"\nbase_url: \"https://api.producthunt.com\"\n"
+ require.NoError(t, os.WriteFile(specPath, []byte(original), 0o644))
+
+ parsed := &spec.APISpec{Name: "producthunt"}
+ renamedFrom, err := reconcileSpecNameWithDir(cliDir, parsed)
+ require.NoError(t, err)
+ assert.Equal(t, "", renamedFrom, "no rename should fire when the suffixed basename strips to the slug")
+ assert.Equal(t, "producthunt", parsed.Name, "parsed.Name must not be mutated to the suffixed form")
+
+ after, err := os.ReadFile(specPath)
+ require.NoError(t, err)
+ assert.Equal(t, original, string(after), "spec.yaml must be byte-identical when no drift exists")
+}
+
+// TestReconcileSpecNameWithDirRejectsDriftEvenOnSuffixedDir — stripping
+// the CLI suffix must not mask legitimate drift. dir = weather-goat-pp-cli
+// reduces to slug "weather-goat", which still mismatches spec.Name "weather".
+func TestReconcileSpecNameWithDirRejectsDriftEvenOnSuffixedDir(t *testing.T) {
+ t.Parallel()
+ cliDir := filepath.Join(t.TempDir(), "weather-goat-pp-cli")
+ require.NoError(t, os.MkdirAll(cliDir, 0o755))
+ specPath := filepath.Join(cliDir, "spec.yaml")
+ original := "name: weather\ndescription: \"Weather GOAT\"\nversion: \"1.0.0\"\nbase_url: \"https://api.example.com\"\n"
+ require.NoError(t, os.WriteFile(specPath, []byte(original), 0o644))
+
+ parsed := &spec.APISpec{Name: "weather"}
+ renamedFrom, err := reconcileSpecNameWithDir(cliDir, parsed)
+ require.NoError(t, err)
+ assert.Equal(t, "weather", renamedFrom, "drift to the slug form must still be reported")
+ assert.Equal(t, "weather-goat", parsed.Name, "parsed.Name must reach the bare slug, not the suffixed binary name")
+
+ after, err := os.ReadFile(specPath)
+ require.NoError(t, err)
+ assert.Contains(t, string(after), "name: weather-goat\n", "spec.yaml must hold the bare slug, not the -pp-cli form")
+ assert.NotContains(t, string(after), "weather-goat-pp-cli", "the suffix must never reach the slug-of-record")
+}
+
// TestEnsureMCPGoMinVersionBumpsOldPin — older library CLIs predating
// the cobratree templates pin a v0.x version that lacks
// WithReadOnlyHintAnnotation/GetArguments. mcp-sync bumps to the floor
← 9f265510 fix(cli): scorer accuracy — insight prefix, MCP dir selectio
·
back to Cli Printing Press
·
fix(cli): scorecard APIName drops binary suffix (#489) c76d3ec9 →